Skip to content

fix(typing): drain 21 modules off the mypy exempt list (64 -> 43) - #93

Merged
stranske merged 1 commit into
mainfrom
claude/mypy-drain-batch-1
Aug 24, 2026
Merged

fix(typing): drain 21 modules off the mypy exempt list (64 -> 43)#93
stranske merged 1 commit into
mainfrom
claude/mypy-drain-batch-1

Conversation

@stranske

Copy link
Copy Markdown
Owner

Batch one of the mypy campaign.

before after
exempt modules 64 43
findings 455 430
modules checked 35 of 99 56 of 99

Targeted the modules with 1–3 findings on purpose. Fixing 50 errors spread across dispatcher (65) and capability_propensity (51) would move the exempt list by zero — and the exempt list is the number the gate reads. The cheap tail is where the leverage is.

Four of the first findings were mine

The Any annotations I added in the previous commit referenced an unimported name in backlog, judge_reliability, watch, features — and later capability_admission, capability_advisor. No runtime impact (all carry from __future__ import annotations, so annotations are never evaluated), but real mypy errors the exempt list was hiding. That's the argument for shrinking the list rather than trusting it.

Real fixes, not casts to quiet a checker

  • consumer_sync_shadowno-redef was pointing at a genuine hazard: two different targets in one function, a str in the entries loop and an optional in the removals loop. My first attempt renamed only the binding and left four later uses resolving to the other loop's stale value — a bug I introduced, which the tests caught. All five uses now renamed together.
  • ux_reviewprocs declared its middle element as object, which is what made every out.write/flush/close unreachable to the checker. Typed as TextIO; the # type: ignore probe I used along the way was removed, not left in.
  • redirect_applylambda *a, **k: spent.append(a) or {} is a deliberate record-then-yield idiom that reads as a bug, since append returns None so the or branch always wins. Same behaviour as a named function that says so.
  • gh_capacity — the ledger row builder returns None when headers carry no rate-limit data, and that None was being appended. Skipped, rather than widening the ledger's type.
  • partitioned_review — a missing assertion_key was appended as None. Detection is preserved (the very next line already flags "not in the adjudication queue"); what goes away is a spurious duplicate report when two decisions both lacked the key.
  • synthesis_promotion — one branch coerced merged with bool() and its sibling six lines up did not. Matched.
  • evidence_acquisition — the parameter is a mapping of env vars, and os.environ is a Mapping[str, str] not a dict; typing it as what the function actually accepts removed both findings.

The report can now see its own subject

With the exempt list in place, scripts/ci_lint_baseline.py reported typecheck-mypy 0 errors — true at the gate and useless for tracking a drain, because the 430 behind the list vanished from the only report that counts them. That's the "green while checking nothing" shape at the reporting layer.

It now measures a second time against a copy of pyproject.toml with the override stripped — a copy, so a crash mid-measure can't leave the real gate config edited — and prints both:

typecheck-mypy: 0 at the gate, but 430 finding(s) remain behind 43 per-module
`ignore_errors` exemption(s).
  That list may only SHRINK — `mypy_exempt_max` in .verify-floor.json fails if it grows.

mypy_exempt_max lowered 64 → 43. Lowering it is the drain; it may never be raised without naming the module and why.

Test gate

python3 src/verify.py442 passed, 0 failed, 0 skipped, 85/85 selftests, 43/43 can-fire, 5/5 gates. ruff check . and black --check --line-length 100 clean. Every touched module re-imported and its selftest run; test_partitioned_review and test_consumer_sync_shadow run specifically, since those two fixes touched validation semantics.

What's left

430 findings across 43 modules, dominated by arg-type (109) and index (104), concentrated in dispatcher (65), capability_propensity (51), runtime_ac_gate (34), capabilities (32). Those are per-module campaigns rather than a tail — next batches should take one big module at a time so each one leaves the list.

🤖 Generated with Claude Code

… of 99 now checked

Batch one of the campaign, targeting the modules with 1-3 findings ON PURPOSE. Fixing 50 errors
spread across `dispatcher` (65) and `capability_propensity` (51) would move the exempt list by
ZERO, and the exempt list is the number the gate reads — so the cheap tail is where the leverage is.

  exempt modules 64 -> 43   |   findings 455 -> 430   |   checked 35 -> 56 of 99

FOUR OF THE FIRST FINDINGS WERE MINE, and worth naming: the `Any` annotations I added in the
previous commit referenced an unimported name in `backlog`, `judge_reliability`, `watch`, `features`
(and later `capability_admission`, `capability_advisor`). No runtime impact — all carry
`from __future__ import annotations`, so annotations are never evaluated — but real mypy errors that
the exempt list was HIDING. That is the argument for shrinking the list rather than trusting it.

REAL FIXES, not casts to make a checker quiet. Representative:
  * `consumer_sync_shadow` — `no-redef` was pointing at a genuine hazard: TWO different `target`s in
    one function, a `str` in the entries loop and an optional in the removals loop. My first attempt
    renamed only the binding and left four later uses resolving to the OTHER loop's stale value — a
    bug I introduced and the tests caught. All five uses now renamed together.
  * `ux_review` — `procs` declared its middle element as `object`, which is what made every
    `out.write/flush/close` unreachable to the checker. Typed as `TextIO`; the earlier
    `# type: ignore` probe was removed rather than left in.
  * `redirect_apply` — `lambda *a, **k: spent.append(a) or {}` is a deliberate record-then-yield
    idiom that reads as a bug, because `append` returns None so the `or` branch always wins. Same
    behaviour as a named function that says so.
  * `gh_capacity` — the ledger row builder returns None when headers carry no rate-limit data, and
    that None was being appended. Skipped instead of widening the ledger's type.
  * `partitioned_review` — a missing `assertion_key` was appended as None. Detection is preserved
    (the very next line already flags "not in the adjudication queue"); what goes away is a spurious
    DUPLICATE report when two decisions both lacked the key.
  * `synthesis_promotion` — one branch coerced `merged` with `bool()` and its sibling six lines up
    did not. Matched.
  * `evidence_acquisition` — the parameter is a mapping of env vars and `os.environ` is a
    `Mapping[str, str]`, not a `dict`; typing it as what the function accepts removed both findings.

AND THE REPORT CAN NOW SEE ITS OWN SUBJECT. With the exempt list in place
`scripts/ci_lint_baseline.py` reported `typecheck-mypy 0 errors` — true at the gate and useless for
tracking a drain, because the 430 behind the list vanished from the only report that counts them.
It now measures a second time against a COPY of pyproject.toml with the override stripped (a copy,
so a crash mid-measure cannot leave the real gate config edited) and prints both:

    typecheck-mypy: 0 at the gate, but 430 finding(s) remain behind 43 per-module
    `ignore_errors` exemption(s).

`mypy_exempt_max` lowered 64 -> 43. Lowering it IS the drain; it may never be raised without naming
the module and why.

Verified: 442 passed, 0 failed, 0 skipped, 85/85 selftests, 43/43 can-fire, 5/5 gates. `ruff check .`
and `black --check --line-length 100` clean. Every touched module re-imported and its selftest run.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 24, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 14 minutes.

View limit details

Limit details: You’ve used the included review currently available. Your 73 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.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: a7d62622-edbf-444a-9660-cc86cd616995

📥 Commits

Reviewing files that changed from the base of the PR and between dd9486d and fd1e2a0.

📒 Files selected for processing (29)
  • .verify-floor.json
  • pyproject.toml
  • scripts/ci_lint_baseline.py
  • src/adversarial.py
  • src/backlog.py
  • src/cadence_registry.py
  • src/capability_admission.py
  • src/capability_advisor.py
  • src/capability_targets.py
  • src/consumer_sync_shadow.py
  • src/epic_lane.py
  • src/evidence_acquisition.py
  • src/execution_profiles.py
  • src/exploration_evidence_plan.py
  • src/exploration_review.py
  • src/features.py
  • src/gh_capacity.py
  • src/human_calibration.py
  • src/issue_quality.py
  • src/judge_reliability.py
  • src/keepalive_shadow.py
  • src/ledger_reconcile.py
  • src/partitioned_review.py
  • src/redirect_apply.py
  • src/redirect_shadow.py
  • src/strategy_experiment.py
  • src/synthesis_promotion.py
  • src/ux_review.py
  • src/watch.py

Comment @coderabbitai help to get the list of available commands.

@agents-workflows-bot

Copy link
Copy Markdown
Contributor

Workflow source needed

PR #93 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:

  • Add <!-- meta:issue:123 --> or a normal Closes #123 / Related to #123 line.
  • Check one Workflow Source option in the PR body.
  • Add a hidden marker such as <!-- workflow-source:local_request -->, <!-- workflow-source:manual_remote -->, <!-- workflow-source:review_followup -->, <!-- workflow-source:sync_campaign -->, or <!-- workflow-source:dependabot -->.
  • Add a workflow source label such as workflow:source-direct-pr, workflow:source-local-request, workflow:source-review-followup, workflow:source-sync, or workflow:no-automation.

Once a valid source is present, this warning will not be reposted.

@stranske
stranske merged commit 4ff9fec into main Aug 24, 2026
17 checks passed
@stranske
stranske deleted the claude/mypy-drain-batch-1 branch August 24, 2026 02:55
stranske pushed a commit that referenced this pull request Aug 24, 2026
THE DEFECT. main has no branch protection, and `gh pr checks` lists what DID
report -- so a check that never started is not red, it is missing, and a PR
with no Gate reads exactly like a PR whose Gate passed. Silence
indistinguishable from success: this repo's founding defect, twice over.
2026-08-23, five python-ci jobs died at a shared install step and #61/#64/#65
merged with all five red. 2026-08-24, #90's Gate run was held at
`action_required` with ZERO jobs and merged with no lint, no format and no
typecheck, landing six F821s found only because somebody ran ruff by hand.

THE CAUSE WILL BE DIFFERENT NEXT TIME, so nothing here models holds. A check
can vanish to a hold, a cancellation, a deleted or renamed workflow, a rate
limit, a mistaken path filter or a GitHub incident. All present identically to
whoever is merging. scripts/check_checks_reported.py asks only: did every check
that NORMALLY reports also report here.

TWO SIMPLER DESIGNS WERE TRIED AND REJECTED BY REAL DATA, both recorded in the
file because the next person will reach for them:
  * one reference PR -- failed on the actual incident. #90 had no Gate, and the
    newest merged PR (#93) had no Gate checks either, so #90 was declared
    healthy. The hold had already swallowed the yardstick.
  * the union across recent merges -- caught #90 (21 absent) but reported 25-26
    absences on entirely healthy PRs, sweeping in event-driven checks. A test
    that cries wolf 25 times gets waived.
Frequency (>=75% of 12 merged PRs) discriminates: #90 exit 1 with 10 absent,
be a second copy of the CI topology.

NOT BRANCH PROTECTION, deliberately. A required check that is HELD never
reports, so the PR could never merge -- the clear path blocked by the very
thing the gate measures. On a solo-maintained repo "unverified but movable"
beats "permanently stuck".

pr-00-gate.yml also gains `push: [main]`. A held PR run cannot be fixed from
inside CI, but the silence AFTER the merge can: #90's F821s would have gone red
on main within one run instead of never. Fail toward noise.

docs/ABSENT_CHECK_LANE_WIRING.md carries the one step this repo cannot land --
the closer's pre-merge call and the prerun `--sweep` line, since the lane TOMLs
live outside any repository. Same shape as docs/MIRROR_SYNC_PATCH.md.

Already found a live one: --sweep flags open PR #91 with 10 absent checks,
which I would have merged on a green-looking list.

Verified: 446 passed, 0 failed, 0 skipped, floor 446, 85/85 selftests, 5 of 5
gates, mypy Success, ruff and black clean. Break -> revert: removing the
max(2, ...) threshold floor fails test_the_threshold_never_falls_to_one;
reverted byte-identical.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
stranske added a commit that referenced this pull request Aug 24, 2026
* feat(ci): an absent check is a red, whatever made it absent

THE DEFECT. main has no branch protection, and `gh pr checks` lists what DID
report -- so a check that never started is not red, it is missing, and a PR
with no Gate reads exactly like a PR whose Gate passed. Silence
indistinguishable from success: this repo's founding defect, twice over.
2026-08-23, five python-ci jobs died at a shared install step and #61/#64/#65
merged with all five red. 2026-08-24, #90's Gate run was held at
`action_required` with ZERO jobs and merged with no lint, no format and no
typecheck, landing six F821s found only because somebody ran ruff by hand.

THE CAUSE WILL BE DIFFERENT NEXT TIME, so nothing here models holds. A check
can vanish to a hold, a cancellation, a deleted or renamed workflow, a rate
limit, a mistaken path filter or a GitHub incident. All present identically to
whoever is merging. scripts/check_checks_reported.py asks only: did every check
that NORMALLY reports also report here.

TWO SIMPLER DESIGNS WERE TRIED AND REJECTED BY REAL DATA, both recorded in the
file because the next person will reach for them:
  * one reference PR -- failed on the actual incident. #90 had no Gate, and the
    newest merged PR (#93) had no Gate checks either, so #90 was declared
    healthy. The hold had already swallowed the yardstick.
  * the union across recent merges -- caught #90 (21 absent) but reported 25-26
    absences on entirely healthy PRs, sweeping in event-driven checks. A test
    that cries wolf 25 times gets waived.
Frequency (>=75% of 12 merged PRs) discriminates: #90 exit 1 with 10 absent,
be a second copy of the CI topology.

NOT BRANCH PROTECTION, deliberately. A required check that is HELD never
reports, so the PR could never merge -- the clear path blocked by the very
thing the gate measures. On a solo-maintained repo "unverified but movable"
beats "permanently stuck".

pr-00-gate.yml also gains `push: [main]`. A held PR run cannot be fixed from
inside CI, but the silence AFTER the merge can: #90's F821s would have gone red
on main within one run instead of never. Fail toward noise.

docs/ABSENT_CHECK_LANE_WIRING.md carries the one step this repo cannot land --
the closer's pre-merge call and the prerun `--sweep` line, since the lane TOMLs
live outside any repository. Same shape as docs/MIRROR_SYNC_PATCH.md.

Already found a live one: --sweep flags open PR #91 with 10 absent checks,
which I would have merged on a green-looking list.

Verified: 446 passed, 0 failed, 0 skipped, floor 446, 85/85 selftests, 5 of 5
gates, mypy Success, ruff and black clean. Break -> revert: removing the
max(2, ...) threshold floor fails test_the_threshold_never_falls_to_one;
reverted byte-identical.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(ci): degrade the absent-check sweep gracefully on a rate limit

The sweep runs every lane round from handoff-prerun.sh. Dumping GitHub's full
rate-limit paragraph hourly would train the reader to skip the whole section,
which is how a health report stops being read. One line instead, and it says
UNKNOWN rather than implying clean -- a reporter that cannot report must not
read as a clean bill of health.

Found by testing the prerun block while genuinely rate-limited, which is the
degraded path I would otherwise have had to simulate.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(ci): ratchet the expected-check set so a sustained outage cannot erode it

FOUND BY DOGFOODING, not by review. Running --pr 91 before merging it, the check
PASSED -- and should not have. While pr-00-gate.yml sat held, every newly merged
PR merged WITHOUT the Gate, so after twelve such merges the Gate's checks no
longer appeared on 75% of the reference window, stopped counting as "normally
reporting", and their absence stopped being flagged. The expected set fell
23 -> 14 names and #91 was pronounced clean by the tool written to catch exactly
that.

A SUSTAINED outage is the case that matters most, and it was the one case the
frequency rule could not see. The erosion test already in this file covered only
PRs that reported NOTHING; a PR reporting some checks but not the Gate's slid
straight through.

config/expected-checks.json is now the high-water mark, seeded from PRs #87/#89
whose Gate demonstrably ran (33 names, 8 of them python-ci). A name that has ever
been expected stays expected until somebody DELETES ITS LINE -- a visible act in
a diff. Same shape as pyproject.toml's mypy exempt ratchet, and the same reason:
an automatic downward move is indistinguishable from the defect.

--update-ratchet raises it and never lowers it.

With the ratchet, PR 91's head correctly reports 19 absences including the whole
python-ci set. PR 89 (Gate ran) reports one, "guard", which is a TRUE positive:
agents-guard.yml was already held by then.

Break -> revert: returning the observed set instead of its union with
ratchet_names() fails test_the_ratchet_is_wired_into_the_expected_set;
byte-identical after.

448 passed, floor 448, 85/85 selftests, 5 of 5 gates, mypy Success, ruff and
black clean.

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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant