feat(reviewer-eval): row-level corpus hashing, split flip stats, FAIL-diff archive, miner outcome labels - #295
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (16)
🚧 Files skipped from review as they are similar to previous changes (14)
📝 WalkthroughWalkthroughChangesBenchmark telemetry and evaluation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ReviewGate
participant DiffArchive
participant BenchmarkMiner
participant GitHubAPI
participant BenchmarkCorpus
ReviewGate->>DiffArchive: archive failed diff bytes
BenchmarkMiner->>GitHubAPI: fetch threads and commits
BenchmarkMiner->>BenchmarkCorpus: write enriched candidates
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
gate-engine/review/eval/reviewers/bench.mts (1)
278-298: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAlign the escalation condition between
overturnRateandrescueRate.
overturnRate.krequiresr.escalateLive.rescueRate.kdoes not. A decoy row that first fails and then passes without a live escalation still counts as a rescue. The two cascade metrics then measure different things.If both metrics describe opus behavior, apply
escalateLiveto both numerators, or drop it from both and restrict the denominators to rows that escalated.🔧 Proposed fix
rescueRate: { - k: count(decoyFirstFail, (r) => r.okFinal), + k: count(decoyFirstFail, (r) => r.okFinal && r.escalateLive), n: decoyFirstFail.length, },🤖 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 `@gate-engine/review/eval/reviewers/bench.mts` around lines 278 - 298, Align the cascade eligibility used by overturnRate and rescueRate in the metrics-building return: update the rescueRate numerator over decoyFirstFail to use the same r.escalateLive condition as overturnRate, preserving the existing denominator unless the surrounding metric contract requires otherwise.
🧹 Nitpick comments (8)
gate-engine/review/evidence/diff-archive.mts (1)
49-67: 🔒 Security & Privacy | 🔵 TrivialConsider a retention policy for the
diffs/store.By design, this store never prunes: the docstring states it "needs no run-id gate and no per-run pruning." Each unique FAIL diff is retained forever under the telemetry directory, and
ARCHIVE_MAX_BYTEScaps only the size of a single file, not the total size of the store.Raw diff text can contain secrets or other sensitive data from the reviewed change. Without any TTL or size-bound sweep, this data accumulates on local disk indefinitely across every FAIL a developer's machine ever produces.
Consider adding a periodic cleanup (age-based or total-size-based) for the
diffs/directory, or documenting the retention expectation for downstream consumers of this archive.🤖 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 `@gate-engine/review/evidence/diff-archive.mts` around lines 49 - 67, Update the diff archive flow around archiveFailedDiff to enforce a retention policy for the diffs store, using periodic cleanup by entry age or total directory size before or after archiving. Preserve hash-based deduplication and best-effort failure handling, and ensure cleanup errors never cause a gate to fail.gate-engine/review/eval/reviewers/mine-bots-lib.mts (2)
61-69: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider accepting a bot-author set in
hasWithdrawal.
hasWithdrawalmatches onlycoderabbitai[bot]. The caller sweeps bothcoderabbitai[bot]andmacroscopeapp[bot]. A withdrawal reply frommacroscopeapp[bot]therefore counts as neither a withdrawal nor a human reply, and the comment staysunresolved.hasHumanReplyalready takes an injectablebotAuthorsset, so the two helpers are asymmetric.♻️ Optional: parameterize the withdrawal author set
-export function hasWithdrawal(replies) { - return (replies ?? []).some( - (r) => r?.author === CODERABBIT_LOGIN && WITHDRAWAL_RE.test(String(r?.body ?? '')), - ); -} +export function hasWithdrawal(replies, botAuthors) { + const bots = botAuthors ?? new Set([CODERABBIT_LOGIN]); + return (replies ?? []).some( + (r) => bots.has(r?.author) && WITHDRAWAL_RE.test(String(r?.body ?? '')), + ); +}🤖 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 `@gate-engine/review/eval/reviewers/mine-bots-lib.mts` around lines 61 - 69, Update hasWithdrawal to accept the same injectable botAuthors set used by hasHumanReply, defaulting to the existing bot author when omitted, and test membership against that set so withdrawals from both swept bot accounts are recognized.
105-114: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
isLineTouchedLaterchecks file-level touches, not line-level touches.The function only tests
c.files.includes(commentPath). Any later commit that touches the same file sets the signal.classifyOutcomethen reportsfixedwith evidenceresolved+line-touchedfor a resolved thread, even if the commented lines never changed. This inflates thefixedlabel in corpus rows.The behavior is a deliberate approximation. Rename the signal or state the granularity in the docstring so corpus authors do not read
lineTouchedLateras line-level evidence.♻️ Optional: clarify the granularity in the docstring
-// commits: [{sha, committedDate: ISOString, files: string[]}] — files already fetched by caller. +// commits: [{sha, committedDate: ISOString, files: string[]}] — files already fetched by caller. +// Granularity is FILE-level, not line-level: a later commit that touches the commented file sets +// the signal even when the commented lines are untouched. Treat it as a weak "fixed" hint. export function isLineTouchedLater(commits, commentPath, commentCreatedAt) {🤖 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 `@gate-engine/review/eval/reviewers/mine-bots-lib.mts` around lines 105 - 114, Clarify the documentation for isLineTouchedLater to state that it detects later file-level touches, not changes to the specific commented lines, and that this is a deliberate approximation. Keep the existing implementation and classifyOutcome behavior unchanged.gate-engine/review/__tests__/mine-bots.test.mts (2)
184-193: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a case for a resolved thread with no supporting evidence.
The suite does not cover
threadResolved: truewithlineTouchedLater: falseandhasHumanReply: false. That input falls past both resolved branches and returnsunresolvedwithoutcomeEvidence: null. The result contradicts the intuitive reading of "resolved", so pin it with a test to prevent an accidental change to the branch order.💚 Proposed additional test
it('unresolved, non-outdated thread with no other signal is also unresolved/null', () => { expect(classifyOutcome({ ...base, threadResolved: false, threadOutdated: false })).toEqual({ outcome: 'unresolved', outcomeEvidence: null, }); }); + + it('resolved with no line touch and no human reply stays unresolved/null', () => { + expect(classifyOutcome({ ...base, threadResolved: true })).toEqual({ + outcome: 'unresolved', + outcomeEvidence: null, + }); + }); + + it('resolved + outdated with no other signal reports outdated-only', () => { + expect(classifyOutcome({ ...base, threadResolved: true, threadOutdated: true })).toEqual({ + outcome: 'unresolved', + outcomeEvidence: 'outdated-only', + }); + });🤖 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 `@gate-engine/review/__tests__/mine-bots.test.mts` around lines 184 - 193, Add a test alongside the existing fallback cases in the classifyOutcome suite for threadResolved: true, lineTouchedLater: false, and hasHumanReply: false, asserting the result remains { outcome: 'unresolved', outcomeEvidence: null } and preserves the current branch-order behavior.
85-92: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd assertions for the untested withdrawal alternatives.
WITHDRAWAL_REhas five alternatives. The suite exercisesyou're right,you are right,agreed,andagreed—. It does not exercisewithdrawordoes not apply. The test title at Line 85 also names theagreed-hyphen form but does not assert it. The regex decides therebuttedlabel, so full alternative coverage is worth the three extra lines.💚 Proposed additional assertions
it('matches the "agreed—" / "agreed," / "agreed-" withdrawal forms', () => { expect( hasWithdrawal([{ author: 'coderabbitai[bot]', body: 'Agreed, closing this out.' }]), ).toBe(true); expect(hasWithdrawal([{ author: 'coderabbitai[bot]', body: 'Agreed—no action needed.' }])).toBe( true, ); + expect(hasWithdrawal([{ author: 'coderabbitai[bot]', body: 'Agreed- reverting.' }])).toBe(true); + }); + + it('matches the "withdraw" and "does not apply" forms', () => { + expect( + hasWithdrawal([{ author: 'coderabbitai[bot]', body: 'I withdraw this comment.' }]), + ).toBe(true); + expect( + hasWithdrawal([{ author: 'coderabbitai[bot]', body: 'This does not apply here.' }]), + ).toBe(true); });🤖 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 `@gate-engine/review/__tests__/mine-bots.test.mts` around lines 85 - 92, Extend the withdrawal-form test around hasWithdrawal to assert the missing WITHDRAWAL_RE alternatives: the “agreed-” hyphen form, “withdraw,” and “does not apply,” using matching bot-authored bodies and expecting true. Keep the existing assertions and test title intact.gate-engine/review/eval/reviewers/mine-bots.mts (1)
60-78: 🚀 Performance & Scalability | 🔵 TrivialConsider an incremental filter to limit GitHub API volume.
listPrsreturns every PR withstate=all. The sweep then callsbotCommentsfor each PR, andfetchReviewThreadsplusfetchPrCommitsfor each PR that has bot comments.commitFilesadds one more request per later commit. On a repository with many PRs this consumes a large share of the REST and GraphQL rate limits, and a secondary rate limit can abort the run mid-sweep.The script already merges into existing
candidates.jsonlrows. Add an optional--sinceor--updated-afterfilter, or skip PRs whoseurlset is already fully present in the merged map, so repeat sweeps only fetch new PRs.🤖 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 `@gate-engine/review/eval/reviewers/mine-bots.mts` around lines 60 - 78, Reduce repeat GitHub API requests in the sweep built around listPrs, botComments, and the merged candidates map by adding an optional since/updated-after filter or skipping PRs whose URLs are already fully represented in the merged map. Preserve processing for new or incomplete PR entries while ensuring repeat sweeps do not fetch comments and related data for already-complete candidates.gate-engine/review/eval/reviewers/README.md (1)
122-124: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winComplete the
gateHashinput list.
benchGateHashingate-engine/review/eval/reviewers/corpus.mtsalso hashesruntime.mtsand every file inSHARED_HELPERS. The list here omits both, so a reader cannot predict which edits invalidate a baseline. Add them.🤖 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 `@gate-engine/review/eval/reviewers/README.md` around lines 122 - 124, Update the gateHash input-list documentation in the reviewer README to include runtime.mts and every file under SHARED_HELPERS, matching the inputs hashed by benchGateHash in corpus.mts so baseline invalidation is fully documented.gate-engine/review/__tests__/reviewer-eval.test.mts (1)
379-385: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFix the truncated test name and give this case a distinct assertion.
The name ends mid-sentence: "…(whole-corpus corpusHash drift) does not". The assertion also repeats the case at lines 359-361, so it adds no coverage. Assert the intended property instead: compute two
rowHashvalues for the same row before and after a sibling row is appended, then pair them.🔧 Proposed fix
- it('a sibling row appended elsewhere in the corpus (whole-corpus corpusHash drift) does not', () => { + it('a sibling row appended elsewhere in the corpus does not break pairing', () => { // Regression guard for the blocker: the stability rerun used to hard-gate on // `section.corpusHash === meta.corpusHash`, a row-SET hash that changes whenever ANY row is // added/removed/edited anywhere in the corpus — defeating row-level pairing entirely once the // corpus grew even by one row. rowUnchanged only ever looks at the two rows being paired. - expect(rowUnchanged({ rowHash: 'h1' }, 'h1')).toBe(true); + const row = goldRow(); + const baseRow = { rowHash: rowHash(row) }; + corpusHashFromRows([row, decoyRow()]); // corpus-set hash drifts; row pairing must not + expect(rowUnchanged(baseRow, rowHash(row))).toBe(true); });🤖 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 `@gate-engine/review/__tests__/reviewer-eval.test.mts` around lines 379 - 385, Update the test case name to clearly describe that whole-corpus corpusHash drift does not block pairing, and replace the duplicate rowUnchanged assertion with coverage of the intended behavior: compute the same row’s rowHash before and after appending a sibling row, then verify those values can be paired successfully.
🤖 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 `@docs/benchmarks/benchmark-methodology.md`:
- Around line 103-106: Update the archive description in
docs/benchmarks/benchmark-methodology.md (lines 103-106) to state that the
fail-open 8 MiB cap and archive errors may leave reviewer FAILs with only
diff_sha256, making them telemetry-only and ineligible for replay-based row
minting. Regenerate docs/decisions/benchmarks-grow-from-telemetry.md (line 11)
with the same exception, then regenerate docs/decisions/INDEX.md (line 11) from
the corrected decision.
- Around line 42-44: Update the benchmark methodology’s “Blind-relabel” step to
define the fallback when a suite has fewer than 40 rows: audit all available
rows rather than selecting a 40–60 item subset, and report the resulting
low-power κ/α estimate alongside the results table.
In `@docs/decisions/benchmarks-grow-from-telemetry.md`:
- Line 17: Update the decision’s Scope entry to include
gate-engine/review/evidence/diff-archive.mts alongside the existing paths,
ensuring the FAIL diff-byte archive helper is covered by the path-based scope.
In `@gate-engine/review/__tests__/diff-archive.test.mts`:
- Around line 14-25: Remove the temporary directory created by mkdtempSync in
the archiveFailedDiff describe block during afterEach, using the existing dir
variable and recursive forced cleanup while preserving the environment-variable
restoration.
In `@gate-engine/review/eval/reviewers/bench.mts`:
- Around line 367-397: Track rows skipped by the row.stable === false check in
the comparison flow, pass the resulting unstable count into buildCompareReport
alongside changed and the other counts, and update buildCompareReport and its
report line in stats.mts to include unstable exclusions so reported
shared/paired totals match the flip table.
- Around line 672-678: Update the results loop in the failMode/abMode section to
default section.rows to an empty object before indexing by res.id. Match
compareReviewer’s existing section.rows ?? {} defensive behavior so sections
without rows are skipped without throwing.
In `@gate-engine/review/eval/reviewers/mine-bots.mts`:
- Around line 106-129: Guard the pagination loop around the cursor update in the
review-thread fetch flow: before continuing after hasNextPage, validate that
pageInfo.endCursor is non-null and differs from the current cursor; break when
it is missing or unchanged, otherwise assign it to cursor and continue fetching.
- Around line 93-95: Update the inner comments connection in fetchReviewThreads
to paginate beyond the first 50 entries, or explicitly detect and report when
comments are truncated. Ensure threadInfoFor receives all comments needed for
hasAddressedMarker, hasWithdrawal, hasHumanReply, byDatabaseId, threadResolved,
and threadOutdated to produce correct results.
- Around line 259-281: Update collectCorpusUrls to wrap each matching corpus
file’s readFileSync call in error handling, skipping unreadable files and
directory entries while continuing to process the remaining entries. Preserve
the existing malformed-line handling and URL collection behavior for readable
files.
- Around line 368-373: Update the scopeForPr call in the scope confirmation
block to pass the full repository slug for cache-key construction while
preserving repoShort for the database query and SQL matching. Ensure
repositories with identical short names but different owners use distinct
scopeCache entries, without changing scopeConfirmed or scopedReviewers behavior.
- Around line 420-431: Update the merge loop around readExistingCandidates so
only newRows entries with a truthy row.url are inserted, preventing missing URLs
from sharing one merge key. Write the serialized rows to a temporary candidates
file, then use renameSync to atomically replace OUT; add renameSync to the
existing node:fs import and preserve the current output contents.
- Around line 172-194: Update commitFiles to paginate the GitHub commit-files
API until all pages are retrieved, aggregating every filename before caching and
returning the result. Preserve the existing cache behavior and error fallback,
and ensure commitsAfter receives the complete file list so isLineTouchedLater
can detect paths on later pages.
In `@README.md`:
- Line 130: Update the dashboard image alt text in README.md to match the labels
and summary represented by dashboard-light.svg and dashboard-dark.svg, removing
the unsupported current-count claim. Use the exact generated summary wording if
the dashboard snapshot remains included.
---
Outside diff comments:
In `@gate-engine/review/eval/reviewers/bench.mts`:
- Around line 278-298: Align the cascade eligibility used by overturnRate and
rescueRate in the metrics-building return: update the rescueRate numerator over
decoyFirstFail to use the same r.escalateLive condition as overturnRate,
preserving the existing denominator unless the surrounding metric contract
requires otherwise.
---
Nitpick comments:
In `@gate-engine/review/__tests__/mine-bots.test.mts`:
- Around line 184-193: Add a test alongside the existing fallback cases in the
classifyOutcome suite for threadResolved: true, lineTouchedLater: false, and
hasHumanReply: false, asserting the result remains { outcome: 'unresolved',
outcomeEvidence: null } and preserves the current branch-order behavior.
- Around line 85-92: Extend the withdrawal-form test around hasWithdrawal to
assert the missing WITHDRAWAL_RE alternatives: the “agreed-” hyphen form,
“withdraw,” and “does not apply,” using matching bot-authored bodies and
expecting true. Keep the existing assertions and test title intact.
In `@gate-engine/review/__tests__/reviewer-eval.test.mts`:
- Around line 379-385: Update the test case name to clearly describe that
whole-corpus corpusHash drift does not block pairing, and replace the duplicate
rowUnchanged assertion with coverage of the intended behavior: compute the same
row’s rowHash before and after appending a sibling row, then verify those values
can be paired successfully.
In `@gate-engine/review/eval/reviewers/mine-bots-lib.mts`:
- Around line 61-69: Update hasWithdrawal to accept the same injectable
botAuthors set used by hasHumanReply, defaulting to the existing bot author when
omitted, and test membership against that set so withdrawals from both swept bot
accounts are recognized.
- Around line 105-114: Clarify the documentation for isLineTouchedLater to state
that it detects later file-level touches, not changes to the specific commented
lines, and that this is a deliberate approximation. Keep the existing
implementation and classifyOutcome behavior unchanged.
In `@gate-engine/review/eval/reviewers/mine-bots.mts`:
- Around line 60-78: Reduce repeat GitHub API requests in the sweep built around
listPrs, botComments, and the merged candidates map by adding an optional
since/updated-after filter or skipping PRs whose URLs are already fully
represented in the merged map. Preserve processing for new or incomplete PR
entries while ensuring repeat sweeps do not fetch comments and related data for
already-complete candidates.
In `@gate-engine/review/eval/reviewers/README.md`:
- Around line 122-124: Update the gateHash input-list documentation in the
reviewer README to include runtime.mts and every file under SHARED_HELPERS,
matching the inputs hashed by benchGateHash in corpus.mts so baseline
invalidation is fully documented.
In `@gate-engine/review/evidence/diff-archive.mts`:
- Around line 49-67: Update the diff archive flow around archiveFailedDiff to
enforce a retention policy for the diffs store, using periodic cleanup by entry
age or total directory size before or after archiving. Preserve hash-based
deduplication and best-effort failure handling, and ensure cleanup errors never
cause a gate to fail.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b0d95ba6-3f0f-4556-982a-52d146989403
⛔ Files ignored due to path filters (2)
docs/benchmarks/assets/dashboard-dark.svgis excluded by!**/*.svgdocs/benchmarks/assets/dashboard-light.svgis excluded by!**/*.svg
📒 Files selected for processing (17)
README.mddocs/benchmarks/README.mddocs/benchmarks/benchmark-methodology.mddocs/decisions/INDEX.mddocs/decisions/benchmarks-grow-from-telemetry.mdgate-engine/review/__tests__/diff-archive.test.mtsgate-engine/review/__tests__/mine-bots.test.mtsgate-engine/review/__tests__/reviewer-eval.test.mtsgate-engine/review/eval/reviewers/README.mdgate-engine/review/eval/reviewers/bench.mtsgate-engine/review/eval/reviewers/corpus.mtsgate-engine/review/eval/reviewers/mine-bots-lib.mtsgate-engine/review/eval/reviewers/mine-bots.mtsgate-engine/review/eval/reviewers/progress.mtsgate-engine/review/eval/reviewers/stats.mtsgate-engine/review/evidence/diff-archive.mtsgate-engine/review/run-review.mts
…, FAIL-diff archive, miner outcome labels PR A of the corpus-growth effort (charter: docs/benchmarks/benchmark-methodology.md, decision: docs/decisions/benchmarks-grow-from-telemetry.md): - corpusHash becomes a row-SET hash (sorted per-row canonical-JSON hashes); baselines, progress checkpoints, and the stability rerun pair per-row via rowHash, so appending corpus rows no longer invalidates comparisons or salvage over retained rows. - compareReviewer pairs on the row-id intersection (changed rows excluded, added/removed counted) and reports pooled + gold-only + decoy-only + clustered-by-case flip tables (pure stats.mts); checkpoint/salvage/baseline IO split into progress.mts (size ratchet); optional caseId/sourcePr/outcomeEvidence/scopeConfirmed row fields with enum lint. - summarize gains cascade-only overturnRate/rescueRate — the metric the July cascade regression was invisible without; sonnet recall floor now gates on effective models, not the env default. - validate compiles reasonPattern (hard fail) and warns non-fatally on comment-leakage tells (pattern-vs-comment match, note-vs-comment Jaccard overlap). - mine-bots: GraphQL thread resolution, CodeRabbit marker parsing, priority-ordered outcome + outcomeEvidence labels, per-PR line-touched checks, collector scope confirmation (scopeConfirmed/scopedReviewers), merge-by-url, alreadyInCorpus flags. - archiveFailedDiff: content-addressed <telemetry>/diffs/<diff_sha256>.diff.gz on reviewer FAIL, fail-open, 8MiB cap, runReviewGate-only (bench can never archive); joins telemetry via the existing diff_sha256 — no event schema change. - docs: living benchmark methodology (checklist + addendum) moved here from frink; tracker views re-rendered. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
norvalbv
force-pushed
the
bench/reviewer-corpus-infra
branch
from
August 1, 2026 16:46
565dafb to
7383e47
Compare
This was referenced Aug 1, 2026
Merged
- methodology: small-suite fallback for the 40-60 blind-relabel window; archive wording aligned with the fail-open/8MiB/telemetry-on contract. - decision scope: diff-archive.mts added via guard-decisions rescope. - bench: unstable-excluded rows reported in the compare line; section.rows guarded on the --against path. - mine-bots: inner thread-comments pagination warning (first:100), hard page cap on the cursor loop, 300-file commit truncation surfaces as lineTouchedTruncated (never a false negative), corpus-url read guarded, scope cache keyed by full repo slug, atomic candidates.jsonl write + merge-key guard. - diff-archive test: temp-dir cleanup in afterEach. README alt-text finding rebutted in-thread: that section is generated by benchmarks:render; the fix belongs in the generator, tracked as follow-up. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Aug 1, 2026
norvalbv
added a commit
that referenced
this pull request
Aug 2, 2026
…from the collector db (#303) * feat(reviewer-eval): mine-telemetry — fail→fix golds + waived decoys from the collector db Capture point 1 of the corpus-growth charter, plus the historical override-valve decoy slice. Read-side design: the miner SELECTs ~/.claude-usage/usage.db (USAGE_DB override) and reads the #295 diff archive — the collector itself is untouched (deviation from the addendum's 'collector correlates' phrasing, recorded in the module docstring; read-side correlation adds zero write paths). - fail→fix: per-(repo,branch) chronological chains (ISO-aware ordering, ship-id tiebreak); PER-LENS correlation when lens data exists, reviewer-level fallback only on lens-data absence; same-diff overrides route to the decoy path; fixed-by-absence recognized on clean ships. Candidate lenses are an ALLOWLIST on disposition ('blocking' or pre-disposition null). Merge keys carry the candidate kind. pickFailReason falls through to the reviewer reason when issues_json filters to nothing (whitespace-only arrays shadowed it via ??). - Evidence honesty: failReason-only rows flagged + counted; repo allowlist defaults to devkit (frink diffs are private — explicit flag + adapt stage). - Shared miner plumbing extracted to mine-common.mts (jscpd: zero clones). - Atomic merge-by-key output to raw/candidates-telemetry.jsonl (gitignored); fail-open on missing db/sqlite3/archive; bypasses propose.mts by design. Live dry-run (devkit scope): 44 candidates incl. 9 with archived bytes; idempotent re-runs. 48 unit tests; 490/490 suite green; tsc/biome clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(reviewer-eval): address CodeRabbit review on #303 Three findings, all valid. - selectFailLensRows (new, in the lib so it is testable): an empty blocking filter used to fall through to `[null]`, the "no lens breakdown recorded" branch. A fail whose only failing lenses were waived or dropped_out_of_charter therefore minted a reviewer-level gold from exactly the lenses the allowlist had just excluded — contradicting the rule the comment above it states. Now three distinct cases, and the skip is COUNTED in the drop histogram (fail-fix:all-failing-lenses-non-blocking) rather than vanishing silently, which is the same evidence-honesty bar as the rest of the funnel. - collectRepoArgs: `--repo --dev` stored "--dev" as the repository. Both callers treat any non-empty result as an explicit scope replacing their defaults, so that silently narrowed the sweep to a repo that cannot exist and reported a clean run. A missing or flag-shaped value is now a usage error. - sqliteJson: the docstring claimed read-only but nothing enforced it. Now opens the collector db with sqlite3 `-readonly`, so the boundary is held by the engine — a write raises "attempt to write a readonly database" instead of mutating the user's telemetry. The miners are strictly read-side. Tests: 47 -> 61 in mine-telemetry.test.mts, covering all three cases of the lens rule, both malformed --repo shapes, and a refused INSERT/DDL that leaves the db intact. Full gate-engine/review suite 503/503. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
norvalbv
added a commit
that referenced
this pull request
Aug 2, 2026
…clone-gate ruling (#310) benchmarks-grow-from-telemetry gains its convergence record before the release that ships it: capture loop closed end-to-end (#295/#302/#303/#309, first 8 pure-telemetry rows, corpus 128), label-trust precondition met (#304: κ 0.735 post-triage, 4.2% noise floor; cleanlab floor still pending bench pred_probs), and the Target's c-CRAB/CR-Bench known-answer path recorded as falsified (#307) with the replacement candidates awaiting ratification. New axis clone-gate-non-import-code ([VALIDATED]): clones are measured over non-import code, excluded at the jscpd tokenizer — with the six-hole failure of post-hoc fragment classification recorded as the rejected road so a future simplifier can't silently re-vacuous the gate (#305/#308). Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR A of the reviewer-benchmark corpus-growth effort. Charter:
docs/benchmarks/benchmark-methodology.md(living checklist + addendum, moved here from frink in this PR); decision:docs/decisions/benchmarks-grow-from-telemetry.md.Engine (bench.mts / corpus.mts / stats.mts / progress.mts)
corpusHash— sorted per-row canonical-JSON hashes. Baselines, progress checkpoints, and the stability rerun all pair per-row viarowHash, so appending corpus rows no longer invalidates comparisons or salvage over retained rows (this was structurally blocking the self-feeding-corpus target).compareReviewerpairs on the row-id intersection (changed rows excluded + counted; added/removed reported) and prints pooled + gold-only + decoy-only + clustered-by-case flip tables. New purestats.mts; checkpoint/salvage IO split toprogress.mts(size ratchet).summarizegains cascade-only overturnRate/rescueRate — the July cascade regression (opus overturning true golds, 0.78→0.67) was invisible without them.BENCH_MODELenv default.validatecompilesreasonPattern(hard fail on bad regex) and warns non-fatally on comment-leakage tells.caseId,sourcePr,outcomeEvidence,scopeConfirmed.Miner (mine-bots.mts + mine-bots-lib.mts)
GraphQL thread resolution, CodeRabbit marker parsing, priority-ordered
outcome/outcomeEvidencelabels (addressed-marker → bot-withdrawal → resolved+line-touched → human-rebuttal → outdated-only), per-PR line-touched checks, collector scope confirmation (scopeConfirmed/scopedReviewersviacommit_ships.pr_number), merge-by-url,alreadyInCorpusflags.Enabler (evidence/diff-archive.mts)
archiveFailedDiff: content-addressed<telemetry>/diffs/<diff_sha256>.diff.gzon reviewer FAIL. Fail-open, 8MiB cap, fires only fromrunReviewGate(bench callsrunCascadedirectly and can never archive). Joins telemetry via the existingdiff_sha256— no event schema change. Stops the ongoing loss of ~12 replayable correctness FAILs/day.Tests: 441 passing in
gate-engine/review/__tests__incl. newdiff-archive,mine-bots, rowHash/pairing/cluster suites.bench.mts validategreen for all five suites (0 LLM calls).Built by a 12-agent workflow (3 scout→code→refute chains + integration barrier); adversarial refuters caught and fixed a real blocker (stability rerun still coupled to the whole-corpus hash).
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests