Skip to content

🤖 refactor: clean up immersive review reveal performance#3473

Open
ammar-agent wants to merge 3 commits into
mainfrom
review-perf-t98b
Open

🤖 refactor: clean up immersive review reveal performance#3473
ammar-agent wants to merge 3 commits into
mainfrom
review-perf-t98b

Conversation

@ammar-agent

@ammar-agent ammar-agent commented Jun 5, 2026

Copy link
Copy Markdown
Collaborator

Summary
Refactors immersive review's reveal pipeline so file-content hydration, overlay identity, highlight readiness, and reveal scheduling live behind a focused hook instead of the monolithic view, and removes the remaining flash paths. The loading cover is now reserved for genuine file switches: iterating hunks within one file — including marking hunks read while read hunks are hidden — is instant, with no loading skeleton. The branch still includes the original immersive hunk-iteration perf benchmark and real Electron coverage for minimap and same-file stability. Rebased onto main.

Background
Immersive review was still visibly flashy. Reveal state was scattered across file-loading effects, highlight callbacks, scroll layout effects, sticky measured overlays, and full-content string keys inside ImmersiveReviewView, making it easy to render or reveal intermediate geometry. A concrete symptom remained after the first cleanup pass: when read hunks are hidden, marking a hunk read removes it from the visible hunk set, which (a) invalidated the full-file content cache because its key included the hunk signature, forcing a redundant file re-read, and (b) changed the overlay contentKey, which the reveal cover was gated on — so every same-file mark-read replayed the full-screen loading skeleton even though the file was already on screen.

Implementation

  • Extracted full-file loading, overlay construction, compact contentKey identity, highlight readiness, and reveal scheduling into useImmersiveOverlay.
  • Extracted the shimmer loading cover into ImmersiveDiffRevealLoadingState; it is an absolute scrollport sibling (no ResizeObserver/measured height).
  • Keyed the full-file content cache by (workspaceId, filePath) only. File bodies do not depend on the hunk set, so hunk-set changes (mark-read, filter toggles) no longer invalidate the cache or trigger a re-read; the overlay is rebuilt in-memory from the cached body plus the live hunks.
  • Split reveal gating: the loading cover, hidden diff stage, and hidden minimap now gate on a file-path change (isActiveFileRevealPending). The content-key pending state is retained only to re-run the hidden scroll pass that keeps the selected hunk in view across same-file rebuilds. Same-file overlay changes reveal in place; only switching to a different file shows the cover.
  • Scoped the overlay deferral (blank stage behind the cover) to genuine new-file loads and resolved already-cached bodies synchronously during render, so revisiting a file or rebuilding the same-file overlay never blanks behind the cover.
  • Made useHighlightedDiff avoid rebuilding fallback chunks on preloaded cache hits and avoid redundant state churn.
  • Added cheap immersive debug attributes used by E2E sampling instead of per-frame textContent reads.

Validation

  • bun test src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.test.tsx (incl. new "keeps same-file hunk-set changes instant without re-reading or re-covering")
  • bun test src/browser/features/Shared/DiffRenderer.test.tsx src/browser/features/Shared/SelectableDiffRenderer.dragSelect.test.tsx
  • make static-check / make build
  • Electron hydration suite (file-switch covers + same-file/mark-read instant), incl. new "keeps same-file mark-read iteration instant while read hunks are hidden": xvfb-run -a env MUX_E2E_LOAD_DIST=1 MUX_E2E_SKIP_BUILD=1 PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD=1 bun x playwright test --project=electron tests/e2e/scenarios/reviewHydration.spec.ts
  • Electron perf specs: xvfb-run -a env MUX_E2E_RUN_PERF=1 MUX_PROFILE_REACT=1 MUX_E2E_LOAD_DIST=1 MUX_E2E_SKIP_BUILD=1 PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD=1 bun x playwright test --project=electron tests/e2e/scenarios/perf.reviewHunkIteration.spec.ts tests/e2e/scenarios/perf.reviewMarkHunkRead.spec.ts

Risks

  • Moderate UX risk around reveal timing and scroll behavior; mitigated by Electron frame-sampling tests asserting the cover still appears for file switches / Shift+M file-read advancement / compact-to-full hydration, and is absent during same-file J/K and same-file mark-read iteration.
  • Moderate shared-renderer risk from the useHighlightedDiff cache-hit change; covered by targeted shared diff renderer tests and static checks.

Generated with mux • Model: anthropic:claude-opus-4-8 • Thinking: max • Cost: $1352.76

@ammar-agent

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. More of your lovely PRs please.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@ammar-agent

Copy link
Copy Markdown
Collaborator Author

@codex review

Updated the perf scenario to wait until all rendered diff rows are syntax-highlighted before resetting profiler samples and measuring hunk iteration.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. What shall we delve into next?

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@ammar-agent

Copy link
Copy Markdown
Collaborator Author

@codex review

Updated this from measurement-only to include the immersive review optimization. The new highlighted harness now reports TaskDuration 0.505s for 60 hunk iterations.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e2ccc6dc06

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@ammar-agent

Copy link
Copy Markdown
Collaborator Author

@codex review

Addressed the read-action sync finding by committing the immersive-local selected hunk back to the parent before hunk read / mark-file-read actions, then revalidated the highlighted perf scenario at TaskDuration 0.565s.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c02d491434

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@ammar-agent

Copy link
Copy Markdown
Collaborator Author

@codex review

Addressed the unmount path by committing the immersive-local selected hunk in cleanup as well as on explicit exit/read actions. Revalidated the highlighted perf scenario at TaskDuration 0.739s.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 87f8f56eb8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.tsx Outdated
@ammar-agent

Copy link
Copy Markdown
Collaborator Author

@codex review

Addressed the mark-file-read stale closure by making the parent handler consult selectedHunkIdRef.current and revalidated the highlighted perf scenario at TaskDuration 0.506s.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2e29f1b412

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@ammar-agent

Copy link
Copy Markdown
Collaborator Author

@codex review

Addressed the visible multi-line hunk range finding by applying an imperative selected-range highlight for J/K hunk jumps without re-rendering the syntax-highlighted diff tree. Revalidated at TaskDuration 0.570s.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dbcc416f01

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/browser/features/RightSidebar/CodeReview/ReviewPanel.tsx
@ammar-agent

Copy link
Copy Markdown
Collaborator Author

@codex review

Addressed persisted selection during page teardown by persisting parent selected-hunk state on change, while immersive hot-path J/K iteration remains local. Revalidated at TaskDuration 0.561s.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3a5c82cc1c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@ammar-agent

Copy link
Copy Markdown
Collaborator Author

@codex review

Addressed parent teardown by writing the immersive-local selected hunk directly to persisted state during immersive cleanup, using a shared selected-hunk storage key helper. Revalidated at TaskDuration 0.605s.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Nice work!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@ammar-agent

Copy link
Copy Markdown
Collaborator Author

@codex review

Addressed the viewport jump feedback by using nearest scroll alignment for same-file J/K hunk iteration while keeping centering for file switches, note jumps, undo, and overlay hydration. Revalidated the highlighted perf scenario at TaskDuration 0.569s.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 10b3e2b810

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep it up!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@ammar-agent

Copy link
Copy Markdown
Collaborator Author

@codex review

Follow-up after three independent review sub-agents: addressed the remaining Shift+M stale-file flash path, updated stale reveal comments, and added a bounded fail-open for full-file hydration/highlight stalls.

@ammar-agent ammar-agent changed the title 🤖 bench: profile immersive hunk iteration 🤖 refactor: clean up immersive review reveal performance Jun 7, 2026
@ammar-agent

Copy link
Copy Markdown
Collaborator Author

@codex review

I refactored the immersive review reveal pipeline and expanded the Electron hydration coverage. Please review the updated branch.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 466e8cef81

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/browser/features/RightSidebar/CodeReview/useImmersiveOverlay.ts
@ammar-agent

Copy link
Copy Markdown
Collaborator Author

@codex review

Addressed the same-file compact→full hydration gate: full-file-context waits now apply to any pending overlay content-key swap, not just file-path swaps, and I added a regression test for the same-file far-hunk-to-near-hunk transition.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f663fb1d3b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/browser/features/RightSidebar/CodeReview/useImmersiveOverlay.ts Outdated
@ammar-agent

Copy link
Copy Markdown
Collaborator Author

@codex review

Pushed the same-file hydration gate fix and regression test. Please take another look.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Swish!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@ammar-agent

Copy link
Copy Markdown
Collaborator Author

@codex review

Addressed the CRLF overlay feedback by normalizing hunk rows in compact overlay construction and added a regression test for CRLF hunk content.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Delightful!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@ammar-agent

Copy link
Copy Markdown
Collaborator Author

@codex review

One more small CI fix: the loading skeleton no longer exposes data-line-index, so happy-dom selectors cannot click skeleton rows while the hidden diff stage is still hydrating.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep them coming!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Add a perf fixture and Electron profiling scenario that reproduces large-file immersive review hunk iteration across 60 J-key steps after the full-file diff is syntax highlighted. Optimize immersive hunk navigation to keep selection local while iterating, avoid per-step localStorage writes, and keep diff renderer callbacks stable so the highlighted diff tree is not re-rendered on each jump.\n\nOptimized highlighted baseline captured locally: 1,500-line file, 150 hunks, 1,650 rendered diff rows syntax-highlighted, 60 iterations, 300 ms wall time, 0.505 s TaskDuration, median 4.70 ms, p95 5.50 ms, max 12.60 ms.\n\n---\n\n_Generated with `mux` • Model: `openai:gpt-5.5` • Thinking: `xhigh` • Cost: `unknown`_\n\n<!-- mux-attribution: model=openai:gpt-5.5 thinking=xhigh costs=unknown -->
@ammar-agent

Copy link
Copy Markdown
Collaborator Author

@codex review

Latest push rebases onto main and fixes excess loading states during same-file hunk iteration: the full-file content cache is now keyed by path only (so marking a hunk read no longer re-reads the file), and the loading cover/hidden-stage/hidden-minimap now gate on file-path changes rather than overlay content-key changes, so same-file hunk iteration (including mark-read while read hunks are hidden) reveals in place without the skeleton. File switches remain covered.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b2831f5c46

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/browser/features/RightSidebar/CodeReview/useImmersiveOverlay.ts Outdated
@ammar-agent

Copy link
Copy Markdown
Collaborator Author

@codex review

Addressed the P2 (stale full-file cache when the file changes on disk).

The full-file body cache is now keyed by (workspaceId, filePath, fileContentVersion). fileContentVersion is an FNV-1a hash of the file's unfiltered diff hunk content (headers + bodies), so:

  • Marking a hunk read while read hunks are hidden filters the visible set but leaves the unfiltered diff content unchanged → version stable → cache hit → still instant, no re-read, no cover.
  • A real file change (tool edits the file, diff is re-fetched) changes the diff content → version changes → cache miss → re-read with fresh context. I hash content rather than hunk ids on purpose, because ids are a hash of path + line ranges and would not change for an in-place edit that preserves line ranges.

Added unit coverage: "re-reads the full-file body when the file's diff content changes" (same hunk id, changed content → re-read) alongside the existing "keeps same-file hunk-set changes instant without re-reading or re-covering".

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4c06a1e402

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/browser/features/RightSidebar/CodeReview/useImmersiveOverlay.ts
Marking a hunk read (or any same-file hunk-set change) used to invalidate the
full-file cache and replay the reveal skeleton. Key the file-content cache by
(path, content-version), scope overlay deferral to genuine file switches, and
gate the loading cover on file-path changes so same-file iteration reveals in
place. The content-version is derived from the file's unfiltered diff content,
so a real file change (tool edit / diff refresh) still busts the cache while
mark-read filtering does not.
@ammar-agent

Copy link
Copy Markdown
Collaborator Author

@codex review

Addressed the second P2 (stale paint on same-path in-place edits).

ActiveFileContentState now tracks the cache key (workspace + path + content version) the body was loaded for, and isActiveFileContentSettled compares that cache key against the current one instead of only the file path. So when a file is edited in place (same path, new content version):

  • the cache key is busted (miss), and
  • the settled check now returns false because the stored body belongs to the previous version,

which means resolvedActiveFileContent no longer falls back to the previous version's body. The overlay drops to the compact hunk (correct, built from the fresh hunks) for the brief window until the new full-file body loads — no stale full-file context is ever painted. Strengthened the unit test to assert the previous body's context line disappears immediately on the edit and only returns after the re-read.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🎉

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant