feat(PLA-120): wire Files-changed tab + persistent file viewed-state#16
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements the "Files changed" tab, enabling users to view file diffs and track their viewed status. It introduces a new file_view database table, API endpoints for managing file view states, and a comprehensive frontend UI featuring a file tree picker, diff viewer, and keyboard navigation. Feedback focuses on unifying inconsistent scroll offsets across components, addressing browser compatibility for the scrollend event, and optimizing scroll-based file tracking by replacing manual DOM lookups with IntersectionObserver. Additionally, a database query optimization was suggested for the run existence check.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
dastratakos
force-pushed
the
dean/pla-120-issue-12-files-changed-tab-vendor-filedifflist-filepicker
branch
from
78a65b3 to
e01c1c0
Compare
Vendors a simplified FileDiffList / FilePicker / SidebarLayout from hosted stage and parses the diff endpoint output via @pierre/diffs. Adds a file_view table + endpoints so the Files-changed tab label can show "N/M viewed" matching the Chapters tab pattern.
Marking a chapter viewed now bulk-inserts file_view rows for every distinct filePath in the chapter's hunkRefs (mirrors hosted-stage's chapter→file behavior, which writes through GitHub's per-file viewed-state — locally we write straight to file_view since that's our store of record). Unmark intentionally does NOT cascade: a file the user explicitly marked from the Files-changed tab (or via another chapter) shouldn't silently disappear when one of its containing chapters is unmarked.
Previously POST /api/chapter-view cascaded to file_view but DELETE did not, so unmarking a chapter left the file count stuck. Make DELETE remove the file_view rows the corresponding POST would have inserted, scoped to the chapter's hunkRef paths within its run. Trade-off: a file marked from the Files-changed tab and then implicitly re-marked via a chapter mark/unmark cycle loses the direct mark. Acceptable to keep the count correct; revisit if direct marks need their own table.
Vendors hosted-stage's CollapsiblePicker pattern: full-height sticky sidebar with a panel-toggle button, auto-collapse on narrow viewports, and a hover-overlay slide-out when collapsed. Replaces the fixed-width sidebar that didn't extend to the bottom of the page. Wires --content-top (measured nav height via ResizeObserver) and --main-height (100vh) on PullRequestLayout so the picker can stick flush below the tab nav and stretch to the viewport bottom. Updates SidebarLayout to mirror hosted's edge-aligned shape (-ml-6 lg:-ml-8). Also extracts the collapse-state machine into useFileCollapseState (XOR-of-defaults model) so deleted files and viewed files start collapsed but the user's manual toggles still survive.
Add `compareFilePaths` and sort `FilesPage` entries with it so the flat list, scroll-spy, and j/k navigation traverse files in the same order the user sees in the sidebar (folders before sibling files at each depth, alphabetical within each group).
…econcile, reset Adds unit tests for `useFileCollapseState` covering initial state, toggle/collapse-all/expand-all, resetKey behavior, and how overrides reconcile when the defaults set changes (manually-collapsed files stay collapsed, manually-expanded files stay expanded).
Mirrors hosted-stage's keyboard-shortcuts setup so future shortcuts can be ported one-to-one. Adds a `KEYBOARD_SHORTCUTS` registry, a `useShortcut` hook, and shared `ShortcutTooltip`/`ShortcutLabel` components, then wires the FilePicker's CollapsiblePicker to `SHORTCUT_KEY.TOGGLE_FILES` via `react-hotkeys-hook`. The toggle button now shows the kbd hint in its tooltip and exposes `aria-keyshortcuts` for screen readers.
Main introduced a stage-cli-native keyboard-shortcut registry (no react-hotkeys-hook dep). The user-edited CollapsiblePicker imports useHotkeys from that missing package, so wire its toggle through a new useShortcutHandler that parses the registry's hotkey strings on top of a window keydown listener — same shape as useFileNavigationKeys. Also pulls in tslib (transitive build-time dep of @radix-ui/react-dropdown-menu via react-remove-scroll) so vite build can resolve it.
dastratakos
force-pushed
the
dean/pla-120-issue-12-files-changed-tab-vendor-filedifflist-filepicker
branch
from
e01c1c0 to
11146ac
Compare
Mirrors hosted-stage's two-table model now that we no longer have GitHub's per-file viewed flag to lean on: - New `chapter_file_view(userId, chapterId, filePath)` table — per-chapter marks. Squashed into 0003_file_view (the prior single-table migration hadn't shipped beyond this branch). - POST /api/chapter-view/:chapterId inserts chapter_file_view rows for every (chapter, file) pair, then promotes file_view only for paths where every containing chapter in the run now has a row. - DELETE /api/chapter-view/:chapterId clears chapter_view, the chapter's chapter_file_view rows, and (per hosted's rule 4) every file_view row for paths the unmarked chapter touched — even if other chapters still cover the path. The next chapter mark on a covering chapter re-promotes via the coverage check. - DELETE /api/runs/:runId/file-views (direct file unmark from the Files tab) now also wipes chapter_file_view rows for that path across the run, so a chapter mark/unmark cycle can't resurrect file_view via coverage after the user explicitly unmarked the file. Trade-off: a file the user marks directly from the Files tab still gets clobbered when a chapter containing it goes through a mark/unmark cycle. Hosted has the same trade-off; preserving direct marks would require a discriminator column.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a426982. Configure here.
Strip out the keyboard registry, shortcut hooks, ShortcutTooltip/Label, j/k file navigation, and react-hotkeys-hook dep so the PR focuses on vendoring FileDiffList + FilePicker. Will reintroduce in a follow-up that mirrors hosted-stage's useHotkeys-based wiring.
Cuts task references ("(PLA-117)", "(PLA-116)"), "mirrors hosted-stage"
shoutouts, and what-the-code-does narration. Keeps comments that explain
non-obvious invariants, hidden constraints, or surprising behavior —
following AGENTS.md "default to no comments".
Drop `behavior: "smooth"` from `window.scrollTo` so the scrollend fires immediately after a click, and revert the manual-selection suppress from 800ms back to 100ms — the longer window was a workaround for the smooth-scroll animation that's no longer needed.
…ted stage The collapsible picker already references these classes, but they had no CSS rules locally — the thin scrollbar on the file picker list and the hidden scrollbar on the collapsed indicator strip were silent no-ops.
The previous hardcoded 64px offset was smaller than `--content-top` (48px topbar + ~48px tab nav ≈ 96px), so clicking a file in the picker positioned the file's outer div above where its sticky header would land. The header then displaced down to its sticky position and covered the first ~30px of code below it. Read `--content-top` dynamically and add 16px of padding so the file header sits just below the sticky tab nav with visible breathing room.
![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: 16a7ae3dc9
ℹ️ 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".
| if (cfvInserts.length === 0) { | ||
| writeJson(res, 200, {}); | ||
| return; |
There was a problem hiding this comment.
When a chapter has no hunkRefs, cfvInserts.length === 0 triggers writeJson(res, 200, {}) inside the transaction callback, but the handler then always calls writeJson(res, 200, {}) again after the transaction. That second write attempts to send headers/body after the response is already ended, which will surface as ERR_HTTP_HEADERS_SENT/write after end at runtime for valid chapters with empty hunks.
Useful? React with 👍 / 👎.
| inArray(fileView.runId, runIds), | ||
| inArray(fileView.filePath, filePaths), |
There was a problem hiding this comment.
This delete predicate combines runId IN (...) and filePath IN (...), which deletes the cartesian product of those sets. In the external-id fan-out path (multiple chapter rows across runs), if touched file paths differ by run, unmarking one chapter can remove unrelated file_view rows from other run/path combinations. This silently corrupts viewed-state across runs and should be constrained to exact (runId, filePath) pairs.
Useful? React with 👍 / 👎.
| let next = current.children.get(part); | ||
| if (!next) { | ||
| next = { | ||
| name: part, | ||
| path: fullPath, |
There was a problem hiding this comment.
buildFileTree reuses any existing node for a path segment without reconciling node type. If a diff contains a deleted file a and an added file a/b.ts in the same patch, a remains a file node but also gets children; flattenFileTree treats file nodes as leaves and never visits those children, so nested files disappear from the Files tab.
Useful? React with 👍 / 👎.
dastratakos
deleted the
dean/pla-120-issue-12-files-changed-tab-vendor-filedifflist-filepicker
branch
1 participant
Summary
Wires the Files-changed tab in the SPA so users can browse the full diff of a run with file picker + per-file diff viewer, and adds persistent per-file viewed-state so the tab label can render
N/M viewed(matching the Chapters tab).Changes
FileDiffList,FilePicker,SidebarLayoutfrom hosted stage; dropped comments / pending-review / GitHub-mark integrations and Virtuoso virtualization.useDiffPatch(shared query),useFileDiffEntries(parses via@pierre/diffs),useActiveFileOnScrollscroll-spy, andj/kuseFileNavigationKeys.file_viewtable + migration;POST/DELETE /api/runs/:runId/file-views(path in JSON body) andGET view-statereturningfilePaths.useViewStateextended with optimisticmarkFileViewed/unmarkFileViewed;PullRequestLayoutre-enables the Files tab and renders both tab count labels off the live patch + view-state.<App>inDiffSettingsProvidersoPierreDiffViewerworks on this route.Testing
pnpm typecheck && pnpm lint && pnpm test— 87 tests across 11 files (added 7 file-view route tests, 1 useViewState write test, 6 patch-parsing hook tests).stage-cli show:chapters/view-state/diff.patch/POST file-views/DELETE file-viewsall return expected shapes.Screenshots