Skip to content

Add focused review navigation controls - #150

Merged
pandemicsyn merged 4 commits into
mainfrom
agent/diff-phase-b-navigation
Jul 18, 2026
Merged

pandemicsyn merged 4 commits into
mainfrom
agent/diff-phase-b-navigation

Conversation

@pandemicsyn

@pandemicsyn pandemicsyn commented Jul 18, 2026

Copy link
Copy Markdown
Owner

Summary

Completes Diff Improvements Plan Phase B step 2 by wiring the merged review-navigation model into the focused PR workbench.

UX and shortcut map

  • Adds a visible traversal selector plus Previous/Next controls for file, hunk, unresolved thread, local draft, Neon finding, and combined attention item.
  • Shows position, empty, loading, filtered, and start/end boundary states in ordinary text.
  • Uses [ for Previous, ] for Next, and ? for the accessible shortcut-help dialog. Every shortcut has the same labeled pointer/touch control, a tooltip/hint, and a visible focus treatment.
  • Keeps the richer chrome in the standalone/focused PR workbench; the embedded dashboard review remains compact.
  • Preserves the existing flat Lit Cockpit language: square hairline surfaces, mono structural chrome, restrained cyan focus/selection, and no decorative cards, shadows, or radii.

Focus, keyboard, and accessibility safeguards

  • Suppresses global review shortcuts for input, textarea, select, contenteditable, native/ARIA dialogs, annotation roots, comment composers, and review submission controls, including focused descendants and shadow-root focus.
  • Ignores already-prevented events, IME/composition and keyCode 229, and Ctrl/Alt/Meta-modified browser behavior.
  • Navigation does not move focus. The help control intentionally moves focus into the dialog, then restores the actual connected invoker on Escape or Close with the visible Help button as fallback.
  • A polite atomic live region announces file, target kind, position, stale/unavailable/severity detail, and relevant loading or boundary status.

Cursor and data flow

  • Builds the workbench projections from the shared canonical/guided cursor model and reconciles filtering/revision changes with exact preservation or deterministic nearest fallback.
  • Exact stable targets are republished when their resolved path or anchor moves, without looping; active tree path, controlled diff selection/anchor, selected annotation/inspector state, and mounted review-surface snapshot update together.
  • An active navigation target is authoritative over an open dirty composer for same-file and cross-file moves. Composer text is preserved, and an explicit human line/range selection clears navigation ownership before opening or re-anchoring the composer.
  • Canonicalizes renamed thread and draft paths so selected annotations remain mounted on the current file. Previous-path-only search matches are added to Pierre's live projection as selected renamed aliases that map back to the current path, preserving the existing model's expansion and virtualization state.
  • Preserves complete multi-line/cross-side draft ranges. Report-only findings select their file and inspector item but do not invent an additions-side diff anchor.
  • Filtered hunk traversal starts at the deterministic directional edge when the prior active file is outside the filter; deletion-only hunks anchor on the deletions side.

Patch-on-demand and performance

  • Initial navigation consumes file metadata plus only already-loaded patch hunks.
  • Cross-file hunk traversal requests one patch at a time, skips explicit unavailable files deterministically, and pauses after eight lazy reads per activation with visible/live status; it does not prefetch every patch.
  • The focused 305-file test constructs 305 file targets while parsing hunks from only one loaded patch.
  • npm run bench:review-fixtures (Node 26.4.0 arm64) recorded a newer large-case run of 81.1 ms tree, 308.7 ms first patch, and 0 ms in-process thread projection, all inside retained fixture targets. The plan records those medians while the tracked baseline JSON preserves the prior raw samples and 75.6/272.4 ms large-case medians. Existing deferred production misses remain recorded and are not reclassified.

Verification

  • Focused navigation/tree/surface/component coverage passes, including target kinds, exact-target movement, composer ownership, multi-line/cross-side ranges, report-only findings, filtered and deletion-only hunks, boundaries/empty states, unavailable and renamed/revision cases, pointer parity, editable/dialog/composer suppression, invoker focus restoration, live announcements, and surface synchronization.
  • Final review-fix loop: 4 focused files / 21 tests passed, plus npm run typecheck, warning-free npm run lint, npm run format:check, and git diff --check.
  • npm run check and the full pre-review npm run verify had completed successfully before the lead testing-policy update: 615 unit, 35 Git/worktree, and 129 integration tests, dashboard/server/docs builds, npm package validation, and packed CLI smoke. The unrelated slow suites were not rerun after review fixes; CI owns that coverage.

Scope and limitations

  • Hunk traversal deliberately pauses after eight unloaded/unavailable files per activation instead of starting an unbounded scan; activating Previous/Next again continues.
  • Typed ephemeral inline Neon-finding rendering, finding-summary tree filtering, and promotion flows remain Phase B steps 3–4; this PR does not claim finding-summary filtering before typed finding rendering exists.
  • No new deviation or deferral was introduced. Existing performance deferrals remain unchanged.

Includes a patch changeset and marks Phase B step 2 completed in place without removing plan or measurement history.

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

ℹ️ 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 web/src/features/pr-review/review-navigation.ts Outdated
Comment thread web/src/features/pr-review/review-navigation.ts Outdated
@pandemicsyn
pandemicsyn merged commit fd1b000 into main Jul 18, 2026
5 checks passed
@pandemicsyn
pandemicsyn deleted the agent/diff-phase-b-navigation branch July 18, 2026 17:29
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