fix(typing): drain 4 more modules (exempt list 20 -> 16, 83 of 99 checked) - #99
Merged
Conversation
…ecked
Batch four: 338 -> 308 findings, exempt bound 20 -> 16.
THE LESSON OF THIS BATCH, and it cost four failing tests to learn properly:
**an ANNOTATION is not a behaviour change; a COERCION is.** Two of my fixes in `feedback.py` were
coercions dressed as type fixes, and both were wrong in the same way:
* `out = selector[role]` became `dict(selector[role])`. The function ends `return {"roles":
selector}`, so the loop mutates `selector[role]` THROUGH `out` — the copy silently discarded
every mutation. Now `out: dict[str, Any] = selector[role]`, which keeps the alias and satisfies
the checker. A coercion should never have been reached for.
* `validate_resolved_worker_model()` returns None for a REJECTED adapter tag, and the write
downstream must still see that None. Wrapping it `str(... or "")` turned a deliberate refusal
into an empty string and broke three provenance tests. Now a separate `validated_model: str |
None` binding, so the parameter stays `str` and the refusal survives.
`tests/test_feedback_model_provenance.py` (x3) and `tests/test_model_profile_trial.py` caught both,
plus `periodic_report`'s selftest. That is the fourth behaviour-changing "type fix" this campaign;
the rule going forward is to prefer annotating a binding and to treat any `dict()`/`list()`/`str()`
as a semantic edit needing its own justification.
AND ONE MORE OF MINE FROM BATCH 3: the `repo` -> `repo_arg` rename in `repo_knowledge` rewrote
KEYWORD ARGUMENT names too, producing `validate_agents_md_export(..., repo_arg=repo_arg)` against a
parameter still called `repo`. Caught by `call-arg`, fixed to `repo=repo_arg`.
Cleared this batch:
* `codemod_lane` — ten `X.strip()` reads of an untyped campaign payload. The validator's job is to
REJECT a bad shape, so `str(X or "").strip()` states what every call already assumed.
* `switch_review` — `stale_runners(now: int | None)` was simply wrong about what it accepts: the
body immediately does `float(now ...)` and four selftest calls pass floats. Signature corrected
rather than the callers coerced, plus the `env -> Mapping` propagation.
* `issue_readiness` — `normalize_title(title: str)` is called with an absent title in four places
INCLUDING its own selftest asserting `normalize_title(None) == ""`. The function always handled
it; the signature did not say so. Two deliberate selftest monkeypatches get narrow reasoned
ignores, and `BOT_AUTHOR` is annotated so the break->revert swap types.
* `feedback` — three heterogeneous `clean = {}` builders annotated, and `_effective`'s declared
`tuple[float, str]` widened to `tuple[float | None, str]` to match the optional median it
already forwarded.
`mypy_exempt_max` lowered 20 -> 16.
Verified: 442 passed, 0 failed, 0 skipped, 85/85 selftests, 43/43 can-fire, 5/5 gates.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 26 minutes. View limit detailsLimit 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. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (7)
Comment |
Contributor
Workflow source neededPR #99 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. |
#94 and #100 landed floor/gate work while this branch was open, conflicting only on .verify-floor.json. Resolved by KEEPING MAIN'S note and re-applying just the two facts this branch owns — the exempt bound (16) and the collected count measured on the MERGE RESULT (448, up from 442 because #94/#100 added tests). Re-verified on the result rather than assumed: 448 passed, 0 failed, 0 skipped, 85/85 selftests, 43/43 can-fire, 5/5 gates; mypy clean over 99 modules.
stranske
temporarily deployed
to
agent-standard
August 24, 2026 05:38 — with
GitHub Actions
Inactive
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 four: 338 -> 308 findings, exempt bound 20 -> 16, 83 of 99 modules checked.
The lesson of this batch, and it cost four failing tests to learn properly
An annotation is not a behaviour change. A coercion is. Two of my fixes in
feedback.pywere coercions dressed as type fixes:out = selector[role]becamedict(selector[role]). The function endsreturn {"roles": selector}, so the loop mutatesselector[role]throughout— the copy silently discarded every mutation. Nowout: dict[str, Any] = selector[role], which keeps the alias and satisfies the checker.validate_resolved_worker_model()returnsNonefor a rejected adapter tag, and the write downstream must still see that None. Wrapping itstr(... or "")turned a deliberate refusal into an empty string. Now a separatevalidated_model: str | Nonebinding.test_feedback_model_provenance(×3),test_model_profile_trialandperiodic_report's selftest caught both. That's the fourth behaviour-changing "type fix" this campaign — the rule going forward is to prefer annotating a binding, and to treat anydict()/list()/str()as a semantic edit needing its own justification.And one more of mine from batch 3: the
repo→repo_argrename inrepo_knowledgerewrote keyword argument names too, producingvalidate_agents_md_export(..., repo_arg=repo_arg)against a parameter still calledrepo. Caught bycall-arg.Cleared this batch
codemod_lane— tenX.strip()reads of an untyped campaign payload. The validator's job is to reject a bad shape, sostr(X or "").strip()states what every call already assumed.switch_review—stale_runners(now: int | None)was simply wrong about what it accepts: the body immediately doesfloat(now ...)and four selftest calls pass floats. Signature corrected rather than callers coerced.issue_readiness—normalize_title(title: str)is called with an absent title in four places including its own selftest assertingnormalize_title(None) == "". The function always handled it; the signature didn't say so.feedback— three heterogeneousclean = {}builders annotated;_effective's declaredtuple[float, str]widened to match the optional median it already forwarded.Test gate
python3 src/verify.py— 442 passed, 0 failed, 0 skipped, 85/85 selftests, 43/43 can-fire, 5/5 gates.What's left
308 findings across 16 modules. The five big ones (
capability_advisor66,dispatcher65,capability_propensity51,runtime_ac_gate34,capabilities32) hold 248; the rest are 11–18 apiece.🤖 Generated with Claude Code