Skip to content

Change impact job lists functions the PR never edited as top cost drivers — mark rows where lizard misreads the function span #1219

Closed

Description

@tobyhede

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):

  1. 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 the formatTable function below it. The PR moved formatTable into 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.
  2. 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

  1. 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.
  2. 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.
  3. 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.
  4. 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

Blocked by

Relationship to other work

Metadata

Metadata

Assignees

No one assigned

    Labels

    SDKenhancementNew feature or requestgithub-actionsPull request modifies GitHub Actionsready-for-agentFully specified and ready for an AFK agent

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions