Skip to content

refactor: trace_impact.py breaches the file and function size caps, with zero test coverage #85

Description

@cdeust

Surfaced while fixing the py/constant-conditional-expression alert in this file (#46). Recorded per §14.3 as a deferral with acceptance criteria, because a behaviour-preserving decomposition of a 329-line async orchestrator belongs in its own PR (§15.1: "never mix the enabling refactor and the feature in one PR"), not inside a CodeQL-triage change.

Current state (measured on fix/46-codeql-tail, after the if True: removal)

cortex_viz/server/trace_impact.py is 559 lines — over the 500-line file cap (§4.1).

Four functions breach the 50-line function cap (§4.2):

Function Lines Limit
_impact_for_graph() 329 50
_run() (nested inside it) 300 50
_ast_and_impact() 74 50
_to_repo_relative() 58 50

Reproduce:

python3 - <<'PY'
import ast, pathlib
src = pathlib.Path("cortex_viz/server/trace_impact.py").read_text()
for n in ast.walk(ast.parse(src)):
    if isinstance(n, (ast.FunctionDef, ast.AsyncFunctionDef)):
        span = n.end_lineno - n.lineno + 1
        if span > 50:
            print(f"{n.name}() = {span} lines")
PY

Why this is more than a style breach

No test references this module. grep -rln "trace_impact\|_impact_for_graph" tests/ returns nothing across 239 test files. The 300-line _run() issues ~12 distinct Cypher queries and a capped get_context fan-out against a live AP bridge; every one of its failure arms (absent file node, empty members, bridge error, the N>20 fallback path) is unexercised. That is the §13.1 A3/F1 gap that makes the decomposition worth doing rather than cosmetic — the size cap is the symptom, the untested query orchestration is the defect.

The if True: that wrapped the whole body (removed in the #46 PR) is evidence the block was never revisited after whatever condition it once carried was dropped.

Acceptance criteria

  • trace_impact.py is under 500 lines, or split along a concern boundary into modules that each are
  • No function in the resulting modules exceeds 50 lines
  • The Cypher query set is separated from the result-shaping so each is testable without a live bridge
  • Every failure arm of the impact path has a test asserting its observable effect: file absent from graph → None; empty members; bridge raising; the >N member fallback to the per-file Cypher path
  • The refactor is behaviour-preserving and ships as its own PR — the existing suite passing unchanged is the proof, with the new tests added on top
  • git diff -w reviewed to confirm no accidental semantic change in moved blocks

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions