Skip to content

Generalize reconstructOldContent into a shared before-content capability on AnalysisContext #4739

Description

@JSONbored

Context

Part of #4737 (parent epic — deterministic tier, REES foundation). Prerequisite
for the complexity-delta and duplication-delta sub-issues.

The problem

REES (review-enrichment/) analyzers normally see only diff hunks, not full before/after file content —
that's why complexity.ts and coverage-delta.ts both explicitly disclaim being true before/after
deltas in their own header comments. But the capability to recover real pre-PR content already exists:
reconstructOldContent(newContent, patch) (review-enrichment/src/analyzers/doc-comment-drift.ts:57-93)
reverse-applies the unified-diff patch onto the fetched post-change (headSha) file content to recover
the actual pre-PR text. It's used today by exactly two analyzers (doc-comment-drift.ts,
exhaustiveness-drift.ts) for narrow structural comparisons (stale @param names, enum/union
exhaustiveness) and is a one-off per-analyzer trick, not shared infrastructure — the shared
AnalysisContext (review-enrichment/src/analysis-context.ts:72-104) exposes only changedFiles
(with GitHub's per-file .patch), addedLines, and patchHunks; no before-content fetch at the context
level.

Fix

Promote reconstructOldContent out of doc-comment-drift.ts into a shared module (or onto
AnalysisContext itself, e.g. context.beforeContentFor(path): string | undefined), so any analyzer can
request the pre-PR text of a changed file without re-deriving the reverse-patch logic. Keep the existing
two call sites working unchanged — this is a pure extraction, not a behavior change for them.

Requirements

  • Preserve exact current behavior for doc-comment-drift.ts and exhaustiveness-drift.ts (byte-faithful
    extraction — verify old vs. new output is identical for both, the same way Break up processors.ts mega-functions #4607's extractions were
    verified this cycle).
  • Handle the failure/edge cases the two existing call sites already handle (patch doesn't cleanly
    reverse-apply, binary file, file didn't exist before the PR / is newly added) — surface these as
    undefined/null, never throw, since a missing "before" is a legitimate outcome (new file) that every
    downstream consumer must handle explicitly.
  • Document the function's real cost (it still needs the post-change file content fetched, same as today
    — this doesn't add a new network call, just relocates existing reverse-patch logic).

Acceptance criteria

  • reconstructOldContent (or equivalent) lives once, callable from any AnalyzerDescriptor, via the
    shared AnalysisContext or a co-located helper module.
  • doc-comment-drift.ts / exhaustiveness-drift.ts migrated to the shared version with zero output
    change (regression tests pin this).
  • New-file / unreconstructable-patch cases return an explicit "no before content" signal, not a
    thrown error or a silent wrong answer.
  • Full branch coverage on the new shared helper.

Metadata

Metadata

Assignees

Labels

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

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions