chore(typing): scope unresolved-import per-file, fix a confidence OverflowError, retract three doc claims - #439
Merged
Conversation
added 3 commits
August 7, 2026 20:54
Three changes to the ty config, all from review of the gate that landed in Jawafdehi#428. `unresolved-import` was off tree-wide. That bought silence about typos and little else: a misspelled module (`from cases.serialzers import ...`) resolves to nothing, is invisible to both gates under a tree-wide ignore, and blows up at import time in whatever environment first exercises that path. It is now ON, with four named files scoped out in two override blocks — two genuinely-optional deps absent from `uv sync`'s venv (cloudpathlib, playwright), and two django-stubs re-export gaps that resolve at runtime (`django.template.loader.engines`, `django.test.testcases.DatabaseOperationForbidden`). The blocks are separate because their exit conditions differ: the stub gaps should be re-checked on every django-stubs bump, the optional deps are permanent. Kept separate from the tests block on purpose, even though both stub-gap files are tests it already matches — that block turns rules off for every test in the tree, whereas naming two files keeps a misspelled import in any OTHER test a failure. The pre-commit ty hook now also fires on `uv.lock`. The lockfile pins ty itself plus every dep whose types it reads, so a dependency bump changes what the checker is and what it knows while touching no `.py` file. Finally, the whole-tree diagnostic totals in the header comment are gone, replaced by the command that reproduces them and the two traps in that recipe (`-c "overrides=[]"` silently no-ops; a 0 from a forced-on rule is indistinguishable from a path that was never analysed). Three of those totals were already wrong. The counts scoped to an explicit `include` list stay — they move when someone edits the list beside them. Verified per-file rather than tree-wide with a deliberate-error probe at two sites: the exempt import stays silent, a fresh bad import in the same file is still reported, and the same bad import elsewhere fails the gate.
`float()` on a large enough int raises OverflowError, which is an
ArithmeticError — not a TypeError and not a ValueError, so it was not in the
except tuple guarding the confidence coercion in `on_result`.
A JSON number has no size limit, so `json.loads` parses a few-hundred-digit
integer into an unbounded Python int and the conversion raises. Because
`jobs.queue.finalize` swallows whatever `on_result` raises, the outcome was a
DONE job with no proposal and no recorded reason — the single outcome this
module's docstring promises cannot happen.
The float spelling is a different case and already worked: JSON `1e400`
parses to `float("inf")`, converts fine, and is stopped by the range check
below. Both are now parametrized, so a fix aimed at one cannot regress the
other.
Verified the tests bite: reverting only the except tuple fails exactly the
two `10**400` cases with "int too large to convert to float". The `inf` case
passes either way by design, and the comment beside it says so.
Three things AGENTS.md asserted after Jawafdehi#428 that re-checking disproved. Each is corrected in place rather than deleted, because the same overstatement is in Jawafdehi#428's body and in its merged commit message, which cannot be edited. The `get_form` override was NOT a bug. AGENTS.md said the override silently dropped Django's `change`; it did not. `_changeform_view` passes `change` as a keyword, so a `**kwargs`-only override forwards it untouched — verified with a spy on `ModelAdmin.get_form`, which reports `change=True` for both the old and the new signature. Spelling the parameter out is a readability and typing change only. The "bugs ty found" bullet is now sorted by whether runtime behaviour actually changed, which separates the one real fix (`case_events.bus`) and the one live-path `None` (`case_scraper`) from the two annotation-only ones (`_die() -> NoReturn`, `build_convert_payload`). The one-warning invariant is conditional, not absolute. Untouched `main` gives 981 warnings in a fresh worktree, not 1. Root cause is not a regression: whitenoise warns `No directory at: .../staticfiles/` from every test that builds a Django handler, and `staticfiles/` is a gitignored `collectstatic` artifact, so a fresh clone does not have one. The count is low in a working tree because the directory happens to exist. Documented both where the baseline is described and where the invariant is stated, with a one-step way to reproduce it. `case_events/bus.py` cited that invariant as a tripwire for its own care around orphaned coroutines; that comment now rests on the orphaned-work argument, which stands on its own. The block ordinals went stale. The list identified override blocks as "THIRD" and "FOURTH", which stopped being true the moment blocks were inserted above them. Blocks are now identified by their opening comment.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
🛠️ Relevant configurations:
[config] enable_ai_metadata: False
custom_model_max_tokens: 200000
git_provider: github
output_relevant_configurations: True
model: openai/cx/gpt-5.5
ENABLE_AUTO_APPROVAL: True
custom_reasoning_model: False
fallback_models: ['openai/cx/gpt-5.4-mini']
is_auto_command: True
publish_output: True
publish_output_progress: True
progress_gif_url:
progress_gif_width: 48
verbosity_level: 0
use_extra_bad_extensions: False
log_level: DEBUG
use_wiki_settings_file: True
use_repo_settings_file: True
use_global_settings_file: True
extra_config_url:
disable_auto_feedback: False
ai_timeout: 120
response_language: en-US
repo_context_files: ['AGENTS.md']
repo_context_from_default_branch: True
repo_context_max_lines: 500
max_description_tokens: 500
max_commits_tokens: 500
max_model_tokens: 32000
model_token_count_estimate_factor: 0.3
patch_extension_skip_types: ['.md', '.txt']
allow_dynamic_context: True
max_extra_lines_before_dynamic_context: 10
patch_extra_lines_before: 5
patch_extra_lines_after: 1
cli_mode: False
large_patch_policy: clip
duplicate_prompt_examples: False
seed: -1
temperature: 0.2
ignore_pr_title: ['^\\[Auto\\]', '^Auto', '^Bump ', '^chore\\(deps\\)']
ignore_pr_target_branches: []
ignore_pr_source_branches: []
ignore_pr_labels: []
ignore_pr_authors: []
ignore_repositories: []
ignore_language_framework: []
restricted_mode: False
reasoning_effort: medium
enable_claude_extended_thinking: False
extended_thinking_budget_tokens: 2048
extended_thinking_max_output_tokens: 4096
claude_extended_thinking_models_override: []
extract_issue_from_branch: True
branch_issue_regex:
enable_custom_labels: False
[pr_reviewer] require_ticket_analysis_review: False
require_score_review: False
require_tests_review: True
require_estimate_effort_to_review: True
require_can_be_split_review: False
require_security_review: True
require_estimate_contribution_time_cost: False
require_todo_scan: False
publish_output_no_suggestions: True
persistent_comment: True
extra_instructions: Focus on: logic errors and edge cases; security/authz regressions; missing error handling;
Django/DRF correctness (migrations, N+1 queries, transaction/atomicity, serializer & permission gaps).
Do NOT comment on formatting, import order, or naming — ruff handles those in CI.
num_max_findings: 3
final_update_message: True
enable_review_labels_security: True
enable_review_labels_effort: True
require_all_thresholds_for_incremental_review: False
minimal_commits_for_incremental_review: 0
minimal_minutes_for_incremental_review: 0
enable_intro_text: True
enable_help_text: False
|
PR Code Suggestions ✨No code suggestions found for the PR. |
PR Agent Walkthrough 🤖Welcome to the PR Agent, an AI-powered tool for automated pull request analysis, feedback, suggestions and more. Here is a list of tools you can use to interact with the PR Agent:
(1) Note that each tool can be triggered automatically when a new PR is opened, or called manually by commenting on a PR. (2) Tools marked with [*] require additional parameters to be passed. For example, to invoke the |
|
Auto-approved PR |
![jawafdehi-pr-agent[bot]](https://avatars.githubusercontent.com/u/241969530?s=60&v=4){kind=small}
Contributor
📝 WalkthroughWalkthroughThe pull request updates ChangesType diagnostics workflow
Confidence validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@case_proposals/job_kind.py`:
- Around line 252-266: In the confidence parsing flow, assign
result.get("confidence") to a raw value and guard it to the accepted numeric
input types before calling float(), preserving the existing absent/invalid
handling path. Keep OverflowError in the exception handling around conversion so
oversized JSON integers still produce the documented failure outcome.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 461e61fe-fadd-4016-a12b-61bb64a4c7da
📒 Files selected for processing (6)
.pre-commit-config.yamlAGENTS.mdcase_events/bus.pycase_proposals/job_kind.pycase_proposals/tests/test_intent_job.pypyproject.toml
Comment on lines
+252
to
+266
| # `except` immediately below is what handles "absent", "not a number" and | ||
| # "too big to be a float", in one place. Narrowing first would just | ||
| # duplicate that branch. | ||
| # | ||
| # OverflowError is in the tuple because a JSON number is not bounded by | ||
| # what a float can hold. `json.loads` parses a few-hundred-digit integer | ||
| # into an unbounded Python int, and `float()` on that raises OverflowError — an | ||
| # ArithmeticError, so NOT covered by TypeError/ValueError. Without it a | ||
| # runaway integer escapes this handler, and because `jobs.queue.finalize` | ||
| # swallows whatever `on_result` raises, the result is a DONE job with no | ||
| # proposal and no recorded reason: the one outcome this module's docstring | ||
| # promises cannot happen. (A JSON `1e400` is different — it parses to | ||
| # float `inf`, converts fine, and is then caught by the range check below.) | ||
| confidence = float(result.get("confidence")) # ty: ignore[invalid-argument-type] | ||
| except (TypeError, ValueError): | ||
| except (TypeError, ValueError, OverflowError): |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
if rg -n 'float\(result\.get\("confidence"\)\)|ty: ignore\[invalid-argument-type\]' case_proposals/job_kind.py; then
echo "Un-narrowed confidence conversion remains." >&2
exit 1
fi
rg -n -C 3 'raw_confidence|float\(raw_confidence\)' case_proposals/job_kind.pyRepository: Jawafdehi/JawafdehiAPI
Length of output: 297
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## file stats"
wc -l case_proposals/job_kind.py
echo
echo "## relevant lines 220-290"
sed -n '220,290p' case_proposals/job_kind.py
echo
echo "## nearby helper definitions"
rg -n -C 4 'def _validation_failure|jobs\.queue\.finalize|result\.get\("confidence"\)' case_proposals/job_kind.pyRepository: Jawafdehi/JawafdehiAPI
Length of output: 5969
Narrow the external confidence value before conversion.
result.get("confidence") is type-unnarrowed from the model output. Add a guard for accepted input types before float(raw_confidence), then keep OverflowError handling for oversized JSON integers.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@case_proposals/job_kind.py` around lines 252 - 266, In the confidence parsing
flow, assign result.get("confidence") to a raw value and guard it to the
accepted numeric input types before calling float(), preserving the existing
absent/invalid handling path. Keep OverflowError in the exception handling
around conversion so oversized JSON integers still produce the documented
failure outcome.
Source: Coding guidelines
damo-da
approved these changes
Aug 7, 2026
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
2 participants
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.
User description
Follow-up to #428, which had already merged when these came in — hence a new PR
rather than an amend. Five items.
1.
unresolved-importis now ON, with four files scoped out in two overrideblocks instead of a tree-wide ignore. A tree-wide ignore bought silence about
typos:
from cases.serialzers import ...resolves to nothing, is invisible toboth gates, and blows up at import time. Two blocks, not one, because the exit
conditions differ — the optional deps (cloudpathlib, playwright) are permanent;
the django-stubs re-export gaps (
django.template.loader.engines,django.test.testcases.DatabaseOperationForbidden) should be re-checked on everystubs bump. Deliberately not folded into the tests block, which would turn the
rule off for every test in the tree. Verified per-file with a deliberate-error
probe: the exempt import stays silent, a fresh bad import in the same file is
still reported.
2.
OverflowErroron an unfloatable confidence.float()on a large enoughint raises
OverflowError— anArithmeticError, so not covered by the(TypeError, ValueError)guard incase_proposals.job_kind.on_result. JSONintegers are unbounded, and because
jobs.queue.finalizeswallows whaton_resultraises, the result was a DONE job with no proposal and no recordedreason: the one outcome that module's docstring promises cannot happen. The float
spelling already worked (
1e400→inf, caught by the range check); both areparametrized so a fix for one can't regress the other.
3. The ty pre-commit hook now fires on
uv.lock, which pins ty itself plusevery dep whose types it reads — a dependency bump changes what the checker is
while touching no
.pyfile.4. Whole-tree diagnostic totals in
pyproject.tomlare gone, replaced by thecommand that reproduces them plus the two traps in that recipe:
-c "overrides=[]"silently no-ops (TOML arrays don't replace), and a 0 from a forced-on rule is
indistinguishable from a path that was never analysed. Three of those totals were
already wrong. Counts scoped to an
include =list stay — they move when someoneedits the list beside them.
5. Three AGENTS.md claims retracted, not deleted, since the same wording is in
#428's body and merged commit message:
get_formoverride was not a bug._changeform_viewpasseschangeasa keyword, so a
**kwargs-only override forwards it untouched — verified with aspy on
ModelAdmin.get_form,change=Trueunder both signatures. The "bugs tyfound" bullet is now sorted by whether runtime behaviour changed.
maingives 981 in afresh worktree. Whitenoise warns
No directory at: .../staticfiles/from everytest that builds a Django handler, and
staticfiles/is a gitignoredcollectstaticartifact.case_events/bus.pycited that invariant as atripwire; it now rests on the orphaned-coroutine argument alone.
inserted above them; blocks are identified by opening comment now.
Gates:
ruff checkclean ·ty checkpasses · all 7 pre-commit hooks pass ·suite 4419 passed / 4 skipped vs 4416 / 4 on untouched main (+3 = the new cases).
Tests verified to bite: reverting only the except-tuple fails exactly the two
10**400cases.PR Type
Documentation, Bug fix, Tests
Description
Gate
unresolved-importper fileCatch huge confidence
OverflowErrorRun
tyonuv.lockCorrect warning/type-debt docs
Diagram Walkthrough
File Walkthrough
bus.py
Clarify publish coroutine failure rationalecase_events/bus.py
AGENTS.md
Update typing and warning guidanceAGENTS.md
staticfiles/unresolved-importpolicyjob_kind.py
Handle unfloatable confidence valuescase_proposals/job_kind.py
OverflowErrorfromfloat()test_intent_job.py
Cover confidence overflow rejection pathscase_proposals/tests/test_intent_job.py
infconfidence regression case.pre-commit-config.yaml
Trigger ty on lockfile changes.pre-commit-config.yaml
uv.locktotytriggertyexecutionpyproject.toml
Scope unresolved import exemptions preciselypyproject.toml
unresolved-importglobally🛠️ Relevant configurations:
These are the relevant configurations for this tool:
[config]
[pr_description]
Summary by CodeRabbit
Bug Fixes
Documentation