refactor(rees): promote reconstructOldContent into a shared analyzer helper

[JSONbored]

Summary

• Implements Generalize reconstructOldContent into a shared before-content capability on AnalysisContext #4739 (sub-issue of epic Epic: PR improvement signal — quality-delta scoring as the positive-axis counterpart to slop-risk #4737). reconstructOldContent(newContent, patch) — the
  unified-diff reverse-patch reconstruction primitive that recovers a changed file's pre-PR text — was
  private to doc-comment-drift.ts (feat(enrichment): Doc-comment-vs-signature drift #1519) and imported cross-file from there by
  exhaustiveness-drift.ts (feat(enrichment): enum / literal-union exhaustiveness-drift analyzer #2028): a one-off trick living in the wrong place, not shared
  infrastructure. This PR promotes it into its own co-located module,
  review-enrichment/src/analyzers/reconstruct-old-content.ts, and migrates both existing callers to it.
• Pure code motion — zero behavior change for both callers. Confirmed mechanically (detail below),
  not by eyeballing, matching the verification discipline Break up processors.ts mega-functions #4607's sibling extraction PRs
  (refactor(queue): extract plan-input builder from runAgentMaintenancePlanAndExecute #4688/refactor(queue): split processGitHubWebhook into per-event handlers #4695/refactor(queue): extract manifest-policy gate from maybePublishPrPublicSurface #4728) established for this kind of change.

Architecture decision: co-located module, not an AnalysisContext method

The issue offered either shape. I looked at both before choosing:

• AnalysisContext (review-enrichment/src/analysis-context.ts) does not expose the GitHub token —
  it's pure authenticated transport (fetchJson/fetchText/fetchStatus); every current caller builds
  its own Authorization header from req.githubToken. A context-based version would still need the
  token threaded in from req, so it buys no real encapsulation over a plain function.
• reconstructOldContent is already pure and I/O-free — the fetch happens separately in each caller. The
  only thing to "promote" is the reverse-patch algorithm itself, which needs no context at all.
• Both current callers use the older (req, fetchFn, options) analyzer-function shape, not the
  AnalysisContext-consuming shape some other analyzers use (codeowners.ts, caller-impact.ts,
  coverage-delta.ts, duplication-scan.ts, etc.). Making this an AnalysisContext method would force
  both callers' exported signatures (and both existing test files' mocking convention — bare fetchFn
  injection, no AnalysisContext construction today) to change just to reach one small helper, and would
  touch registry.ts's wiring for both analyzers — a materially larger blast radius than the issue's own
  "pure extraction, not a behavior change" framing calls for.
• A plain co-located module is callable from either convention — the raw-req analyzers this issue
  touches today, and the AnalysisContext-based analyzers Real complexity-delta analyzer (true before/after comparison) #4740/Real duplication-delta analyzer (before/after comparison) #4741 will likely extend
  (coverage-delta.ts/duplication-scan.ts already receive AnalysisContext) — without forcing either
  style on the other. This matches the existing precedent in the same directory for shared analyzer
  helpers: diff-lines.ts, binary-extensions.ts, github-headers.ts are all plain sibling modules, not
  context methods.

Byte-faithful verification (mechanical, not eyeballed)

• Extracted the original function body (both locations, signature line through closing brace) into two
  files and diffed them directly: the only difference anywhere in the 37-line function is one inline
  comment (2 lines), reworded from doc-comment-drift-specific language ("bail so we never trust a bad old
  signature") to caller-agnostic language ("bail so we never trust a reconstructed result that isn't
  provably faithful to the real pre-PR file") — a comment has zero runtime effect.
• Stripped whole-line comments from both extracted bodies and compared: identical MD5 hash
  (7b195bf14c422e114a54d91da3d3e4a6) — the executable code is provably byte-identical, not just
  eyeballed-similar.
• exhaustiveness-drift.ts's diff is a single line: the import path
  (./doc-comment-drift.js → ./reconstruct-old-content.js). Nothing else in the file changed.
• doc-comment-drift.ts's diff is exactly: one import line added, and the function's 40 lines (doc
  comment + body) removed. The call site (reconstructOldContent(content, file.patch!) inside
  scanDocCommentDrift) is untouched — same name, now resolved via import instead of a local definition.
• The full pre-existing test suites for both analyzers (14 scanDocCommentDrift tests, 4
  scanExhaustivenessDrift tests) pass unmodified, proving both callers' end-to-end observable
  behavior is unchanged.

Edge cases (patch doesn't cleanly reverse-apply / binary file / newly-added file)

• Patch doesn't cleanly reverse-apply: unchanged — still returns null (a hunk starting before the
  cursor or past the content's end, or a context/added line that doesn't match newContent at the
  expected position).
• Binary file: this function only ever sees two text blobs and has no path/extension to inspect, so
  it cannot detect this itself — both callers already filter to known source extensions and require
  file.patch to be present before calling it (GitHub omits .patch entirely for binary/oversized
  files), and that filtering is untouched by this PR (see the byte-faithful diffs above).
• File didn't exist before the PR (newly added): unchanged — a "wholly added" patch (-0,0 old
  range) reconstructs to an empty string, not null. I deliberately did not normalize this to null
  in this PR: doing so is 100% safe for both current callers (they already branch on if (!oldContent),
  which treats "" and null identically) but it's a real, separate behavior decision on the shared
  function's own contract, and this issue is explicitly scoped as pure extraction. I instead pinned the
  current contract with an explicit regression test and documented it plainly in the module's doc comment
  so every future caller (including Real complexity-delta analyzer (true before/after comparison) #4740/Real duplication-delta analyzer (before/after comparison) #4741) knows to check falsiness, not === null. Happy to open a
  small separate follow-up if a strict-null contract is preferred later.

Coverage

Added review-enrichment/test/reconstruct-old-content.test.ts: the four existing
reconstructOldContent unit tests move over verbatim (removed from doc-comment-drift.test.ts, which no
longer imports the function directly), plus five new tests closing every branch the original four didn't
already exercise — a non-hunk preamble line before the first @@ header, out-of-order/overlapping hunks,
a \ No newline at end of file marker line, the trailing-flush no-op case (last hunk already reaches
EOF), and the wholly-new-file case. Verified with node's own coverage instrumentation
(node --test --experimental-test-coverage --test-coverage-include='dist/analyzers/reconstruct-old-content.js'):
100.00% line / 100.00% branch / 100.00% function coverage on the new module.

Scope

• The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
• This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
• This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
• I linked a currently open issue this PR resolves (e.g. Closes #123) — a linked open issue is required for every contributor PR.

Note on the issue link: this implements #4739, a sub-issue of epic #4737 — not Fixes/Closes
wording, per the epic's multi-part sub-issue convention.

Validation

• git diff --check — clean.
• npm run actionlint — not run; no .github/workflows/** files touched.
• npm run typecheck — clean (root-level; review-enrichment has its own separate tsconfig.json
  and is not part of this project reference, but its own build/typecheck is covered by rees:test
  below).
• npm run test:coverage — not run; review-enrichment/ is a standalone project (its own
  package.json/package-lock.json, not an npm workspace member) tested via node's built-in test
  runner, not vitest — it is not part of the root Codecov codecov/patch gate (that gate's
  coverage comes only from the root vitest run --coverage → coverage/lcov.info, which never
  touches review-enrichment/src/**). review-enrichment/ changes are gated entirely by
  npm run rees:test (CI's own "REES build, source-map validation, and tests" step, path-filtered on
  review-enrichment/**), run below.
• npm run test:workers — not run; no test/workers/**-relevant code touched.
• npm run build:mcp / npm run test:mcp-pack — not run; no MCP package changes.
• npm run ui:openapi:check / npm run ui:lint / npm run ui:typecheck / npm run ui:build — not
  run; no apps/gittensory-ui/** or API/schema changes.
• npm audit --audit-level=moderate — not run; no dependency changes (no package.json/lockfile
  touched in either the root or review-enrichment).
• New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries — no new behavior (pure extraction); the promoted helper has a dedicated test file with 100% line/branch/function coverage (verified with node's built-in coverage instrumentation, detail above), and both existing callers' full pre-existing test suites pass unmodified.

If any required check was skipped, explain why:

• This is a review-enrichment/-only, behavior-preserving extraction with no root src/**, UI, MCP,
  workers, schema, OpenAPI, or dependency surface touched, so those gates are left to CI rather than
  duplicated locally (they path-filter out for this diff anyway). The checks that matter for this change
  were run directly, in full, in the foreground:
  • npm ci (root) and npm run rees:install (fresh worktree, no prior node_modules): both clean.
  • npm run rees:test (npm --prefix review-enrichment test — build, source-map validation, analyzer
    metadata drift check, then the full node-test suite): 1229/1229 tests passed, including all 14
    scanDocCommentDrift tests, all 4 scanExhaustivenessDrift tests, the 4 relocated
    reconstructOldContent tests, and the 5 new tests — zero failures.
  • npm run typecheck (root): clean.
  • git diff --check: clean.
  • Coverage: node's built-in --experimental-test-coverage, scoped to the new module via
    --test-coverage-include, reported 100.00% line / 100.00% branch / 100.00% function — see Coverage
    above.
  • Byte-faithful diff + MD5 comparison of the moved function's body against its original location — see
    the Summary section above for the full method and result.

Safety

• No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
• Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
• Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests. — N/A, no auth/cookie/CORS/session code touched.
• API/OpenAPI/MCP behavior is updated and tested where needed. — N/A, no API/OpenAPI/MCP surface touched.
• UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. — N/A, no UI changes.
• Visible UI changes include a UI Evidence section below... — N/A, no visible/UI changes (backend-only refactor, internal to review-enrichment/).
• Public docs/changelogs are updated where needed... — N/A, no doc-affecting behavior change; CHANGELOG.md intentionally not touched.

UI Evidence

Not applicable — this PR has no visible/UI/frontend surface; it is an internal review-enrichment/
(REES) refactor with no API, schema, or behavior change.

Notes

• Prerequisite for the complexity-delta and duplication-delta sub-issues under epic Epic: PR improvement signal — quality-delta scoring as the positive-axis counterpart to slop-risk #4737 (per the
  epic's own text, likely extending coverage-delta.ts/duplication-scan.ts, which already receive
  AnalysisContext — either of those can import this shared helper directly regardless of which analyzer
  calling convention they end up using).
• Deliberately scoped to only this extraction — did not also add an AnalysisContext.beforeContentFor-style
  convenience method with no real caller today; see the architecture-decision writeup above for why, and
  happy to add one in a follow-up once a concrete AnalysisContext-based consumer exists.

[superagent-security]

Superagent didn't find any vulnerabilities or security issues in this PR.

[loopover-orb]

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

_{Review updated: 2026-07-10 22:44:48 UTC}

5 files · 1 AI reviewer · 2 blockers · readiness 93/100 · CI green · unstable

⏸️ Suggested Action - Manual Review

• No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
• Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.

Review summary
This is a clean, mechanical extraction: `reconstructOldContent` moves verbatim from `doc-comment-drift.ts` into its own module `reconstruct-old-content.ts`, and both callers (`doc-comment-drift.ts`, `exhaustiveness-drift.ts`) are repointed to the new import path with no logic changes — confirmed by diffing the moved function body against its prior form, which is byte-identical aside from comments. Tests are relocated verbatim (4 tests) plus 6 new tests added to pin previously-implicit branches (out-of-order hunks, preamble skip, no-newline marker, trailing-flush no-op, wholly-added-file empty-string case), giving the promoted helper full branch coverage in its own file. The PR explicitly ties to issue #4739 (sub-issue of epic #4737) and follows the established extraction pattern from #4688/#4695/#4728, and CI is green across build/typecheck/tests.

Nits — 4 non-blocking

• The 5-deep nesting in `reconstruct-old-content.ts:55` (`while > while > if > if > if`) is inherited unchanged from the prior location, not introduced by this PR, but since it's getting its own home now might be a good time to flatten the inner `if (sign === "-") {...} else {...}` with an early-continue for the `-` case to shave a level.
• The new module's header comment and JSDoc are quite long (17 lines of doc for a ~25-line function) — reads more like a design doc than a code comment; consider trimming to the parts that aren't already restated in the removed-vs-added-line logic itself.
• `review-enrichment/test/reconstruct-old-content.test.ts` imports from `../dist/analyzers/reconstruct-old-content.js` — worth double-checking the build config picks up the new `src/analyzers/` file into `dist/` the same way its siblings are (CI passing is good evidence it does, but flagging since it's easy to silently miss a new file in an explicit `include`/`files` list).
• Nothing structural to change — this is a good-faith, narrowly-scoped extraction with matching test migration; ship as-is.

Concerns raised — review before merging

• No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
• Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.

Signal	Result	Evidence
Code review	❌ 2 blockers	1 reviewer
Linked issue	⚠️ Missing	No linked issue or no-issue rationale found.
Related work	✅ No active overlap found	No same-issue or scoped active PR overlap found.
Change scope	✅ 20/20	Low review scope from cached public metadata (no linked issue context).
Validation posture	✅ 25/25	PR body includes validation/test evidence.
Contributor workload	✅ 10/10	Author activity: 48 registered-repo PR(s), 40 merged, 285 issue(s).
Contributor context	✅ Confirmed Gittensor contributor	JSONbored; Gittensor profile; 48 PR(s), 285 issue(s).
Gate result	❌ Blocking	Repo-configured hard blocker found.

Review context

• Author: JSONbored
• Role context: owner (maintainer lane)
• Public audience mode: oss maintainer
• Lane context: Repository is configured for direct PR review.
• Public profile languages: not available
• Official Gittensor activity: 48 PR(s), 285 issue(s).
• PR-specific overlap: none found.

Contributor next steps

• Treat this as maintainer-lane context rather than normal contributor-lane activity.
• Explain no-issue PR.
• Link the issue being solved, or explicitly explain why this is a no-issue PR.

Signal definitions

• Related work = same linked issue, overlapping active PRs, or title/path similarity.
• Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
• Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
• Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
• Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

_{🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed}

---

💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

• Re-run Gittensory review