Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,14 @@ Please choose versions by [Semantic Versioning](http://semver.org/).
* MINOR version when you add functionality in a backwards-compatible manner, and
* PATCH version when you make backwards-compatible bug fixes.

## Unreleased

- fix: golden-set identity is the signature alone. `rule_id` is now an additional constraint when both sides carry one, never a short-circuit, and neither `path` nor `line` is read. Both defects were demonstrated on real rows before the change: the `github-pr-review-agent#11` CHANGELOG entry (a bullet bundling three concerns) matched an unrelated `fix:`-vs-`chore:` prefix finding purely because both carried `changelog/conventional-prefix-required`, so any configuration finding *some* instance of a rule was credited with the specific one; and the "write-scoped token still minted under `--skip-post`" issue, keyed to `pkg/factory/runner.go`, scored a miss when a later run anchored it at `cmd/run-task/main.go` while naming `runner.go` in its own body
- fix: all 42 golden entries re-keyed from `path:line` strings to semantic keywords drawn from code identifiers (`S104`, `apt-key`, `ErrorRequestHandler`) rather than prose, which is one author's phrasing and drifts between runs. 37 of the 43 signatures embedded a line reference, so `recall` measured whether a configuration cited the same *line* rather than whether it found the issue. Measured, not estimated: the curated-1 Opus pass — the identical model/effort/mode that seeded the set — scored recall **0.262** against it; after the re-key, **0.463**, while the run the set was seeded from still self-matches **1.000**
- fix: two `docs/pr-post-back.md` entries merged into one. Both described the stale "backward compatibility" framing in a single paragraph, and a later run reported them as one finding — which would have matched two entries and double-counted recall
- feat: `load_golden` rejects a signature that embeds a line reference or carries fewer than two keywords. The prior convention was documented (`line is NEVER used for identity`) but unenforced, and the seeded signatures smuggled the line back in as a substring; one entry had already been hand-fixed line-free in an earlier adjudication without the convention propagating to the other 42
- fix: the scoring-mutates-nothing test compared `git diff --exit-code` against the working tree, so it failed for any developer holding a legitimate uncommitted fixture edit and would have passed a mutation that was staged. It now compares testdata digests before and after the scoring subprocess, the same way the ledger is already checked

## v0.40.0

- fix: findings citing an extensionless file are no longer dropped. `PATH_LINE_RE` required a literal dot, so `Dockerfile:8`, `Makefile:127`, `Jenkinsfile:4` and `LICENSE:1` could not be attributed and every such item was discarded. Measured live on the 2026-08-09 curated-1 pass: `backup#15` produced four correctly-formed findings, all on Dockerfile/Makefile, and lost **all four** — the row scored 0 findings and read as a **clean PR**, the worst available outcome given that "clean" is a designation the benchmark cannot otherwise establish. Across the pass, 5 of 133 findings (4%) were being dropped, concentrated so that one row lost 100% of its content
Expand Down
8 changes: 5 additions & 3 deletions bench/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -158,15 +158,17 @@ The four ledger slices under `bench/testdata/` are verbatim extracts of real led

## Scoring

**Matching and the three golden states.** The match rule is exact and deterministic. For a `(golden entry, finding)` pair within the same `pr_id`: if both sides carry a non-null `rule_id`, they match when those ids are equal and do not match otherwise — a rule-id disagreement is decisive and no fallback is attempted. Otherwise they match when the `path` field is string-equal and **every** keyword in the entry's `signature` appears as a case-insensitive substring of the finding's `body`. Otherwise they do not match. `line` is never used for identity — it is display data only. Path comparison is whole-string equality with no extension requirement, so an extensionless path such as `Dockerfile` compares like any other.
**Matching and the three golden states.** The match rule is exact and deterministic. For a `(golden entry, finding)` pair within the same `pr_id`: they match when **every** keyword in the entry's `signature` appears as a case-insensitive substring of the finding's `body`. When both sides carry a non-null `rule_id`, the ids must also be equal — `rule_id` is an additional constraint, never a short-circuit. An entry with an empty `signature` never matches.

Neither `path` nor `line` is part of identity. Both are coordinates the reviewer chooses when citing an issue, not properties of the issue: the same defect is routinely anchored at the line that defines a flag or at the line that suffers its consequence, in two different files. Both remain on the entry and the finding as provenance and display data. Signatures must carry at least two keywords and may not embed a line reference; `load_golden` rejects a set that violates either.

Every golden entry carries one of three states, and the state governs what a hit or miss costs. An `accepted` entry — one the reviewer should produce — costs recall when unmatched and costs nothing when matched. A `rejected` entry — a known false positive — costs precision when matched and costs nothing when unmatched. An `unreviewed` entry — not yet adjudicated — is excluded from both numerator and denominator of both ratios, and a finding matching one is neither a penalty nor a candidate.

**Gap-triage candidates.** A finding matching no golden entry at all is a **gap-triage candidate**. It is quoted verbatim on the report page under its own heading, with its `pr_id`, `path`, `line` and `body`, and it is counted against nothing. It is **not a precision failure**. The golden set is bootstrapped from a single strong-model run, not hand-curated ground truth, and the reason this classification matters is load-bearing: when several independent configurations agree on a finding the baseline missed, that is evidence the golden set is incomplete and the entry is a candidate for promotion to `accepted`. When nothing ever reproduces a baseline finding, it is a candidate for demotion to `rejected`. The recorded case is `tts-mcp#20`, which was annotated "clean — correct answer is zero findings" on the strength of six zero-finding runs from a weaker model; the strong model then found a real, hand-verified defect in it.

**What precision currently measures.** `golden-dev-1` carries zero `rejected` entries, so precision cannot be lost by any configuration. A precision of `1.000` is a property of the golden set's adjudication state and is **not yet a result**.

**What recall currently measures.** 36 of the 42 signatures embed a line reference, so a re-report of the same issue at a different line does not match and surfaces as a gap-triage candidate rather than a hit. On this golden set, `recall` measures whether a configuration cited the same line, not whether it found the issue. Across the four runs of config `9ce66e05…` only 5 of 16 findings hit an entry, and the `apt-key` issue in `.github/workflows/ci.yml` is reported in all four runs at lines 13, 29, 9 and 27 and is a gap candidate every time.
**What recall measures.** Issue detection. This was not always true: until 2026-08-10, 37 of the 43 signatures embedded a line reference, so `recall` measured whether a configuration cited the same *line*. The cost was measured, not estimated — the `curated-1` Opus run, at the identical model/effort/mode that seeded the golden set, scored **0.262** against it; after re-keying to semantic signatures, **0.463**, while the run the set was seeded from still self-matches **1.000**. The report page recomputes this caveat from the entries on every render, so a future adjudication that reintroduces a `path:line` key says so on the page instead of silently degrading recall.

**Runs are an occurrence index, never a timestamp cluster.** Within one `config_hash`, the k-th ledger row for a given `pr_id`, in ledger file order, belongs to run k. Run boundaries need no clock, no threshold, and no tuning, and they survive clock skew, a slow PR, and an operator pausing between PRs. The boundary between run 2 and run 3 of config `9ce66e05…` is 48 seconds while gaps *within* runs 1, 2 and 4 reach 140, 128 and 110 seconds — no time threshold separates them in either direction. A run that does not cover every PR in the manifest is labelled **partial** on the report page, and a partial run is scored over the PRs it actually covers: the missing PRs' golden entries are **out of scope for that run, not misses**, so a 3-PR run scores against 24 entries and a 1-PR run against 1.

Expand Down Expand Up @@ -208,7 +210,7 @@ These are deliberately not configurable:
- **Inline rule tag:** `*(rule: \`<id>\`)*` is read positionally from the item and recorded verbatim; it is **not** validated against `rules/index.json`; the head-anchored backtick fallback is **index-gated** (only used when no inline tag is present and the token is a known rule id)
- **Unattributable-item rejection:** `UNATTRIBUTABLE FINDING` fires when an item inside a severity section yields neither `path` nor `rule_id`; no ledger row, no row marker, no opt-out
- **Stderr excerpt bound:** at most 2,000 bytes of rejected output are printed to stderr (truncation is marked)
- **Match rule:** `rule_id` exact when both sides carry one; otherwise `path` string equality plus every signature keyword present case-insensitively in body; `line` is never used for identity; path comparison is whole-string with no extension requirement
- **Match rule:** every signature keyword present case-insensitively in body; `rule_id` an additional constraint when both sides carry one, never a short-circuit; neither `path` nor `line` is part of identity; signatures need >= 2 keywords and may not embed a line reference
- **Golden states:** `accepted` (recall miss on no-match), `rejected` (precision penalty on match), `unreviewed` (excluded from both ratios); a finding matching no entry is a gap-triage candidate, **not a precision failure**
- **Run chunking:** per-PR occurrence index in ledger file order; the k-th row for a given `pr_id` belongs to run k; no clock, no threshold
- **Report location:** `bench/reports/<64-lowercase-hex>.md`; full hash, no truncation; tracked in git
Expand Down
Loading
Loading