Skip to content

perf(server): reuse origin metadata within PR lookups - #11389

Closed
yashranaway wants to merge 1 commit into
pingdotgg:mainfrom
yashranaway:perf/pr-origin-lookup
Closed

yashranaway wants to merge 1 commit into
pingdotgg:mainfrom
yashranaway:perf/pr-origin-lookup

Conversation

@yashranaway

@yashranaway yashranaway commented Sep 12, 2026 •

Copy link
Copy Markdown
Contributor

Repeated PR lookups resolve the head remote and origin independently, even when both are origin. This spawns two identical Git config reads during cache verification.

Reuse the origin repository result within each lookup when it is also the head remote. Distinct remotes still resolve concurrently, and every request still checks current remote identity. This addresses the duplicate-read portion of #11220.

Validation: all 113 GitManager tests pass, including a regression that fails before the fix (two origin reads) and passes afterward (one). Server typecheck and targeted lint pass.

Model: GPT-6
Harness: Codex in T3 Code

Summary by CodeRabbit

  • Bug Fixes

    • Improved repeated pull request lookups for the same branch by reusing cached repository data.
    • Reduced redundant remote repository checks during branch and pull request resolution.
  • Tests

    • Added regression coverage to verify cached data is reused for repeated lookups.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Sep 12, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 12, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 221d8a2

Macroscope's review found this PR approvable — This is a small, self-contained server performance refactor that reuses origin metadata during PR lookups while preserving distinct-remote behavior. The accompanying test verifies the reduced Git configuration reads, and no production defaults, schemas, infrastructure, or sensitive areas are changed.

Notes:

  • Diff unchanged. Approvability was decided on eligibility alone.

You can add or adjust custom eligibility rules. Learn more.

macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Sep 12, 2026
@coderabbitai

coderabbitai Bot commented Sep 12, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 8a2e4ded-5fc7-4b9f-ab25-93f050c483a3

📥 Commits

Reviewing files that changed from the base of the PR and between ca6416e and 221d8a2.

📒 Files selected for processing (2)
  • apps/server/src/git/GitManager.test.ts
  • apps/server/src/git/GitManager.ts

Limit details: You’ve used all 10 included reviews currently available.


📝 Walkthrough

Walkthrough

GitManager now shares the origin repository context when resolving pull-request identity and branch-head context. A regression test verifies that repeated warm lookups avoid GitHub calls and read the origin URL once.

Suggested reviewers: juliusmarminge

Changes

Git repository context caching

Layer / File(s) Summary
Shared context resolution and regression coverage
apps/server/src/git/GitManager.ts, apps/server/src/git/GitManager.test.ts
GitManager reuses one origin repository lookup when the head remote is origin. Pull-request identity and branch-head resolution use the shared resolver. Tests verify cached lookups and Git configuration reads.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Merge Risk: ⚪ Minimal · up to 221d8

The origin metadata reuse preserves repository context while avoiding duplicate configuration reads, with no current merge-blocking risk identified.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main performance change: reusing origin metadata during server pull request lookups.
Description check ✅ Passed The description explains what changed, why it changed, the expected behavior for distinct remotes, and the validation results. It does not use the template headings or include the checklist, but the c…
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Usage-based review receipt

Note

This review was completed with usage-based billing: files reviewed beyond your plan's included limits are billed at $0.25/file. View usage-based billing.


Comment @coderabbitai help to get the list of available commands.

@juliusmarminge juliusmarminge added the macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews label Oct 1, 2026 — with ChatGPT Codex Connector
@macroscopeapp
macroscopeapp Bot dismissed their stale review October 1, 2026 02:48

Dismissing prior approval to re-evaluate 221d8a2

@t3dotgg

t3dotgg commented Oct 6, 2026

Copy link
Copy Markdown
Member

Note

🤖 Claude Opus 5.5 responding on behalf of Theo

Closing as superseded. The same fix landed in #16272: when a branch tracks origin, the PR lookup now reads remote.origin.url once instead of twice (resolveHeadAndOriginContexts in apps/server/src/git/GitManager.ts), with a test that asserts one read.

A rebase of this branch onto main conflicts only with that identical code, so nothing is left to land. Thanks @yashranaway for finding and fixing this.

@t3dotgg t3dotgg closed this Oct 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews size:S 10-29 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants