Repository navigation
Change impact job lists functions the PR never edited as top cost drivers — mark rows where lizard misreads the function span #1219
Description
Background
The "Change impact (advisory)" job (.github/workflows/impact.yml, added in #1137) gives every pull request a complexity cost score. It uses lizard, an open-source code-complexity tool, to split each file into functions and measure their cyclomatic complexity (roughly, how many separate paths run through the function). It then charges a cost for every function the PR changes, weighted by how complex that function is. The "Top cost drivers" table lists the most expensive ones and tells reviewers to "simplify or refactor these".
Problem
For TypeScript, lizard sometimes gets a function's line span wrong. When it does, the function is reported as changed even though the PR never edited it, and it can top the cost-driver table. The report header already warns that "TypeScript function-span defects … can distort scores", but nothing marks which rows are affected. A reviewer has no way to tell a real hotspot from a parsing error without running lizard by hand.
Verified on #1193 (lizard 1.23.0, base 576b8986 against head 106ea9082):
encrypt/status.ts:formatProgress: cost 23,920, the feat: add changesets for releasing #1 cost driver. The PR does not edit this function. In the base version lizard reads its span as lines 226–291, which takes in theformatTablefunction below it. The PR movedformatTableinto a shared module, so the span shrank and lizard's complexity count fell from 15 to 13. The tool scores that as an expensive change.migrate/src/version.ts:pickEncryptedColumn: cost 11,130, the fix(ci): set up bun in github actions for release #2 cost driver. The PR only appends new functions after it. Lizard's span grows from lines 202–235 to 202–342, swallowing the three new functions. The function itself is untouched.
Together these two rows account for more cost than the next three drivers combined. There is a related effect that is not a span error: migrate/src/backfill.ts:runBackfill is listed as a cost driver even though the PR cut its complexity from 25 to 9. That is the tool charging for any edit to a complex function, and it is out of scope here.
Why nothing catches it: the job prints whatever the impact-gate tool produces. Our wrapper (scripts/impact-gate.py) never checks a reported function against what the diff actually touched.
Proposal
- In the report step, for each function in the cost-driver and refactoring tables, compare lizard's line span on the base and head versions with the lines the diff actually changes.
- When the span grew or shrank by more lines than the PR changed inside it, or the PR changed no line inside the base span at all, mark the row (for example
⚠ span may be misattributed). Add one sentence under the table explaining the mark. - Do not change the score or the grade. This only labels rows. Changing the score belongs upstream in lizard or
impact-gate; if a fix there is feasible, file it upstream and link it here. - Add a fixture test with the two shapes above: a function directly above one that gets removed, and a function directly above code that gets appended. Check that both are marked and that a function the PR really edited is not.
Acceptance criteria
- Re-running the report for feat(cli): reverse migration (stash decrypt) for moving columns back to plaintext #1193 marks
formatProgressandpickEncryptedColumn, and does not markrenderRoworstatusCommandinencrypt/status.ts(the PR really edited those). - The score, the grade and the job's pass/warn outcome do not change.
- A test in
test_impact_gate.pycovers both span shapes.
Blocked by
- Change impact job scores commits from main as part of the PR when the PR base is behind main #1218 (same report path; fix the base range first so the marking is checked against the right diff)
Relationship to other work
- Add advisory ImpactGate reports calibrated against Stack history #1137 added the job.
- Change impact job scores commits from main as part of the PR when the PR base is behind main #1218 fixes the stale base found in the same audit.
stash decryptlifecycle rules are re-derived in each command — give the migration lifecycle one owning module #1217 holds the refactor findings from the feat(cli): reverse migration (stash decrypt) for moving columns back to plaintext #1193 audit. This issue stops reviewers being pointed at the misattributed rows.
Activity