Skip to content

feat(web): edit saved review comments in place - #12087

Closed
cmaciasjimenez wants to merge 1 commit into
pingdotgg:mainfrom
cmaciasjimenez:feat/edit-review-comments
Closed

cmaciasjimenez wants to merge 1 commit into
pingdotgg:mainfrom
cmaciasjimenez:feat/edit-review-comments

Conversation

@cmaciasjimenez

@cmaciasjimenez cmaciasjimenez commented Sep 16, 2026 •

Copy link
Copy Markdown

Once a review comment is submitted in the diff panel, its text is frozen: hovering it offers only a delete button, so fixing a typo means deleting the comment and writing it again from scratch. Comments already posted to a pull request host have been editable all along (pullRequests.updateComment), which makes the local ones the odd surface out.

Give a saved comment a pencil beside its delete action. It reopens the same inline composer seeded with the comment's text, and buffers the rewrite locally so cancelling leaves the saved comment untouched. Saving upserts the record by id, so the draft keeps one inline reference and the comment keeps its place among the draft's context.

addReviewComment already replaced a record with a matching id, but by filtering and re-appending, which moved an edited comment to the end. It now replaces in place, so an edit no longer reorders the draft's context.

No contract, server, or mobile change: review comments are composer-draft state, and mobile dissolves them straight into the draft text rather than keeping an editable bubble. Pending (unsubmitted) pull-request review comments are still delete-only — a separate surface, left for a follow-up.

Before

Hovering a saved comment offers only delete.

Before: hovering a saved review comment in the diff panel reveals only a delete button

After

The same comment now offers edit beside delete, and the pencil reopens the composer with the comment's text, Cancel and Save.

After: hovering the same comment reveals an edit button beside delete After: the inline composer reopened on the saved comment, prefilled with its text, with Cancel and Save
Edit.comment.mov

Verification

  • vp test run apps/web/src/composerDraftStore.test.ts apps/web/src/components/diffs/DiffCommentAnnotation.test.tsx — 143 passed, including a new case covering in-place replacement.
  • vp test run apps/web/src/components/files/FilePreviewPanel.test.ts apps/web/src/components/diffs/StyledDiffCodeView.test.tsx apps/web/src/components/diffs/commentSubmitShortcut.test.ts — 17 passed.
  • Web typecheck and targeted lint clean on the changed files.
  • Walked the flow in the running web app on both the thread diff panel and the file preview: add, hover, edit, cancel, save, delete.

Written by Claude Opus 5 in T3 Code.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added the ability to edit saved review comments directly within code annotations.
    • Added Save and Cancel controls, keyboard escape handling, and an edit action for existing comments.
  • Bug Fixes

    • Updated edited comments without changing their original position in the review comment list.
    • Preserved comment ordering when replacing existing review comments.

A review comment could only be deleted and rewritten from scratch once
submitted, while comments already posted to a pull request host have been
editable all along.

Give the saved comment a pencil beside its delete action. It reopens the
same inline composer seeded with the comment, buffering the rewrite so
cancelling leaves the saved text untouched. Saving upserts the record in
place, so an edit no longer reorders the draft's context.
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Sep 16, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR adds a new saved-comment editing workflow across the diff and file-preview surfaces and changes shared composer state replacement semantics. The change is contained but affects production user interactions in multiple places, so human review is appropriate.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 378e2eeb-7566-4c74-a797-50d65e822c94

📥 Commits

Reviewing files that changed from the base of the PR and between ccf220b and b4c07e5.

📒 Files selected for processing (5)
  • apps/web/src/components/diffs/AnnotatableCodeView.tsx
  • apps/web/src/components/diffs/DiffCommentAnnotation.tsx
  • apps/web/src/components/files/FilePreviewPanel.tsx
  • apps/web/src/composerDraftStore.test.ts
  • apps/web/src/composerDraftStore.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The change adds inline editing for saved review comments. Edited text flows through existing annotation persistence paths, and replacing a comment preserves its position in the review comment list.

Changes

Review comment editing

Layer / File(s) Summary
Comment edit form
apps/web/src/components/diffs/DiffCommentAnnotation.tsx
Saved comments can enter edit mode through a Pencil button. The textarea, Escape key, Cancel button, and Save button use edit-specific handlers.
Comment replacement persistence
apps/web/src/composerDraftStore.ts, apps/web/src/composerDraftStore.test.ts
addReviewComment replaces an existing comment in place. A regression test verifies that the comment order remains unchanged.
Edit callback wiring
apps/web/src/components/diffs/AnnotatableCodeView.tsx, apps/web/src/components/files/FilePreviewPanel.tsx
Annotation edit callbacks update the matching review comment through the existing persistence functions.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant Reviewer
  participant DiffCommentAnnotation
  participant AnnotatableCodeView
  participant ComposerDraftStore
  Reviewer->>DiffCommentAnnotation: Select saved comment edit
  DiffCommentAnnotation->>DiffCommentAnnotation: Update edited text
  Reviewer->>DiffCommentAnnotation: Select Save
  DiffCommentAnnotation->>AnnotatableCodeView: Send edited text
  AnnotatableCodeView->>ComposerDraftStore: Upsert comment by entry ID
  ComposerDraftStore-->>AnnotatableCodeView: Preserve comment position
Loading

Suggested reviewers: bil0000

Merge Risk: ⚪ Minimal · up to b4c07

Saved-comment editing uses the intended persistence path and does not expose editing for delete-only comments. The change is ready to merge.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 5 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title is concise, specific, and accurately summarizes the main change: editing saved review comments in place.
Description check ✅ Passed The description clearly explains the change, motivation, UI behavior, scope, screenshots, video, and verification steps. It does not include the template's explicit Checklist section, but the required…
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@cmaciasjimenez

Copy link
Copy Markdown
Author

Leaving the CodeRabbit Docstring Coverage pre-merge warning (33.33% vs its 80% default) unaddressed on purpose.

Satisfying it would mean adding a docstring to saveEdit in apps/web/src/components/diffs/AnnotatableCodeView.tsx. None of its four sibling callbacks in that file carries one:

removeEntry    = useCallback(...)
submitEntry    = useCallback(...)
saveEdit       = useCallback(...)   <- added here
beginComment   = useCallback(...)

Documenting only the new one would make it the single commented callback in the file. The convention here is that the exported surface gets a doc comment and internal callbacks do not — DiffCommentAnnotation's /** The shared inline comment treatment for file previews, thread diffs, and pull-request diffs. */ is untouched, and this change adds comments only where the reasoning is not visible from the code: why a rewrite is buffered before saving, and why addReviewComment now replaces in place.

The 80% threshold is CodeRabbit's generic default rather than anything this repository asks for, so I have treated it as a false positive rather than changing the code to match it. Happy to reverse that if a maintainer disagrees.

The CodeRabbit check itself reports Review completed with no actionable comments, and the review body puts merge risk at Minimal.

Copy link
Copy Markdown
Member

Note

This comment is posted by Julius' dot

This adds an Edit/Save/Cancel workflow for saved local review comments across the diff and file-preview surfaces. No maintainer approval for that direction and scope is linked. Closing under prior approval. Discuss the proposed workflow in Ideas and link explicit approval for reconsideration.

@cmaciasjimenez

Copy link
Copy Markdown
Author

Opened an Ideas discussion to get approval for the direction and scope: #14570. Once a maintainer approves it there, I'll link the approval here and ask for this PR to be reopened.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants