Skip to content

Comments

🤖 feat: immersive keyboard-first code review mode#2448

Open
ibetitsmike wants to merge 45 commits intomainfrom
mike/immersive-review
Open

🤖 feat: immersive keyboard-first code review mode#2448
ibetitsmike wants to merge 45 commits intomainfrom
mike/immersive-review

Conversation

@ibetitsmike
Copy link
Contributor

@ibetitsmike ibetitsmike commented Feb 17, 2026

Summary

Introduces immersive, keyboard-first code review and hardens it through iterative UX/performance/layout fixes:

  • global immersive toggle shortcut (works outside Review tab)
  • coherent whole-file immersive rendering (full file + hunk overlays)
  • file/hunk/line keyboard navigation with centered hunk jumps
  • inline line-level quick feedback (Shift+L comment, Shift+D dislike)
  • immersive notes sidebar with note list + sentiment icons
  • notes-panel E shortcut to jump to selected note and start editing immediately
  • regression fixes for inline edit cancel (Esc) and bottom-of-viewport comment targeting
  • Tab panel-toggle now ignores editable fields (won’t steal focus while editing notes)
  • large-file navigation performance improvements
  • follow-up correctness and accessibility fixes from Codex review
  • final diff-row height regression fixes (highlighted vs plain rows now match)

Background

Immersive mode started as a full-screen overlay for fast keyboard review. Follow-up feedback required coherent whole-file context, better line-level composition, robust note navigation under filters, and smoother behavior on large files. Codex review then surfaced several edge cases in fallback rendering, overlay state, and focus isolation. Recent UX feedback required tightening keyboard-event isolation and line/hunk sync so inline edits and bottom-of-screen comment actions reliably target the intended rows.

Implementation

Immersive review UX

  • Shift+I opens/toggles immersive mode globally via RightSidebar.
  • n/p navigate files, k/j navigate hunks, arrow keys move/extend line selection.
  • Hunk jumps target first modified line and center viewport.

Inline composition + notes workflow

  • Removed header quick-action buttons.
  • Quick actions now open the existing inline composer at selected lines.
  • Composer request lifecycle was hardened to avoid stale/replayed requests after hunk/file changes.
  • Added immersive notes-panel E key flow: with a selected note, it navigates to the note and opens that inline review note directly in edit mode.
  • Added explicit edit-request plumbing from immersive view into inline review rows so the exact targeted note enters edit mode once.

Sentiment icon consistency

  • Shift+D quick notes retain thumbs-down behavior.
  • Shift+L quick notes now render a thumbs-up icon in the immersive notes list.

Regression hardening (latest)

  • Inline note editor now stops keyboard propagation for save/cancel keys so Esc cancels local editing without triggering higher-level immersive shortcuts.
  • Diff line interactions now keep immersive line selection synchronized for click/drag/comment actions.
  • Composer open flow now carries the intended target line through hunk resolution, preventing fallback jumps to hunk-start when commenting near the viewport bottom.
  • Immersive Tab panel switching now ignores editable targets, so typing/editing in inline note textareas keeps focus stable.

Performance and correctness

  • Reduced cursor-move re-render pressure in immersive mode.
  • Added high-line-count syntax-highlighting guard for very large overlays.
  • Fixed fallback overlay/index mapping to keep rendered rows and selectable indices aligned.
  • Normalized highlighted per-line HTML to strip embedded CR/LF that could inflate row layout.
  • Normalized immersive file-content splitting for CRLF input.
  • Kept per-line comment controls as absolute overlays so they never contribute to row height.

Accessibility + layout stability

  • Immersive safety now clears persisted immersive state when Review panel is not mounted/collapsed.
  • Background panes are hidden from AT and made inert while immersive mode is active.
  • To avoid broad shell layout regressions, inert/aria handling is applied directly at the ChatPane and Sidebar container roots (instead of adding extra wrapper layout nodes in WorkspaceShell).

Debug/verification story

  • Added a Storybook immersive-review scenario that mixes syntax-highlighted rows with an oversized plain fallback chunk to make row-height drift easy to detect.

Validation

  • make static-check
  • make typecheck
  • make lint
  • bun test src/browser/utils/review/navigation.test.ts src/browser/utils/review/quickReviewNotes.test.ts
  • bun run eslint src/browser/components/RightSidebar/CodeReview/ImmersiveReviewView.tsx src/browser/components/shared/DiffRenderer.tsx src/browser/components/shared/InlineReviewNote.tsx
  • Storybook manual measurement on ReviewTabImmersiveHighlightVsPlainHeight confirms highlighted and plain rows both render at ~15.391px.

Risks

  • inert support is broad in modern browsers/Electron, but still an integration-sensitive behavior around focus in complex nested interactive trees.
  • Mobile/stacked immersive UX remains less exercised than desktop.

Generated with mux • Model: openai:gpt-5.3-codex • Thinking: xhigh • Cost: $25.18

Copy link

@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: 5a91db7631

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@ibetitsmike ibetitsmike force-pushed the mike/immersive-review branch from 5a91db7 to cf6e7d9 Compare February 17, 2026 09:18
@ibetitsmike
Copy link
Contributor Author

@codex review

Addressed both unresolved review threads:

  • clear immersive flag when no review panel is mounted
  • skip empty files during immersive file navigation

Please take another look.

Copy link

@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: 205e01d084

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@ibetitsmike
Copy link
Contributor Author

@codex review

Addressed both latest comments:

  • aligned fallback hunk overlay indices with rendered rows
  • note navigation now targets the matching hunk by parsed line range

Please re-review.

Copy link

@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: fe46eb0182

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@ibetitsmike
Copy link
Contributor Author

@codex review

Addressed latest feedback:

  • fallback @@ header rows are no longer included in immersive line index mapping
  • note-to-hunk navigation by parsed line range remains in place

Please review again.

Copy link

@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: ca76bb1d52

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@ibetitsmike
Copy link
Contributor Author

@codex review

Addressed latest issue by resolving notes->hunk targeting against the unfiltered hunk set (allHunks) so note navigation still works when filters hide the corresponding hunk.

Please review again.

Copy link

@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: 012686220e

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@ibetitsmike
Copy link
Contributor Author

@codex review

Addressed latest feedback by treating collapsed sidebar as no mounted review panel when deciding whether immersive mode can stay enabled.

Please re-review.

Copy link

@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: 34782b7821

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Copy link

@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: 34782b7821

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@ibetitsmike
Copy link
Contributor Author

@codex review

Addressed latest P1s:

  • immersive note navigation now preserves context when target hunks are filtered out
  • external-selection composer state is now cleared when request becomes null to prevent stale composer submission

Please review again.

Copy link

@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: 9241a37c30

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@ibetitsmike
Copy link
Contributor Author

@codex review

Addressed latest threads:

  • external inline composer requests now reapply when mapped selection changes and clear when request is removed
  • immersive inline composer requests are cleared when switching away from the target hunk to avoid stale reopen
  • added explicit aria-hidden wrappers in WorkspaceShell so immersive overlay semantically isolates background panes
  • note navigation continues to preserve filtered/unfiltered context

Please re-review.

Copy link

@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: c11dacf72c

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@ibetitsmike
Copy link
Contributor Author

@codex review

Addressed latest focus/interaction issue by applying/removing inert imperatively on WorkspaceShell background pane wrappers during immersive mode (while keeping aria-hidden), so hidden panes cannot steal focus.

Please review again.

Copy link

@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: c741a9aec5

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Copy link

@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: c741a9aec5

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@ibetitsmike
Copy link
Contributor Author

@codex review

Addressed latest inert mount-path issue by rerunning inert synchronization when the shell transitions out of placeholder/loading state.

Please re-review.

@chatgpt-codex-connector
Copy link

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

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@ibetitsmike
Copy link
Contributor Author

@codex review

Addressed the workspace-switch case by rerunning inert synchronization on workspace changes, so newly mounted pane wrappers receive inert when immersive is already active.

Please review again.

@chatgpt-codex-connector
Copy link

Codex Review: Didn't find any major issues. Another round soon, please!

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

The Tooltip's open prop was toggling between false (controlled) and
undefined (uncontrolled), triggering a React warning on every diff line
render (~206 warnings). Use conditional spread so the prop is either
{ open: false } or absent entirely.
…ctedHunk

When the user arrow-keys into a different hunk and presses Shift+C, the
openComposer callback was using the previously-selected hunk (set via j/k
navigation) instead of the hunk at the actual cursor position. This caused
isSelectionInsideRange to fail and the composer to default to
selectedHunkRange.startIndex (often line 0 = top of file).

Fix: openComposer now uses overlayData.lineHunkIds to resolve the hunk
containing the cursor, updates selectedHunkId via onSelectHunk to keep
state in sync, and uses the resolved range for offset calculation.
Strip CR/LF from per-line highlighted HTML fragments before rendering.
Some highlighter outputs can include hard line breaks in a single line
fragment, which causes each diff row to render with an extra blank line
when using whitespace-pre in the code cell.

Also keep diff code-cell classes aligned with pre-regression behavior in
both DiffRenderer and SelectableDiffRenderer.
Normalize full-file content line splitting with CRLF-aware parsing before
building immersive overlays. Trailing carriage returns in per-line content
can render as extra breaks inside whitespace-preserving diff cells, which
made every row appear to have extra spacing.

This keeps immersive row height consistent with baseline rendering.
…neHeight override

Chromatic regressions across Diff stories indicated shared row layout drift.
Restore DiffRenderer to the known-good baseline from perf tooltip removal
phase and drop the immersive-only lineHeight override.

This keeps the tooltip performance win while removing recent global row
height/normalization experiments that altered visual snapshots.
… inflation

Storybook/Chromatic debugging showed selectable diff rows rendering at ~27px instead of ~15px because the per-line comment button could participate in layout in the story runtime, inflating each grid row.

Set inline style with absolute positioning and inset=0 on the comment button (while keeping utility classes) so overlay positioning is unambiguous across runtimes. Also set type=button for explicit semantics.

Verified in Storybook via DevTools: row heights return to single-line (~15.39px) and the comment button computes to absolute positioning.
@ibetitsmike ibetitsmike force-pushed the mike/immersive-review branch from 3784854 to 452fc7f Compare February 20, 2026 17:42
@ibetitsmike
Copy link
Contributor Author

@codex review

Pushed the latest immersive review follow-ups, including row-height stabilization and a new mixed highlight-vs-plain Storybook verification story.

Local make static-check is passing.

Copy link

@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: 763bd8be89

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@ibetitsmike
Copy link
Contributor Author

@codex review

Addressed the Shift+I stale-closure toggle issue in RightSidebar using a functional state updater and pushed the fix.

Local make static-check is passing.

@chatgpt-codex-connector
Copy link

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

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@ibetitsmike
Copy link
Contributor Author

@codex review

Added two immersive review UX updates:

  • E now edits the selected note from the notes panel (jump + inline edit mode)
  • Shift+L notes now render a thumbs-up icon (keeping Shift+D as thumbs-down)

Local make static-check passes.

Copy link

@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: b5c5a88792

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@ibetitsmike
Copy link
Contributor Author

@codex review

Addressed two follow-up immersive regressions:

  • Esc now cancels inline note editing locally again
  • bottom-of-viewport comment actions now keep line/hunk targeting stable (no jump to hunk start)

Also retained the new E note-edit shortcut and thumbs-up icon for Shift+L notes.

Local make static-check passes.

@ibetitsmike
Copy link
Contributor Author

@codex review

Addressed the latest feedback: immersive Tab panel toggle is now ignored when focus is in editable fields (e.g., inline note textarea), so it no longer steals focus while editing.

Also includes the prior follow-up fixes:

  • Esc cancels inline edit locally again
  • bottom-of-viewport comment placement no longer jumps to hunk start

Local make static-check passes.

Copy link

@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: 4c57dd1baa

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +1028 to +1032
const renderSelection: LineSelection | null = externalSelectionRequest
? {
startIndex: externalSelectionRequest.selection.startIndex,
endIndex: externalSelectionRequest.selection.endIndex,
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Clear external composer request after initial sync

When immersive mode opens the inline composer via Shift+C/Shift+L/Shift+D, this branch keeps renderSelection permanently tied to externalSelectionRequest as long as the parent prop stays non-null. Because canceling the composer only clears internal selection state, Esc/cancel cannot actually close the composer, and subsequent local selection changes are ignored until some unrelated state (like hunk change) clears the external request. This makes keyboard quick-comment flows effectively sticky and hard to exit.

Useful? React with 👍 / 👎.

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