feat(web): edit and number chat annotations - #5263
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
size:XXL
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| const useRightSide = rightAvailable >= minWidth || rightAvailable >= leftAvailable; | ||
| const availableWidth = useRightSide ? rightAvailable : leftAvailable; | ||
| const width = Math.max(minWidth, Math.min(maxWidth, availableWidth)); | ||
| const left = useRightSide ? rightLeft : Math.max(edgeGap, leftRight - width); |
There was a problem hiding this comment.
🟠 High chat/ChatTextSelectionPopover.tsx:56
When the right side of the selection has less space than minWidth, popoverPosition still returns left set to rightLeft and forces width to minWidth, so the comment editor overflows past the right viewport edge and the submit button is clipped. For example, on a 400px-wide viewport with a selection at left=180,width=20, it chooses the right side with only 168px available but renders a 220px editor at left=212, pushing the controls beyond the viewport. The final left needs to be clamped so the forced-width editor stays within the viewport.
| const left = useRightSide ? rightLeft : Math.max(edgeGap, leftRight - width); | |
| const left = useRightSide ? Math.min(rightLeft, window.innerWidth - edgeGap - width) : Math.max(edgeGap, leftRight - width); |
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/chat/ChatTextSelectionPopover.tsx around line 56:
When the right side of the selection has less space than `minWidth`, `popoverPosition` still returns `left` set to `rightLeft` and forces `width` to `minWidth`, so the comment editor overflows past the right viewport edge and the submit button is clipped. For example, on a 400px-wide viewport with a selection at `left=180,width=20`, it chooses the right side with only 168px available but renders a 220px editor at `left=212`, pushing the controls beyond the viewport. The final `left` needs to be clamped so the forced-width editor stays within the viewport.
| return trimmedPrompt.length > 0 ? `${trimmedPrompt}\n\n${blocks}` : blocks; | ||
| } | ||
|
|
||
| export function parseChatSelectionMessageSegments( |
There was a problem hiding this comment.
🟡 Medium src/chatSelectionAnnotation.ts:103
parseChatSelectionMessageSegments treats any <chat_selection> XML in the prompt as a parsed annotation. When a user types or pastes that XML as ordinary text (e.g., while discussing the format), the content is returned as a selection segment and later stripped from userPromptText, so the user's actual message is silently hidden and replaced with an annotation indicator. There is no marker distinguishing formatter-generated blocks from user-typed content, so the parser cannot tell them apart. Consider using a sentinel or delimiter that the user would not naturally type, or restricting parsing to formatter-injected regions.
🤖 Copy this AI Prompt to have your agent fix this:
In file @packages/shared/src/chatSelectionAnnotation.ts around line 103:
`parseChatSelectionMessageSegments` treats any `<chat_selection>` XML in the prompt as a parsed annotation. When a user types or pastes that XML as ordinary text (e.g., while discussing the format), the content is returned as a `selection` segment and later stripped from `userPromptText`, so the user's actual message is silently hidden and replaced with an annotation indicator. There is no marker distinguishing formatter-generated blocks from user-typed content, so the parser cannot tell them apart. Consider using a sentinel or delimiter that the user would not naturally type, or restricting parsing to formatter-injected regions.
![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 using high effort 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 5f7ad83. Configure here.
| const [editorAnchorRect, setEditorAnchorRect] = useState< | ||
| ChatTextSelectionPopoverProps["rect"] | null | ||
| >(null); | ||
| const indicators = useMemo(() => deriveChatSelectionIndicators(annotations), [annotations]); |
There was a problem hiding this comment.
Source markers use local numbers
Medium Severity
Annotation markers in ChatMarkdown are numbered locally per message via deriveChatSelectionIndicators, showing 1, 2, ... on each. This conflicts with the composer's global numbering, causing mismatches when annotations span multiple assistant messages.
Reviewed by Cursor Bugbot for commit 5f7ad83. Configure here.
| ? { start: validSourceStart, end: validSourceEnd } | ||
| : null | ||
| : null; | ||
| const match = offsetMatch ?? findChatMarkdownTextMatch(index.text, target); |
There was a problem hiding this comment.
Duplicate text matches first only
Medium Severity
When sourceStart or sourceEnd are missing or invalid, resolveChatMarkdownTextRange falls back to finding the first occurrence of the selected text. This can cause highlights, markers, and active selections to attach to the wrong passage, particularly with repeated phrases or multiple annotations.
Reviewed by Cursor Bugbot for commit 5f7ad83. Configure here.
ApprovabilityVerdict: Needs human review 2 blocking correctness issues found. This PR introduces a substantial new feature for editing and numbering chat annotations, including multiple new UI components, state management changes, and XML-based annotation serialization. The scope and complexity of new user-facing behavior, combined with unresolved review comments identifying potential bugs (viewport overflow, silent content hiding), warrant human review. You can customize Macroscope's approvability policy. Learn more. |
|
Closing because this follow-up needs to be explicitly stacked on #5224 rather than opened directly against main. |
1 participant
What Changed
Adds editing and numbered indicators to response-text annotations.
Users can now:
Why
The first annotation flow made it possible to attach selected response text, but pending annotations were difficult to locate again and could not be corrected without removing and recreating them. Numbered markers make the relationship between the composer chips and assistant response obvious, while the editor keeps small comment changes fast.
UI Changes
Before
The first annotation flow exposed selection and composer actions, but did not show numbered source markers or an edit affordance.
After
Selecting response text still opens the existing Add to chat action.
Pending annotations now show numbered markers beside the source response and a matching composer summary.
Clicking a marker highlights the source text and opens an editor for the comment.
Multiple annotations receive stable sequential numbers and remain visible alongside the prompt.
After sending, the conversation keeps the compact annotation summary without exposing serialized markup.
Validation
./node_modules/.bin/vp test run apps/web/src/chatSelectionAnnotation.test.ts apps/web/src/components/ChatView.logic.test.ts apps/web/src/components/chat/MessagesTimeline.test.tsx apps/web/src/composerDraftStore.test.ts apps/web/src/proposedPlan.test.ts(150 passed)./node_modules/.bin/vp run --filter @t3tools/web typecheck./node_modules/.bin/vp run --filter @t3tools/shared typecheckChecklist
Model: OpenAI Codex · Harness: Codex desktop
Note
Medium Risk
Large chat/composer UI and prompt serialization changes affect what users send to providers; DOM text indexing for highlights may misalign on edge-case markdown layouts.
Overview
Adds numbered markers on assistant markdown for pending text selections, with amber highlights and an edit/delete flow before send.
ChatMarkdownmaps selections to source offsets in rendered text so markers stay aligned after layout changes.Composer and send path store
chatSelectionAnnotationsin drafts (persist v9), show pending chips, and append sharedchat_selectionblocks (with optionalmessage_idand offsets) to outgoing prompts. Plan follow-up can restore cleared prompt + annotations on failed send when the composer was still empty.Timeline ties pending markers to source assistant messages; sent user turns show a compact “N annotations” summary without exposing serialized markup. Shared
@t3tools/shared/chatSelectionAnnotationowns format/parse and indicator derivation; user docs describe the flow.Reviewed by Cursor Bugbot for commit 5f7ad83. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add editable, numbered text-selection annotations to chat assistant messages
ChatSelectionAnnotationstored in the composer draft and serialized into the outgoing message via<chat_selection>XML blocks.ChatSelectionAnnotationEditor) to update or delete the annotation before sending.ComposerPendingChatSelectionAnnotations), counts them toward "has content" checks, and restores them on send failure.composerDraftStore(storage version bumped 8 → 9) and grouped by message id for display inMessagesTimeline.📊 Macroscope summarized 5f7ad83. 13 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.