Skip to content

refactor(rees): migrate three more analyzers off hand-rolled bounded-fetch onto boundedFetchText - #4947

Merged
loopover-orb[bot] merged 1 commit into
mainfrom
refactor/4824-rees-boundedfetchtext-migration
Jul 11, 2026
Merged

refactor(rees): migrate three more analyzers off hand-rolled bounded-fetch onto boundedFetchText#4947
loopover-orb[bot] merged 1 commit into
mainfrom
refactor/4824-rees-boundedfetchtext-migration

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

Follow-up to #4759 / PR #4821 (discovered drift, deliberately not bundled into that PR to keep it
scoped to the 3 files #4759 named). Same migration, applied to the 3 remaining REES analyzers that
still hand-rolled their own bounded-fetch logic:

Each file's private readBoundedText is deleted outright (never relocated), and the fetch path now
goes through boundedFetchText / options.analysis.fetchText, mirroring duplication-delta.ts's own
fetchFileAtHead (endpointCategory: "github-contents", a per-file phase string, subcall,
maxBytes, maxCallsPerCategory). undocumented-export.ts is switched onto githubHeaders().
unused-export.ts and caller-impact.ts already used githubHeaders() and only needed the
boundedFetchText migration. No new shared module.

Test plan

  • All three files' existing test suites pass unchanged (no mocking updates were needed — the
    injected fetchFn/fetchImpl still flows through to boundedFetchText's own fetchImpl option).
  • Added one new test per file exercising the options.analysis.fetchText branch (asserting the
    bare fetch path is never invoked when an analysis context is supplied), for full branch coverage
    on the new ternary — mirrors the tests PR refactor(rees): migrate three analyzers off hand-rolled bounded-fetch onto boundedFetchText #4821 added for its own 3 files.
  • npm run rees:test (review-enrichment build + sourcemap validation + metadata check + full node
    test suite): 1297/1297 passing.
  • Root npm run test:ci (all steps, run individually): git diff --check, actionlint, DB
    migration/schema-drift checks, selfhost checks, cf-typegen check, root typecheck,
    test:coverage (702 files / 13899 tests), test:engine-parity (15/15) + engine-parity:drift-check
    (22 hand-duplicated pairs agree — confirmed unaffected by this REES-internal-only change),
    test:driver-parity (18/18), gittensory-engine workspace tests (349/349), test:workers,
    MCP/miner build+pack checks, OpenAPI/docs/manifest/command-reference drift checks, UI lint/typecheck/test/build.
  • npm audit --audit-level=moderate: 0 vulnerabilities.

Closes #4824

…fetch onto boundedFetchText

undocumented-export.ts, unused-export.ts, and caller-impact.ts each still hand-rolled their own
private readBoundedText + fetchFileAtHead wrapper for fetching a changed file's content at headSha,
the same pattern doc-comment-drift.ts, exhaustiveness-drift.ts, and complexity-delta.ts had before
#4759/PR #4821 migrated them onto the shared boundedFetchText helper. undocumented-export.ts also
hand-built its own GitHub auth headers inline instead of the shared githubHeaders() helper, the same
inconsistency doc-comment-drift.ts had before that PR.

Deletes each file's private readBoundedText outright (never relocated), replaces the fetch path with
boundedFetchText / options.analysis.fetchText (mirroring duplication-delta.ts's own fetchFileAtHead),
and switches undocumented-export.ts onto githubHeaders(). unused-export.ts and caller-impact.ts
already used githubHeaders() and only needed the boundedFetchText migration. No new shared module.

Closes #4824
@superagent-security

Copy link
Copy Markdown
Contributor

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

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 11, 2026
@loopover-orb

loopover-orb Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Tip

🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩

✅ Gittensory review result - approve/merge recommended

Review updated: 2026-07-11 02:27:11 UTC

6 files · 1 AI reviewer · no blockers · readiness 100/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This migrates the three remaining REES analyzers (undocumented-export.ts, unused-export.ts, caller-impact.ts) off their hand-rolled readBoundedText/fetchFileAtHead onto the shared boundedFetchText helper, mirroring the pattern PR #4821 already applied to the other three analyzers — each private readBoundedText is deleted outright (not relocated) and fetchFileAtHead now branches on options.analysis.fetchText vs boundedFetchText with matching endpointCategory/phase/subcall/maxBytes/maxCallsPerCategory options. undocumented-export.ts additionally switches from hand-built auth headers to the shared githubHeaders() helper, bringing it in line with the other two files. Each file gets a new test asserting the analysis.fetchText/fetchJson branch is taken and the bare fetch path is never invoked, giving real coverage on the new ternary; CI is fully green and the change is confined to analyzer internals with no schema/migration surface.

Nits — 5 non-blocking
  • undocumented-export.ts's old hand-built headers included "X-GitHub-Api-Version": "2022-11-28" explicitly; verify githubHeaders(token, { raw: true }) still sets that header (github-headers.js wasn't in the provided diff/file set, so this couldn't be directly confirmed) — it's an established helper already used by the sibling files, so low risk, but worth a quick confirm.
  • The PR description doesn't explicitly say "Closes Migrate 3 more REES analyzers onto boundedFetchText #4824" — the linkage is only visible via the test comments (`// Migrate 3 more REES analyzers onto boundedFetchText #4824: ...`); add an explicit Closes/Fixes line so the issue auto-closes on merge.
  • Consider a one-line note in the PR description cross-linking refactor(rees): migrate three analyzers off hand-rolled bounded-fetch onto boundedFetchText #4821 as "same pattern, see that PR's review for the header/options rationale" to speed up re-review.
  • If a fourth hand-rolled bounded-fetch analyzer ever surfaces, it may be worth extracting fetchFileAtHead's fetchOptions-building into a tiny shared helper (owner/repo/path/headSha/token/phase → fetchOptions) since the three files now duplicate that block verbatim — not worth doing in this PR, just a note for if a fourth migration ever comes up.
  • PR author also opened the linked issue — Link an issue that was opened by a different contributor, or provide a rationale for why this self-authored issue represents genuine discovery work.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #4824
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 (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 48 registered-repo PR(s), 40 merged, 312 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 48 PR(s), 312 issue(s).
Gate result ✅ Passing No configured 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: Python, TypeScript, Ruby, Go, JavaScript, MDX, Shell, Solidity
  • Official Gittensor activity: 48 PR(s), 312 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
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

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gittensory approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit 6954202 into main Jul 11, 2026
11 checks passed
@loopover-orb
loopover-orb Bot deleted the refactor/4824-rees-boundedfetchtext-migration branch July 11, 2026 02:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Migrate 3 more REES analyzers onto boundedFetchText

1 participant