chore(sonar): Tier 1 cleanup — 31 code smells across 5 rules#16
Merged
Conversation
Mechanical hygiene pass on Sonar's lower-effort code-smell rules.
Targeted: S1192 (literal duplicates), S2772 (unneeded pass), S5713
(redundant exception classes), S8409 (redundant FastAPI response_model
kwarg), S7503 (async-without-await false positives in framework code).
Changes per rule:
S1192 — extract module-level constants:
* api.py: _SSE_MEDIA_TYPE, _SESSION_NOT_FOUND_DETAIL
(replaces 5 + 4 literal duplicates)
* orchestrator.py: _AGENT_FAILURE_MARKER (replaces 3)
* storage/models.py: _ACTIVE_ROW_SQL (replaces 4)
S2772 — remove redundant ``pass`` after type-checking-only imports:
* orchestrator.py, triggers/base.py, triggers/transports/schedule.py,
triggers/transports/webhook.py — pass removed; the bundler's
``_ORPHANED_TYPE_CHECKING_RE`` rewrite injects pass when imports
get stripped
* orchestrator.py: also moved the explanatory comment OUT of the
``if TYPE_CHECKING:`` block so the bundler regex sees the body
as empty after stripping (comments inside the block prevent the
regex from matching, which would break the dist bundle)
* policy.py: pass removed; trailing ``# pragma: no cover`` comment
also dropped from the ``if`` line so the bundler regex matches
S5713 — drop redundant exception subclasses from ``except`` tuples:
* agents/turn_output.py + graph.py: ``(json.JSONDecodeError, ValueError)``
-> ``ValueError`` (JSONDecodeError is a ValueError subclass)
* api.py x4: ``(FileNotFoundError, ValueError, KeyError, LookupError)``
-> ``(FileNotFoundError, ValueError, LookupError)`` (KeyError is
a LookupError subclass)
* triggers/transports/webhook.py: ``(ValueError, TypeError, ValidationError)``
-> ``(ValueError, TypeError)`` (pydantic ValidationError is a
ValueError subclass)
S8409 — remove FastAPI ``response_model=`` kwargs already implied by
the route handler's return-type annotation:
* api.py x5 + api_dedup.py x1
S7503 — expand the suppression list (sonar-project.properties) to
cover the remaining 9 ``async def`` sites whose async signature is
required by the surrounding framework contract:
* learning/scheduler.py + tools/approval_watchdog.py — APScheduler
* llm.py — langchain BaseChatModel.ainvoke
* agents/supervisor.py — LangGraph node
* api.py — FastAPI handler / Depends callable
* service.py — service.submit_async() awaitable contract
* triggers/auth.py — FastAPI Depends bearer dependency
Verification:
ruff check src/ tests/ passed
pyright src/runtime 0 errors / 0 warnings
pytest -x 1310 passed / 8 skipped
pytest --cov=src/runtime --cov-fail-under=85 88.90%
build_single_file.py dist/* regenerated
Projected SonarCloud after merge:
code_smells 97 -> ~66 (32% drop)
|
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
Mechanical hygiene pass on Sonar's lower-effort code-smell rules. Targets ~32% of the 97 open smells in a single PR with no behavior change.
What's removed
python:S1192python:S2772passpython:S5713excepttuplepython:S8409response_model=python:S7503sonar.issue.ignore.multicriteriaConstants extracted (S1192)
api.py_SSE_MEDIA_TYPE"text/event-stream"×5api.py_SESSION_NOT_FOUND_DETAIL"session not found"×4orchestrator.py_AGENT_FAILURE_MARKER"agent failed:"×3storage/models.py_ACTIVE_ROW_SQL"deleted_at IS NULL"×4Bundler interaction (heads-up for review)
S2772's removal exposed a constraint in the bundler's import-stripping pass.
scripts/build_single_file.py:_ORPHANED_TYPE_CHECKING_REinjects apassbody when anif TYPE_CHECKING:block becomes empty after intra-import stripping. Two cases broke that auto-fix:policy.pyhadif TYPE_CHECKING: # pragma: no cover ...— the trailing comment prevented the regex from matching. Trimmed the comment (TYPE_CHECKING blocks aren't executed at runtime, so the pragma is redundant).orchestrator.pyhad explanatory comments INSIDE theif TYPE_CHECKING:block — the regex doesn't see the body as empty when it contains comment lines (Python treats comments as no-ops, but the regex is structural). Moved the comments above theifline.The bundle tests (
tests/test_build_*.py,tests/test_bundle_*.py) caught both cases pre-commit.Test plan
uv run ruff check src/ tests/— passeduv run pyright src/runtime— 0 errors / 0 warningsuv run pytest -x— 1,310 passed / 8 skippeduv run pytest --cov=src/runtime --cov-fail-under=85 -x— 88.90%uv run python scripts/build_single_file.py—dist/*regenerated and committedcode_smellsdrops from 97 toward ~66Out of scope
🤖 Generated with Claude Code