fix(typing): drain the last module to ZERO, and unlatch the ratchet gate that would have fired on it - #120
Merged
Merged
Conversation
…ate that would have fired on it
capability_advisor's 24 findings typed and the `[[tool.mypy.overrides]]` block removed
entirely. 99 of 99 modules checked; the campaign total is 240 findings -> 0 with ZERO net
`# type: ignore` added.
mypy ratchet: 0/0 max of 99 module(s) exempt, 99 checked — fully drained
THE LAST MODULE EXPOSED A LATCHED GATE IN THE RATCHET'S OWN REPORTING, and that is the part
worth keeping. `verify.mypy_exempt_modules()` returned None for TWO different situations --
"pyproject.toml unreadable" and "readable, no ignore_errors override" -- so the very run
that FINISHED the drain would have printed `mypy ratchet: NOT COUNTED`. The gate goes silent
at the exact moment it succeeds, which is the failure verify.py exists to prevent.
Three things show this was unintended rather than a choice:
* the function's OWN docstring already argues against it -- "a ratchet that stops being
counted is indistinguishable from one that emptied, and only one of those is good news";
* `_format_mypy_exempt_line` carries a " -- fully drained" branch that NO INPUT COULD
REACH, because [] was never returned;
* the selftest asserted the function was TRUTHY, so an empty list failed it. The ratchet's
own test forbade its drained state.
That is CLAUDE.md's latched-gate pattern exactly -- a gate whose clear path is blocked by the
thing it measures. Its three questions all PASS here (what decrements it: typing a module;
can that run while closed: yes; do the measuring and draining windows match: one list), which
is why it survived review: the latch was not in the ratchet's logic but in its REPORTING, and
it could only ever fire at zero. Worth remembering when applying that checklist -- a gate can
answer all three and still be unable to announce its own success.
Fixed: [] now means answered-and-empty, None stays genuinely unanswerable, and the selftest
asserts all THREE renderings (NOT COUNTED / fully drained / both-numbers) plus `is not None`
rather than truthiness. Break->revert: restoring `return None` fails with "pyproject.toml
does not parse; the ratchet cannot be counted".
THE MODULE DIFF, reviewed before applying, as with every batch:
* `out: list[Any] = {` was annotating a DICT literal -- a plainly false annotation, so
correcting it to dict[str, Any] changes nothing at runtime;
* `consult_target` already did `str(repository or "").strip()`, so widening the parameter
to `str | None` documents existing behaviour instead of adding a coercion;
* `now_types` now calls `classify_task` ONCE where the old `set(X and [...X...])` called it
twice -- equivalent because classify_task is pure ("Deterministic, order-stable", regex
over a constant table, no writes), and half the work;
* the `cap` -> `bound_cap` rename is complete within its block and every use is positional,
so it cannot repeat the earlier `repo` -> `repo_arg` regression that rewrote keyword
argument names at call sites;
* HOW_TO_USE's strings and `_selftest_how_to_use` were fenced off in the brief and are
untouched.
Also: this was the first offload in the campaign whose pytest step actually RAN (453
deselected). Every earlier agent reported "no module named tomllib" and fell back to
selftests, because a non-interactive login zsh on this machine resolves python3 to
/usr/bin/python3 (3.9.6) -- macOS path_helper puts /usr/bin ahead of anaconda, and the conda
init that fixes it lives in ~/.zshrc, which such a shell does not source. The brief now opens
with `export PATH="/opt/anaconda3/bin:$PATH"`. The launchd fleet is unaffected: it runs
/bin/bash -lc, and ~/.bash_profile does carry the prepend.
Verification: `python3 src/verify.py`
pytest: 453 passed, 0 failed, 0/26 max skipped (453 collected; floor 453)
selftests: 85 of 85 modules ran, 0/7 max skipped
mypy ratchet: 0/0 max of 99 module(s) exempt, 99 checked — fully drained
VERIFIED -- 453 tests actually executed and passed, 85 selftests spoke, 5 of 5 gates green
`collected` is unchanged at 453: the new coverage lives in verify.py's own selftest.
ruff and black clean at CI's settings.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…atever shape it takes Raised by CodeRabbit on PR #117 and worth doing. `validate_current_refs` declares `refs: list[dict]`, and its caller in capability_compiler used to pass `cast(dict, contract.get("current_refs") or {})` -- a cast naming the wrong type over a fallback of the wrong type. That PR corrected both to list/[]. WHY THIS NEEDED A TEST RATHER THAN JUST THE FIX. The old code was harmless in practice: a dict and an empty list both fail `not isinstance(refs, list) or not refs` and take the same branch, so the annotation was wrong while the behaviour was right. A coincidence holding two wrong things in agreement is precisely what should be pinned rather than trusted. The guard is load-bearing and the demonstration shows why: with `isinstance` removed, `{"path": "README.md"}` is ITERATED -- yielding the string key "path" -- and fails a DIFFERENT check ("current_refs[0] must contain path and optional symbol") instead of being rejected as malformed. So a dict would be silently processed as a list of its keys. The test asserts the exact error, not merely `valid is False`, which is what makes that distinction visible. Covers dict, populated dict, empty list, bare string and None, plus a well-formed case so the negative assertions cannot pass vacuously. repo_knowledge.py --selftest: OK. Break->revert demonstrated as described above. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 24 minutes. View limit detailsLimit details: You’ve used the included review currently available. Your 70 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 (5)
Comment |
stranske
temporarily deployed
to
agent-standard
August 25, 2026 01:16 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
Workflow source neededPR #120 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. |
Automated Status SummaryHead SHA: 30fc23f
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
added a commit
that referenced
this pull request
Aug 25, 2026
…"measured zero" (#121) * fix(gates): stop three gates from confusing "could not measure" with "measured zero" Follow-up to the ratchet latch (#120), from an audit of every gate in this tree that reports a blocking/drainable pair. Six were clean. Three were not, and one of them is the ratchet fix itself being only half done. ===== 1. verify.py — THE CEILING PASSED BY BEING BLIND ===== #120 fixed the ratchet's RENDERER. Its ENFORCEMENT still coerced the uncountable case to zero: # None (uncountable) must not read as 0 — that would let the ceiling pass by being blind. "mypy_exempt_max": len(exempt) if exempt is not None else 0, The comment states the rule the line below it breaks, which is worse than no comment: it tells a reader the case is handled. So with an unreadable pyproject.toml the run printed `mypy ratchet: NOT COUNTED` while `_ceiling_problems` returned [] — two readers of one quantity disagreeing, and the permissive one deciding the exit code. Verified by construction before fixing: renderer `NOT COUNTED`, ceiling `[]`. `None` is now carried through, and `_ceiling_problems` FAILS on an uncountable-but-bounded quantity ("CEILING UNCHECKABLE") instead of skipping it. An UNSET ceiling over an uncountable quantity stays fine — nothing was agreed there. THE RULE MOVED INTO A ONE-LINE PURE FUNCTION, and that is the load-bearing part. `_exempt_ceiling_input` exists because the first version of this fix PASSED ITS OWN BREAK->REVERT: the selftest asserted `_ceiling_problems` handles None, which proves the CHECKER is right and says nothing about what the checker is FED — and the feed was the broken half. Restoring the `else 0` coercion inline left every assertion green. Extracted, the same break now fails on `assert _exempt_ceiling_input(None) is None`. Same lesson as the unreachable " -- fully drained" branch one level up: a test that cannot fail on the defect is not coverage. ===== 2. verify.py — AN UNREADABLE FLOOR READ AS AN UNSET ONE ===== `load_floor()` returned {} both for "no floor recorded" and "recorded but does not parse", and `floor_state` rendered each as `unset`. So a corrupted .verify-floor.json looked like a repo that had never agreed a floor, and every count-based check silently stopped applying — the permissive direction, silently, which is the exact hole that file exists to close. Unreadable now carries FLOOR_UNREADABLE, renders as `UNREADABLE (.verify-floor.json exists but does not parse)`, and is a PROBLEM rather than a note. Absent stays absent. Wired on purpose: a marker nothing reads would be this repo's founding defect in miniature. ===== 3. backlog.py — A ZERO THAT COULD MEAN HEALTH OR BLINDNESS ===== `scoped_blocker_entries()` returns {} for FIVE situations and only one means "there are no blockers"; the rest are failures to measure, and all five rendered as `scoped_blockers_live: 0`. Scoped blockers are latched-gate instance #2, where stale ones emptied the fleet backlog for 78 days, so an ambiguous zero here has an expensive history. Found by chasing the one UNSURE from the silent-empty triage: `handoff.sh:127` validates the sentinel with `jq -e .`, which accepts ANY valid JSON including a bare array — so a structurally wrong sentinel is neither reinitialised by the writer nor reported by the reader. `scoped_blocker_source()` now names it: ok / absent / unreadable / unparseable / wrong_shape, published beside the counts. Behaviour deliberately UNCHANGED — every case still yields {} so work proceeds (fail toward motion). Only the silence speaks. ===== 4. ux_review.py — UNREACHABLE ON PURPOSE, SO SAY SO ===== The audit flagged `without_base_sha: 0` as unreachable for a nonempty registration set. It is, and that is CORRECT: `discover_historical_panels` marks every panel `base_sha_unrecoverable` because none recorded the commit under review, and inferring one from today's checkout would fuse two states of one app into a single subject (CLAUDE.md §2; the selftest already asserts "must never borrow today's HEAD"). So no behaviour changed. What was missing is that the OUTPUT did not say the number is a permanent floor rather than a backlog — and the obvious way to "drain" a drainable-looking number here is to invent a SHA, which corrupts provenance. The report now states it, and the selftest asserts the statement. "It cannot reach zero, deliberately, and here is why" is a valid answer to the new fourth latched-gate question; "it cannot, and nobody noticed" is not, and the two must not look alike. ===== ALSO ===== The global rule and skill gained that fourth question and instance #9 (machine-local files, not in this diff): "What does it PRINT when fully drained — and has any input ever produced that output?" Questions 1-3 interrogate a gate's logic and instance #9 passed all three; the latch was in its voice. Two mechanical tells: a rendering branch no input can reach, and a test asserting TRUTHINESS on a countable quantity, which forbids zero. Break->revert demonstrated on all four: coercing None back to 0 fails the feed assertion; returning {} for an unreadable floor fails "unreadable must be distinguishable from absent"; collapsing wrong_shape into ok fails the sentinel-source assertion; dropping the ux_review permanence field fails its assertion. Verification: `python3 src/verify.py` pytest: 458 passed, 0 failed, 0/26 max skipped (458 collected; floor 458) selftests: 85 of 85 modules ran, 0/7 max skipped mypy ratchet: 0/0 max of 99 module(s) exempt, 99 checked — fully drained VERIFIED -- 458 tests actually executed and passed, 85 selftests spoke, 5 of 5 gates green mypy clean over all 99 modules; ruff and black clean at CI's settings. `collected` unchanged at 458 — the new coverage is in module selftests, which pytest does not collect. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(autofix): formatting/lint --------- Co-authored-by: Tim Stranske <tim@stranskemo.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.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.
Finishes the campaign.
capability_advisor's 24 findings typed and the[[tool.mypy.overrides]]block removed entirely.Campaign total across #93/#95/#98/#99/#109/#117 and this PR: 240 findings → 0, 64 → 0 exempt modules, with zero net
# type: ignoreadded.The last module exposed a latched gate in the ratchet's own reporting
This is the part worth keeping.
verify.mypy_exempt_modules()returnedNonefor two different situations — "pyproject.toml unreadable" and "readable, noignore_errorsoverride" — so the very run that finished the drain would have printedmypy ratchet: NOT COUNTED. The gate goes silent at the exact moment it succeeds, which is the failureverify.pyexists to prevent.Three things show it was unintended rather than a choice:
_format_mypy_exempt_linecarries a" — fully drained"branch that no input could reach, because[]was never returned;That is CLAUDE.md's latched-gate pattern exactly — a gate whose clear path is blocked by the thing it measures. Note that its three questions all pass here (what decrements it: typing a module; can that run while closed: yes; do the measuring and draining windows match: one list). The latch was not in the ratchet's logic but in its reporting, and it could only ever fire at zero. Worth remembering when applying that checklist: a gate can answer all three questions correctly and still be unable to announce its own success.
Fixed so
[]means answered-and-empty whileNonestays genuinely unanswerable, with the selftest asserting all three renderings (NOT COUNTED/fully drained/ both-numbers) andis not Nonerather than truthiness. Break→revert: restoringreturn Nonefails withpyproject.toml does not parse; the ratchet cannot be counted.The module diff, reviewed before applying
out: list[Any] = {was annotating a dict literal — a plainly false annotation, so correcting it todict[str, Any]changes nothing at runtime.consult_targetalready didstr(repository or "").strip(), so widening the parameter tostr | Nonedocuments existing behaviour rather than adding a coercion.now_typesnow callsclassify_taskonce where the oldset(X and [...X...])called it twice — equivalent becauseclassify_taskis pure ("Deterministic, order-stable", regex over a constant table, no writes), and half the work.cap→bound_caprename is complete within its block and every use is positional, so it cannot repeat the earlierrepo→repo_argregression that rewrote keyword argument names at call sites.HOW_TO_USE's strings and_selftest_how_to_usewere fenced off in the brief and are untouched.Also here
A regression test for malformed
current_refs, raised by CodeRabbit on #117 and fair.validate_current_refsdeclaresrefs: list[dict]while its caller passedcast(dict, ... or {})— the annotation was wrong and the behaviour was right, because a dict and an empty list both fail the same guard. A coincidence holding two wrong things in agreement is exactly what should be pinned. With theisinstanceremoved,{"path": "README.md"}is iterated into its string keys and fails a different check instead of being rejected — so the test asserts the exact error, not merelyvalid is False.The ratchet immediately caught that new test, which is the design working on its author: with
repo_knowledgeno longer exempt, the heterogeneous fixture tuple failed withNeed type annotation for "_bad". Annotatedtuple[Any, ...].Verification
collectedis unchanged at 458: the new coverage lives in module selftests, whichverify.pyruns as subprocesses and pytest does not collect. ruff and black clean at CI's settings.🤖 Generated with Claude Code