Skip to content

Commit 2355202

Browse files
authored
chore(sonar): Tier 1 cleanup — 31 code smells across 5 rules (#16)
Mechanical hygiene pass: S1192 literal-duplicate constants, S2772 unneeded pass removal, S5713 redundant exception subclass cleanup, S8409 redundant FastAPI response_model kwargs, S7503 suppression-list expansion. 97 -> ~66 code smells. No behavior change. Closes #16.
1 parent 158e9ed commit 2355202

14 files changed

Lines changed: 249 additions & 238 deletions

File tree

dist/app.py

Lines changed: 56 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -690,8 +690,11 @@ class IncidentState(Session):
690690

691691
# Phase 11 (FOC-04): forward-reference imports for the should_gate
692692
# signature only; kept inside ``TYPE_CHECKING`` so the bundle's
693-
# intra-import stripper does not remove a load-bearing import. The
694-
# ``pass`` keeps the block syntactically valid after stripping.
693+
# intra-import stripper sees them. The bundler's
694+
# ``_ORPHANED_TYPE_CHECKING_RE`` rewrite injects a ``pass`` body when
695+
# the imports get stripped (build_single_file.py:292) — but its regex
696+
# requires the ``if TYPE_CHECKING:`` line to have no trailing comment,
697+
# so do not add one here.
695698
# ----- imports for runtime/agents/responsive.py -----
696699
"""Responsive agent kind — the today-default LLM agent.
697700

@@ -1397,6 +1400,9 @@ async def _poll(self, registry):
13971400

13981401

13991402

1403+
# Forward-import: ``runtime.triggers.base`` only defines a dataclass and
1404+
# the type appears in a method annotation. Kept inside ``TYPE_CHECKING``
1405+
# to avoid a runtime circular import.
14001406
# ----- imports for runtime/api.py -----
14011407
"""FastAPI app — health, listings, incident, and multi-session endpoints.
14021408

@@ -3590,6 +3596,12 @@ class Base(DeclarativeBase):
35903596
pass
35913597

35923598

3599+
# SQL fragment used as the partial-index predicate so soft-deleted
3600+
# rows don't bloat the indexes (mirrors the application-layer
3601+
# "exclude deleted" filter in SessionStore queries).
3602+
_ACTIVE_ROW_SQL = "deleted_at IS NULL"
3603+
3604+
35933605
class IncidentRow(Base):
35943606
__tablename__ = "incidents"
35953607

@@ -3641,11 +3653,11 @@ class IncidentRow(Base):
36413653

36423654
__table_args__ = (
36433655
Index("ix_incidents_status_env_active", "status", "environment",
3644-
postgresql_where=text("deleted_at IS NULL"),
3645-
sqlite_where=text("deleted_at IS NULL")),
3656+
postgresql_where=text(_ACTIVE_ROW_SQL),
3657+
sqlite_where=text(_ACTIVE_ROW_SQL)),
36463658
Index("ix_incidents_created_at_active", "created_at",
3647-
postgresql_where=text("deleted_at IS NULL"),
3648-
sqlite_where=text("deleted_at IS NULL")),
3659+
postgresql_where=text(_ACTIVE_ROW_SQL),
3660+
sqlite_where=text(_ACTIVE_ROW_SQL)),
36493661
Index("ix_incidents_parent_session_id", "parent_session_id"),
36503662
)
36513663

@@ -6799,7 +6811,7 @@ def parse_envelope_from_result(
67996811
continue
68006812
try:
68016813
payload = json.loads(content)
6802-
except (json.JSONDecodeError, ValueError):
6814+
except ValueError: # JSONDecodeError is a ValueError subclass
68036815
continue
68046816
if not isinstance(payload, dict):
68056817
continue
@@ -8265,12 +8277,8 @@ async def _resume_with_timeout(
82658277

82668278
# ====== module: runtime/policy.py ======
82678279

8268-
if TYPE_CHECKING: # pragma: no cover -- type checking only
8269-
8270-
8271-
pass # noqa: PIE790 -- bundle survives even if imports are stripped
8272-
8273-
8280+
if TYPE_CHECKING:
8281+
pass
82748282
GateReason = Literal[
82758283
"auto",
82768284
"high_risk_tool",
@@ -9974,7 +9982,7 @@ def _try_recover_envelope_from_raw(raw: str) -> AgentTurnOutput | None:
99749982
for candidate in candidates:
99759983
try:
99769984
payload = json.loads(candidate)
9977-
except (json.JSONDecodeError, ValueError):
9985+
except ValueError: # JSONDecodeError is a ValueError subclass
99789986
continue
99799987
if not isinstance(payload, dict):
99809988
continue
@@ -10997,9 +11005,6 @@ async def make_checkpointer(
1099711005

1099811006
if TYPE_CHECKING:
1099911007
pass
11000-
11001-
11002-
1100311008
@dataclass(frozen=True)
1100411009
class TriggerInfo:
1100511010
"""Provenance attached to every session started via a trigger.
@@ -11469,9 +11474,6 @@ async def stop(self) -> None:
1146911474

1147011475
if TYPE_CHECKING:
1147111476
pass
11472-
11473-
11474-
1147511477
_log = logging.getLogger(__name__)
1147611478

1147711479

@@ -11564,7 +11566,7 @@ async def handler(
1156411566
)
1156511567
except KeyError as exc:
1156611568
raise HTTPException(status_code=404, detail=str(exc)) from exc
11567-
except (ValueError, TypeError, ValidationError) as exc:
11569+
except (ValueError, TypeError) as exc: # pydantic ValidationError is a ValueError subclass
1156811570
_log.warning(
1156911571
"trigger %r transform/dispatch failed: %s", name, exc
1157011572
)
@@ -11577,8 +11579,6 @@ async def handler(
1157711579

1157811580
if TYPE_CHECKING:
1157911581
pass
11580-
11581-
1158211582
_log = logging.getLogger(__name__)
1158311583

1158411584

@@ -13540,15 +13540,7 @@ def gc_orphaned_checkpoints(engine: Engine) -> int:
1354013540
# ====== module: runtime/orchestrator.py ======
1354113541

1354213542
if TYPE_CHECKING:
13543-
# Avoid a runtime circular import — ``runtime.triggers.base`` only
13544-
# defines a dataclass, and the type appears in a method annotation.
1354513543
pass
13546-
13547-
13548-
13549-
13550-
13551-
1355213544
from langgraph.errors import GraphInterrupt
1355313545
from langgraph.types import Command
1355413546

@@ -13567,6 +13559,12 @@ def gc_orphaned_checkpoints(engine: Engine) -> int:
1356713559
_log = logging.getLogger("runtime.orchestrator")
1356813560

1356913561

13562+
# Marker that ``runtime.graph._handle_agent_failure`` writes onto the
13563+
# AgentRun.summary of the failing turn. Read by the retry / extract-error
13564+
# helpers below.
13565+
_AGENT_FAILURE_MARKER = "agent failed:"
13566+
13567+
1357013568
def _assert_envelope_invariant_on_finalize(session: "Session") -> None:
1357113569
"""Phase 10 (FOC-03) defence-in-depth log sweep.
1357213570

@@ -14484,9 +14482,9 @@ def _extract_last_error(inc: "Session") -> Exception | None:
1448414482
import pydantic as _pydantic
1448514483
for run in reversed(inc.agents_run):
1448614484
summary = (run.summary or "")
14487-
if not summary.startswith("agent failed:"):
14485+
if not summary.startswith(_AGENT_FAILURE_MARKER):
1448814486
continue
14489-
body = summary.removeprefix("agent failed:").strip()
14487+
body = summary.removeprefix(_AGENT_FAILURE_MARKER).strip()
1449014488
if "EnvelopeMissingError" in body:
1449114489
return _EnvelopeMissingError(
1449214490
agent=run.agent or "unknown",
@@ -15064,7 +15062,7 @@ async def _retry_session_locked(self, session_id: str) -> AsyncIterator[dict]:
1506415062
# successful runs. Retry attempts then append fresh runs.
1506515063
inc.agents_run = [
1506615064
r for r in inc.agents_run
15067-
if not (r.summary or "").startswith("agent failed:")
15065+
if not (r.summary or "").startswith(_AGENT_FAILURE_MARKER)
1506815066
]
1506915067
# Bump retry counter for unique LangGraph thread id (the prior
1507015068
# thread's checkpoint sits at a terminal node and would
@@ -15207,6 +15205,13 @@ def _event_ts() -> str:
1520715205
_log = logging.getLogger("runtime.api")
1520815206

1520915207

15208+
# Wire-format constants (extracted to keep S1192 — duplicated literal
15209+
# strings — in check; every SSE endpoint uses _SSE_MEDIA_TYPE, every
15210+
# session-not-found path raises with _SESSION_NOT_FOUND_DETAIL).
15211+
_SSE_MEDIA_TYPE = "text/event-stream"
15212+
_SESSION_NOT_FOUND_DETAIL = "session not found"
15213+
15214+
1521015215
# HTTP status -> structured error code. Used by the global exception
1521115216
# handler to keep React's error UI from having to switch on every
1521215217
# integer status code.
@@ -15619,7 +15624,7 @@ async def _events():
1561915624
):
1562015625
yield f"data: {json.dumps(ev, default=str)}\n\n"
1562115626

15622-
return StreamingResponse(_events(), media_type="text/event-stream")
15627+
return StreamingResponse(_events(), media_type=_SSE_MEDIA_TYPE)
1562315628

1562415629
@fastapi_app.post("/incidents/{incident_id}/resume")
1562515630
async def resume_incident(incident_id: str, req: ResumeRequest) -> StreamingResponse:
@@ -15647,15 +15652,14 @@ async def _events():
1564715652
}
1564815653
yield f"data: {json.dumps(err, default=str)}\n\n"
1564915654

15650-
return StreamingResponse(_events(), media_type="text/event-stream")
15655+
return StreamingResponse(_events(), media_type=_SSE_MEDIA_TYPE)
1565115656

1565215657
# ------------------------------------------------------------------
1565315658
# Multi-session endpoints
1565415659
# ------------------------------------------------------------------
1565515660

1565615661
@fastapi_app.post(
1565715662
"/sessions",
15658-
response_model=SessionStartResponse,
1565915663
status_code=201,
1566015664
)
1566115665
async def start_session_endpoint(
@@ -15685,7 +15689,7 @@ class is matched by name so this handler does not depend on a
1568515689
raise
1568615690
return SessionStartResponse(session_id=sid)
1568715691

15688-
@fastapi_app.get("/sessions", response_model=list[SessionStatus])
15692+
@fastapi_app.get("/sessions")
1568915693
async def list_sessions_endpoint(request: Request) -> list[SessionStatus]:
1569015694
"""Snapshot of in-flight sessions (running / awaiting_input / error)."""
1569115695
svc = request.app.state.service
@@ -15695,10 +15699,7 @@ async def list_sessions_endpoint(request: Request) -> list[SessionStatus]:
1569515699
# HITL approval endpoints (risk-rated tool gateway)
1569615700
# ------------------------------------------------------------------
1569715701

15698-
@fastapi_app.get(
15699-
"/sessions/{session_id}/approvals",
15700-
response_model=list[PendingApproval],
15701-
)
15702+
@fastapi_app.get("/sessions/{session_id}/approvals")
1570215703
async def list_pending_approvals(
1570315704
session_id: str, request: Request
1570415705
) -> list[PendingApproval]:
@@ -15713,13 +15714,13 @@ async def list_pending_approvals(
1571315714
orch = request.app.state.orchestrator
1571415715
try:
1571515716
inc = orch.store.load(session_id)
15716-
except (FileNotFoundError, ValueError, KeyError, LookupError) as e:
15717+
except (FileNotFoundError, ValueError, LookupError) as e: # KeyError is a LookupError subclass
1571715718
# ``ValueError`` covers the SessionStore id-format guard
1571815719
# (``Invalid incident id ...``) which we treat as a 404
1571915720
# at the API boundary — the client passed an id that
1572015721
# cannot exist, semantically equivalent to "not found".
1572115722
raise HTTPException(
15722-
status_code=404, detail="session not found"
15723+
status_code=404, detail=_SESSION_NOT_FOUND_DETAIL
1572315724
) from e
1572415725
# Defensive: ``svc`` is unused here today — the read goes through
1572515726
# the orchestrator's store. We keep the reference so a future
@@ -15761,9 +15762,9 @@ async def submit_approval_decision(
1576115762
orch = request.app.state.orchestrator
1576215763
try:
1576315764
orch.store.load(session_id)
15764-
except (FileNotFoundError, ValueError, KeyError, LookupError) as e:
15765+
except (FileNotFoundError, ValueError, LookupError) as e: # KeyError is a LookupError subclass
1576515766
raise HTTPException(
15766-
status_code=404, detail="session not found"
15767+
status_code=404, detail=_SESSION_NOT_FOUND_DETAIL
1576715768
) from e
1576815769

1576915770
decision_payload = {
@@ -15874,9 +15875,9 @@ async def get_session_detail(session_id: str, request: Request) -> dict:
1587415875
orch = request.app.state.orchestrator
1587515876
try:
1587615877
return orch.get_session(session_id)
15877-
except (FileNotFoundError, ValueError, KeyError, LookupError) as e:
15878+
except (FileNotFoundError, ValueError, LookupError) as e: # KeyError is a LookupError subclass
1587815879
raise HTTPException(
15879-
status_code=404, detail="session not found",
15880+
status_code=404, detail=_SESSION_NOT_FOUND_DETAIL,
1588015881
) from e
1588115882

1588215883
@fastapi_app.post("/sessions/{session_id}/resume")
@@ -15911,7 +15912,7 @@ async def _events():
1591115912
}
1591215913
yield f"data: {json.dumps(err, default=str)}\n\n"
1591315914

15914-
return StreamingResponse(_events(), media_type="text/event-stream")
15915+
return StreamingResponse(_events(), media_type=_SSE_MEDIA_TYPE)
1591515916

1591615917
@fastapi_app.post("/sessions/{session_id}/retry")
1591715918
async def retry_session_sse(
@@ -15934,12 +15935,9 @@ async def _events():
1593415935
}
1593515936
yield f"data: {json.dumps(err, default=str)}\n\n"
1593615937

15937-
return StreamingResponse(_events(), media_type="text/event-stream")
15938+
return StreamingResponse(_events(), media_type=_SSE_MEDIA_TYPE)
1593815939

15939-
@fastapi_app.get(
15940-
"/sessions/{session_id}/retry/preview",
15941-
response_model=RetryDecisionPreview,
15942-
)
15940+
@fastapi_app.get("/sessions/{session_id}/retry/preview")
1594315941
async def preview_retry(
1594415942
session_id: str, request: Request,
1594515943
) -> RetryDecisionPreview:
@@ -15949,19 +15947,16 @@ async def preview_retry(
1594915947
orch = request.app.state.orchestrator
1595015948
try:
1595115949
decision = orch.preview_retry_decision(session_id)
15952-
except (FileNotFoundError, ValueError, KeyError, LookupError) as e:
15950+
except (FileNotFoundError, ValueError, LookupError) as e: # KeyError is a LookupError subclass
1595315951
raise HTTPException(
15954-
status_code=404, detail="session not found",
15952+
status_code=404, detail=_SESSION_NOT_FOUND_DETAIL,
1595515953
) from e
1595615954
return RetryDecisionPreview(
1595715955
retry=bool(decision.retry),
1595815956
reason=str(decision.reason),
1595915957
)
1596015958

15961-
@fastapi_app.get(
15962-
"/sessions/{session_id}/lessons",
15963-
response_model=list[LessonResponse],
15964-
)
15959+
@fastapi_app.get("/sessions/{session_id}/lessons")
1596515960
async def list_session_lessons(
1596615961
session_id: str, request: Request,
1596715962
) -> list[LessonResponse]:
@@ -16057,7 +16052,7 @@ async def _stream():
1605716052
last_seq = ev.seq
1605816053
yield f"data: {envelope.model_dump_json()}\n\n"
1605916054

16060-
return StreamingResponse(_stream(), media_type="text/event-stream")
16055+
return StreamingResponse(_stream(), media_type=_SSE_MEDIA_TYPE)
1606116056

1606216057
@fastapi_app.websocket("/ws/sessions/{session_id}/events")
1606316058
async def ws_events(websocket: WebSocket, session_id: str) -> None:
@@ -16165,7 +16160,6 @@ def register_dedup_routes(
1616516160

1616616161
@app.post(
1616716162
"/sessions/{session_id}/un-duplicate",
16168-
response_model=UnDuplicateResponse,
1616916163
status_code=200,
1617016164
tags=["dedup"],
1617116165
)

0 commit comments

Comments
 (0)