chore(typing): promote pyright reportUnknown* warnings to errors (#165)#166
Merged
Conversation
pyrightconfig.json was running typeCheckingMode=strict but explicitly downgraded reportUnknownVariableType and reportUnknownArgumentType to warnings, hiding 116 latent type-information gaps. This restores the strict-mode default and adds explicit annotations / casts / TypedDicts to fix every warning that surfaced. Net effect: - pyrightconfig.json: drop the two warning overrides - ~30 files: add list/dict/tuple element type annotations, dataclass field type annotations, cast() at boundary calls, TypedDict where dict[str, Any] was load-bearing - No logic changes, no new helpers, no refactors Tests still pass (sandbox-known port-bind exception remains). Closes #165
Follow-up to f1fcfab. The previous commit accidentally: - Stripped two `# noqa: BLE001` comments from intentional broad `except Exception:` blocks (daemon/src/daemon/app.py speech playback, memory/review/commands.py mem0 export retry path) → ruff lint failed - Left five files unformatted: audit/local_sink.py, memory/mem0_bridge/backfill.py, memory/read/search.py, memory/retention/rollup_generator.py, memory/review/commands.py → ruff format --check failed Restoring the noqa markers and reformatting brings ruff format/check back to clean. pyright still 0/0/0 and mypy strict still passes. Refs #165 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough多数のモジュールで JSON / list / dict や dataclass/Pydantic の空リテラルに対し typing.cast と明示的な型付き default_factory を導入し、型を絞った。制御フロー・公開 API・振る舞いに変更はない。 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 OpenGrep (1.17.0)tests/memory/mem0_bridge/test_backfill.py┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m �[1m Loading rules from local config...�[0m 🔧 Semgrep (1.157.0)tests/memory/mem0_bridge/test_backfill.pyComment |
![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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@daemon/src/daemon/contracts/memory.py`:
- Line 38: The Field default_factory for matches uses an unnecessary cast:
replace the lambda cast pattern in the matches: list[MemoryMatch] =
Field(default_factory=lambda: cast("list[MemoryMatch]", [])) with a simple
default_factory=list to match other fields and remove complexity; update the
declaration for matches (type MemoryMatch) to use Field(default_factory=list)
and remove the cast/imports that become unused.
In `@daemon/src/daemon/memory/mem0_bridge/backfill.py`:
- Line 274: Replace the invalid Python2-style exception clause in backfill.py by
changing the line "except TypeError, ValueError:" to the Python3-compatible
tuple form "except (TypeError, ValueError):" so the file parses correctly;
locate the clause in daemon/src/daemon/memory/mem0_bridge/backfill.py (the
except block handling TypeError and ValueError) and update the syntax only,
preserving the existing handler body and indentation.
🪄 Autofix (Beta)
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ab1bec07-23b5-40fa-a536-0ed487b56c9b
📒 Files selected for processing (30)
daemon/src/daemon/adapters/camera/onvif_client.pydaemon/src/daemon/adapters/llm/event_parser.pydaemon/src/daemon/adapters/llm/openai_transport.pydaemon/src/daemon/app.pydaemon/src/daemon/audit/langfuse_export.pydaemon/src/daemon/audit/local_sink.pydaemon/src/daemon/audit/redactor.pydaemon/src/daemon/config/loader.pydaemon/src/daemon/contracts/adapters.pydaemon/src/daemon/contracts/archive.pydaemon/src/daemon/contracts/audio.pydaemon/src/daemon/contracts/config_docs.pydaemon/src/daemon/contracts/memory.pydaemon/src/daemon/core/logging.pydaemon/src/daemon/harness/tool_loop.pydaemon/src/daemon/harness/tool_registry.pydaemon/src/daemon/memory/mem0_bridge/backfill.pydaemon/src/daemon/memory/mem0_bridge/store.pydaemon/src/daemon/memory/read/search.pydaemon/src/daemon/memory/retention/rollup_generator.pydaemon/src/daemon/memory/review/commands.pydaemon/src/daemon/memory/write/embeddings.pydaemon/src/daemon/memory/write/extractor.pydaemon/src/daemon/memory/write/tool.pydaemon/src/daemon/pipeline/latency_metrics.pydaemon/src/daemon/security/websocket_auth.pydaemon/src/daemon/tools/evaluate_detection.pydocs/adr/decision-log.mddocs/adr/decisions/2026-04-08-promote-pyright-reportunknown-warnings-to-errors-and-fix-116-latent-issues.mdpyrightconfig.json
Address CodeRabbit findings on PR #166: - contracts/memory.py: simplify `Field(default_factory=lambda: cast("list[MemoryMatch]", []))` to `Field(default_factory=list[MemoryMatch])`. The generic-alias factory satisfies pyright strict without the redundant cast/lambda wrapper and removes the unused `cast` import. - memory/mem0_bridge/backfill.py: restore the parenthesized `except (TypeError, ValueError):` form. ruff 0.15.1's Python 3.14 formatter strips the parens to `except TypeError, ValueError:` — syntactically valid in 3.14 but unidiomatic. Re-parenthesizing with an `as _exc` binding (unused; required because parens-only is what ruff drops) keeps the conventional reading. Refs #165 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
daemon/src/daemon/memory/mem0_bridge/backfill.py (1)
267-275:⚠️ Potential issue | 🟡 MinorNone、float、Decimalの入力に対する明示的なテストケースを追加してください
Exception path（line 273-275）はすでに
test_float_or_none_with_unparseable_stringでテスト済みです。ただし以下のケースが不足しています：
- None 入力（line 268の早期リターンをテストする明示的なケースなし）
- float 入力（isinstance チェックでカバーされているが、テストケースなし）
- Decimal 入力（isinstance チェックでカバーされているが、テストケースなし）
各入力型に対して明示的なテストを追加してください。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@daemon/src/daemon/memory/mem0_bridge/backfill.py` around lines 267 - 275, Add explicit unit tests for the _float_or_none function to cover the missing input types: create tests named like test_float_or_none_with_none (assert _float_or_none(None) is None), test_float_or_none_with_float (assert _float_or_none(1.23) == 1.23), and test_float_or_none_with_decimal (use Decimal('1.23') and assert the return equals float(Decimal('1.23'))). Keep the existing test_float_or_none_with_unparseable_string unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@daemon/src/daemon/memory/mem0_bridge/backfill.py`:
- Around line 267-275: Add explicit unit tests for the _float_or_none function
to cover the missing input types: create tests named like
test_float_or_none_with_none (assert _float_or_none(None) is None),
test_float_or_none_with_float (assert _float_or_none(1.23) == 1.23), and
test_float_or_none_with_decimal (use Decimal('1.23') and assert the return
equals float(Decimal('1.23'))). Keep the existing
test_float_or_none_with_unparseable_string unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 02d8d229-274f-4320-98ef-b79920ac1dda
📒 Files selected for processing (2)
daemon/src/daemon/contracts/memory.pydaemon/src/daemon/memory/mem0_bridge/backfill.py
Address CodeRabbit outside-diff comment on PR #166: the existing suite covered bool, str, unparseable str, and int but left the None early-return and the float/Decimal isinstance branches without explicit assertions. Add three small parameterless tests so each input type has a named regression case. Refs #165 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/memory/mem0_bridge/test_backfill.py`:
- Around line 382-397: Add tests to cover special float values and Decimal→float
precision edge cases for _float_or_none: add a parametrized test (e.g.,
test_float_or_none_with_special_floats) that passes float('inf'), float('-inf'),
and float('nan') and asserts identity for infinities and NaN-preserving behavior
(use math.isnan for NaN check), and add a test (e.g.,
test_float_or_none_decimal_precision_loss) that converts a very high-precision
Decimal and asserts the result equals pytest.approx(float(high_precision));
reference the existing _float_or_none import in
tests/memory/mem0_bridge/test_backfill.py and reuse pytest.mark.parametrize and
math for the special-value assertions.
🪄 Autofix (Beta)
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0732206b-f464-4a36-affe-aff4035f32b4
📒 Files selected for processing (1)
tests/memory/mem0_bridge/test_backfill.py
…_float_or_none Address CodeRabbit nitpick on PR #166: extend the new _float_or_none suite with the special-value branches the implementation passes through unchanged: - ±inf via parametrized assert (round-trip equality) - NaN via math.isnan (NaN != NaN so direct compare is unsafe) - 30-digit Decimal via pytest.approx (verifies float() loss is bounded) Refs #165 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.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.
Summary
pyrightconfig.jsonwas runningtypeCheckingMode=strictwhile explicitly downgradingreportUnknownVariableTypeandreportUnknownArgumentTypetowarning, hiding 116 latent type-information gaps. This PR restores the strict-mode default and adds explicit annotations / casts to fix every warning that surfaced.Changes
pyrightconfig.json: drop the two warning overrides (now strict default =error)cast()at boundary calls# noqa: BLE001markers and reformatted 5 files in fixup commit (6cb1cab)Stats
pyrightconfig.jsoncast(...)additions: ~64castimports: 15Test plan
uv run ruff format --check daemon tests scripts— passuv run ruff check daemon tests scripts— passuv run mypy --strict daemon/src tests— passuv run pyright daemon/src— 0 errors, 0 warnings, 0 informationsuv run pytest tests/— 1230 passed, 11 deselected.claude/.simplify-donemarker createdadr_required: false)Why this matters
Strict mode is meaningless if individual checks are silently relaxed. The downgrade was a Trojan horse — it allowed the strict label to stay green while letting unknown-typed values flow through tool argument paths, audit redactor, harness loops, and more. Restoring the defaults makes future regressions visible at PR time instead of accumulating.
Closes #165
🤖 Generated with Claude Code
Summary by CodeRabbit
リリースノート
保守作業
ドキュメント
テスト