Skip to content

Commit c2ca9de

Browse files
juliusmarmingejuliusmarminge
authored
Add file preview comments and task toggles (pingdotgg#3115)
1 parent 60b546c commit c2ca9de

30 files changed

Lines changed: 2585 additions & 333 deletions

‎apps/mobile/src/features/review/reviewCommentSelection.test.ts‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,4 +78,68 @@ describe("review comment serialization", () => {
7878
);
7979
expect(segments[2]).toEqual(expect.objectContaining({ kind: "text", text: "\nAfter" }));
8080
});
81+
82+
it("parses source-language review comments created by the web file viewer", () => {
83+
const [segment] = parseReviewCommentMessageSegments(
84+
[
85+
'<review_comment sectionId="file:docs/plan.md" sectionTitle="File comment" filePath="docs/plan.md" startIndex="0" endIndex="1" rangeLabel="L1 to L2">',
86+
"Clarify this.",
87+
"```md",
88+
"# Plan",
89+
"- Step one",
90+
"```",
91+
"</review_comment>",
92+
].join("\n"),
93+
);
94+
95+
expect(segment).toEqual(
96+
expect.objectContaining({
97+
kind: "review-comment",
98+
comment: expect.objectContaining({
99+
filePath: "docs/plan.md",
100+
fenceLanguage: "md",
101+
diff: "# Plan\n- Step one",
102+
}),
103+
}),
104+
);
105+
});
106+
107+
it("keeps fenced examples in comment prose separate from the context fence", () => {
108+
const [segment] = parseReviewCommentMessageSegments(
109+
[
110+
'<review_comment sectionId="section-1" sectionTitle="Working tree" filePath="src/app.ts" startIndex="0" endIndex="0" rangeLabel="+1">',
111+
"Try this:",
112+
"```ts",
113+
"const value = 1;",
114+
"```",
115+
"Then retry.",
116+
"```diff",
117+
"@@ -0,0 +1,1 @@",
118+
"+one",
119+
"```",
120+
"</review_comment>",
121+
].join("\n"),
122+
);
123+
124+
expect(segment).toEqual(
125+
expect.objectContaining({
126+
kind: "review-comment",
127+
comment: expect.objectContaining({
128+
text: ["Try this:", "```ts", "const value = 1;", "```", "Then retry."].join("\n"),
129+
diff: "@@ -0,0 +1,1 @@\n+one",
130+
}),
131+
}),
132+
);
133+
});
134+
135+
it("round-trips greater-than signs in review attributes", () => {
136+
const serialized = formatReviewCommentContext(
137+
{ ...makeTarget(), sectionTitle: "Changes > 5" },
138+
"Check this.",
139+
);
140+
const [comment] = parseReviewInlineComments(serialized);
141+
142+
expect(serialized).toContain('sectionTitle="Changes &gt; 5"');
143+
expect(comment?.sectionTitle).toBe("Changes > 5");
144+
});
81145
});

‎apps/mobile/src/features/review/reviewCommentSelection.ts‎

Lines changed: 34 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ export interface ReviewInlineComment {
2121
readonly rangeLabel: string;
2222
readonly text: string;
2323
readonly diff: string;
24+
readonly fenceLanguage?: string;
2425
}
2526

2627
export type ReviewCommentMessageSegment =
@@ -38,6 +39,7 @@ let currentTarget: ReviewCommentTarget | null = null;
3839
const listeners = new Set<() => void>();
3940
const REVIEW_COMMENT_BLOCK_PATTERN = /<review_comment\b([^>]*)>\s*([\s\S]*?)<\/review_comment>/g;
4041
const REVIEW_COMMENT_ATTRIBUTE_PATTERN = /([a-zA-Z][a-zA-Z0-9_-]*)="([^"]*)"/g;
42+
const REVIEW_COMMENT_FENCE_PATTERN = /(`{3,})([^\s`]*)[^\n]*\n([\s\S]*?)\n\1/g;
4143

4244
function emitChange() {
4345
listeners.forEach((listener) => listener());
@@ -170,12 +172,17 @@ function formatReviewSelectedDiff(target: ReviewCommentTarget): string {
170172
}
171173

172174
function escapeReviewCommentAttribute(value: string): string {
173-
return value.replace(/&/g, "&amp;").replace(/"/g, "&quot;").replace(/</g, "&lt;");
175+
return value
176+
.replace(/&/g, "&amp;")
177+
.replace(/"/g, "&quot;")
178+
.replace(/</g, "&lt;")
179+
.replace(/>/g, "&gt;");
174180
}
175181

176182
function unescapeReviewCommentAttribute(value: string): string {
177183
return value
178184
.replace(/&lt;/g, "<")
185+
.replace(/&gt;/g, ">")
179186
.replace(/&quot;/g, '"')
180187
.replace(/&amp;/g, "&");
181188
}
@@ -195,15 +202,19 @@ function readNonNegativeInteger(value: string | undefined): number | null {
195202
return Number(value);
196203
}
197204

198-
function extractReviewCommentText(rawBody: string): string {
199-
const fenceIndex = rawBody.indexOf("```diff");
200-
const commentBody = fenceIndex >= 0 ? rawBody.slice(0, fenceIndex) : rawBody;
201-
return commentBody.trim();
202-
}
203-
204-
function extractReviewCommentDiff(rawBody: string): string {
205-
const match = rawBody.match(/```diff\s*\n([\s\S]*?)\n```/);
206-
return match?.[1]?.trim() ?? "";
205+
function extractReviewCommentBody(rawBody: string): {
206+
text: string;
207+
language: string;
208+
contents: string;
209+
} {
210+
const matches = Array.from(rawBody.matchAll(REVIEW_COMMENT_FENCE_PATTERN));
211+
const match = matches.at(-1);
212+
const fenceIndex = match?.index;
213+
return {
214+
text: rawBody.slice(0, fenceIndex ?? rawBody.length).trim(),
215+
language: match?.[2]?.trim() || "diff",
216+
contents: match?.[3] ?? "",
217+
};
207218
}
208219

209220
function parseReviewInlineComment(
@@ -219,6 +230,7 @@ function parseReviewInlineComment(
219230
if (!filePath || !sectionId || startIndex === null || endIndex === null) {
220231
return null;
221232
}
233+
const body = extractReviewCommentBody(rawBody);
222234

223235
return {
224236
id: `review-comment:${index}:${sectionId}:${filePath}:${startIndex}:${endIndex}`,
@@ -228,13 +240,20 @@ function parseReviewInlineComment(
228240
startIndex: Math.min(startIndex, endIndex),
229241
endIndex: Math.max(startIndex, endIndex),
230242
rangeLabel: attributes.rangeLabel?.trim() || "line",
231-
text: extractReviewCommentText(rawBody),
232-
diff: extractReviewCommentDiff(rawBody),
243+
text: body.text,
244+
diff: body.contents,
245+
fenceLanguage: body.language,
233246
};
234247
}
235248

236249
export function formatReviewCommentContext(target: ReviewCommentTarget, comment: string): string {
237250
const rangeLabel = formatReviewSelectedRangeLabel(target);
251+
const diff = formatReviewSelectedDiff(target);
252+
const longestBacktickRun = Math.max(
253+
0,
254+
...Array.from(diff.matchAll(/`+/g), (match) => match[0].length),
255+
);
256+
const fence = "`".repeat(Math.max(3, longestBacktickRun + 1));
238257
return [
239258
[
240259
"<review_comment",
@@ -247,9 +266,9 @@ export function formatReviewCommentContext(target: ReviewCommentTarget, comment:
247266
">",
248267
].join(""),
249268
comment.trim(),
250-
"```diff",
251-
formatReviewSelectedDiff(target),
252-
"```",
269+
`${fence}diff`,
270+
diff,
271+
fence,
253272
"</review_comment>",
254273
].join("\n");
255274
}

‎apps/mobile/src/features/threads/ThreadFeed.tsx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1193,6 +1193,9 @@ const ReviewCommentCard = memo(function ReviewCommentCard(props: {
11931193
});
11941194

11951195
function buildReviewCommentPatch(comment: ReviewInlineComment): string {
1196+
if ((comment.fenceLanguage ?? "diff") !== "diff") {
1197+
return "";
1198+
}
11961199
const diff = comment.diff.trim();
11971200
if (!diff) {
11981201
return "";

‎apps/web/src/components/ChatMarkdown.browser.tsx‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,26 @@ describe("ChatMarkdown", () => {
6969
document.body.innerHTML = "";
7070
});
7171

72+
it("makes task-list checkboxes interactive when a change handler is provided", async () => {
73+
const onTaskListChange = vi.fn();
74+
const screen = await render(
75+
<ChatMarkdown
76+
text={"- [ ] Ship it"}
77+
cwd="/repo/project"
78+
onTaskListChange={onTaskListChange}
79+
/>,
80+
);
81+
82+
try {
83+
const checkbox = page.getByRole("checkbox", { name: "Toggle task" });
84+
await expect.element(checkbox).not.toBeDisabled();
85+
await checkbox.click();
86+
expect(onTaskListChange).toHaveBeenCalledWith({ markerOffset: 2, checked: true });
87+
} finally {
88+
await screen.unmount();
89+
}
90+
});
91+
7292
it("rewrites file uri hrefs into direct paths before rendering", async () => {
7393
const filePath =
7494
"/Users/yashsingh/p/sco/claude-code-extract/src/utils/permissions/PermissionRule.ts";

‎apps/web/src/components/ChatMarkdown.tsx‎

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ interface ChatMarkdownProps {
9191
text: string;
9292
cwd: string | undefined;
9393
threadRef?: ScopedThreadRef | undefined;
94+
onTaskListChange?: ((input: { markerOffset: number; checked: boolean }) => void) | undefined;
9495
isStreaming?: boolean;
9596
skills?: ReadonlyArray<Pick<ServerProviderSkill, "name" | "displayName">>;
9697
className?: string;
@@ -108,6 +109,17 @@ const highlightedCodeCache = new LRUCache<string>(
108109
MAX_HIGHLIGHT_CACHE_MEMORY_BYTES,
109110
);
110111
const highlighterPromiseCache = new Map<string, Promise<DiffsHighlighter>>();
112+
113+
function findTaskListMarkerOffset(markdown: string, listItemStart: number): number | null {
114+
const firstLineEnd = markdown.indexOf("\n", listItemStart);
115+
const firstLine = markdown.slice(
116+
listItemStart,
117+
firstLineEnd === -1 ? markdown.length : firstLineEnd,
118+
);
119+
const match = firstLine.match(/^(?:\s*(?:[-+*]|\d+[.)])\s+)(\[[ xX]\])/);
120+
if (!match?.[1]) return null;
121+
return listItemStart + firstLine.indexOf(match[1]);
122+
}
111123
const CHAT_MARKDOWN_SANITIZE_SCHEMA = {
112124
...defaultSchema,
113125
attributes: {
@@ -1170,6 +1182,7 @@ function ChatMarkdown({
11701182
text,
11711183
cwd,
11721184
threadRef,
1185+
onTaskListChange,
11731186
isStreaming = false,
11741187
skills = EMPTY_MARKDOWN_SKILLS,
11751188
className,
@@ -1215,8 +1228,44 @@ function ChatMarkdown({
12151228
p({ node: _node, children, ...props }) {
12161229
return <p {...props}>{renderSkillInlineMarkdownChildren(children, skills)}</p>;
12171230
},
1218-
li({ node: _node, children, ...props }) {
1219-
return <li {...props}>{renderSkillInlineMarkdownChildren(children, skills)}</li>;
1231+
li({ node, children, ...props }) {
1232+
const listItemStart = node?.position?.start.offset;
1233+
const markerOffset =
1234+
typeof listItemStart === "number" ? findTaskListMarkerOffset(text, listItemStart) : null;
1235+
return (
1236+
<li {...props} data-task-marker-offset={markerOffset ?? undefined}>
1237+
{renderSkillInlineMarkdownChildren(children, skills)}
1238+
</li>
1239+
);
1240+
},
1241+
input({ node: _node, type, checked, disabled: _disabled, ...props }) {
1242+
if (type !== "checkbox" || !onTaskListChange) {
1243+
return (
1244+
<input
1245+
{...props}
1246+
type={type}
1247+
checked={checked}
1248+
disabled={_disabled}
1249+
readOnly={type === "checkbox"}
1250+
/>
1251+
);
1252+
}
1253+
return (
1254+
<input
1255+
{...props}
1256+
type="checkbox"
1257+
name="markdown-task"
1258+
aria-label="Toggle task"
1259+
checked={checked}
1260+
onChange={(event) => {
1261+
const markerOffset = Number(
1262+
event.currentTarget.closest("li")?.dataset.taskMarkerOffset,
1263+
);
1264+
if (!Number.isSafeInteger(markerOffset)) return;
1265+
onTaskListChange({ markerOffset, checked: event.currentTarget.checked });
1266+
}}
1267+
/>
1268+
);
12201269
},
12211270
a({ node, href, children, ...props }) {
12221271
const normalizedHref = href ? normalizeMarkdownLinkHrefKey(href) : "";
@@ -1330,9 +1379,11 @@ function ChatMarkdown({
13301379
fileLinkParentSuffixByPath,
13311380
isStreaming,
13321381
markdownFileLinkMetaByHref,
1382+
onTaskListChange,
13331383
threadRef,
13341384
resolvedTheme,
13351385
skills,
1386+
text,
13361387
],
13371388
);
13381389

0 commit comments

Comments
 (0)