feat: vendor shared file tree + resizable picker from monorepo#63
Open
dastratakos wants to merge 2 commits into
Open
feat: vendor shared file tree + resizable picker from monorepo#63dastratakos wants to merge 2 commits into
dastratakos wants to merge 2 commits into
Conversation
Syncs the CLI's file picker / chapter sidebar with the monorepo overhauls that landed after the fork: - #980: extract a single shared FileTree (+ FileFilterInput) rendered by both the Files-changed picker and the chapter detail sidebar. Replaces the chapter sidebar's flat FileViewRow list and the file picker's own inlined tree reimplementation. - #984: order the collapsed picker indicators by file-tree order rather than raw API order. - #985: add a shared useResizablePanel hook + chapter-panel-constants; CollapsiblePicker gains an opt-in resize handle and the chapter side panel drops its hand-rolled drag logic for the hook. Adaptations for the CLI: imports rewritten to @/lib/*; FILE_VIEWED_STATE moved into lib/diff-types; comment counts are absent so callers pass an empty map; CollapsiblePicker drops the command-board event the CLI lacks. file-view-row.tsx is deleted (its last consumer, the chapters index page, now renders a small local FilePathRow). Typecheck, lint, test (299), and build all pass.
|
Ready to review this PR? Stage has broken it down into 10 individual chapters for you: 
Chapters generated by Stage for commit 63b3f16 on Jun 3, 2026 3:29am UTC. |
* feat(web): vendor explicit file-focus navigation + viewed/collapse shortcuts
Syncs the CLI with the monorepo's keyboard-navigation overhaul that
landed after the fork:
- #943: replace scroll-derived "active file" with an explicit focused-file
state machine (useCurrentFile) + a shared orchestrator (useFileDiffNavigation)
that owns the diff-viewer ref and wires every file shortcut. Adds a
"file keyboard mode" so keyboard focus shows an outline ring while
click/scroll selection only highlights the picker row.
- #975: add v (mark file viewed + advance, pinning the marked file to top),
; / Shift+; (collapse file / all files), Esc (exit keyboard mode), and
re-bind chapter-viewed from v to Shift+V (#910). file-header gains
ShortcutTooltip hints on the viewed + collapse controls.
New hooks (vendored, imports adapted to @/lib/*): use-current-file,
use-file-viewed-key, use-file-collapse-keys, use-file-keyboard-mode-exit-key,
use-file-diff-navigation; use-file-navigation-keys rewritten to the focus
signature. The old scroll heuristic use-active-file-on-scroll is deleted.
Both routes now delegate to useFileDiffNavigation; FileDiffList gains a
focusedFilePath outline. Ported the monorepo's file-focus-shortcuts test
(16 cases).
CLI simplification: the canToggleViewed gate stays at its default (the CLI's
viewed state is client-side and always writable).
Typecheck, lint, web tests (105), and build all pass.
* feat(web): advance to next chapter on view + auto-mark chapter when all files viewed
Mirrors the hosted app's chapter mark-complete flow (chapter-context.tsx
handleMarkComplete / toggleChapterFileViewed), which the CLI lacked:
- Marking a chapter viewed (Shift+V or the navigator toggle) now advances
to the next chapter — or returns to the run's chapter list once every
chapter is viewed.
- Marking a chapter's last unviewed file now auto-marks the chapter viewed
and advances, so finishing a chapter by its files behaves like Shift+V.
Both routes through a shared advanceAfterChapterComplete helper. The all-
viewed/next-chapter check reads the pre-mark view-state snapshot (the
optimistic cache patch lands a tick later), matching the monorepo's
current-snapshot logic. Confetti is intentionally omitted (CLI has none).
Verified locally via stagereview show: completing a chapter's files marks
it viewed and navigates to the next chapter. typecheck, lint, test (315) pass.
* fix(web): preserve scroll position when navigating between chapters
The chapter detail page scrolls the window, and TanStack Router defaults
to resetScroll: true, so every chapter-to-chapter navigation jumped back
to the top. The hosted app passes resetScroll: false for on-detail-page
chapter nav (chapter-context.tsx navigateToPrev/Next/Chapter); the CLI's
navigations omitted it.
Add resetScroll={false} to the navigator's prev/next/jump <Link>s and to
the keyboard + auto-advance navigate() calls. Navigating to the run's
chapter list once all chapters are viewed keeps the default reset, matching
the hosted app's mark-complete-all flow.
Verified locally: scrolling down a chapter and advancing keeps the scroll
position instead of jumping to the top.
* fix(web): align to the new chapter's first file on chapter change
resetScroll: false alone left the window at the prior chapter's offset, so
advancing could strand the reader deep in (or past) the next chapter. The
hosted app realigns the diff column to the new chapter's first file under
the sticky header on chapter change, gated on the side panel being pinned
(i.e. only when scrolled into the content).
Port that to the CLI's window-scroll model: a useLayoutEffect keyed on
chapter id scrolls to the first file when the carried-over offset left it
above --content-top, and leaves the view alone when already near the top
(so the chapter summary stays visible).
Verified locally: scrolled to the bottom of a chapter then advanced -> the
next chapter's first file snaps under the header; advancing from the top
leaves the view in place.
* fix(web): sync file focus on route-driven scrolls + robust collapse-all test
Addresses PR review (Bugbot/codex):
- Deep-link scrollTo on the Files tab now selects the linked file (not just
scrolls), so the picker highlights it and j/k/v/; start from it.
- Key-change jumps and the chapter-change first-file realign now select the
file they scroll to, so shortcuts act on the file shown at the top instead
of a stale path carried over from the previous chapter. This also makes the
orchestrator's selectFile return value a live consumer (was unused).
- allCollapsed now tests membership (files.every(has)) instead of size >=
length, so Shift+; can't flip to expand-all when collapsedFiles carries
paths outside the current file list.
typecheck, lint, test (315) pass.
* refactor(web): align collapse-state + chapter realign with hosted monorepo
Match the hosted app rather than diverging:
- allCollapsed: revert to collapsedFiles.size >= files.length (the monorepo's
test). Instead of a membership test, scope the Files-tab collapse defaults to
the current files (only mark deleted/viewed paths that are in the diff), so
collapsedFiles stays a subset of files exactly like the monorepo guarantees.
- Chapter-change realign now only scrolls to the first file (no selectFile),
matching the monorepo's reset effect which repositions without touching
file-focus state.
Key-change jumps still selectFile(target.filePath) — that DOES match the
monorepo (its chapter route effect selects the focused key change's file).
typecheck, lint, test (315) pass.
* refactor(web): remove scrollTo deep-link, matching the hosted monorepo
The hosted app has no scrollTo flow on the Files tab, so drop the CLI's:
- files route: remove the scrollTo search param + validateSearch schema
- FilesPage: remove the scrollTo prop and the select-on-scrollTo effect
- prologue Review Focus: the concern location is now plain text (matching
pull-request-summary-section), not a link into the Files tab
PrologueSection no longer needs runId. typecheck, lint, test (315), build pass.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 63b3f16. Configure here.
| viewed={viewed} | ||
| commentCountsByPath={NO_COMMENT_COUNTS} | ||
| filter={filter} | ||
| /> |
There was a problem hiding this comment.
Chapter sidebar breaks reading order
Medium Severity
The chapter sidebar now renders FileTree, which sorts files by tree order, while the chapter diff and j/k navigation still follow hunkRef first-appearance order from filterFilesForChapter. The previous flat list matched that reading order, so sidebar order no longer aligns with the main diff or keyboard navigation.
Reviewed by Cursor Bugbot for commit 63b3f16. Configure here.
| const el = document.getElementById(`file-${firstPath}`); | ||
| if (!el) return; | ||
| const contentTop = Number.parseFloat(getComputedStyle(el).getPropertyValue("--content-top")); | ||
| if (el.getBoundingClientRect().top < contentTop) { |
There was a problem hiding this comment.
Chapter scroll align wrong threshold
Medium Severity
After a chapter change, the layout effect compares the first file’s viewport top to parseFloat of --content-top, but that CSS variable is 6rem, so the parsed value is 6 rather than the sticky offset in pixels. The realignment scroll often never runs when the first file sits under the sticky header but above that incorrect threshold.
Reviewed by Cursor Bugbot for commit 63b3f16. Configure here.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 63b3f16b75
ℹ️ 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 on lines
82
to
+85
| {area.locations[0] && ( | ||
| <Link | ||
| to="/runs/$runId/files" | ||
| params={{ runId }} | ||
| search={{ scrollTo: area.locations[0] }} | ||
| className="ml-auto shrink-0 text-xs text-muted-foreground transition-colors hover:text-foreground" | ||
| > | ||
| {getFileName(area.locations[0])} → | ||
| </Link> | ||
| <span className="ml-auto shrink-0 text-xs text-muted-foreground"> | ||
| {getFileName(area.locations[0])} | ||
| </span> |
There was a problem hiding this comment.
When a prologue focus area includes a location, this now renders the file name as inert text instead of the previous link to /runs/$runId/files?scrollTo=..., and the files route no longer consumes that search param. In runs with prologue review-focus locations, users lose the direct path from the concern to the relevant diff and must manually find the file.
Useful? React with 👍 / 👎.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 30 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/web/src/components/chapter/chapter-file-list.tsx">
<violation number="1" location="packages/web/src/components/chapter/chapter-file-list.tsx:45">
P2: `FileTree` internally re-sorts files into alphabetical directory-tree order (`sortFileTree(buildFileTree(files))`), but the chapter diff list and `j`/`k` keyboard navigation still render and traverse files in hunkRef first-appearance order. The sidebar file order no longer matches the main diff reading order, which can confuse users navigating by keyboard or visually correlating the sidebar with the diff.</violation>
</file>
<file name="packages/web/src/routes/chapter-detail-page.tsx">
<violation number="1" location="packages/web/src/routes/chapter-detail-page.tsx:229">
P1: `getComputedStyle(el).getPropertyValue("--content-top")` returns the raw CSS custom property value (e.g. `"6rem"`), not a resolved pixel value. `Number.parseFloat("6rem")` yields `6`, but `getBoundingClientRect().top` is in pixels. The comparison `top < 6` almost never fires correctly — the sticky header is ~96px (6rem × 16px), so the scroll realignment after a chapter change is effectively dead code. Use `parseFloat(getComputedStyle(document.documentElement).fontSize) * 6` or read from a non-custom property to get the pixel offset.</violation>
</file>
<file name="packages/web/src/components/chapter/chapter-panel-constants.ts">
<violation number="1" location="packages/web/src/components/chapter/chapter-panel-constants.ts:8">
P2: resolve functions don't clamp to declared CHAPTER_PANEL_MIN_WIDTH and max bounds, contradicting the documented contract. For a 400px viewport, the default (120px) and max (200px) both fall below the declared floor of 280px, and the max can be less than the min.</violation>
</file>
<file name="packages/web/src/components/prologue/prologue-section.tsx">
<violation number="1" location="packages/web/src/components/prologue/prologue-section.tsx:83">
P2: Prologue concern file references were changed from navigable links to plain text, removing direct navigation to the corresponding file diff.</violation>
</file>
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
| if (!firstPath) return; | ||
| const el = document.getElementById(`file-${firstPath}`); | ||
| if (!el) return; | ||
| const contentTop = Number.parseFloat(getComputedStyle(el).getPropertyValue("--content-top")); |
There was a problem hiding this comment.
P1: getComputedStyle(el).getPropertyValue("--content-top") returns the raw CSS custom property value (e.g. "6rem"), not a resolved pixel value. Number.parseFloat("6rem") yields 6, but getBoundingClientRect().top is in pixels. The comparison top < 6 almost never fires correctly — the sticky header is ~96px (6rem × 16px), so the scroll realignment after a chapter change is effectively dead code. Use parseFloat(getComputedStyle(document.documentElement).fontSize) * 6 or read from a non-custom property to get the pixel offset.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/web/src/routes/chapter-detail-page.tsx, line 229:
<comment>`getComputedStyle(el).getPropertyValue("--content-top")` returns the raw CSS custom property value (e.g. `"6rem"`), not a resolved pixel value. `Number.parseFloat("6rem")` yields `6`, but `getBoundingClientRect().top` is in pixels. The comparison `top < 6` almost never fires correctly — the sticky header is ~96px (6rem × 16px), so the scroll realignment after a chapter change is effectively dead code. Use `parseFloat(getComputedStyle(document.documentElement).fontSize) * 6` or read from a non-custom property to get the pixel offset.</comment>
<file context>
@@ -187,9 +198,57 @@ function ChapterDetailContent({
+ if (!firstPath) return;
+ const el = document.getElementById(`file-${firstPath}`);
+ if (!el) return;
+ const contentTop = Number.parseFloat(getComputedStyle(el).getPropertyValue("--content-top"));
+ if (el.getBoundingClientRect().top < contentTop) {
+ diffListRef.current?.scrollToFile(firstPath);
</file context>
| })} | ||
| </div> | ||
| <FileFilterInput value={filter} onChange={setFilter} className="mb-2" /> | ||
| <FileTree |
There was a problem hiding this comment.
P2: FileTree internally re-sorts files into alphabetical directory-tree order (sortFileTree(buildFileTree(files))), but the chapter diff list and j/k keyboard navigation still render and traverse files in hunkRef first-appearance order. The sidebar file order no longer matches the main diff reading order, which can confuse users navigating by keyboard or visually correlating the sidebar with the diff.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/web/src/components/chapter/chapter-file-list.tsx, line 45:
<comment>`FileTree` internally re-sorts files into alphabetical directory-tree order (`sortFileTree(buildFileTree(files))`), but the chapter diff list and `j`/`k` keyboard navigation still render and traverse files in hunkRef first-appearance order. The sidebar file order no longer matches the main diff reading order, which can confuse users navigating by keyboard or visually correlating the sidebar with the diff.</comment>
<file context>
@@ -1,45 +1,55 @@
- })}
- </div>
+ <FileFilterInput value={filter} onChange={setFilter} className="mb-2" />
+ <FileTree
+ files={files}
+ focusedFilePath={focusedFilePath}
</file context>
| export const CHAPTER_PANEL_MIN_WIDTH = 280; | ||
| export const CHAPTER_PANEL_MAX_WIDTH_FRACTION = 0.5; | ||
|
|
||
| export const resolveChapterPanelDefaultWidth = (viewportWidth: number) => |
There was a problem hiding this comment.
P2: resolve functions don't clamp to declared CHAPTER_PANEL_MIN_WIDTH and max bounds, contradicting the documented contract. For a 400px viewport, the default (120px) and max (200px) both fall below the declared floor of 280px, and the max can be less than the min.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/web/src/components/chapter/chapter-panel-constants.ts, line 8:
<comment>resolve functions don't clamp to declared CHAPTER_PANEL_MIN_WIDTH and max bounds, contradicting the documented contract. For a 400px viewport, the default (120px) and max (200px) both fall below the declared floor of 280px, and the max can be less than the min.</comment>
<file context>
@@ -0,0 +1,12 @@
+export const CHAPTER_PANEL_MIN_WIDTH = 280;
+export const CHAPTER_PANEL_MAX_WIDTH_FRACTION = 0.5;
+
+export const resolveChapterPanelDefaultWidth = (viewportWidth: number) =>
+ Math.round(viewportWidth * 0.3);
+
</file context>
| > | ||
| {getFileName(area.locations[0])} → | ||
| </Link> | ||
| <span className="ml-auto shrink-0 text-xs text-muted-foreground"> |
There was a problem hiding this comment.
P2: Prologue concern file references were changed from navigable links to plain text, removing direct navigation to the corresponding file diff.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/web/src/components/prologue/prologue-section.tsx, line 83:
<comment>Prologue concern file references were changed from navigable links to plain text, removing direct navigation to the corresponding file diff.</comment>
<file context>
@@ -81,14 +80,9 @@ function PrologueDisplay({ prologue, runId }: { prologue: Prologue; runId: strin
- >
- {getFileName(area.locations[0])} →
- </Link>
+ <span className="ml-auto shrink-0 text-xs text-muted-foreground">
+ {getFileName(area.locations[0])}
+ </span>
</file context>
dastratakos
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Syncs the CLI's file picker and chapter detail sidebar with the monorepo file-tree overhauls that landed after the CLI was forked (~May 27). Brings the CLI up to the shared file tree architecture and the resizable panel hook.
Ports three monorepo PRs:
FileTree(+FileFilterInput) rendered by both the Files-changed picker and the chapter detail sidebar. Previously the CLI's chapter sidebar used a flatFileViewRowlist and the file picker had its own inlined tree reimplementation — two divergent copies, now one.useResizablePanelhook +chapter-panel-constants;CollapsiblePickergains an opt-in resize handle and the chapter side panel drops its hand-rolled drag logic.Changes
lib/use-resizable-panel.ts,components/chapter/chapter-panel-constants.ts,components/files/file-tree.tsx,components/files/file-filter-input.tsx(vendored from the monorepo, imports adapted to@/lib/*).file-picker.tsx(thin wrapper overFileTree),chapter-file-list.tsx(rendersFileTreeinstead of a flat list),collapsible-picker.tsx(internal collapse state +shortcutKey+resize+ShortcutTooltip),chapter-side-panel.tsx(usesuseResizablePanel).components/chapter/file-view-row.tsx— its last consumer (the chapters index page) now renders a small localFilePathRow.FILE_VIEWED_STATEmoved intolib/diff-types; comment counts are absent so callers pass an empty map;CollapsiblePickerdrops the command-board event the CLI lacks. The chapter sidebar now passesfocusedFilePathso the tree highlights the active file.Testing
pnpm typecheck✅pnpm lint✅ (clean, no warnings)pnpm test✅ 299/299pnpm build✅Summary by cubic
Ports the shared file tree and resizable picker to the CLI and adds explicit file-focus navigation with keyboard shortcuts. This makes file review faster, aligns picker/sidebar behavior, and smooths chapter completion and navigation.
New Features
Refactors
useFileDiffNavigation; removed scroll-derived active-file heuristic; addeduse-current-file,use-file-viewed-key,use-file-collapse-keys,use-file-keyboard-mode-exit-key;FileDiffListacceptsfocusedFilePath.scrollTodeep-link; Prologue “Review Focus” now shows the filename only.Written for commit 63b3f16. Summary will update on new commits.