From 468244d6dcc2df4fd43022def863ca07cf08a426 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Sun, 7 Jun 2026 20:40:49 -0700 Subject: [PATCH] Improve markdown review card sizing, quoting, and note grouping (#4856) * Improve markdown review card sizing, quoting, and note grouping - Narrow the annotation rail and editor padding to reduce visual weight - Replace webkit line-clamp quote truncation with single-line nowrap; shorten MAX_CARD_QUOTE_LENGTH to 60 - Remove FileText icon from the quote block - Tone down active card border/background highlight opacity - Always reserve quote height in stacked layout (cards show a fallback excerpt even without selected text) - Group multiple markdown review notes per file under a single header in the formatted prompt * fix test --- src/renderer/src/assets/main.css | 9 +-- src/renderer/src/assets/markdown-preview.css | 6 +- .../src/assets/rich-markdown-editor.css | 9 +-- .../diff-comments/DiffCommentCard.tsx | 3 +- .../rich-markdown-review-note-layout.test.ts | 15 +++++ .../rich-markdown-review-note-layout.ts | 5 +- .../src/lib/markdown-review-note-copy.test.ts | 1 + .../src/lib/markdown-review-notes.test.ts | 30 ++++++++- src/renderer/src/lib/markdown-review-notes.ts | 63 ++++++++++++------- 9 files changed, 102 insertions(+), 39 deletions(-) diff --git a/src/renderer/src/assets/main.css b/src/renderer/src/assets/main.css index 8e389248460..cea65bb8ef6 100644 --- a/src/renderer/src/assets/main.css +++ b/src/renderer/src/assets/main.css @@ -2837,12 +2837,12 @@ html.onboarding-tour-start-transition::view-transition-new(root) { display: flex; align-items: flex-start; gap: 8px; - margin: 4px 0 6px 0; + margin: 2px 0 4px 0; padding-left: 8px; border-left: 2px solid color-mix(in srgb, #e7b10a 35%, transparent); color: var(--muted-foreground); font-size: 12.5px; - line-height: 1.4; + line-height: 1.35; } .orca-diff-comment-quote-text { @@ -2850,10 +2850,7 @@ html.onboarding-tour-start-transition::view-transition-new(root) { min-width: 0; overflow: hidden; text-overflow: ellipsis; - display: -webkit-box; - -webkit-line-clamp: 2; - -webkit-box-orient: vertical; - word-break: break-all; + white-space: nowrap; } /* Comment Body */ diff --git a/src/renderer/src/assets/markdown-preview.css b/src/renderer/src/assets/markdown-preview.css index 5a3df9025d7..231bf6ec69b 100644 --- a/src/renderer/src/assets/markdown-preview.css +++ b/src/renderer/src/assets/markdown-preview.css @@ -281,7 +281,7 @@ .markdown-annotation-list-block { position: relative; display: grid; - grid-template-columns: minmax(0, 1fr) minmax(260px, min(34cqw, 360px)); + grid-template-columns: minmax(0, 1fr) minmax(220px, min(28cqw, 300px)); column-gap: 32px; align-items: start; } @@ -367,8 +367,8 @@ } .markdown-annotation-card.is-active > .orca-diff-comment-card { - border-color: color-mix(in srgb, var(--foreground) 32%, transparent); - background: color-mix(in srgb, var(--foreground) 8%, var(--editor-surface)); + border-color: color-mix(in srgb, var(--foreground) 18%, transparent); + background: color-mix(in srgb, var(--foreground) 4%, var(--editor-surface)); } .markdown-annotation-card.is-attention > .orca-diff-comment-card { diff --git a/src/renderer/src/assets/rich-markdown-editor.css b/src/renderer/src/assets/rich-markdown-editor.css index 948207dc586..7f85b880f89 100644 --- a/src/renderer/src/assets/rich-markdown-editor.css +++ b/src/renderer/src/assets/rich-markdown-editor.css @@ -123,7 +123,7 @@ } .rich-markdown-editor-shell.has-rich-markdown-review-notes .rich-markdown-editor { - padding-right: min(360px, 34%); + padding-right: clamp(252px, 28%, 308px); } .rich-markdown-review-rail-actions { @@ -178,7 +178,8 @@ position: absolute; top: 0; right: 16px; - width: min(300px, 30%); + /* Why: the card action pill needs the same minimum rail width as preview mode. */ + width: clamp(220px, 24%, 250px); pointer-events: none; z-index: 30; } @@ -191,8 +192,8 @@ } .rich-markdown-review-note-card.is-active > .orca-diff-comment-card { - border-color: color-mix(in srgb, var(--foreground) 32%, transparent); - background: color-mix(in srgb, var(--foreground) 8%, var(--editor-surface)); + border-color: color-mix(in srgb, var(--foreground) 18%, transparent); + background: color-mix(in srgb, var(--foreground) 4%, var(--editor-surface)); } .rich-markdown-review-note-card.is-attention > .orca-diff-comment-card { diff --git a/src/renderer/src/components/diff-comments/DiffCommentCard.tsx b/src/renderer/src/components/diff-comments/DiffCommentCard.tsx index e5e6d23647c..c309183ca65 100644 --- a/src/renderer/src/components/diff-comments/DiffCommentCard.tsx +++ b/src/renderer/src/components/diff-comments/DiffCommentCard.tsx @@ -1,4 +1,4 @@ -import { CornerDownLeft, Pencil, Trash, FileText } from 'lucide-react' +import { CornerDownLeft, Pencil, Trash } from 'lucide-react' import { useLayoutEffect, useRef, useState, type ReactNode } from 'react' import { Button } from '@/components/ui/button' import { getDiffCommentLineLabel } from '@/lib/diff-comment-compat' @@ -213,7 +213,6 @@ export function DiffCommentCard({ {/* Quote Block */} {quote ? (
-
{quote}
) : null} diff --git a/src/renderer/src/components/editor/rich-markdown-review-note-layout.test.ts b/src/renderer/src/components/editor/rich-markdown-review-note-layout.test.ts index 029770e5e29..98e47f5214e 100644 --- a/src/renderer/src/components/editor/rich-markdown-review-note-layout.test.ts +++ b/src/renderer/src/components/editor/rich-markdown-review-note-layout.test.ts @@ -36,6 +36,21 @@ describe('stackRichMarkdownReviewNotePositions', () => { expect(stacked[0].top).toBe(100) expect(stacked[1].top).toBeGreaterThan(stacked[0].top) }) + + it('reserves quote height for fallback source-line excerpts', () => { + const stacked = stackRichMarkdownReviewNotePositions([ + { + comment: makeComment({ id: 'first', body: 'note' }), + top: 100 + }, + { + comment: makeComment({ id: 'second', body: 'note' }), + top: 100 + } + ]) + + expect(stacked[1].top).toBe(210) + }) }) describe('shouldExpandRichMarkdownReviewRail', () => { diff --git a/src/renderer/src/components/editor/rich-markdown-review-note-layout.ts b/src/renderer/src/components/editor/rich-markdown-review-note-layout.ts index a0bca9b7e2f..3052891046f 100644 --- a/src/renderer/src/components/editor/rich-markdown-review-note-layout.ts +++ b/src/renderer/src/components/editor/rich-markdown-review-note-layout.ts @@ -14,7 +14,7 @@ export type RichMarkdownReviewRailState = { const REVIEW_NOTE_GAP_PX = 8 const REVIEW_NOTE_BASE_HEIGHT_PX = 58 // new Notion-style card base height (padding, border, gap, header) const REVIEW_NOTE_BODY_LINE_HEIGHT_PX = 20 // new body line-height -const REVIEW_NOTE_QUOTE_HEIGHT_PX = 32 // new quote height (border, margins, text) +const REVIEW_NOTE_QUOTE_HEIGHT_PX = 24 // single-line quote height (line-height plus margins) function getReviewNoteStartLine(comment: Pick): number { return comment.startLine ?? comment.lineNumber @@ -46,10 +46,11 @@ export function stackRichMarkdownReviewNotePositions( return [...positions].sort(compareRichMarkdownReviewNotePositions).map((position) => { const top = Math.max(position.top, nextOpenTop) const measured = measuredHeights?.get(position.comment.id) + // Why: cards render a fallback source-line quote even when no exact text was selected. const estimatedHeight = REVIEW_NOTE_BASE_HEIGHT_PX + position.comment.body.split('\n').length * REVIEW_NOTE_BODY_LINE_HEIGHT_PX + - (position.comment.selectedText ? REVIEW_NOTE_QUOTE_HEIGHT_PX : 0) + REVIEW_NOTE_QUOTE_HEIGHT_PX const height = measured ?? estimatedHeight nextOpenTop = top + height + REVIEW_NOTE_GAP_PX return { ...position, top } diff --git a/src/renderer/src/lib/markdown-review-note-copy.test.ts b/src/renderer/src/lib/markdown-review-note-copy.test.ts index 73c8f515304..bfac91ff209 100644 --- a/src/renderer/src/lib/markdown-review-note-copy.test.ts +++ b/src/renderer/src/lib/markdown-review-note-copy.test.ts @@ -36,6 +36,7 @@ describe('copyMarkdownReviewNotesForAgent', () => { [ 'File: README.md', 'Source: markdown', + '', 'Line 2', 'Excerpt:', '> specific phrase', diff --git a/src/renderer/src/lib/markdown-review-notes.test.ts b/src/renderer/src/lib/markdown-review-notes.test.ts index 3de9bf7abd8..d1ab0dfdbf2 100644 --- a/src/renderer/src/lib/markdown-review-notes.test.ts +++ b/src/renderer/src/lib/markdown-review-notes.test.ts @@ -70,7 +70,7 @@ describe('markdown review notes', () => { expect( getMarkdownReviewCardQuote('one\ntwo broad line\nthree', note({ selectedText: 'broad' })) ).toBe('broad') - expect(formatMarkdownReviewCardQuote('a'.repeat(120))).toBe(`${'a'.repeat(93)}...`) + expect(formatMarkdownReviewCardQuote('a'.repeat(120))).toBe(`${'a'.repeat(57)}...`) }) it('formats a deterministic prompt for terminal agents', () => { @@ -83,6 +83,7 @@ describe('markdown review notes', () => { [ 'File: README.md', 'Source: markdown', + '', 'Lines 2-3', 'Excerpt:', '> two', @@ -100,4 +101,31 @@ describe('markdown review notes', () => { expect(formatted).toContain('Excerpt:\n> specific phrase') }) + + it('groups multiple notes for one markdown file under a single header', () => { + const formatted = formatMarkdownReviewNotes( + [ + note({ id: 'a', lineNumber: 2, body: 'is this part of the command?' }), + note({ id: 'b', lineNumber: 3, body: 'what are these fields?' }) + ], + 'one\ntwo\nthree' + ) + + expect(formatted).toBe( + [ + 'File: README.md', + 'Source: markdown', + '', + 'Line 2', + 'Excerpt:', + '> two', + 'User comment: "is this part of the command?"', + '', + 'Line 3', + 'Excerpt:', + '> three', + 'User comment: "what are these fields?"' + ].join('\n') + ) + }) }) diff --git a/src/renderer/src/lib/markdown-review-notes.ts b/src/renderer/src/lib/markdown-review-notes.ts index eea1a7e7b99..a34cf19d0a9 100644 --- a/src/renderer/src/lib/markdown-review-notes.ts +++ b/src/renderer/src/lib/markdown-review-notes.ts @@ -2,7 +2,7 @@ import type { DiffComment } from '../../../shared/types' import { getDiffCommentLineLabel } from './diff-comment-compat' const MAX_EXCERPT_LINES = 8 -const MAX_CARD_QUOTE_LENGTH = 96 +const MAX_CARD_QUOTE_LENGTH = 60 export type MarkdownReviewNote = DiffComment & { source: 'markdown' } @@ -84,31 +84,52 @@ export function getMarkdownReviewCardQuote( return formatMarkdownReviewCardQuote(getMarkdownReviewHighlightedText(content, note)) } +function escapeMarkdownReviewNoteBody(body: string): string { + return body + .replace(/\\/g, '\\\\') + .replace(/"/g, '\\"') + .replace(/\r/g, '\\r') + .replace(/\n/g, '\\n') +} + +function formatMarkdownReviewNoteDetails(note: MarkdownReviewNote, content: string): string { + const excerpt = note.selectedText + ? getMarkdownReviewHighlightedText(content, note) + .split(/\r\n|\r|\n/) + .map((line) => `> ${line}`) + .join('\n') + : getMarkdownReviewExcerpt(content, note) + const parts = [ + getDiffCommentLineLabel(note), + excerpt ? `Excerpt:\n${excerpt}` : null, + `User comment: "${escapeMarkdownReviewNoteBody(note.body)}"` + ] + return parts.filter((part): part is string => part !== null).join('\n') +} + export function formatMarkdownReviewNotes( notes: readonly MarkdownReviewNote[], content: string ): string { - return sortMarkdownReviewNotes(notes) - .map((note) => { - const escapedBody = note.body - .replace(/\\/g, '\\\\') - .replace(/"/g, '\\"') - .replace(/\r/g, '\\r') - .replace(/\n/g, '\\n') - const excerpt = note.selectedText - ? getMarkdownReviewHighlightedText(content, note) - .split(/\r\n|\r|\n/) - .map((line) => `> ${line}`) - .join('\n') - : getMarkdownReviewExcerpt(content, note) - const parts = [ - `File: ${note.filePath}`, + const groups = new Map() + for (const note of sortMarkdownReviewNotes(notes)) { + const group = groups.get(note.filePath) + if (group) { + group.push(note) + } else { + groups.set(note.filePath, [note]) + } + } + + return [...groups.entries()] + .map(([filePath, fileNotes]) => { + // Why: agents need the file once; repeated markdown note blocks waste prompt context. + return [ + `File: ${filePath}`, 'Source: markdown', - getDiffCommentLineLabel(note), - excerpt ? `Excerpt:\n${excerpt}` : null, - `User comment: "${escapedBody}"` - ] - return parts.filter((part): part is string => part !== null).join('\n') + '', + fileNotes.map((note) => formatMarkdownReviewNoteDetails(note, content)).join('\n\n') + ].join('\n') }) .join('\n\n') }