Skip to content

fix(web): PR code view keeps its scroll when a turn finishes - #13835

Open
sameerr03 wants to merge 1 commit into
pingdotgg:mainfrom
sameerr03:fix/pr-code-soft-turn-refresh
Open

sameerr03 wants to merge 1 commit into
pingdotgg:mainfrom
sameerr03:fix/pr-code-soft-turn-refresh

Conversation

@sameerr03

@sameerr03 sameerr03 commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

What Changed

A turn finishing no longer resets an open pull request's Code tab. The tab re-reads the first diff page in the background and keeps its viewer, its loaded pages, and its scroll position on screen.

The Code tab now receives two refresh signals instead of one combined token:

  • Refresh button, or a new PR revision (updatedAt). Unchanged: the diff starts over from page one.
  • A turn finishing (the server's environment-wide refresh epoch). The tab points back at the first page without clearing what is shown. The existing page merge does the rest. An unchanged answer keeps every loaded page. A changed one replaces that page and the pages after it.

Why

An open PR Code view jumped back to the top whenever a turn finished anywhere in the environment: the same thread, another worktree, or another project. The server publishes one refresh epoch per finished turn. The panel added that epoch to the same token as the Refresh button, so the Code tab cleared its pages, showed "Loading pull request diff...", and mounted a new viewer at scroll 0.

That signal means "something may have changed", and usually nothing has. A soft re-read handles both cases. A push that really changes the diff also changes the PR's updatedAt, so it still gets the full reload, and pages after the first cannot go stale.

UI Changes

Setup: thread A runs a turn while thread B shows a PR's Code tab, scrolled to 820 px.

Before: B jumps to the top when A finishes.

abc-reset.mp4

After: B stays at 820 px. The viewer stays the same and no loading state appears.

abc-fixed.mp4

Verification

Real web UI on 95030dc674, with real provider turns:

  • A/B/C case: 820 → 820 px. Before this change: 820 → 0.
  • Turns finishing in the same thread, another thread in the same project, and another project: scroll kept in all three.
  • 351-file PR with two diff pages loaded, scrolled to 200,000 px: both pages and the scroll position kept. Page 3 still loads afterwards.
  • Refresh button: still shows the loading state and restarts from page one.
  • Single-commit scope, viewed-file ticks, the Summary tab, the chat, the unsent draft, Files, and Diff: all unchanged.
  • Not tested: a real push to a PR (the updatedAt path, which this change leaves as it was) and the desktop app (same components).
  • tsc --noEmit for apps/web, lint on the changed files (no new findings), and PullRequestDetailPanel.test.tsx all pass.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for any UI changes
  • I included a video for animation/interaction changes

Implemented with Claude Opus 5.5 in Claude Code, running in T3 Code. UI verification by a GPT-6 Astra computer-use agent.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Background updates to pull request code now refresh diff and viewed-file information while retaining already loaded pages when the first page is unchanged. If the diff has changed, later pages are updated accordingly.
    • After a background update, the diff view returns to its first page.

A finished turn anywhere in the environment bumped the same token as the
Refresh button, so the Code tab cleared its diff pages, showed the loading
state, and remounted the viewer at the top. Turn refreshes now re-read the
first page in place; the Refresh button and a new PR revision still start over.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Sep 26, 2026
@sameerr03
sameerr03 marked this pull request as ready for review September 26, 2026 12:34
@coderabbitai

coderabbitai Bot commented Sep 26, 2026

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: a78da863-f382-4ae2-8003-206c5a95ca16

📥 Commits

Reviewing files that changed from the base of the PR and between 95030dc and 3472adc.

📒 Files selected for processing (2)
  • apps/web/src/components/pullRequest/PullRequestCodeTab.tsx
  • apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The Code tab now receives manual and background refresh tokens separately. When the background token changes, it refreshes diff and viewed-file data from the first diff page while retaining accumulated slices.

Changes

Pull request code refresh

Layer / File(s) Summary
Pass and handle separate refresh signals
apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx, apps/web/src/components/pullRequest/PullRequestCodeTab.tsx
The detail panel passes separate manual and background refresh tokens. The Code tab resets its requested cursor to the first page and refreshes diff and viewed-file data when the background token changes. It retains later pages if the first-page response is unchanged; otherwise, existing slice-update logic replaces subsequent pages.

Priority: ⬇️ Low

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

Change: Bug fix

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 3472a

Turn completion can refresh the Code tab without resetting its scroll position, and no actionable merge-blocking issue remains after normal checks.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 3472a

Background refreshes keep the Code tab stable, but a switch between environments showing the same pull request may leave pages from the previous environment on screen. No new server access or authorization bypass is evident; the concern is which cached pages the UI displays.

Retained concerns

  • Medium · security · inferred: A background refresh can retain diff pages under the same reference and commit when the environment changes. If the new environment returns an identical first page, later pages from the previous environment can remain displayed; unlike the former turn-triggered clearing path, the new path does not clear them.
Security review details

Security Blast Radius

  • inferred — The identified exposure is previously loaded diff content in the current browser view, not evidence of a new cross-environment server read or gained privilege.

Security Findings and Attack Paths

  • inferred — If the Code tab remains mounted across an environment switch with the same reference and commit, its old later pages can survive a soft refresh when the new first page compares equal. Actual reuse and the resulting display have not been verified.

Trust Boundaries and Controls

  • observed — The refresh token is a read trigger: the diff query continues to carry the selected environment and PR reference, and viewed-file reads retain their capability gate.

Hardening Proposals

  • proposed — Include environment identity in the retained diff-slice scope and verify that an older refresh completion cannot replace data for a newer context or refresh intent.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main fix: preserving the pull request Code view scroll position when a turn finishes.
Description check ✅ Passed The description is complete and focused. It explains what changed, why it changed, UI behavior before and after, verification results, untested cases, and checklist status. The UI evidence is provided…
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.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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

@macroscopeapp

macroscopeapp Bot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 3472adc

Macroscope's review found this PR approvable — This is a small, focused UI bug fix that separates background turn refreshes from explicit diff reloads, preserving the existing viewer and scroll position while retaining hard reload behavior when needed. It introduces no schema, deployment, security, billing, default, or static-analysis changes.

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

This branch has not been deployed

No deployments
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:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants