feat(PRO-546): add a local review-comments model to the CLI#68
feat(PRO-546): add a local review-comments model to the CLI#68dastratakos wants to merge 33 commits into
Conversation
|
Ready to review this PR? Stage has broken it down into 9 individual chapters for you: 
Chapters generated by Stage for commit 05aab2b on Jun 20, 2026 8:06am UTC. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
cubic analysis
5 issues found across 35 files
Linked issue analysis
Linked issue: PRO-546: Add a local review-comments model to the CLI
| Status | Acceptance criteria | Notes |
|---|---|---|
| ✅ | Add database schema for comments and threads (comment and comment_thread tables), with a scopeKey anchor and appropriate indexes | Migration and schema files add comment and comment_thread tables, scopeKey column, and indexes; snapshot and migration files present. |
| ✅ | Expose API routes to list/create threads, reply, resolve/reopen, edit, and delete comments/threads | New routes implement CRUD and resolve actions and are wired into the server routes. |
| ✅ | Provide a web UI for inline threads (render in diff), a composer with markdown toolbar, and text-selection + gutter affordances to start comments | Web components and plumbing added: inline rendering integrated into the diff viewer, composer/editor and toolbar, selection popup, and gutter affordances, plus provider/context wiring. |
| ✅ | Anchor threads to a deterministic diff scope so threads persist across re-imports (scope-key survival) | deriveScopeKey was added and comment_thread stores scopeKey; server routes resolve and query by scope key; tests exercise scope-key survival. |
| ✅ | Operate locally without GitHub sync and provide viewer identity fallbacks | PR is explicit that comments are local-only; viewer resolution uses gh when available but falls back to git config or a generic label; local author default is used in DB schema. |
| ✅ | Include tests covering routes and selection helpers (CRUD, cascade, scope-key survival, and text-selection unit tests) | New integration and unit tests were added and test suite reported passing in PR testing notes. |
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
![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: 2e785edd3d
ℹ️ 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".
…on't reach Pierre's slot)
Pierre creates its <diffs-container> shadow root asynchronously, so caching it once on mount left getSelection() reading a null root when the diff rendered late, silently breaking select-to-comment. Resolve it per-event and bind shadow listeners as soon as the root appears (bounded poll), with the container capture listeners as fallback. Addresses Cursor Bugbot finding.
… on hover Wire Pierre's onLineSelected so dragging across the line-number gutter opens the composer for the whole range (multi-line comments). Hovering a thread now highlights its anchored lines via selectedLines; an isHoveringRef guard keeps that highlight from triggering onLineSelected and opening a composer.
Resolve the local reviewer's identity via `gh api user` (falling back to git config user.name, then a generic 'You') and render their name + avatar in the comment byline instead of a hardcoded placeholder. Adds a /api/viewer route, a useViewer hook, and the Viewer wire type. Degrades gracefully when gh is unauthenticated or the repo isn't on GitHub.
dastratakos
force-pushed
the
dean/pro-546-add-a-local-review-comments-model-to-the-cli
branch
from
bdedb69 to
8889afc
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8889afc48b
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7f64cf323e
ℹ️ 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".
A split-view gutter drag that crosses from one column to the other emits a SelectedLineRange whose side and endSide differ. handleLineSelected opened a draft using only norm.side, so the thread would cover unrelated old/new line numbers on a single side. Extract toSingleSideSelection, which mirrors buildSelectedLineRange's null-on-cross-side contract, and use it for the gutter path too.
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
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/lib/use-text-selection.ts">
<violation number="1" location="packages/web/src/lib/use-text-selection.ts:66">
P2: `toSingleSideSelection` can anchor to the wrong diff side when only `endSide` is present, because it defaults directly to additions instead of falling back to `endSide`.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db4b45acd3
ℹ️ 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".
The threads query exposed error/isLoading, but no consumer read them, so a load failure rendered identically to having no comments — the diff still shows, the overlay is just silently empty. Toast the error from the provider (one query instance → one toast); React Query only sets error once its retries exhaust, so transient blips stay quiet. Add a regression test covering the error-toast and the no-toast-on-empty-success paths.
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: debad2ab73
ℹ️ 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".
Three review-flagged gaps in the comment text-selection path: - toSingleSideSelection fell straight through to additions when only endSide was set; fall back to endSide first (mirrors normalizeSelected- LineRange), so a side-less range anchors to the side actually known. - mouseup was bound on the diff container, so releasing a multi-line drag past the last visible row (outside the diff) never fired — the drag stuck open and no popup appeared. Capture mouseup on the document instead. - A unified-view triple-click on a replacement line extends into the addition row that shares the deletion's number; the endLine>startLine guard missed it, so the cross-side range was dropped. Detect the trailing row by element identity and clamp to the clicked side. Add toSingleSideSelection unit cases for the endSide fallback.
Give the failed-fetch toast a stable id so a re-fire (StrictMode double- mount, remount with a cached error, or refetch failing with a new error reference) updates one toast instead of stacking duplicates.
The text-selection popup is suppressed while a composer is open (draft !== null), but the gutter '+' and gutter drag-select both replaced the draft anchor unconditionally. Since the composer's text lives in CommentForm's own state, moving the anchor unmounts the form and drops in-progress text. Guard both gutter paths on a draftRef (a render-synced mirror, so the Pierre callbacks keep stable identity and don't re-init the diff).
Replace the single moving draft with a collection of independent composers: opening a comment on another line (gutter +, gutter drag, or text selection) now adds a composer instead of relocating the existing one. Each composer submits, cancels, and shows errors independently. This supersedes the earlier single-draft guard (7117d9f): rather than block a second comment to avoid discarding the first, both stay open, so nothing is ever discarded. Implementation notes: - Draft state is a keyed collection; one composer per (side, endLine) row. - Composer text lives in a parent draftBodies ref keyed by anchor, so a form keeps its text if it remounts when another draft opens or closes. - Pierre keys its annotation rows by array index, so a row can be reused for a different anchor when a draft is added/removed; the composer carries a stable per-anchor key to force a clean remount (re-reading its own text) rather than inheriting another composer's in-progress state. - Extract the draft/annotation logic into lib/comment-drafts.ts with unit tests. Known trade-off: opening/closing a composer briefly flashes the other annotation rows (Pierre re-applies the annotation layer); accepted as-is for now.
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
openDraft treated an existing composer on the same (side, endLine) row as a no-op, so re-dragging a range that shares the end line but changes the start (e.g. 3-10 -> 7-10) left the draft's startLine stale and created the thread with the wrong span. Upsert instead: re-opening the row adopts the new startLine (and clears any stale submit error). Extracted to upsertDraft with unit tests.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7dbe216. Configure here.
The failed-fetch toast has a stable id but nothing cleared it when a later refetch (or navigating to another run) succeeded, so a stale "Couldn't load comments" could linger while comments were visible. Dismiss it by id once `error` clears. Add a regression test for the recover-and-dismiss path.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 933ac40a63
ℹ️ 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".
A triple-click on the last row before a collapsed hunk gap extends the range to offset 0 of the first row after the gap (e.g. 50 -> 200). Decrementing that row anchored the draft at an unrendered line (199), so Pierre had no row for the composer and no comment box appeared. Resolve the previous rendered line from the DOM instead of assuming endLine - 1; this also subsumes the unified replacement-line case. Add previousRenderedLine with unit tests.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8e8d05430e
ℹ️ 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".
A reply's edit/delete menu was gated only by !isEditing for that reply, so while editing the root (or another reply) or composing a reply, other replies still showed actions — clicking Edit then dropped the in-progress form's unsaved text. Gate reply actions on the thread being idle, matching how the root comment's actions already behave.
|
You're iterating quickly on this pull request. To help protect your rate limits, cubic has paused automatic reviews on new pushes for now—when you're ready for another review, comment |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…backs Pre-merge review cleanups: - Consolidate the request-body boundary parser that had been copied into comments, pull-request-mutations, and view-state routes into a single parseJsonBody<T> in routes/json.ts. All 14 call sites now use it. (view-state's file-view 400 now surfaces the Zod message instead of a custom string — no test or behavior depends on the exact text.) - Add regression tests for the viewer route's degraded paths that previously had none: the synthesized github.com/<login>.png avatar when gh omits avatar_url, and the generic "You" fallback when run outside a git repo (readRepoRoot throws).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 94994dc3c7
ℹ️ 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".
The comment mutation routes (create/reply/resolve/edit/delete) were state- changing but, unlike the PR mutation routes, skipped the enforceSameOrigin guard. Since the loopback port is predictable and parseJsonBody ignores Content-Type, a malicious page could drive comment writes via a no-preflight POST (or DNS rebinding). Guard every comment write route with enforceSameOrigin and add a cross-origin 403 regression test.
handleResolveToggle collapsed the thread on resolve via setIsOpen(false), bypassing the handleOpenChange guard — so resolving while editing a comment or composing a reply unmounted CommentForm and dropped unsaved text. Skip the collapse when a reply/edit/delete form is active (the thread still resolves).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05aab2b4f0
ℹ️ 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".
| side: draft.side, | ||
| startLine: draft.startLine, | ||
| endLine: draft.endLine, |
There was a problem hiding this comment.
When this viewer is used on a chapter detail page, filterFilesForChapter can render a synthetic intermediate-vs-new diff after applying the other hunks in the file. For deletion-side rows after an earlier out-of-chapter insertion/deletion, Pierre's displayed old line numbers are therefore intermediate-file coordinates, but persisting draft.startLine/draft.endLine directly stores them as if they were the original diff's old-line coordinates. A left-side comment created in that chapter can then disappear or attach to the wrong line in the full-file view or a later re-import; convert deletion-side coordinates back to the original diff before creating the thread, or disable creation on those synthetic left-side rows.
Useful? React with 👍 / 👎.
1 participant
Summary
Adds a local, line-anchored review-comments model to the
stagereviewCLI so reviewers can write comment threads — reply, resolve/reopen, edit, delete — alongside the existing checkmark review state, entirely on-machine with no GitHub sync yet (PRO-546). Threads anchor to the diff scope rather than a run, so they survive re-runningshowon the same diff, and the shape maps cleanly onto GitHub's review-comment model for the planned sync work. The comment UI is vendored to match the hosted review experience: inline threads in the diff, a composer with a markdown formatting toolbar, and text-selection + gutter affordances for starting a comment.Changes
comment_thread+commenttables (migration0006); threads keyed by a deterministicscopeKey(extracted sharedderiveScopeKey) so comments persist across re-imports of the same diff.routes/comments.ts— list/create threads, reply, resolve (PATCH), edit & delete comments/threads; wire types in@stagereview/types/comments.useCommentThreads(React Query) + a run-level context provider; threads render inline via Pierre line annotations; create via a text-selection popup and a gutter "+".Testing
pnpm typecheck && pnpm lint && pnpm testall pass (347 tests, incl. 11 new comment-route tests).stagereview show: create/reply/resolve/edit/delete, inline rendering, persistence across re-runs, and the selection/gutter/toolbar affordances.Summary by cubic
Adds local, line-anchored review comments to the CLI and diff UI so reviewers can create, reply, edit, delete, and resolve threads on-device. Also shows the gh-authenticated user's GitHub login and avatar in comment bylines with graceful fallbacks, and supports multiple comment composers open at once (PRO-546).
New Features
comment_threadandcommenttables with deterministicscopeKeyvia extractedderiveScopeKey(migration0006).routes/comments.tsto list/create/reply/edit/delete and resolve/reopen threads; wire types in@stagereview/types/comments./api/viewerresolves identity viagh api user(fallback to gituser.nameor "You"); webuseViewerrenders GitHub login + avatar; types in@stagereview/types/viewer.comment-drafts.comment-draftsunit tests, and viewer fallback regressions (missingavatar_url, non-git repo).parseJsonBody<T>for request validation used by comments, view-state, and pull-request mutation routes.react-textarea-autosize; export./commentsand./viewerfrom@stagereview/types.Bug Fixes
readRepoRootfails and keeps identity cached for the session withgcTime; bylines show the GitHub login, not the display name.endSidewhenstartSideis unknown, capturemouseupondocumentfor long drags, and clamp triple-click selections to the previous rendered line so drafts anchor to visible rows (covers unified replacement lines and hunk gaps).upsertDraft) so the created thread matches the new span.enforceSameOriginon all mutation routes (403 on violation).Written for commit 05aab2b. Summary will update on new commits.