Skip to content

Real complexity-delta analyzer (true before/after comparison) #4740

Description

@JSONbored

Context

Part of #4737 (parent epic — deterministic tier, REES). Depends on the shared
before-content primitive (sibling sub-issue in this epic, "Generalize reconstructOldContent...").

The problem

review-enrichment/src/analyzers/complexity.ts (lines 1-214) is not a before/after delta despite living
in a "quality" analyzer set — it computes an approximate McCabe complexity for newly-added functions
only (whose opening line is visible in the diff) against a fixed threshold
(DEFAULT_MAX_COMPLEXITY = 10). A function whose signature isn't in the diff (only its body edited) gets
no score at all, and there is no comparison to what the function's complexity was before the change — a
PR that meaningfully simplifies a gnarly existing function currently gets no credit at all from this
analyzer, which is exactly backwards for an "improvement" signal.

Fix

Using the shared before-content primitive, compute real before/after complexity for every function whose
body changed (not just newly-added ones): reconstruct the pre-PR file text, parse both versions, match
functions by name/position, and diff their approximate-McCabe scores. Where a function is new (no
"before" to compare, per the prior sub-issue's explicit "no before content" signal), keep the existing
absolute-threshold behavior as a fallback rather than silently dropping it — this analyzer needs to keep
covering the case it already handles correctly.

Requirements

  • Reuse the existing approximate-McCabe counting logic in complexity.ts for both before and after
    parses — don't reimplement complexity counting, just run it twice and diff.
  • Output shape needs a clear delta representation per function (e.g. { name, before, after, delta }),
    not just a single number — this feeds the aggregate sub-score (sibling sub-issue) which needs
    structured findings, not an opaque total.
  • Match this analyzer's existing AnalyzerDescriptor contract (review-enrichment/src/analyzers/types.ts)
    — same run/render shape, register any behavior change through registry.ts's existing entry rather
    than adding a parallel analyzer, unless the before/after version is different enough in cost/behavior
    to warrant a distinct AnalyzerName (implementer's call — document the reasoning either way).
  • Cost class: this does more work than the current absolute-only version (two parses instead of one) —
    reassess and, if needed, update its cost: AnalyzerCostClass classification honestly rather than
    leaving a stale cheap label on a heavier analyzer.

Acceptance criteria

  • A function with an unchanged signature but a simplified body (fewer branches/nesting) shows a
    negative (improving) complexity delta — the case the current analyzer cannot see at all.
  • Newly-added functions keep their existing absolute-threshold finding behavior (no regression).
  • New/reconstructable-only-partially files degrade gracefully (documented, tested), never crash the
    analyzer run.
  • Full branch coverage on the new delta logic; a regression test for the "simplifies an existing
    gnarly function" case specifically, since that's the whole point of this sub-issue.

Metadata

Metadata

Assignees

Labels

maintainer-onlyOwner-only work — yields no Gittensor points.

Projects

Status
Done

Relationships

None yet

Development

No branches or pull requests

Issue actions