fix(verify): bound the exec mirror by its own skip ceiling, not the runner's - #152
Merged
Conversation
…unner's
`.verify-floor.json`'s `skipped_max` = 26 is, by the floor file's own note,
"exactly what a machine with none of this instance's local prerequisites skips
(a GitHub runner)". It was ALSO applied to the exec mirror, which is the
opposite deprivation: every local prerequisite present (it exists only on the
machine the system runs on) but a flat file copy, so no `.github/` and no
`.git`. The mirror skips 31 tests a runner skips none of — 12 from
`repo_files_absent(".github/workflows", ...)` in test_ci_gate_config.py and 19
from `git_repo_absent()` in test_repo_artifact_hygiene.py.
So `python3 verify.py` from `~/.codex/orchestrator-mirror` was RED on every
input, correct trees included. Measured on unmodified main: 471 passed, 0
failed, 31/26 max skipped, sole problem "CEILING exceeded". Confirmed
structural rather than caused by any PR by syncing the PREVIOUS main into a
scratch mirror and getting the same 31. CLAUDE.md §1 makes the mirror run the
verdict ("cmp-clean is not agreement"), so the one instrument that catches
cross-tree divergence had stopped protecting anything, and a gate red whatever
you do is a gate that gets switched off.
NOT fixed by raising `skipped_max` to 31: CI runs on a runner where 26 is the
measured bound, and the extra five would let a runner skip five more checks in
silence — one gate going quiet to un-stick another. Instead each shape carries
its own agreed number, measured where it is enforced, which is latched-gate
question 3 taken literally: the measuring window equal to the draining window.
* `env_prereq.exec_mirror_shape()` detects the tree — no `.github/` AND git
reports no repository — the PREREQUISITE and never `$CI`, as every other
detector there does. BOTH marks required, because the mirror number is the
looser one, so an ambiguous tree falls back to the base agreement.
* `verify.mirror_key()` derives the floor key from the base key: ONE rule
consumed by both the check and `--update-floor`, so any ceiling gets a
`mirror_` variant for free and an unset or misspelt one falls back to base.
The mirror value REPLACES the base rather than adding to it — 26+31 would buy
26 units of headroom nothing on the mirror can legitimately spend.
* `_ceiling_report()` returns the verdict AND the count/limit rendering from ONE
shape decision. They used to reach it separately, which is the mypy-ratchet
defect exactly (the line said NOT COUNTED while the enforcement cleared); the
selftest now holds both halves against each other.
* `_preserved_ceilings()` carries both shapes through `--update-floor`. The
reconcile job rewrites this file on every push to main FROM A CHECKOUT, where
the mirror keys are never read, so a writer keeping only the keys it used
would have deleted `mirror_skipped_max` and re-latched the mirror silently.
* The summary's first line now names the tree in BOTH shapes, and the pytest
line reads `31/31 max [mirror_skipped_max]` — the key named whenever it is not
the base one, so a mirror reader is never sent to raise the number CI needs.
Deliberately NOT asserted: that `mirror_skipped_max >= skipped_max`. It holds
today by coincidence — the two populations are disjoint, not nested — and the
drain named in the floor note (teach orch-sync-mirror.sh to copy `.github/`,
dropping 12) would take it to 19, so the assertion would refuse the drain it
exists to encourage. The reasoning is left in place beside the omission.
Verified: `python3 src/verify.py` green from BOTH trees. Checkout: 502 passed,
0 failed, 0/26 max skipped, 87/87 selftests, 5/5 gates. Scratch exec mirror
(synced with ORCH_MIRROR so the live mirror is untouched): 471 passed, 0
failed, 31/31 max [mirror_skipped_max], 87/87 selftests, 5/5 gates, VERIFIED.
`collected` is 502 in both and unchanged — the new coverage lives in the
verify.py and env_prereq.py selftests, which pytest does not collect. Six
deliberate-break→revert demonstrations, each confirmed to discriminate: AND→OR
in the shape detector, `ceiling_limit` ignoring the mirror key,
`_preserved_ceilings` dropping it, the renderer reaching the shape separately,
`verify()` hardcoding `mirror=False` (the single wiring pin, formatter-checked
against black 26.5.1), and the real floor file losing the key.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 8 minutes. View limit detailsLimit details: You’ve used all 2 included reviews currently available. Your 57 included PR review attempts over the past 7 days set your current allowance at 2 reviews 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 (5)
Comment |
stranske
temporarily deployed
to
agent-standard
August 29, 2026 18:21 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
Workflow source neededPR #152 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
Automated Status SummaryHead SHA: 3aaa62e
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. |
stranske
pushed a commit
that referenced
this pull request
Aug 29, 2026
…erge 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>
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>
stranske
added a commit
that referenced
this pull request
Aug 29, 2026
…loor writer must not whitelist (#154) Two generalisable remainders from #152. Both are the same shape: a guard that was correct in the instance and silent in the general case. 1. A WIRING PIN THAT SILENTLY STOPS FIRING. `assert needle in src` proves the wiring only while the needle matches ONE site. Add a second call site and deleting the one the pin MEANS leaves it matching the other — the guard passes over the fully restored defect, which is this repo's founding failure wearing the costume of a guard against it. Not hypothetical. #152's exec-mirror pin FAILED TO FIRE against a deliberate break, because the detected shape was reaching the ceiling check and the summary renderer through two separate calls. The break demo caught it; an `in` pin never would have. CLAUDE.md's pin rule covered only the false POSITIVE (a pin firing on a correct reformat) and had nothing to say about this direction. Audited all six pins in the repo — verify.py's `--update-floor` guard and exec-mirror shape, `test_checks_reported`'s ratchet union, and `test_verify_coverage_mode`'s three runner needles. EVERY ONE matched exactly once, so there is no live false negative; what this change fixes is that uniqueness decays QUIETLY. All six now assert `count(...) == 1` and report the observed count, so the decay is a red that names the choice to make. A count above 1 is usually a design smell rather than a pin problem: two consumers reaching one decision separately can disagree. #152 already routed the shape through `_ceiling_report`, which returns the verdict and its rendering from one decision — and that is what makes the pin unique again for free. 2. THE FLOOR WRITER WAS A WHITELIST, SO A NEW KEY WAS ONE FORGOTTEN REGISTRATION FROM SILENT DELETION. `--update-floor` wrote only the keys it recognised (`CEILINGS` plus their mirror variants). Anything else in `.verify-floor.json` was discarded — and ci.yml's reconcile job runs it on EVERY push to main, so the discard would arrive as a bot commit nobody reads. `mirror_skipped_max` escaped that only because #152 remembered to register it. Registering each key was never the fix; the registration step IS the trap, and forgetting it is silent. Inverted: `REMEASURED_FLOOR_KEYS` names the three keys the writer rewrites (`collected`, `passed`, `note`) and `_carried_forward` passes everything else through verbatim. A key the writer has never heard of now survives by default — failing toward keeping evidence rather than losing it — and values are no longer coerced to `int`, which was a type assumption a general carry-forward has no business making. Ceilings are still never re-measured, and that is now asserted directly rather than implied by the whitelist. CLAUDE.md §1's wiring-pin rule gains the false-negative mode, the design smell, and the preference for a registration-free design over a pin where one exists. Verified: `python3 src/verify.py` green from BOTH trees. Checkout: 502 passed, 0 failed, 0/26 max skipped, 87/87 selftests, 5/5 gates. Scratch exec mirror: 471 passed, 0 failed, 31/31 max [mirror_skipped_max], 87/87 selftests, 5/5 gates, VERIFIED. `collected` unchanged at 502 — no new pytest tests; the new coverage is in verify.py's selftest and in strengthened assertions inside two existing test files. Four deliberate-break→revert demonstrations, each confirmed to discriminate: the floor writer reverted to a CEILINGS whitelist (only the unknown-key assert fails); a second consumer of the detected shape added (the uniqueness pin fires where `in` would have stayed green); a duplicated `child_argv` gates call site (same, in test_verify_coverage_mode); and the ratchet union removed outright (count 0). 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.
The defect
.verify-floor.json'sskipped_max= 26 is, by the floor file's own note, "exactly what a machine with none of this instance's local prerequisites skips (a GitHub runner: no agent CLIs, no~/.codex/skills, no/Applications/ChatGPT.app, no populated capability ledger)".That same number was also applied to the exec mirror, which is the opposite deprivation: every local prerequisite is present (it exists only on the machine the system runs on) but it is a flat file copy, so it is not a git repository and has no
.github/. It skips 31 tests a runner skips none of:.github/workflowsabsentrepo_files_absent(...)tests/test_ci_gate_config.pygit_repo_absent()tests/test_repo_artifact_hygiene.pySo
python3 verify.pyfrom~/.codex/orchestrator-mirrorwas RED on every input, correct trees included — and CLAUDE.md §1 makes that run the verdict ("cmp-clean is not agreement"), so the one instrument that catches cross-tree divergence had stopped protecting anything. A gate red whatever you do is a gate that gets ignored, then switched off.Measured, not inferred. Unmodified main:
471 passed, 0 failed, 31/26 max skipped (502 collected; floor 502), sole problemCEILING exceeded. Syncing the previous main into a scratch mirror gave the same 31, so the breach is structural, not caused by any PR.Why not just raise
skipped_maxto 31CI runs on a runner where 26 is the measured bound. Raising it would hand the runner five units of slack — one gate going quiet to un-stick another. Each shape gets its own agreed number, measured where it is enforced: latched-gate question 3 taken literally, the measuring window equal to the draining window.
The change
env_prereq.exec_mirror_shape()detects the tree — no.github/and git reports no repository. The PREREQUISITE, never$CI, exactly as every other detector there does. Both marks required, because the mirror number is the looser one, so an ambiguous tree falls back to the base agreement. It is the same two detectors that cause the skips, so there is no second literal to drift.verify.mirror_key()derives the floor key from the base key — one rule consumed by both the check and--update-floor, so any ceiling gets amirror_variant for free and an unset or misspelt one falls back to base (the strict direction). The mirror value replaces the base rather than adding to it: 26+31 would buy 26 units of headroom nothing on the mirror can legitimately spend._ceiling_report()returns the verdict and the count/limit rendering from one shape decision. They reached it separately in my first draft, which is the mypy-ratchet defect exactly (the line saidNOT COUNTEDwhile the enforcement cleared) — caught because a break→revert demo failed to fire, the pinned fragment matching at both wiring points._preserved_ceilings()carries both shapes through--update-floor.ci.yml's reconcile job rewrites this file on every push to main from a checkout, where the mirror keys are never read — a writer keeping only the keys it used would have deletedmirror_skipped_maxand re-latched the mirror silently, as a bot commit nobody reads.31/31 max [mirror_skipped_max]— the key named whenever it is not the base one, so a mirror reader is never sent to raise the number CI depends on.Deliberately not asserted: that
mirror_skipped_max >= skipped_max. It holds today (31 vs 26) by coincidence — the two populations are disjoint, not nested — and the drain named in the floor note (teachorch-sync-mirror.shto copy.github/, dropping 12) would take it to 19, so that assertion would refuse the drain it exists to encourage. The reasoning is left in place beside the omission.Verification
python3 src/verify.py, green from both trees, measured on the rebase ontob80e6e4:502 passed, 0 failed, 0/26 max skipped (502 collected; floor 502), 87/87 selftests, 5/5 gates471 passed, 0 failed, 31/31 max [mirror_skipped_max], 87/87 selftests, 5/5 gates, VERIFIEDThe mirror was synced with
ORCH_MIRROR=<tmp>so the live mirror is untouched — it still needs the manualorch-sync-mirror.shafter merge, per CLAUDE.md §1.collectedstays 502 in both and is unchanged: the new coverage lives inverify.py's andenv_prereq.py's selftests, which pytest does not collect.Six deliberate-break→revert demonstrations, each confirmed to discriminate:
AND→ORin the shape detector;ceiling_limitignoring the mirror key;_preserved_ceilingsdropping it; the renderer reaching the shape separately;verify()hardcodingmirror=False(the single wiring pin — smallest fragment, split so it cannot match its own line, verified stable under black 26.5.1, which reflowed the call and left the fragment intact); and the real floor file losing the key.Formatted and linted at the pinned CI versions (black 26.5.1, ruff 0.16.4, isort 8.0.1, mypy 2.3.1 — clean, no new exemptions).
🤖 Generated with Claude Code