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
This commit is contained in:
Jinjing
2026-06-07 20:40:49 -07:00
committed by GitHub
parent 880fd964ac
commit 468244d6dc
9 changed files with 102 additions and 39 deletions
+3 -6
View File
@@ -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 */
+3 -3
View File
@@ -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 {
@@ -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 {
@@ -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 ? (
<div className="orca-diff-comment-quote">
<FileText className="size-3.5 flex-shrink-0 text-amber-500 mt-0.5" />
<div className="orca-diff-comment-quote-text">{quote}</div>
</div>
) : null}
@@ -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', () => {
@@ -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<DiffComment, 'lineNumber' | 'startLine'>): 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 }
@@ -36,6 +36,7 @@ describe('copyMarkdownReviewNotesForAgent', () => {
[
'File: README.md',
'Source: markdown',
'',
'Line 2',
'Excerpt:',
'> specific phrase',
@@ -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')
)
})
})
+42 -21
View File
@@ -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<string, MarkdownReviewNote[]>()
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')
}