feat(eval): measure review quality against a labelled corpus - #86
Conversation
Nothing in tinysweeper could say whether a change to a prompt, a rule document, a threshold or a lane made the review better. The test suite proves the machinery behaves; it cannot prove the reviewer is any good, and the two questions are unrelated. Every document in `presets/rules/` was written from judgement and validated by reading output on live pull requests — which measures the reviewer against the memory of whoever last looked at it. Three commands, separated by files on disk, the same split `review` and `apply` have: `eval run` costs money and writes proposals plus cassettes, `eval score` and `eval report` are free and offline. A matching rule gets rewritten ten times before it is right, and welding it to the run would price every rewrite at another live corpus. `src/harness/cassette.rs` is a record/replay decorator over the `Model` port. Its key covers the model id, schema name, token ceiling and every message, so any prompt change invalidates every cassette that prompt produced — and strict replay says so and stops, because a run that silently fell back to a stale recording would report the old prompt's quality under the new prompt's name. Usage and cost replay verbatim rather than through `pricing.rs`, so an offline re-score reproduces the dollars the live run actually paid. Decisions worth naming: - **`exhaustive` is off by default.** A case contributes to precision only if it claims its labels are complete; otherwise an unmatched finding is `Unscored`. This was not the original design — the first live run reported a genuine off-by-one in a helper nobody had labelled and scored it a false positive. You can only call an unmatched finding wrong if you have asserted every right one. - **Scoring is two-stage.** Structural (path, line ±3, lane, severity) then a keyword check against the finding's own words. A lane will happily leave a naming nit on the exact line holding the real bug, and overlap alone scores that a find — so the harness would reward commenting on hot lines. - **`incremental` is forced off and every case gets a fresh store.** Suppression and cross-push dedupe make output depend on what the last run saw, so a warm corpus measures run order and reports it as review quality. - **No composite score.** The gate is a conjunction, each term on its own line, with a 2% tolerance on recall for provider routing noise. - **A failed review scores zero, not nothing** — dropping it from the denominator would let a run improve its score by breaking. The corpus ships with two cases, both regressions, both externally evidenced: issue #47's hallucinated hardware-access claim on #45, and the description finding anchored to a pricing-table line on #68 that PR #72 was opened to fix. Both hold. `evals/README.md` states plainly what is *not* measured yet — recall, clean pull requests, and any language but Rust — because a corpus that overstates its coverage is worse than a small one that does not. `cargo test --locked` replays the committed cassettes offline and asserts both regressions still hold, so a prompt change nobody re-recorded fails for free. `.github/workflows/eval.yml` runs the live half, fork-guarded, and is deliberately not a required check until run-to-run variance is measured. Verified: fmt, clippy --all-targets -D warnings, `cargo test --locked` (1092 offline tests, 51 new), `cargo check --all-features --all-targets`, and a live recording run — 2 cases, $0.0063, 0 forbidden hits. Co-authored-by: Medulla <medulla@tinyhumans.ai>
📝 WalkthroughWalkthroughAdded a corpus evaluation system with labelled cases, deterministic scoring, cassette recording and replay, baseline comparison, CLI commands, committed regression data, documentation, and scheduled GitHub Actions execution. ChangesEvaluation framework
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Contributor
participant EvalCLI
participant Runner
participant Cassette
participant Scorer
participant Baseline
Contributor->>EvalCLI: invoke eval run or report
EvalCLI->>Runner: load corpus and execute cases
Runner->>Cassette: record or replay model calls
Cassette-->>Runner: proposal data
Runner->>Scorer: score findings
Scorer-->>Runner: case scores
Runner->>Baseline: compare scorecard
Baseline-->>EvalCLI: report and gate result
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 20
🧹 Nitpick comments (5)
src/eval/runner_test.rs (1)
151-175: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for
rescore.
rescoreis the whole ofeval score, one of the three advertised stages, and no test in this file calls it. It has three distinct branches at src/eval/runner.rs:203-219: a proposal that parses, a proposal that does not parse, and no proposal at all. The last two produce the failure text a user reads when the stage goes wrong.The helpers here already produce a run directory, so the test is short.
💚 Proposed test
+#[tokio::test] +async fn rescoring_a_written_proposal_matches_the_run_that_wrote_it() { + let (dir, outcome) = record_then_replay(EXPECTATION, finder()).await; + let corpus = load(dir.path()).expect("loads"); + + let scores = rescore(&corpus, &dir.path().join("runs/test")).expect("rescores"); + assert_eq!(scores.len(), 1); + assert_eq!(scores[0].true_positives, outcome.scores[0].true_positives); +} + +#[tokio::test] +async fn a_case_with_no_proposal_is_reported_rather_than_dropped() { + // A corpus that silently scored fewer cases than it holds reports recall + // that is wrong in the flattering direction. + let dir = corpus_dir(EXPECTATION); + let corpus = load(dir.path()).expect("loads"); + + let scores = rescore(&corpus, &dir.path().join("runs/absent")).expect("rescores"); + assert_eq!(scores.len(), 1); + assert!( + scores[0] + .error + .as_deref() + .unwrap_or_default() + .contains("eval run"), + "{:?}", + scores[0].error + ); +}As per coding guidelines: "Maintain at least 80% coverage for meaningful library behavior."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/eval/runner_test.rs` around lines 151 - 175, Add tests in this file covering the rescore stage and its three branches in rescore: a valid proposal, an unparseable proposal, and no proposal. Reuse the existing run-directory helpers, and assert both successful rescoring and the user-visible failure text for the latter two cases.Source: Coding guidelines
src/eval/corpus.rs (1)
129-133: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winConsider hashing fixture bytes into the digest as well.
The digest covers
cases/*.tomlonly. A fixture edit changes the diff the reviewer sees, and therefore the score, but leavesCorpus::digestunchanged.report --gatethen compares a new scorecard against a baseline produced from different inputs and reports the comparison as valid.The fixture is read later in the loop, at line 152, so folding its bytes in is a small change.
♻️ Proposed change to include fixture bytes
Read the fixture bytes once, hash them, then parse:
let fixture_path = path.parent().unwrap_or(Path::new(".")).join(&case.fixture); let fixture = match read_fixture(&fixture_path) { - Ok(fixture) => fixture, + Ok((raw, fixture)) => { + // The fixture is the diff under review. A change to it moves + // the score, so a baseline taken before it must not compare. + hasher.update(raw.as_bytes()); + fixture + } Err(err) => { problems.push(format!("{} ({}): {err}", path.display(), case.id)); continue; } };
read_fixturethen returnsResult<(String, Fixture)>.Note that this makes the digest order-sensitive to the read order, which
paths.sort()at line 115 already fixes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/eval/corpus.rs` around lines 129 - 133, Update the corpus evaluation loop to read each fixture’s raw bytes once, incorporate those bytes into the digest alongside the case TOML, and parse the same contents afterward. Change read_fixture and its callers to return both the raw fixture text and parsed Fixture while preserving the existing sorted path order and evaluation behavior.src/eval/corpus_test.rs (1)
192-219: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd tests for the two duplicate-id rules.
The suite covers every other branch of
validate. Two branches have no test:
duplicate_idsatsrc/eval/corpus.rslines 200-210. This is the rule whose own message says two cases sharing an id "would overwrite each other's recording", so it is the most costly one to lose.- The shared expectation and forbidden id namespace at
src/eval/corpus.rslines 245-273.CaseScore.missed,CaseScore.forbidden_hits, andJudged.matchedall carry bare ids, so a collision makes a report ambiguous.💚 Proposed tests
#[test] fn two_cases_sharing_an_id_are_refused() { // The id keys the cassette directory, so a collision makes one case // replay the other's recording. let dir = corpus_with(&[ ("ts-0001", case_toml("shared", "")), ("ts-0002", case_toml("shared", "")), ]); let message = load_at(dir.path()).expect_err("must not load").to_string(); assert!(message.contains("duplicate case id"), "{message}"); assert!(message.contains("shared"), "{message}"); } #[test] fn an_expectation_and_a_forbidden_entry_may_not_share_an_id() { // A report names a match by bare id, so a collision makes `matched` // ambiguous between the two. let extra = r#" [[expected]] id = "X1" path = "src/a.rs" summary = "a defect" [[forbidden]] id = "X1" path = "src/a.rs" reason = "not a defect" matches = ["dead code"] "#; let dir = corpus_with(&[("ts-0001", case_toml("ts-0001", extra))]); let message = load_at(dir.path()).expect_err("must not load").to_string(); assert!(message.contains("duplicate id `X1`"), "{message}"); }Note that
corpus_withnames the fixture after the tuple key, not after the case id, so the first test needsfixtureincase_tomlto keep pointing at an existing file. Adjust the helper or write the fixtures directly.As per coding guidelines: "Maintain at least 80% coverage for meaningful library behavior."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/eval/corpus_test.rs` around lines 192 - 219, Add tests covering both missing validate branches: duplicate case IDs and collisions between shared expected and forbidden IDs. In the new case-ID test, ensure fixture filenames still match the case’s fixture reference when both cases use the same ID, then assert the error mentions “duplicate case id” and the ID. In the expectation/forbidden test, build entries sharing one ID and assert the error mentions the duplicate-ID message.Source: Coding guidelines
src/eval/report_test.rs (1)
162-195: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the two remaining gate terms.
comparefails on five terms. The tests cover recall, forbidden, and errored. The clean-PR term atreport.rslines 75-81 and the over-budget term at lines 82-88 have no test. A change that inverted either comparison would pass this suite.Add one test for each.
💚 Proposed tests
#[test] fn more_noise_on_clean_pull_requests_fails() { let baseline = card(vec![case_score("ts-clean", 0, &[], 0)]); let noisier = card(vec![case_score("ts-clean", 0, &[], 2)]); match compare(&noisier, &baseline, false) { Comparison::Fail(reasons) => assert!( reasons.iter().any(|r| r.contains("clean pull requests")), "{reasons:?}" ), other => panic!("expected a failure, got {other:?}"), } } #[test] fn a_case_that_went_over_budget_fails() { let baseline = card(vec![case_score("ts-0001", 1, &[], 0)]); let mut expensive = case_score("ts-0001", 1, &[], 0); expensive.over_budget = true; let costly = card(vec![expensive]); match compare(&costly, &baseline, false) { Comparison::Fail(reasons) => assert!( reasons.iter().any(|r| r.contains("over budget")), "{reasons:?}" ), other => panic!("expected a failure, got {other:?}"), } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/eval/report_test.rs` around lines 162 - 195, Add two tests alongside the existing compare gate tests: one using a clean baseline case whose noise count increases, asserting Comparison::Fail with a reason containing “clean pull requests,” and another setting over_budget on an otherwise matching case, asserting failure with a reason containing “over budget.”src/eval/report.rs (1)
38-55: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename the CLI flag to match what it disables.
One flag suppresses both checks. Line 40 guards the corpus digest and line 47 guards the config digest, and both are skipped when
allow_driftis true. The CLI spells that flag--allow-config-drift, as shown insrc/bin/tinysweeper.rs:366-484. The message at lines 42-45 then tells a user to pass--allow-config-driftto bypass a corpus change.The corpus guard is the stronger of the two: a moved corpus means the labels changed. A user who reads the flag name would not expect it to disable that guard. Rename it now, while the CLI surface is new, or split it into two flags. A rename after release breaks runbooks.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/eval/report.rs` around lines 38 - 55, Rename the shared drift-control flag and its `allow_drift` parameter to reflect that it disables both corpus and configuration digest checks, updating the CLI definition and all references including `compare`. Keep both guards under the renamed option and update the user-facing corpus-change message to recommend the new flag name.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/eval.yml:
- Around line 56-67: Insert a separate `eval score` workflow step between the
recording command in “Record and score the corpus” and the `eval report`
command, using the same locked Cargo harness configuration and required
environment so it generates a fresh `evals/runs/latest/scorecard.json` before
reporting.
- Around line 49-51: Update the actions/checkout@v5 step to set
persist-credentials to false while preserving the existing recursive submodules
configuration.
In @.gitignore:
- Line 27: Remove the already tracked generated files under evals/runs/latest/
from version control while retaining the evals/runs/ ignore rule, so future eval
run outputs remain untracked and are not modified as repository content.
In `@evals/cases/ts-0045-kernel-bypass-hallucination.toml`:
- Around line 20-22: Set exhaustive = true in the case configuration before the
[provenance] table so the declared noise-measurement behavior is applied to
unmatched findings. Keep the existing forbidden entries and prose unchanged,
ensuring the key is parsed as a top-level case field rather than part of the
provenance table.
In
`@evals/cassettes/ts-0068-description-anchored-to-code/0008-e01abc2e7b65aa1c.json`:
- Around line 13-16: The recorded tinysweeper tests summary is truncated at the
output-token limit and must be regenerated as a complete response. Increase the
generation token limit or reject incomplete responses before recording, then
regenerate both
evals/cassettes/ts-0068-description-anchored-to-code/0008-e01abc2e7b65aa1c.json
lines 13-16 and
evals/runs/latest/ts-0068-description-anchored-to-code/proposal.json lines
47-65; apply the same correction to both artifacts.
In `@evals/README.md`:
- Around line 3-6: Update the README statement describing corpus loading to
distinguish execution modes: live evaluation uses tinysweeper eval, while cargo
test loads and replays committed cases offline through committed_test.rs. Remove
the inaccurate claim that cargo test never loads the corpus.
- Line 13: Update the fenced code block in the README opening fence to specify
the text language label, changing the unlabeled fence to use text and resolving
the markdownlint MD040 warning.
In `@src/bin/tinysweeper.rs`:
- Around line 424-432: Update the Score arm’s eval::Scorecard construction to
preserve the existing scorecard’s loose_replays count when one is already
present, using zero only when no prior scorecard exists. Keep the re-scoring
behavior and newly computed fields unchanged while carrying the run’s original
loose-mode metadata into eval::write_scorecard.
- Around line 458-473: Update the eval report argument validation so using
--gate without --baseline is rejected instead of silently skipping comparison.
Enforce the dependency in the command definition or equivalent command logic,
while preserving the existing comparison behavior when both options are
supplied.
- Around line 585-623: Update case_stub to serialize the title using the
existing toml encoder before interpolating it into the TOML template, replacing
the manual title.replace("\"", "'") handling. Preserve the encoded value as a
valid TOML basic string so quotes, backslashes, newlines, and other special
characters round-trip without changing the title.
In `@src/eval/corpus.rs`:
- Around line 226-229: Update the corpus validation logic alongside the existing
case.id guard to validate case.fixture before it is joined with the case file
directory. Reject fixture values containing path separators or "..", using the
same problem-reporting behavior as the id validation, so read_fixture cannot
access paths outside the corpus.
In `@src/eval/report.rs`:
- Around line 226-239: Thread the user’s config-drift flag through the markdown
report path: update markdown’s signature and its compare call to use the passed
flag instead of false, then update all four call sites in tinysweeper.rs and the
calls in report_test.rs to supply it, keeping rendered report results consistent
with the gate.
- Around line 212-221: Update the finding formatting in the judged-results loop
to render path and line when judged.line is present, but render only judged.path
when it is None; do not use 0 as a fallback line number. Preserve the existing
title and reason output and the behavior exercised by the unanchored finding
case.
In `@src/eval/runner.rs`:
- Around line 155-164: Align the documentation above prepare with the
implementation: either add the missing intended configuration override to
prepare alongside config.review.incremental = false, or revise the comment to
describe one override and its rationale. Ensure the final documentation
accurately states which load-bearing setting(s) the harness pins.
- Around line 105-113: Handle errors from open_cassette in the corpus loop
instead of propagating them with ?. When opening a cassette fails, score that
case through score::failed, matching the existing review_case failure handling
near line 122, then continue processing subsequent cases and preserve the normal
cassette flow for successful opens.
- Around line 136-153: Update review_case to apply the case-specific lanes from
case.case.lanes using the existing with_lanes helper before calling
review_with_state, and pass the resulting configuration into that call. Preserve
the existing repository, model, state, and review flow, and update corpus
cassettes if the resulting reviews change.
- Around line 265-289: Update digest_of to hash every score-affecting
configuration value with explicit separators between fields and entries,
preventing concatenation collisions; include config.review.lanes in the digest
even though it is currently only used for iteration. Add separators around the
review fields, model fields, and each path_instructions glob/instructions pair
while preserving the existing hashed field coverage and digest output format.
In `@src/eval/score_test.rs`:
- Around line 211-230: Update the far-case finding in
a_finding_three_lines_off_still_matches_and_four_does_not to use line 14 instead
of line 20, so the test explicitly verifies that four lines from expectation
(10, 10) does not match while retaining the existing assertions.
In `@src/eval/score.rs`:
- Around line 89-118: Update the expectation selection in the scoring logic
around `case.expected.iter().find` to first choose a matching expectation whose
ID is not present in `claimed`, then fall back to the first matching expectation
when all matches are already claimed. Preserve the existing true-positive and
duplicate handling, and add coverage for two overlapping expectations being
claimed by two findings.
In `@src/harness/cassette.rs`:
- Around line 303-322: The cassette key function key must hash the complete JSON
schema, not just schema_name. In src/harness/cassette.rs lines 303-322, add
request.schema to the hash with separators while retaining the existing
schema_name component; in src/harness/cassette_test.rs lines 26-52, extend
the_key_covers_everything_that_can_change_an_answer with a schema mutation
assertion so schema changes produce different keys.
---
Nitpick comments:
In `@src/eval/corpus_test.rs`:
- Around line 192-219: Add tests covering both missing validate branches:
duplicate case IDs and collisions between shared expected and forbidden IDs. In
the new case-ID test, ensure fixture filenames still match the case’s fixture
reference when both cases use the same ID, then assert the error mentions
“duplicate case id” and the ID. In the expectation/forbidden test, build entries
sharing one ID and assert the error mentions the duplicate-ID message.
In `@src/eval/corpus.rs`:
- Around line 129-133: Update the corpus evaluation loop to read each fixture’s
raw bytes once, incorporate those bytes into the digest alongside the case TOML,
and parse the same contents afterward. Change read_fixture and its callers to
return both the raw fixture text and parsed Fixture while preserving the
existing sorted path order and evaluation behavior.
In `@src/eval/report_test.rs`:
- Around line 162-195: Add two tests alongside the existing compare gate tests:
one using a clean baseline case whose noise count increases, asserting
Comparison::Fail with a reason containing “clean pull requests,” and another
setting over_budget on an otherwise matching case, asserting failure with a
reason containing “over budget.”
In `@src/eval/report.rs`:
- Around line 38-55: Rename the shared drift-control flag and its `allow_drift`
parameter to reflect that it disables both corpus and configuration digest
checks, updating the CLI definition and all references including `compare`. Keep
both guards under the renamed option and update the user-facing corpus-change
message to recommend the new flag name.
In `@src/eval/runner_test.rs`:
- Around line 151-175: Add tests in this file covering the rescore stage and its
three branches in rescore: a valid proposal, an unparseable proposal, and no
proposal. Reuse the existing run-directory helpers, and assert both successful
rescoring and the user-visible failure text for the latter two cases.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 52ba35ca-daf0-4e96-a2fd-b56e2e77f8aa
📒 Files selected for processing (50)
.github/workflows/eval.yml.gitignoreREADME.mddocs/modules/eval/README.mdevals/README.mdevals/baselines/current.jsonevals/cases/ts-0045-kernel-bypass-hallucination.tomlevals/cases/ts-0068-description-anchored-to-code.tomlevals/cassettes/ts-0045-kernel-bypass-hallucination/0001-4497e3eda1a56fbb.jsonevals/cassettes/ts-0045-kernel-bypass-hallucination/0002-4497e3eda1a56fbb.jsonevals/cassettes/ts-0045-kernel-bypass-hallucination/0003-6da662fd47b391d2.jsonevals/cassettes/ts-0045-kernel-bypass-hallucination/0004-90bbbd8c54959140.jsonevals/cassettes/ts-0045-kernel-bypass-hallucination/0005-ef46cd18cec1eacc.jsonevals/cassettes/ts-0045-kernel-bypass-hallucination/0006-5c59b67cccc1271c.jsonevals/cassettes/ts-0045-kernel-bypass-hallucination/0007-5fadecc54c179112.jsonevals/cassettes/ts-0045-kernel-bypass-hallucination/0008-38b8337bcd81f141.jsonevals/cassettes/ts-0045-kernel-bypass-hallucination/0009-bd905b514c3a937b.jsonevals/cassettes/ts-0045-kernel-bypass-hallucination/0010-0ccd7b85a38ee0c9.jsonevals/cassettes/ts-0045-kernel-bypass-hallucination/0011-3ae4722b8634b1f6.jsonevals/cassettes/ts-0045-kernel-bypass-hallucination/0012-c54bac2d556ce3a9.jsonevals/cassettes/ts-0068-description-anchored-to-code/0001-e48466f676c0cf56.jsonevals/cassettes/ts-0068-description-anchored-to-code/0002-71b25fd167fb909c.jsonevals/cassettes/ts-0068-description-anchored-to-code/0003-667311b57815c78f.jsonevals/cassettes/ts-0068-description-anchored-to-code/0004-9bf13617aa402c43.jsonevals/cassettes/ts-0068-description-anchored-to-code/0005-ac8fe7c78fe11446.jsonevals/cassettes/ts-0068-description-anchored-to-code/0006-0c796b973502c916.jsonevals/cassettes/ts-0068-description-anchored-to-code/0007-fe0eaf653fb2034f.jsonevals/cassettes/ts-0068-description-anchored-to-code/0008-e01abc2e7b65aa1c.jsonevals/cassettes/ts-0068-description-anchored-to-code/0009-767a0901d483f09d.jsonevals/fixtures/ts-0045-kernel-bypass-hallucination.jsonevals/fixtures/ts-0068-description-anchored-to-code.jsonevals/runs/latest/scorecard.jsonevals/runs/latest/ts-0045-kernel-bypass-hallucination/proposal.jsonevals/runs/latest/ts-0068-description-anchored-to-code/proposal.jsonsrc/bin/tinysweeper.rssrc/eval/committed_test.rssrc/eval/corpus.rssrc/eval/corpus_test.rssrc/eval/mod.rssrc/eval/report.rssrc/eval/report_test.rssrc/eval/runner.rssrc/eval/runner_test.rssrc/eval/score.rssrc/eval/score_test.rssrc/eval/types.rssrc/harness/cassette.rssrc/harness/cassette_test.rssrc/harness/mod.rssrc/lib.rs
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Adds a test covering an edge case in the scoring logic to ensure correct behavior when inputs are at boundary values. This guards against regressions in future changes. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Adds a test covering edge cases in the scoring logic, including empty inputs and boundary values, to ensure the function behaves correctly under unusual conditions. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The runner now reinitializes its internal state after a reset, ensuring that subsequent evaluations start from a clean and consistent baseline. Previously, stale state could persist and affect the results of later runs. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The runner now re-applies the initial configuration and environment after a reset, ensuring subsequent evaluations behave consistently instead of running with stale or cleared state. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The runner now correctly restores its state after evaluating an expression, ensuring that subsequent evaluations start from a clean slate. Previously, the runner could retain stale state between calls, which caused incorrect results when evaluating multiple expressions in sequence. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The report generation previously skipped evaluations with no results, leaving users without any output. This change ensures that an empty evaluation still produces a report, so the tool remains informative even when there is nothing to summarize. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The report generation previously skipped evaluations with no results, leaving users without any output. This change ensures that an empty evaluation still produces a report, so the tool remains informative even when nothing was executed. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The report generation previously skipped evaluations with no results, leaving users without any output. This change ensures that an empty evaluation still produces a report, so the tool remains informative even when there is nothing to summarize. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The markdown report function now accepts an `allow_config_drift` parameter, which is threaded through from the eval command's format handling. This ensures that when the `md` format is requested with config drift permitted, the rendered report reflects that setting rather than always defaulting to disallowing drift. The test suite has been updated to pass the new argument explicitly. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Adds a trailing newline to the source file to comply with POSIX standards and avoid potential compiler warnings. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The file previously lacked a trailing newline, which can cause issues with some tools and version control systems. This change adds the newline to ensure the file ends properly. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The file previously lacked a trailing newline, which can cause issues with some tools and version control systems. This change adds the newline to ensure the file ends properly. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The corpus loader previously skipped files that could not be read, but a recent change made it fail hard instead. This restores the original behavior of falling back to an empty corpus when a file is missing, so evaluation runs do not abort unexpectedly. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The score aggregation logic was inadvertently dropped during a previous refactor, causing scores to be calculated without combining all contributing components. This change restores the aggregation step so that the final score correctly reflects the sum of all individual scoring criteria. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Adds a test covering an edge case in the scoring logic to ensure correct behavior when inputs are at boundary values. This guards against regressions in the evaluation path. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Adds a test covering an edge case in the scoring logic to ensure correct behavior when inputs are at boundary values. This guards against regressions in the evaluation path. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The cassette harness previously stopped replaying recorded interactions once a write attempt failed, which prevented subsequent reads from being served. This change restores playback capability after a write error so the harness can continue using the cassette for later requests. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The cassette test file was no longer being used by the test suite, so it has been removed to keep the repository clean and avoid confusion. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add a new evaluation case that tests for hallucinated kernel bypass claims, ensuring the model does not fabricate details about bypassing kernel security mechanisms. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Adds a README for the evals directory explaining the purpose of the suite, how to execute the evaluations, and the process for adding new evaluation cases. This makes the evaluation workflow accessible to new contributors and clarifies the expected structure for future tests. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
This change introduces a README file to the evals directory, providing documentation and context for the evaluation scripts and resources contained within it. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Adds a GitHub Actions workflow to run evaluation checks on the repository, ensuring code quality and correctness are verified automatically on pushes and pull requests. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Adds a GitHub Actions workflow to run evaluation checks on the repository, ensuring code quality and correctness are verified automatically on pushes and pull requests. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Adds a test case covering the runner's behavior when given an empty input, ensuring it completes without error and produces no output. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Reformat code across the tinysweeper binary, evaluation report, and runner modules to conform to rustfmt's standard style. This includes wrapping long lines and adjusting indentation in function calls, string formatting, and test assertions. No behavioral changes are introduced. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The runner now reinitializes its internal state after a reset, ensuring subsequent evaluations start from a clean baseline instead of carrying over stale data. This fixes incorrect results when the runner is reused across multiple evaluation cycles. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Adds a test case covering the runner's behavior when given an empty input, ensuring it completes without error and produces no output. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Adds a test case covering the runner's behavior when given an empty input, ensuring it completes without error and produces no output. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Adds a test case covering the runner's behavior when given an empty input, ensuring it completes without error and produces no output. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Refresh the corpus and config digests in the current baseline to match the latest evaluation inputs, and add a trailing newline to the file. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Reformatted the failure reason handling in the runner and the path instruction setup in the test to use more compact, consistent formatting. No behavioral changes were made; this is purely a code style cleanup. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add a GitHub Actions workflow to run evaluation checks on the repository. This provides automated validation of changes through the eval pipeline. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The source file was missing a trailing newline, which can cause issues with some tooling and version control systems. Added the newline to ensure the file ends properly. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The source file was missing a trailing newline, which can cause issues with some tooling and version control systems. This change adds the newline to ensure the file ends properly. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The file previously lacked a trailing newline, which can cause issues with some tools and version control systems. This change adds the newline to ensure the file ends properly. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The cassette harness previously stopped replaying recorded interactions once a write attempt failed, which prevented subsequent requests from being served from the cassette. This change ensures playback continues even after a write error, so the harness remains usable for read-only scenarios. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The cassette harness previously stopped replaying recorded interactions once a write attempt failed, which prevented subsequent reads from being served. This change ensures that playback continues even after a write error, allowing the harness to remain usable for read-heavy scenarios. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The cassette harness previously removed the playback file when a write attempt failed, which prevented subsequent reads from succeeding. This change keeps the cassette in place after a failed write so that playback can still occur, preserving the original recorded data for later use. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The cassette harness previously stopped replaying recorded interactions once a write attempt failed, which prevented subsequent reads from being served. This change restores playback capability after a write error so the harness can continue serving recorded responses. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The cassette harness previously stopped replaying recorded interactions once a write attempt failed, which prevented subsequent reads from being served. This change restores playback capability after a write error so the harness can continue serving recorded responses. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The runner previously skipped restoring its saved state when exiting early due to an error, leaving the evaluator in an inconsistent condition. It now always restores the state before returning, ensuring subsequent evaluations start from a clean baseline. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The cassette test file was no longer being used by the test suite, so it has been removed to keep the repository clean and avoid confusion. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The source file was missing a trailing newline, which can cause issues with some tooling and version control systems. This change adds the newline to ensure the file ends properly. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The source file was missing a trailing newline, which can cause issues with some tooling and version control systems. Added the newline to ensure the file ends properly. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
There was a problem hiding this comment.
Requesting changes: 1 lane(s) blocking, worst finding is high.
Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.
$0.0190 · 400,863 in / 33,674 out · 289,776 cached (72%) · openrouter/openai/text-embedding-3-small, z-ai/glm-5.2 · 820 embedded
critique: $0.0080 · 76,272 in / 14,521 out · 8,921 cached (12%) · z-ai/glm-5.2
security: $0.0039 · 80,532 in / 10,483 out · 70,965 cached (88%) · z-ai/glm-5.2
tests: $0.0037 · 125,754 in / 4,604 out · 108,148 cached (86%) · z-ai/glm-5.2
description: $0.0034 · 118,305 in / 4,066 out · 101,742 cached (86%) · z-ai/glm-5.2
The runner previously dropped its internal state when handling certain evaluation paths, causing subsequent calls to behave incorrectly. This change restores that state so the runner continues to operate consistently across evaluations. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Adds a test case covering the runner's behavior when given an empty input, ensuring it handles the edge case gracefully without errors. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Refresh the baseline evaluation results to reflect the latest model performance, capturing updated scores and metrics from the most recent run. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/eval/runner.rs (1)
170-171: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPersist direct review failures before scoring them.
Line 171 creates only an in-memory failure. It does not call
write_failure.If a previous run created
proposal.json, then a later directreview_casefailure leaves that proposal in place.rescorethen reads the stale proposal and reports a normal score.Write a
failure.jsonmarker in this branch before callingscore::failed. Add a test that records a proposal, forces a direct review error, and verifies thatrescoreremains failed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/eval/runner.rs` around lines 170 - 171, Update the Err branch in the review-case flow to persist a failure marker via write_failure before invoking score::failed, ensuring stale proposal.json data cannot be used by rescore. Add a test covering an existing proposal followed by a direct review error and verify that rescore remains failed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/eval/runner.rs`:
- Around line 138-143: Update the failed-case scoring path around score::failed
so failed CaseScore values preserve the cassette’s served cost instead of always
reporting 0.0; pass cassette usage or at minimum cassette.cost_usd() into
score::failed while keeping the existing corpus budget accumulation and failure
behavior unchanged.
---
Outside diff comments:
In `@src/eval/runner.rs`:
- Around line 170-171: Update the Err branch in the review-case flow to persist
a failure marker via write_failure before invoking score::failed, ensuring stale
proposal.json data cannot be used by rescore. Add a test covering an existing
proposal followed by a direct review error and verify that rescore remains
failed.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0b6547f0-91a9-4497-b89c-572af2f4577d
📒 Files selected for processing (9)
.github/workflows/eval.ymlevals/baselines/current.jsonsrc/bin/tinysweeper.rssrc/eval/corpus.rssrc/eval/corpus_test.rssrc/eval/runner.rssrc/eval/runner_test.rssrc/harness/cassette.rssrc/harness/cassette_test.rs
🚧 Files skipped from review as they are similar to previous changes (6)
- evals/baselines/current.json
- .github/workflows/eval.yml
- src/eval/corpus_test.rs
- src/eval/runner_test.rs
- src/bin/tinysweeper.rs
- src/eval/corpus.rs
| // Spend is what actually left the account, not what a successful | ||
| // proposal reported: a case that failed after its calls has still spent | ||
| // them, and charges must not walk the corpus ceiling on the strength of | ||
| // never having produced a proposal to sum. Recorded usage replayed | ||
| // verbatim, so the ceil works offline too. | ||
| spent += cassette.cost_usd(); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Preserve served cost in failed case scores.
Line 143 correctly adds cassette.cost_usd() to the corpus budget. Line 171 then calls score::failed, which sets CaseScore.cost_usd to 0.0.
A case that receives model answers and later fails therefore reports zero cost in RunOutcome.scores and offline reports. Extend the failed-score path to carry cassette usage or at least cassette.cost_usd().
Also applies to: 170-171
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/eval/runner.rs` around lines 138 - 143, Update the failed-case scoring
path around score::failed so failed CaseScore values preserve the cassette’s
served cost instead of always reporting 0.0; pass cassette usage or at minimum
cassette.cost_usd() into score::failed while keeping the existing corpus budget
accumulation and failure behavior unchanged.
The anchor fallback suppressed on lane, file and line proximity alone, so two defects a few lines apart in one function were one repeat and one silent deletion — and a deleted finding can flip a verdict, which is the failure this branch exists to stop. It was the same mistake corrected in `council::agree` here (merge on evidence, never on its absence) left standing in the dedupe itself. `PostedAnchor` now carries the title and `covers_anchor` requires it to match. The title rather than the `rule`, on the evidence: the four repeats of one concern on #86 carried the rules `untrusted-repo-rules`, `Pin third-party actions to a commit SHA`, and twice nothing at all, while the title was identical every time. Keying on the rule would leave the fallback catching nothing in the case it exists for. A comment whose title cannot be read anchors nothing rather than everything. Adds the review-flow coverage the helper's unit tests did not give: `a_finding_whose_anchor_moved_is_not_posted_twice` drives two real reviews over one defect, the second quoting a different snippet so it hashes to a fresh fingerprint, and asserts one posted comment. It also asserts the two identities differ, so it cannot pass by the fingerprint having matched. Co-authored-by: Medulla <medulla@tinyhumans.ai>
Nothing in tinysweeper could say whether a change to a prompt, a rule document, a threshold or a lane made the review better. The test suite proves the machinery behaves — it cannot prove the reviewer is any good, and the two questions are unrelated. Every document in
presets/rules/was written from judgement and validated by reading output on live pull requests, which measures the reviewer against the memory of whoever last looked at it.This is the prerequisite for the agent-council work: without it, "the council improved recall" is an opinion.
Shape
Same propose/dispose split as
review/apply, for the same reason: the expensive, irreversible half happens once and everything downstream is a pure function of what it wrote. A matching rule gets rewritten ten times before it is right.src/harness/cassette.rsis a record/replay decorator over theModelport (one method, so wrapping it is free). Its key covers the model id, schema name, token ceiling and every message — so any prompt change invalidates every cassette that prompt produced, and strict replay says so and stops. A run that silently fell back to a stale recording would report the old prompt's quality under the new prompt's name. Usage and cost replay verbatim rather than throughpricing.rs, so an offline re-score reproduces the dollars the live run actually paid.Decisions worth reviewing
exhaustiveis off by default, and this changed mid-build. A case contributes to precision only if it claims its labels are complete; otherwise an unmatched finding isUnscored— neither credit nor defect. The first live run of the corpus reported a genuine off-by-one inshorten(returnslimit + 1chars when the firstlimitchars hold no space) and scored it a false positive, because nobody had labelled it. You can only call an unmatched finding wrong if you have asserted every right one. Consequence: a largeunscoredcount means the corpus is thin, not that the reviewer is noisy.Scoring is two-stage: structural (path, line ±3, lane, severity) then a keyword check against the finding's own title, body and rule. The second stage looks like overreach and is not — a lane will happily leave a naming nit on the exact line that holds the real bug, and scoring on overlap alone counts that as a find, so the harness would reward commenting on hot lines. Every match decision is written into the report with its reason, so a wrong one is arguable rather than invisible.
incrementalis forced off and every case gets a freshMemoryState. Suppression, cross-push dedupe and prior-review loading make output depend on what the last run saw, so a warm corpus measures run order and reports it as review quality — silently, because a suppressed finding looks exactly like one that was never made.No composite score. A number folding recall, precision and cost together can be improved by trading the one that matters for the two that do not. The gate is a conjunction, each term on its own line, with a 2% tolerance on recall — a gate with no tolerance fails on provider routing noise and teaches people to re-run CI rather than read it. Two runs are only comparable when the corpus and config digests match; a stricter gate finds fewer things without the reviewer having got worse.
A failed review scores zero, not nothing. Dropping it from the denominator would let a run improve its own score by breaking.
The corpus
Two cases, both regressions, both evidenced outside this bot — which the loader enforces, because an expectation written from tinysweeper's own output measures whether tinysweeper still agrees with itself and cannot expose a blind spot:
ts-0045-kernel-bypass-hallucinationts-0068-description-anchored-to-codedescriptionfinding must not anchor to implementation codeBoth hold on the current tree. Building them surfaced a schema gap — PR #68's defect is structural (which lane landed where), not textual — so
[[forbidden]]gained alanesscope and the "must match something" rule became "must narrow something".evals/README.mdstates plainly what is not measured yet: recall (both cases are forbidden-only, so it rendersn/a), clean pull requests, and any language but Rust. I deliberately did not label recently-merged PRs as "clean" — the only reviewers on this repo are tinysweeper itself and a CodeRabbit that was rate-limited on all three candidates, so "nobody objected" is close to circular. A corpus that overstates its coverage is worse than a small one that does not.Verification
cargo testincludesthe_committed_corpus_replays_and_holds_its_regressions, which runs the real engine over the real corpus against the committed cassettes with no model at all — free, offline, and it is what catches "somebody changed a prompt and never re-recorded".Live recording run: 2 cases, $0.0063, 0 forbidden hits, 1 unscored.
.github/workflows/eval.ymlruns the live half on dispatch, weekly, and on PRs touching anything that can move a score. Fork-guarded soOPENROUTER_API_KEYnever reaches a fork's branch. Deliberately not a required check until run-to-run variance is measured.Follow-up, not in scope here
The
shortenoff-by-one the corpus surfaced is insrc/findings/summarise.rsas it existed at PR #45; the file no longer exists onmain, so there is nothing live to fix.Summary by CodeRabbit
New Features
Documentation
Tests