fix(bench-gate): the H1 directive gate can be cleared by a one-token lookup table - #1343
Conversation
There was a problem hiding this comment.
Sorry @robotrocketscience, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
Warning Review limit reached
Next review available in: 27 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
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 |
Reviewer's GuideAdds a bench-gate validity guard that ensures the directive-detection corpus cannot be cleared by a trivial first-token classifier, and updates design docs, re-entry queue, and changelog to reflect corrected H1 precision/recall numbers and the newly recognized corpus problem without changing detector code. Flow diagram for the H1 directive corpus validity bench gateflowchart TD
A[Run test_directive_corpus_defeats_a_first_token_baseline] --> B[Load lab corpus v0_1 or v0_2]
B --> C[Partition corpus into train and held_out]
C --> D[Train first_token_baseline on train]
D --> E[Evaluate first_token_baseline on held_out]
E --> F{P >= 0.80 and R >= 0.60?}
F -->|yes| G[Fail test: corpus can be cleared by one_token baseline]
F -->|no| H[Pass test: corpus defeats one_token baseline]
subgraph Gate_reentry_conditions
I[Detector_precision >= 0.80]
J[Detector_recall >= 0.60]
K[Corpus_defeats_one_token_baseline test green]
I --> L[H1 gate may be treated as valid]
J --> L
K --> L
end
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
PR-size soft capThis PR is over the advisory size threshold:
Bigger PRs collide with more open work, which under the parallel-session workflow tends to produce repeated This is advisory only — nothing is blocked. If the size is intentional (large refactor, module removal, generated code), apply the |
|
[claim:review:Garsecg:2026-08-05T02:10:23Z] |
|
[claim:review:Kulili:2026-08-05T02:10:40Z] |
|
[release:review:Kulili:2026-08-05T02:10:48Z] |
…aseline (#1341) The H1 gate is supposed to certify that directive_detector distinguishes a durable rule from a one-shot task. It cannot: a classifier that reads only the first word of the prompt and answers with that word's majority training label clears P>=0.80 / R>=0.60 on held-out rows at P=0.912 / R=0.795, while the real detector scores 0.706 and fails. Of 114 distinct opening words in lab corpus v0.1 only 7 appear in both classes. The new guard trains that baseline on a deterministic 60/40 split of whatever corpus is mounted and fails when it clears the gate. It is a statement about the corpus, not the detector. Red against v0.1 by design; skips in public CI with the rest of the bench gate.
…ding (#1341) Both docs carried P=0.664 as the detector's standing measurement. That was the pre-Path-A number; Path A shipped in #467 and the confirming re-run this memo asked for never happened. The shipped detector measures P=0.706 / R=0.937, and Path A removed 8 false positives rather than the ~45 estimated. The larger correction is that the gate cannot presently certify anything: v0.1 separates its classes by opening vocabulary, so head-position rules buy free precision. Records the measurement, replaces the monolithic-cluster failure analysis with the six families actually present, and makes corpus v0.2 the blocking item ahead of any further detector iteration.
6c5c60e to
4bec0e7
Compare
Review — the conclusion holds, the guard that encodes it is under-poweredReviewed the diff, the two design memos and the new bench-gate test. The The conclusion is not at riskWorth saying plainly before the criticism, so it does not get over-read: the The guard rests on 34 predictions from one partition
Recall is comfortable. Precision is not: the interval's lower bound is below That matters more here than it usually would, because of what this guard is Recommended fix: sweep K salted partitions ( I did not make that change myself. It needs a corpus run to pick K and to Not blocking, and whyThe guard as written is under-powered, not wrong: it currently fails, correctly, Two things I fixed on the branch
One thing left as-is, deliberatelyThe memo and the CHANGELOG both state |
|
merge-train: merged 4bec0e7 → |
|
[release:review:Garsecg:2026-08-05T02:18:58Z] |
|
Both points taken and fixed in PR #1350 (closes #1349). The guard. Swept to K=200 salted partitions, failing if the baseline clears on any one of them. Your predicted failure was not hypothetical for long — I landed corpus v0.2 shortly after this merged and declared the guard green off a single partition, which is exactly the false clearance you described. The sweep is what actually establishes it: v0.1 clears on 196/200, the union on 0/200 with a maximum precision of 0.754. Chose fail-on-any over pooled counts because pooled predictions across overlapping partitions are not independent, so the Wilson interval on them is too tight in the direction that makes passing easier — and for a guard, a false alarm costing a corpus inspection beats a false clearance blessing an overfit detector. The point estimate. You were right that it should not have shipped bare, and right to leave it to me. Thanks for the rebase and the § What's being decided catch — that section was carrying both the stale number and a superseded framing, and I had missed it. One thing your review did not ask for but follows from it: against the union the detector is P=0.665 / R=0.636. Recall was 0.937 with 34 points of headroom on v0.1, which is the premise every iteration path in that memo was written on. It is now barely above the floor, because v0.2's hard positives are durable rules the 29-verb regex never fires on. Recall is the binding constraint from here, not precision. |
Closes #1341.
The H1 bench gate was never re-run after the fix it was waiting on shipped. Re-running it turned up a stale published number, and then a larger problem: the gate cannot certify what it claims to certify.
1. The published number was stale
docs/design/v2_directive_detection.mdanddocs/design/V2_REENTRY_QUEUE.mdhave both carried P=0.664 since May. That is the pre-Path-A figure. Path A shipped in #467 and the confirming re-run the memo itself asked for ("needs corpus re-run to confirm") never happened.Measured against the shipped detector, lab corpus v0.1, 285 rows:
P=0.706 / R=0.937. Path A removed 8 false positives, not the ~45 estimated — off by about 5x. And the surviving errors are not the "single dominant pattern" the failure analysis described; they fall into six structurally distinct families (interrogatives the filters miss, attribution, third-person descriptive, use/mention, task verbs outside the prefix bank, and stance/affirmation frames).
2. The gate can be cleared by a one-token lookup table
This is the finding that matters, and it is why this PR ships no detector change.
Partition the corpus deterministically, 163 train / 122 held out. Build the weakest classifier that can be written: read the first word of the prompt, look up the majority label for that word among training rows, answer with it. No grammar, no mood, no notion of durability or task-versus-rule.
It clears the gate. The real detector scores 0.706 and does not.
The cause is in the corpus. Of 114 distinct opening words, only 7 appear in both classes — 87.9% of rows are decided by their first token. Positives open with deontic/policy vocabulary (
always,never,don't,avoid,prefer,use); negatives open with task, question and discourse vocabulary (write,run,check,what,should,ok,please). No row labelleddirectiveopens with a one-shot task verb.So any rule keyed on head position buys precision for free. It is never charged for the durable directives it would wrongly suppress, because the corpus contains none of them — no "Check every PR for a changelog entry before you approve it", no "Run the full suite before pushing to a shared branch". Those are ordinary standing rules that open with a task verb, and a head-verb filter eats them in production while scoring clean in-corpus.
3. How that was established, and what did not ship
Six family-scoped suppression rules were built against the train split only, each self-measured, then each put through independent adversarial review. Composed, they took the corpus to P=0.953 / R=0.853 — a comfortable pass, and it held up on the held-out split (P=0.944).
Every one of the six was judged both overfit and over-reaching, unanimously, with concrete minimal pairs. Representative: a rule claiming to detect interrogative force by subject-auxiliary inversion actually keyed on a closed pronoun list —
should we only run the migration on Postgresis suppressed,should the migration only run on the Postgres shardis not, identical syntax. Another suppressed genuine directives outright:Check each incoming PR for a changelog entry before you approve it.None of them are in this PR. The one-token baseline explains why they measured so well and reviewed so badly, and a gate that a lookup table passes will bless an overfit detector — it nearly did here.
What this PR contains
test_directive_corpus_defeats_a_first_token_baseline— a bench-gate guard that trains the baseline on whatever corpus is mounted and fails if it clears the gate. A statement about the corpus, not the detector. Red against v0.1 by design; skips in public CI with the rest of the bench gate.No change to
src/. Detector behaviour is byte-identical.Verification
Both reds are the intended state against v0.1 and both are invisible to public CI.
What this unblocks, and what it does not
#1315 (natural-language capture for time-boxed locks) is blocked on "H1 gate met, or a narrower detector proposed with its own gate". This PR does not clear that blocker — it establishes that clearing it against v0.1 would have meant nothing. The path is corpus v0.2: minimal pairs that break the head-word/class correlation, hard positives (durable rules opening with task verbs, absent today) and hard negatives (one-shot requests opening with policy verbs). Target: class-ambiguous opening words rise from 12.1% to a majority.
Corpus rows stay lab-side throughout. Only aggregate statistics cross the boundary.
Two lab-side corpus-hygiene notes, neither blocking: the label vocabulary is
not-directivewhiletests/corpus/v2_0/README.mddocumentsnot_directive, and theprovenancefield the same README requires to be non-empty is absent from all 285 rows.Summary by Sourcery
Add a corpus-validity guard for the H1 directive-detection bench gate and correct related documentation and changelog entries.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests:
Chores: