Skip to content

perf(review): skip duplicate full-file grounding fetch for added files#3918

Merged
gittensory-orb[bot] merged 1 commit into
mainfrom
perf/skip-duplicate-added-file-grounding
Jul 7, 2026
Merged

perf(review): skip duplicate full-file grounding fetch for added files#3918
gittensory-orb[bot] merged 1 commit into
mainfrom
perf/skip-duplicate-added-file-grounding

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • fetchFullFileContents (grounding, on by default in the self-host starter env via GITTENSORY_REVIEW_GROUNDING=true) fetched and inlined the full post-change content of every changed file whose status !== "removed", including newly ADDED files.
  • Every line of a newly added file is already a + line in the diff, which is sent in the same prompt (up to 120,000 chars). So the full-file fetch for an added file was a byte-for-byte duplicate of content already present — a wasted GitHub Contents API round-trip and wasted prompt budget on every AI-reviewed PR under the recommended grounding config.
  • Added status !== "added" to the existing candidate filter (same treatment as "removed"), and extended the existing "skips removed/binary" test to also assert an added file is excluded and never fetched.
  • Left modified-file grounding untouched — PullRequestFile doesn't currently carry patch/line-count data, so a "skip when the diff already covers most of a modified file" refinement would need a bigger interface change; out of scope here.

Found via a fresh performance/scalability/accuracy hardening audit of the self-host ORB stack. Tracked under #1667.

Scope

  • src/review/review-grounding.ts — exclude status === "added" from the full-file-content candidate filter
  • test/unit/review-grounding.test.ts — cover the new exclusion

Validation

  • npm run typecheck
  • npx vitest run test/unit/review-grounding.test.ts --coverage — 20/20 passing; 100% line coverage, changed lines fully covered (remaining uncovered branches are pre-existing and untouched by this diff)
  • git diff --check clean

Safety

  • No behavior change for modified/removed files. An added file's content still reaches the reviewer in full via the diff — this only removes the duplicate second copy.

Closes #3897

Every line of a newly added file is already a '+' line in the diff
sent in the same prompt, so fetchFullFileContents re-fetching and
re-inlining its full content duplicated that content byte-for-byte --
a wasted GitHub API round-trip and wasted prompt budget on every
AI-reviewed PR under the recommended self-host grounding config.

Closes #3897
@superagent-security

Copy link
Copy Markdown
Contributor

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

@gittensory-orb gittensory-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.5x multiplier. label Jul 7, 2026
@gittensory-orb

gittensory-orb Bot commented Jul 7, 2026

Copy link
Copy Markdown

Tip

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

✅ Gittensory review result - approve/merge recommended

Review updated: 2026-07-07 06:59:43 UTC

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

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This diff adds `file.status !== "added"` to the candidate filter in `fetchFullFileContents` (src/review/review-grounding.ts:141) so newly added files skip the full-file GitHub fetch, since their entire content is already present as `+` lines in the diff sent in the same prompt. The change is a one-line, well-scoped filter addition with a matching test update (`test/unit/review-grounding.test.ts`) that asserts an added file is excluded from `reads` and from the output, and the full file content confirms the filter and test are wired correctly (no other candidate-filtering logic depends on `status === "added"` elsewhere in the shown file). The logic is sound: `status !== "removed" && status !== "added"` correctly narrows to modified/renamed files only, matching the stated intent, and CI is green.

Nits — 4 non-blocking
  • The added-file skip assumes the diff itself wasn't truncated by the 120k-char prompt budget; for a very large new file whose diff patch gets truncated, dropping the full-file fetch could leave the reviewer with only partial content — worth a one-line comment acknowledging this edge case if not already considered acceptable.
  • The three-line comment block in review-grounding.ts:138-140 duplicates most of the PR description almost verbatim — could be tightened to a single line referencing perf(review): skip duplicate full-file-content fetch for added files already covered by the diff #3897 per the file's existing terse comment style.
  • Consider a fast-follow issue for the acknowledged modified-file refinement (skip fetch when the diff already covers most of a modified file) once `PullRequestFile` carries patch/line-count data, as the PR description itself flags as out of scope.
  • If not already covered elsewhere, a test asserting behavior when an added file's diff is truncated could preempt the edge case above.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #3897
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: 51 registered-repo PR(s), 43 merged, 343 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 51 PR(s), 343 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: not available
  • Official Gittensor activity: 51 PR(s), 343 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

@codecov

codecov Bot commented Jul 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.58%. Comparing base (61109fe) to head (a1751b9).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3918   +/-   ##
=======================================
  Coverage   93.58%   93.58%           
=======================================
  Files         358      358           
  Lines       34342    34342           
  Branches    12570    12570           
=======================================
  Hits        32138    32138           
  Misses       1580     1580           
  Partials      624      624           
Files with missing lines Coverage Δ
src/review/review-grounding.ts 92.85% <100.00%> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@gittensory-orb gittensory-orb Bot left a comment

Copy link
Copy Markdown

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.

@gittensory-orb gittensory-orb Bot merged commit 4942547 into main Jul 7, 2026
10 checks passed
@gittensory-orb gittensory-orb Bot deleted the perf/skip-duplicate-added-file-grounding branch July 7, 2026 06:59
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.5x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

perf(review): skip duplicate full-file-content fetch for added files already covered by the diff

1 participant