fix(typing): drain the <=5-finding tail (exempt list 43 -> 26, 73 of 99 checked) - #95
Merged
Merged
Conversation
… 73 of 99 checked
Batch two. The cheap tail is now gone, so future batches take ONE big module at a time.
exempt modules 43 -> 26 | findings 430 -> 374 | checked 56 -> 73 of 99
THREE OF MY OWN FIXES WERE WRONG, and the suite caught each. Recording them because the pattern
matters more than the diffs:
1. `research_subjects` — I wrapped a bad-input probe as `domain_target(str(bad))`. That loop
deliberately passes None to prove it is REJECTED, and `str(None)` is the perfectly valid slug
"None", so the rejection never fired and the selftest failed on "blank slug accepted: None".
A type fix that changed behaviour. Reverted to a narrow `# type: ignore[arg-type]` WITH the
reason — which is what a deliberately-wrong-typed probe actually needs.
2. `dict(x)` / `list(x)` do not satisfy mypy for an `object`, and would add a real copy at
runtime. Replaced with `cast(...)`, which states the shape the surrounding code has already
validated and compiles to nothing.
3. Per-file `mypy src/X.py` is NOT the project run. Eleven modules I had confirmed clean per-file
still carried cross-module findings, which is why this batch cleared 17 modules rather than the
21 the tail projected. Measure with the project command.
WHERE AN IGNORE IS THE RIGHT ANSWER, it is narrow and it says why: `agent_auth_check` monkeypatches
`Path.read_text` inside its selftest to force the OSError path, which is exactly what `method-assign`
exists to flag and exactly where it is wrong. Three of those, each annotated. No error code is
disabled anywhere, and `test_mypy_config_silences_nothing_by_error_code` still forbids that.
REPEATED PATTERNS, fixed at the source rather than the symptom:
* `env: dict | None` where the argument is `os.environ` — that is a `Mapping[str, str]`, not a
dict. Four modules (`tick` x2 sites, `capability_opportunity`, `exploration_backfill`, after
`evidence_acquisition` in batch 1).
* `claims.holder()` returns None by design, and three selftests indexed straight through it.
Binding and asserting the holder EXISTS is a stronger check than the one-liner it replaced.
* heterogeneous dict literals whose FIRST entry pins a narrow value type, then conflict later —
annotated once at the declaration (`decision`, `out`, `checks` x2, `bucket`, `base_kwargs`,
`candidates`).
* two lambdas with loop-captured default args, which mypy cannot infer. The capture is
LOAD-BEARING, so it survives in the replacement functions' signatures.
`mypy_path` gained `tests`, so mypy can RESOLVE the recurrence-fixture roster that
`capability_admission` legitimately imports (declared via `paths.TESTS_DIR`). The check target stays
`src` — but mypy then follows into that one test file, and its two findings were FIXED rather than
excluded. `tick.py` also lost a dead `_unused = ([], [], ...)` tuple my first rewrite left behind.
`mypy_exempt_max` lowered 43 -> 26.
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's selftest run.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 47 minutes. View limit detailsLimit details: You’ve used the included review currently available. Your 74 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 (24)
Comment |
Contributor
Workflow source neededPR #95 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. |
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
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.
Batch two. The cheap tail is now gone — future batches take one big module at a time.
Three of my own fixes were wrong, and the suite caught each
Recording these because the pattern matters more than the diffs:
domain_target(str(bad)). That loop deliberately passesNoneto prove it's rejected — andstr(None)is the perfectly valid slug"None", so the rejection never fired and the selftest failed on "blank slug accepted: None". Reverted to a narrow# type: ignore[arg-type]with the reason, which is what a deliberately-wrong-typed probe actually needs.dict(x)/list(x)don't satisfy mypy for anobject— and they'd add a real copy at runtime. Replaced withcast(...), which states the shape the surrounding code already validated and compiles to nothing.mypy src/X.pyis not the project run. Eleven modules I'd confirmed clean per-file still carried cross-module findings — which is why this batch cleared 17 modules rather than the 21 the tail projected. Measure with the project command.Where an ignore is the right answer, it's narrow and says why
agent_auth_checkmonkeypatchesPath.read_textinside its selftest to force theOSErrorpath. That is exactly whatmethod-assignexists to flag, and exactly where it's wrong. Three of those, each annotated.No error code is disabled anywhere, and
test_mypy_config_silences_nothing_by_error_codestill forbids that.Repeated patterns, fixed at the source
env: dict | Nonewhere the argument isos.environ— that's aMapping[str, str], not a dict. Four modules (tick×2 sites,capability_opportunity,exploration_backfill, afterevidence_acquisitionin batch 1).claims.holder()returnsNoneby design, and three selftests indexed straight through it. Binding it and asserting the holder exists is a stronger check than the one-liner it replaced.decision,out,checks×2,bucket,base_kwargs,candidates).One config change, and its cost paid rather than dodged
mypy_pathgainedtests, so mypy can resolve the recurrence-fixture roster thatcapability_admissionlegitimately imports (declared viapaths.TESTS_DIR). The check target stayssrc— but mypy then follows into that one test file, and its two findings were fixed, not excluded.tick.pyalso lost a dead_unused = ([], [], ...)tuple my first rewrite left behind.Test gate
python3 src/verify.py— 442 passed, 0 failed, 0 skipped, 85/85 selftests, 43/43 can-fire, 5/5 gates.ruff check .andblack --check --line-length 100clean; every touched module's selftest run.mypy_exempt_maxlowered 43 → 26.What's left
374 findings across 26 modules, and it's now concentrated:
capability_advisor(66),dispatcher(65),capability_propensity(51),runtime_ac_gate(34),capabilities(32) hold ~248 of them. Those are five per-module campaigns; skimming is finished.🤖 Generated with Claude Code