feat(testgen): the acceptance gate now rejects tests that pass when the code is broken - #131
Merged
Merged
Conversation
|
Warning Review limit reachedNext included review available in 14 minutes. View limit detailsLimit details: You’ve used the included review currently available. Your 80 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
Comment |
…he code is broken
DEDUP (CLAUDE.md §0). Grepped src/ and tests/ for hollow/deliberate/break/local_verify/
node_verdict; searched the improvement log; read features.py's testgen entry. testgen_gate.py
EXISTS and is the right home -- an assured-acceptance gate with collect/import, baseline
non-regression, reliability and --min-covered-lines-delta -- and had NO hollowness notion.
This EXTENDS it; no capability is registered.
WHAT WAS WRONG. The gate's strongest criterion was `coverage_delta`. A test that calls the
function and asserts nothing raises covered lines exactly as much as one that pins the
result, so the gate guarding GENERATED tests could be satisfied by tests that can never
fail. Proven end-to-end on a fixture of one real and two hollow tests, all three passing
normally:
PASS coverage_delta covered-lines delta 4 >= required 1
FAIL no_hollow_nodes 2 test(s) pass against a broken base: ...test_hollow_smoke
The old gate accepts that set. The selftest now asserts BOTH facts in one case, so the
reason this check exists cannot be lost to a later tidy-up.
BUILT ON #124's CONVENTION, not beside it. That PR drained this module to "misuse is not a
bad-test verdict": every check carries `could_not_measure`, and a blind check names the
misuse AND the remedy. The hollowness probe is the same shape -- it can be prevented from
running (no --base-ref, no local_verify.py, a timeout) -- so it returns `measured: False`
with a reason and a remedy, and `_hollow_check_row` renders the same COULD NOT MEASURE
wording. A blind probe never passes, for the reason #124 gives for coverage_delta: this is
the strongest check here, so letting "could not run" read as ok would make it the easiest
one to switch off silently. Break -> revert: widening `ok` to `not nodes` fails two asserts;
reverted byte-identical.
It reads node_verdict/node_analysis, NOT the exit code -- per-node grading is advisory by
construction in local_verify and deliberately leaves the process result alone, so a gate
reading the exit code would accept hollow tests while believing it had checked.
testgen_lane.py forwards --base-ref/--test-path when given and never guesses one: the gate
fails closed without it, so an omitted ref surfaces as a failed check rather than as a
silently weaker gate. Its selftest asserts both the absent and present forms.
No un-gating was needed. capability_advisor reports these as "matched but a gate blocked
invocation", but dispatch_ready is false fleet-wide by construction (status=active plus
immutable version lineage, which 0 of 33 capabilities have). That governs automatic ROUTER
dispatch; both modules are CLIs and run today.
VERIFY: 457 passed, 1 failed, 0 skipped, 85/85 selftests, 5/5 gates. The one failure is
PRE-EXISTING ON CLEAN MAIN and machine-local, confirmed by stashing this branch and
re-running: test_model_tier_resolution.test_capacity_gate_is_seat_level_not_gemini_special
unpacks three values from capacity.compute(), whose docstring says "(state, reason[, meta])"
-- meta optional. `_shed(agent)` is `(SHED_DIR / agent).exists()`, a file OUTSIDE
$ORCH_STATE_DIR, and this machine has a real codex 429 shed flag, so compute() takes the
two-value early return. main's CI is green because a runner has no shed file. The test
should neutralise _shed rather than depend on the host; filed separately, not bundled here.
Floor unchanged: these are selftest cases, which add no collected pytest tests.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
stranske
force-pushed
the
claude/testgen-gate-rejects-hollow-tests
branch
from
August 26, 2026 01:00
e69f697 to
73edb06
Compare
stranske
had a problem deploying
to
agent-standard
August 26, 2026 01:00 — with 
GitHub Actions
Failure
GitHub Actions
Failure
stranske
temporarily deployed
to
agent-standard
August 26, 2026 01:00 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
Workflow source neededPR #131 needs either a linked GitHub issue or one valid non-issue Workflow Source before PR metadata automation can manage it safely. Please do one of:
Once a valid source is present, this warning will not be reposted. |
Contributor
|
Workflow state fingerprint for Keepalive Loop Reporter. Do not edit. |
Automated Status SummaryHead SHA: 8a150ef
Coverage Overview
Coverage Trend
Top Coverage Hotspots (lowest coverage)
Low Coverage Files (<50.0%)
Updated automatically; will refresh on subsequent CI/Docker completions. Keepalive checklistScopeNo scope information available Tasks
Acceptance criteria
|
Contributor
|
Workflow state fingerprint for Agents Gate Followups. Do not edit. |
Owner
Author
|
Merging with one red check, and stating why rather than waving it through. The failure is That is the fleet automation's own token, not this PR. The job's purpose is to merge OTHER automerge-labelled PRs; it neither builds nor tests anything here, and it would have failed identically on an empty commit. Every substantive check is green: Local verification: 457 passed, 1 failed, 85/85 selftests, 5/5 gates. That one local failure is pre-existing on clean main and machine-local — |
stranske
pushed a commit
that referenced
this pull request
Aug 29, 2026
…D, not by uncovered lines
DEDUP (CLAUDE.md §0). Checked src/ for priorit/rank/hotspot/target — only capability_targets.py,
which ranks CAPABILITIES not test targets. Grepped the tree for hotspot/prioriti/rank_. Searched
the improvement log for "escaped defect" and "test priority": both reported a real absence with
the log read in full. Nearest existing thing is coverage_trend.compute_top_files, which ranks by
LOWEST COVERAGE — precisely the ordering this exists not to lead with. Not present; building new.
`capability_admission.py --preflight` clears with zero blocking failures; caller, heartbeat and
fixture remain obligations, and the fixture is discharged here.
WHY NOT MOST-UNCOVERED-FIRST. That maximises percentage-per-PR and points agents at the largest
uncovered files, which are the glue modules where a meaningful test is hardest to write. It is the
ordering most likely to produce hollow tests, so uncovered mass is the LAST tier, not the first.
THREE TIERS, lexicographic:
1. ESCAPED DEFECTS — a file that later needed a fix is a file whose tests missed something. The
only tier reporting observed failure of the TESTS rather than a property of the code.
2. CHURN — where regressions actually arrive.
3. UNCOVERED MASS — how far the metric moves. Last, deliberately.
All three multiplied by (1 - hollow_rate), so a module where agents keep producing tests that pass
against a broken base sinks however much uncovered code it has. That term only became measurable
when testgen_gate grew no_hollow_nodes in #131.
LEXICOGRAPHIC, NOT A WEIGHTED SUM, and the first version was the latter. Multiplying the tiers
apart (1e6/1e3/1) and summing holds only while the lower tiers stay small: at 1,000,000 uncovered
statements tier 3 exactly equals one escaped defect and the ordering inverts. A scoring function
whose correctness depends on inputs staying under a magic threshold is a defect waiting for a big
repository. Two of the new tests caught it during review; the tuple holds at any magnitude and the
tests now assert at 10**9. No blended "score" is reported at all — one number formed from three
incomparable tiers invites exactly the trade-off the tuple forbids.
TIER 1 IS A GIT PROXY TODAY AND SAYS SO. The Brain has the better signal in outcomes.durability,
where broke_later means merged, CI green, broke afterwards. Measured across 4,665 outcome rows:
durable 2842, abandoned 1300, pending 517, reverted 4, reworked 2, broke_later ZERO —
durability_sweep assigns "reopened, reverted, or durable" and never broke_later, while
pattern_miner.TERMINAL_FAILURE_DURABILITY consumes it. A consumer for a label nothing produces.
Six escaped-defect rows cannot order a queue, so tier 1 reads git history, which needs no
instrumentation and exists in every repo. brain_signal_status() reports which source is live and
returns "unknown" rather than zero when the store cannot be read, so the day the Brain signal
becomes usable is visible rather than assumed.
RANKING IS NOT TRAINING. A fix commit touching a file is decent evidence for ORDERING work and
poor evidence for a learner: code changes for many reasons. This module ranks; it never writes
durability labels. A broke_later producer must clear a higher bar and is deliberately separate.
A SECOND DEFECT OF MY OWN, caught while validating on real data: the CLI treated a missing
--coverage-json as an empty report, so tier 3 rendered as a column of zeros that reads as
"everything is covered" rather than "nothing was read" — the same could-not-measure-as-measured-
zero shape this module's own notes describe. It now names the file and the failure. Found when a
scratch path was cleaned up between runs and the ranking carried cheerfully on.
VALIDATED ON REAL DATA. stranske/Trend_Model_Project, its real Gate coverage payload and its real
history: multi_period/engine.py ranks first on 9 fix commits, 28 churn, 335 uncovered — while
3_Results.py, which has the MOST uncovered statements at 759, ranks third. That inversion is the
design working.
Break -> revert on both invariants: restoring the weighted sum fails the two tier-ordering tests;
removing the hollow discount fails the two hollow tests. Byte-identical revert, 13 pass.
Tests are pytest, not selftest cases, on purpose: local_verify grades per pytest NODE, so a
selftest is one node and its assertions are invisible to hollow detection. A module that orders
test-writing work should have its own tests gradeable by the gate judging that work. The module
keeps a --selftest as well (88/88), exercising the CLI as it ships including live git parsing.
Verified: 515 passed, 0 failed, 0 skipped, 88/88 selftests, 5/5 gates. Floor 502 -> 515.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
stranske
added a commit
that referenced
this pull request
Aug 29, 2026
…D, not by uncovered lines (#153) * feat(testgen): rank test-writing work by where testing actually FAILED, not by uncovered lines DEDUP (CLAUDE.md §0). Checked src/ for priorit/rank/hotspot/target — only capability_targets.py, which ranks CAPABILITIES not test targets. Grepped the tree for hotspot/prioriti/rank_. Searched the improvement log for "escaped defect" and "test priority": both reported a real absence with the log read in full. Nearest existing thing is coverage_trend.compute_top_files, which ranks by LOWEST COVERAGE — precisely the ordering this exists not to lead with. Not present; building new. `capability_admission.py --preflight` clears with zero blocking failures; caller, heartbeat and fixture remain obligations, and the fixture is discharged here. WHY NOT MOST-UNCOVERED-FIRST. That maximises percentage-per-PR and points agents at the largest uncovered files, which are the glue modules where a meaningful test is hardest to write. It is the ordering most likely to produce hollow tests, so uncovered mass is the LAST tier, not the first. THREE TIERS, lexicographic: 1. ESCAPED DEFECTS — a file that later needed a fix is a file whose tests missed something. The only tier reporting observed failure of the TESTS rather than a property of the code. 2. CHURN — where regressions actually arrive. 3. UNCOVERED MASS — how far the metric moves. Last, deliberately. All three multiplied by (1 - hollow_rate), so a module where agents keep producing tests that pass against a broken base sinks however much uncovered code it has. That term only became measurable when testgen_gate grew no_hollow_nodes in #131. LEXICOGRAPHIC, NOT A WEIGHTED SUM, and the first version was the latter. Multiplying the tiers apart (1e6/1e3/1) and summing holds only while the lower tiers stay small: at 1,000,000 uncovered statements tier 3 exactly equals one escaped defect and the ordering inverts. A scoring function whose correctness depends on inputs staying under a magic threshold is a defect waiting for a big repository. Two of the new tests caught it during review; the tuple holds at any magnitude and the tests now assert at 10**9. No blended "score" is reported at all — one number formed from three incomparable tiers invites exactly the trade-off the tuple forbids. TIER 1 IS A GIT PROXY TODAY AND SAYS SO. The Brain has the better signal in outcomes.durability, where broke_later means merged, CI green, broke afterwards. Measured across 4,665 outcome rows: durable 2842, abandoned 1300, pending 517, reverted 4, reworked 2, broke_later ZERO — durability_sweep assigns "reopened, reverted, or durable" and never broke_later, while pattern_miner.TERMINAL_FAILURE_DURABILITY consumes it. A consumer for a label nothing produces. Six escaped-defect rows cannot order a queue, so tier 1 reads git history, which needs no instrumentation and exists in every repo. brain_signal_status() reports which source is live and returns "unknown" rather than zero when the store cannot be read, so the day the Brain signal becomes usable is visible rather than assumed. RANKING IS NOT TRAINING. A fix commit touching a file is decent evidence for ORDERING work and poor evidence for a learner: code changes for many reasons. This module ranks; it never writes durability labels. A broke_later producer must clear a higher bar and is deliberately separate. A SECOND DEFECT OF MY OWN, caught while validating on real data: the CLI treated a missing --coverage-json as an empty report, so tier 3 rendered as a column of zeros that reads as "everything is covered" rather than "nothing was read" — the same could-not-measure-as-measured- zero shape this module's own notes describe. It now names the file and the failure. Found when a scratch path was cleaned up between runs and the ranking carried cheerfully on. VALIDATED ON REAL DATA. stranske/Trend_Model_Project, its real Gate coverage payload and its real history: multi_period/engine.py ranks first on 9 fix commits, 28 churn, 335 uncovered — while 3_Results.py, which has the MOST uncovered statements at 759, ranks third. That inversion is the design working. Break -> revert on both invariants: restoring the weighted sum fails the two tier-ordering tests; removing the hollow discount fails the two hollow tests. Byte-identical revert, 13 pass. Tests are pytest, not selftest cases, on purpose: local_verify grades per pytest NODE, so a selftest is one node and its assertions are invisible to hollow detection. A module that orders test-writing work should have its own tests gradeable by the gate judging that work. The module keeps a --selftest as well (88/88), exercising the CLI as it ships including live git parsing. Verified: 515 passed, 0 failed, 0 skipped, 88/88 selftests, 5/5 gates. Floor 502 -> 515. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(floor): resolve against #152 as the union, re-measured on the merge result #152 landed underneath this branch and added a NEW floor key, mirror_skipped_max, bounding the exec mirror by its own skip ceiling rather than the runner's. Resolved as the UNION per the rule this file states: main's structure kept in full including that key, this branch's rationale appended rather than either note replacing the other, and the count RE-MEASURED on the merge result rather than carried forward from either side. 515 from verify.py's own run after the rebase, which confirms #152 added no collected tests. That makes 502 + 13 correct -- but measured, not assumed. Taking a side would have been silently right this time, and it is what put the floor 8 below reality on an earlier occasion. Verified on the merge result: 515 passed, 0 failed, 0 skipped, 88/88 selftests, 5/5 gates. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Tim Stranske <tim@stranskemo.com> Co-authored-by: Claude Opus 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.
Dedup finding (CLAUDE.md §0)
Checked
src/andtests/for hollow, deliberate, break, local_verify, node_verdict; searched the improvement log; readfeatures.py's testgen entry.testgen_gate.pyexists and is exactly the right place — an "assured-acceptance gate for generated pytest tests" with collect/import, baseline non-regression, repeated-run reliability and--min-covered-lines-delta. It had no reference to local_verify, hollowness or node grading. So this extends an existing capability rather than building a new one, and registers nothing new.What was wrong
The gate's strongest criterion was
coverage_delta— "covered lines went up by at least N". A test that calls the function and asserts nothing raises covered lines exactly as much as one that pins the result. So the gate guarding generated tests could be satisfied by tests that can never fail.Proven end-to-end on a fixture of one real and two hollow tests, all three passing normally:
The old gate accepts that set. The new one names both hollow nodes and refuses.
How
hollow_check()shells out tolocal_verify.py, which reverts to--base-refand re-runs the candidates per node: a node that still passes did not notice the break, and is hollow.It reads
node_verdict/node_analysis, not the exit code — per-node grading is advisory by construction in local_verify and deliberately leaves the process result alone. A gate reading the exit code would accept hollow tests while believing it had checked.NOT EVALUATED is not a pass
This is the load-bearing decision. The probe can be prevented from running — no
--base-ref, nolocal_verify.py, a timeout. In that state the gate has no evidence at all, sookisFalseand the detail saysNOT EVALUATED: <reason>.Reporting that as ok would make the strongest check the easiest one to switch off silently — the exact defect class this gate exists to catch, one level up.
Break → revert
okwidened tonot hollow_nodes(unevaluated passes)Byte-identical revert both times.
Also pinned:
coverage_deltais asserted green in the same synthetic case whereno_hollow_nodesis red — so the selftest states in code why the new check cannot be replaced by the old one.Lane
testgen_lane.pypasses--base-ref/--test-paththrough when given; its selftest asserts both the absent and present forms. It never guesses a ref — the gate fails closed when one is missing, so omitting it cannot quietly buy a green run.No un-gating was required
capability_advisorreports both as "matched but a gate blocked invocation", butdispatch_readyis false fleet-wide by construction: it needsstatus=activeplus immutable version lineage, which 0 of 33 capabilities have. That governs automatic router dispatch only. Both modules are CLIs and run today.One gap worth naming
CLAUDE.md §0 asks for the dedup finding in the capability's ledger notes.
capabilities.py's only write path istransition, which requires a lifecycle state change — and extending a capability isn't a promotion, so there is no honest transition to make and no note-only command exists. Recorded in the commit instead.Verified
458 passed, 0 failed, 0 skipped, 85/85 selftests, 5/5 gates. Floor unchanged — these are selftest cases, which add no collected pytest tests.🤖 Generated with Claude Code