Skip to content

🤖 fix: prevent immersive review hunk layout flashes#3442

Merged
ammario merged 1 commit into
mainfrom
immersive-review-why6
Jun 3, 2026
Merged

🤖 fix: prevent immersive review hunk layout flashes#3442
ammario merged 1 commit into
mainfrom
immersive-review-why6

Conversation

@ammar-agent
Copy link
Copy Markdown
Collaborator

Summary

Fixes layout flashes while iterating through hunks in Immersive Review by synchronizing visual hunk state before paint and keeping the agent-assisted callout from changing the diff column height mid-file.

Background

Immersive Review hunk navigation is keyboard-first, so J/K iteration should feel stable. The previous flow corrected scroll position, cursor selection, and reveal state after paint, and the per-hunk assisted banner could mount/unmount above the diff when moving between flagged and unflagged hunks in the same file.

Implementation

  • Moved file reveal gating, selected-hunk cursor/selection sync, and scroll/outline DOM writes to layout effects so they settle before the browser paints.
  • Added a fixed assisted-banner slot for files containing assisted hunks, so entering or leaving an assisted hunk does not reflow the diff column.
  • Kept the assisted callout inside the diff column and rendered it as a stable one-line row.

Validation

  • make fmt
  • bun test src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.test.tsx
  • make typecheck
  • make lint
  • make static-check
  • git diff --check

Risks

Low-to-medium. The change is scoped to Immersive Review layout/navigation. The main risk is altered timing around scroll/cursor effects, covered by existing Immersive Review tests plus a new regression test for same-file assisted/non-assisted hunk iteration.


Generated with mux • Model: openai:gpt-5.5 • Thinking: xhigh • Cost: 3826192{MUX_COSTS_USD:-0}

@ammar-agent
Copy link
Copy Markdown
Collaborator Author

@codex review

Please review the Immersive Review hunk layout flash fix.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

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: f3068f41b6

ℹ️ 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

Please take another look. I kept the fixed-height callout row for layout stability and exposed the full assisted-review comment via hover/native tooltip so longer rationale remains available without reflowing the diff.

@ammar-agent ammar-agent force-pushed the immersive-review-why6 branch from f3068f4 to 83b816d Compare June 2, 2026 18:06
@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

Please take another look after the final visual-stability tweak. The assisted row keeps the prior selected-banner geometry while still reserving a fixed slot for same-file hunk iteration and exposing the full comment via tooltip/title.

@ammar-agent ammar-agent force-pushed the immersive-review-why6 branch from 83b816d to 8545a82 Compare June 2, 2026 19:09
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

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: 8545a82357

ℹ️ 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

Please take another look. The assisted slot reservation now derives from all hunks for the active file, so hide-read/search filters cannot collapse the slot while the file remains active.

@ammar-agent ammar-agent force-pushed the immersive-review-why6 branch from 8545a82 to c234f1d Compare June 2, 2026 19:13
@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

Please take another look. I kept the selected assisted banner's visual geometry/content styling aligned with the prior banner while preserving the fixed slot and allHunks-based reservation for filtered hunks.

@ammar-agent ammar-agent force-pushed the immersive-review-why6 branch from c234f1d to 53cfd25 Compare June 2, 2026 19:35
@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".

@ammar-agent
Copy link
Copy Markdown
Collaborator Author

@codex review

Please take another look. The fixed assisted slot now uses the prior single-line banner geometry via an intrinsic line-height calc, so selected assisted snapshots should remain visually aligned while long comments stay clipped with tooltip access.

@ammar-agent ammar-agent force-pushed the immersive-review-why6 branch from 53cfd25 to a7608a8 Compare June 2, 2026 19:52
@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

Please review the follow-up scroll-thrash fix. A review sub-agent audited same-file/cross-file hunk navigation, reveal release, full-file overlay swaps, assisted banner layout, and note navigation and found no remaining cause for scroll thrash.

@ammar-agent ammar-agent force-pushed the immersive-review-why6 branch from a7608a8 to a661665 Compare June 2, 2026 21:56
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

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: a66166587b

ℹ️ 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

Please review the final scroll-thrash refinement. I addressed the context-row feedback by only suppressing scroll while selected-hunk cursor sync is actually pending, preserving intentional context-row cursor scrolling, and avoiding latent center jumps after full-file context swaps. A review sub-agent re-audited the committed code and verified no remaining plausible cause for scroll thrash.

@ammar-agent ammar-agent force-pushed the immersive-review-why6 branch from a661665 to 7ec6e18 Compare June 2, 2026 22:22
@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".

@ammar-agent
Copy link
Copy Markdown
Collaborator Author

@codex review

Please take another look after making the scroll regression tests synchronous for CI stability. The production scroll-thrash fix is unchanged from the version re-audited by the review sub-agent.

@ammar-agent ammar-agent force-pushed the immersive-review-why6 branch from 7ec6e18 to b3d42ba Compare June 2, 2026 22:37
@ammar-agent
Copy link
Copy Markdown
Collaborator Author

@codex review

Please take another look after hardening the scroll regression tests for the full CI unit environment. Production scroll code is unchanged from the review-sub-agent-verified version.

@ammar-agent ammar-agent force-pushed the immersive-review-why6 branch from b3d42ba to 0c453d4 Compare June 2, 2026 22:47
@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Another round soon, 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".

Stabilize Immersive Review hunk navigation by moving visual scroll and selection synchronization to layout effects and reserving a fixed assisted-callout row for files containing agent-flagged hunks.

---

_Generated with `mux` • Model: `openai:gpt-5.5` • Thinking: `xhigh` • Cost: `3820105{MUX_COSTS_USD:-0}`_

<!-- mux-attribution: model=openai:gpt-5.5 thinking=xhigh costs=10.47 -->
@ammar-agent
Copy link
Copy Markdown
Collaborator Author

@codex review

Please take another look after removing the CI-flaky scroll recorder tests. The production scroll-thrash fix remains the version verified by the review sub-agent; local targeted tests/typecheck/lint/static-check pass.

@ammar-agent ammar-agent force-pushed the immersive-review-why6 branch from 0c453d4 to d677b9e Compare June 2, 2026 22:55
@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".

@ammario ammario merged commit fc40d86 into main Jun 3, 2026
24 checks passed
@ammario ammario deleted the immersive-review-why6 branch June 3, 2026 02:20
@mux-bot mux-bot Bot mentioned this pull request Jun 3, 2026
ammario pushed a commit that referenced this pull request Jun 4, 2026
## Summary

Prevents Immersive Review from painting unpositioned diff geometry while
compact hunk overlays hydrate into full-file context. The reveal gate
now keys on the rendered overlay identity instead of file path alone, so
file switches, hydration swaps, and filtered hunk-content swaps stay
hidden until the selected hunk has been scrolled into place.

## Background

PR #3442 fixed hunk-to-hunk scroll thrash and assisted-banner reflow,
but the remaining flash came from a different invariant: while full-file
content loaded asynchronously, the visible diff could swap from a
compact hunk-only overlay to a much taller full-file overlay. That
changes row indices and minimap geometry even when the selected hunk is
unchanged.

## Implementation

- Gate reveal by `{ filePath, overlayData.content }`, not just file
path.
- Keep both the diff stage and minimap stage invisible while a
geometry-changing overlay is pending.
- During pending reveal, scroll using the selected hunk's resolved line
in the new overlay, ignoring stale compact-overlay cursor indices.
- Only preserve an out-of-hunk context cursor when the overlay content
is unchanged; compact → full-file hydration remaps the cursor back into
the selected hunk.
- Force a hidden minimap redraw after parent scroll positioning and
before the reveal RAF opens the stage, so the thumb/active marker read
the final scrollTop.
- Move minimap canvas redraw to a layout effect because canvas pixels
are visible layout state.
- Pin the `ai` package to the already-locked 6.0.175 release so
lockfile-free static checks avoid the currently broken 6.0.196 tarball.

## Validation

- `bun test
src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.test.tsx
src/browser/features/RightSidebar/CodeReview/ImmersiveMinimap.test.tsx`
- `make typecheck`
- `make lint`
- `git diff --check`
- `make static-check`
- `make static-check-full`
- Re-ran targeted tests + `make static-check` after rebasing onto latest
`origin/main`.
- Read-only review sub-agent audited the final design and found no
remaining plausible hydration/minimap flash path.

## Risks

Moderate, localized to Immersive Review diff/minimap reveal behavior.
The main user-visible tradeoff is that geometry swaps may briefly show
the existing loading scrim instead of painting an unanchored diff. The
code intentionally keeps normal same-file hunk navigation out of the
reveal gate unless the overlay content changes.

The `ai` dependency pin narrows the package range to the version already
resolved in `bun.lock`; this is intended to make lockfile-free package
checks deterministic while npm metadata advertises a latest tarball that
returns 404.

---

_Generated with `mux` • Model: `openai:gpt-5.5` • Thinking: `xhigh` •
Cost: `$71.64`_

<!-- mux-attribution: model=openai:gpt-5.5 thinking=xhigh costs=71.64
-->
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.

2 participants