diff --git a/packages/ui/components/ListMarker.tsx b/packages/ui/components/ListMarker.tsx new file mode 100644 index 00000000..b5507d97 --- /dev/null +++ b/packages/ui/components/ListMarker.tsx @@ -0,0 +1,73 @@ +import React from 'react'; + +/** + * Shared list-item marker used by both the main Viewer and the plan-diff + * clean view. Renders the appropriate glyph(s) for a list item given its + * level, ordered flag, display index, and checkbox state. + * + * Rendering rules: + * - Ordered items render `${orderedIndex}.` with tabular-nums so digit + * widths stay stable across e.g. `9.` → `10.`. + * - Checkbox items render the circle / checkmark SVG. + * - Ordered + checkbox renders BOTH: numeral first, checkbox second + * (matches GitHub's `1. [ ] task` rendering). + * - Plain bullets fall back to `•` / `◦` / `▪` by level. + * + * Interactivity is opt-in: the Viewer passes `interactive` + `onToggle` + * for click-to-toggle checkboxes; the diff view leaves both undefined. + */ +interface ListMarkerProps { + level: number; + ordered?: boolean; + orderedIndex?: number | null; + checked?: boolean; // undefined = not a checkbox + interactive?: boolean; + onToggle?: () => void; +} + +const BULLET_BY_LEVEL = ['\u2022', '\u25E6', '\u25AA']; + +export const ListMarker: React.FC = ({ + level, + ordered, + orderedIndex, + checked, + interactive, + onToggle, +}) => { + const isCheckbox = checked !== undefined; + const showNumeral = !!ordered && orderedIndex != null; + const bullet = BULLET_BY_LEVEL[Math.min(level, BULLET_BY_LEVEL.length - 1)]; + + const handleClick = interactive && onToggle + ? (e: React.MouseEvent) => { e.stopPropagation(); onToggle(); } + : undefined; + + return ( + + {showNumeral && ( + + {orderedIndex}. + + )} + {isCheckbox ? ( + checked ? ( + + + + ) : ( + + + + ) + ) : !showNumeral ? ( + {bullet} + ) : null} + + ); +}; diff --git a/packages/ui/components/Viewer.tsx b/packages/ui/components/Viewer.tsx index c00c854e..5f53ec88 100644 --- a/packages/ui/components/Viewer.tsx +++ b/packages/ui/components/Viewer.tsx @@ -3,7 +3,8 @@ import { createPortal } from 'react-dom'; import hljs from 'highlight.js'; import 'highlight.js/styles/github-dark.css'; import { Block, Annotation, AnnotationType, EditorMode, type InputMethod, type ImageAttachment, type ActionsLabelMode } from '../types'; -import { Frontmatter } from '../utils/parser'; +import { Frontmatter, computeListIndices } from '../utils/parser'; +import { ListMarker } from './ListMarker'; import { AnnotationToolbar } from './AnnotationToolbar'; import { FloatingQuickLabelPicker } from './FloatingQuickLabelPicker'; @@ -530,11 +531,25 @@ export const Viewer = forwardRef(({ {!frontmatter && blocks.length > 0 && blocks[0].type !== 'heading' &&
} {groupBlocks(blocks).map(group => group.type === 'list-group' ? ( -
- {group.blocks.map(block => ( - setLightbox({ src, alt })} key={block.id} block={block} onOpenLinkedDoc={onOpenLinkedDoc} onToggleCheckbox={onToggleCheckbox} checkboxOverrides={checkboxOverrides} /> - ))} -
+ (() => { + const indices = computeListIndices(group.blocks); + return ( +
+ {group.blocks.map((block, i) => ( + setLightbox({ src, alt })} + key={block.id} + block={block} + orderedIndex={indices[i]} + onOpenLinkedDoc={onOpenLinkedDoc} + onToggleCheckbox={onToggleCheckbox} + checkboxOverrides={checkboxOverrides} + /> + ))} +
+ ); + })() ) : group.block.type === 'code' && isMermaidLanguage(group.block.language) ? ( ) : group.block.type === 'code' && isGraphvizLanguage(group.block.language) ? ( @@ -967,7 +982,8 @@ const BlockRenderer: React.FC<{ onImageClick?: (src: string, alt: string) => void; onToggleCheckbox?: (blockId: string, checked: boolean) => void; checkboxOverrides?: Map; -}> = ({ block, onOpenLinkedDoc, imageBaseDir, onImageClick, onToggleCheckbox, checkboxOverrides }) => { + orderedIndex?: number | null; +}> = ({ block, onOpenLinkedDoc, imageBaseDir, onImageClick, onToggleCheckbox, checkboxOverrides, orderedIndex }) => { switch (block.type) { case 'heading': const Tag = `h${block.level || 1}` as React.ElementType; @@ -979,15 +995,23 @@ const BlockRenderer: React.FC<{ return ; - case 'blockquote': + case 'blockquote': { + // Content may span multiple merged `>` lines. Split on blank-line + // paragraph breaks so `> a\n>\n> b` renders as two

children. + const paragraphs = block.content.split(/\n\n+/); return (

- + {paragraphs.map((para, i) => ( +

0 ? 'mt-2' : ''}> + +

+ ))}
); + } case 'list-item': { const indent = (block.level || 0) * 1.25; // 1.25rem per level @@ -1002,28 +1026,14 @@ const BlockRenderer: React.FC<{ data-block-id={block.id} style={{ marginLeft: `${indent}rem` }} > - { e.stopPropagation(); onToggleCheckbox!(block.id, !isChecked); } : undefined} - role={isInteractive ? 'checkbox' : undefined} - aria-checked={isInteractive ? isChecked : undefined} - > - {isCheckbox ? ( - isChecked ? ( - - - - ) : ( - - - - ) - ) : ( - - {(block.level || 0) === 0 ? '•' : (block.level || 0) === 1 ? '◦' : '▪'} - - )} - + onToggleCheckbox!(block.id, !isChecked) : undefined} + /> diff --git a/packages/ui/components/plan-diff/PlanCleanDiffView.tsx b/packages/ui/components/plan-diff/PlanCleanDiffView.tsx index 3c150138..e1747ce0 100644 --- a/packages/ui/components/plan-diff/PlanCleanDiffView.tsx +++ b/packages/ui/components/plan-diff/PlanCleanDiffView.tsx @@ -8,7 +8,8 @@ import React, { useEffect, useRef, useState, useCallback } from "react"; import hljs from "highlight.js"; -import { parseMarkdownToBlocks } from "../../utils/parser"; +import { parseMarkdownToBlocks, computeListIndices } from "../../utils/parser"; +import { ListMarker } from "../ListMarker"; import type { Block, Annotation, EditorMode, ImageAttachment } from "../../types"; import { AnnotationType } from "../../types"; import type { PlanDiffBlock } from "../../utils/planDiffEngine"; @@ -409,17 +410,29 @@ const MarkdownChunk: React.FC<{ content: string }> = ({ content }) => { () => parseMarkdownToBlocks(content), [content] ); + // Compute ordered-list display indices across the entire chunk so every + // list-item gets the right numeral even though we don't group here. + // Non-list blocks pass through as `null` and act as streak-breaks — same + // behavior as the main Viewer's per-group counter. + const orderedIndices = React.useMemo( + () => computeListIndices(blocks), + [blocks] + ); return ( <> - {blocks.map((block) => ( - + {blocks.map((block, i) => ( + ))} ); }; -const SimpleBlockRenderer: React.FC<{ block: Block }> = ({ block }) => { +const SimpleBlockRenderer: React.FC<{ block: Block; orderedIndex?: number | null }> = ({ block, orderedIndex }) => { switch (block.type) { case "heading": { const Tag = `h${block.level || 1}` as keyof React.JSX.IntrinsicElements; @@ -437,12 +450,20 @@ const SimpleBlockRenderer: React.FC<{ block: Block }> = ({ block }) => { ); } - case "blockquote": + case "blockquote": { + // Split on blank-line paragraph breaks so merged `> a\n>\n> b` + // renders as two

children instead of collapsing to one line. + const paragraphs = block.content.split(/\n\n+/); return (

- + {paragraphs.map((para, i) => ( +

0 ? "mt-2" : ""}> + +

+ ))}
); + } case "list-item": { const indent = (block.level || 0) * 1.25; @@ -452,43 +473,12 @@ const SimpleBlockRenderer: React.FC<{ block: Block }> = ({ block }) => { className="flex gap-3 my-1.5" style={{ marginLeft: `${indent}rem` }} > - - {isCheckbox ? ( - block.checked ? ( - - - - ) : ( - - - - ) - ) : ( - - {(block.level || 0) === 0 - ? "\u2022" - : (block.level || 0) === 1 - ? "\u25E6" - : "\u25AA"} - - )} - + diff --git a/packages/ui/types.ts b/packages/ui/types.ts index 31a8322b..5bf81515 100644 --- a/packages/ui/types.ts +++ b/packages/ui/types.ts @@ -58,6 +58,8 @@ export interface Block { level?: number; // For headings (1-6) or list indentation language?: string; // For code blocks (e.g., 'rust', 'typescript') checked?: boolean; // For checkbox list items (true = checked, false = unchecked, undefined = not a checkbox) + ordered?: boolean; // For list items: true when source marker was \d+. + orderedStart?: number; // For ordered list items: integer parsed from the marker (e.g. 5 for "5.") order: number; // Sorting order startLine: number; // 1-based line number in source } diff --git a/packages/ui/utils/parser.test.ts b/packages/ui/utils/parser.test.ts index 1c718a0e..74e055e2 100644 --- a/packages/ui/utils/parser.test.ts +++ b/packages/ui/utils/parser.test.ts @@ -1,5 +1,18 @@ import { describe, test, expect } from "bun:test"; -import { parseMarkdownToBlocks } from "./parser"; +import { parseMarkdownToBlocks, computeListIndices } from "./parser"; +import type { Block } from "../types"; + +/** Tiny factory for list-item blocks used by computeListIndices tests. */ +const li = (level: number, ordered: boolean, orderedStart?: number): Block => ({ + id: `b${Math.random()}`, + type: "list-item", + content: "", + level, + ordered: ordered || undefined, + orderedStart, + order: 0, + startLine: 1, +}); describe("parseMarkdownToBlocks — code fences", () => { /** @@ -274,3 +287,329 @@ describe("parseMarkdownToBlocks — list continuation lines", () => { expect(blocks[1].type).toBe("heading"); }); }); + +describe("parseMarkdownToBlocks — ordered lists", () => { + test("numeric markers produce ordered list items with orderedStart", () => { + const md = "1. first\n2. second"; + const blocks = parseMarkdownToBlocks(md); + expect(blocks).toHaveLength(2); + expect(blocks[0].type).toBe("list-item"); + expect(blocks[0].ordered).toBe(true); + expect(blocks[0].orderedStart).toBe(1); + expect(blocks[0].content).toBe("first"); + expect(blocks[1].ordered).toBe(true); + expect(blocks[1].orderedStart).toBe(2); + expect(blocks[1].content).toBe("second"); + }); + + test("ordered list can start at an arbitrary number", () => { + const md = "5. five\n6. six"; + const blocks = parseMarkdownToBlocks(md); + expect(blocks).toHaveLength(2); + expect(blocks[0].orderedStart).toBe(5); + expect(blocks[1].orderedStart).toBe(6); + }); + + test("bullet markers stay unordered (no ordered flag)", () => { + const md = "- a\n* b"; + const blocks = parseMarkdownToBlocks(md); + expect(blocks).toHaveLength(2); + expect(blocks[0].ordered).toBeUndefined(); + expect(blocks[0].orderedStart).toBeUndefined(); + expect(blocks[1].ordered).toBeUndefined(); + }); + + test("mixed bullet/numeric/bullet run preserves per-item ordered flag", () => { + const md = "- a\n1. b\n- c"; + const blocks = parseMarkdownToBlocks(md); + expect(blocks).toHaveLength(3); + expect(blocks[0].ordered).toBeUndefined(); + expect(blocks[1].ordered).toBe(true); + expect(blocks[1].orderedStart).toBe(1); + expect(blocks[2].ordered).toBeUndefined(); + }); + + test("nested ordered item inside an unordered parent keeps its ordered flag", () => { + const md = "- parent\n 1. child\n 2. child two"; + const blocks = parseMarkdownToBlocks(md); + expect(blocks).toHaveLength(3); + expect(blocks[0].ordered).toBeUndefined(); + expect(blocks[0].level).toBe(0); + expect(blocks[1].ordered).toBe(true); + expect(blocks[1].level).toBe(1); + expect(blocks[1].orderedStart).toBe(1); + expect(blocks[2].ordered).toBe(true); + expect(blocks[2].level).toBe(1); + expect(blocks[2].orderedStart).toBe(2); + }); + + test("continuation line on an ordered item merges into content and preserves ordered flag", () => { + const md = "1. first item\n continuation\n2. second"; + const blocks = parseMarkdownToBlocks(md); + expect(blocks).toHaveLength(2); + expect(blocks[0].ordered).toBe(true); + expect(blocks[0].content).toBe("first item\ncontinuation"); + expect(blocks[1].ordered).toBe(true); + expect(blocks[1].orderedStart).toBe(2); + }); + + test("numeric checkbox sets both ordered and checked", () => { + const md = "1. [ ] todo task\n2. [x] done task"; + const blocks = parseMarkdownToBlocks(md); + expect(blocks).toHaveLength(2); + expect(blocks[0].ordered).toBe(true); + expect(blocks[0].checked).toBe(false); + expect(blocks[0].content).toBe("todo task"); + expect(blocks[1].ordered).toBe(true); + expect(blocks[1].checked).toBe(true); + expect(blocks[1].content).toBe("done task"); + }); + + test("'1.5 second' is not parsed as a list item (no whitespace after the dot)", () => { + const md = "1.5 second response time"; + const blocks = parseMarkdownToBlocks(md); + expect(blocks).toHaveLength(1); + expect(blocks[0].type).toBe("paragraph"); + expect(blocks[0].ordered).toBeUndefined(); + }); + + test("heading branch wins over list branch for '### 1. Foo'", () => { + const md = "### 1. Foo"; + const blocks = parseMarkdownToBlocks(md); + expect(blocks).toHaveLength(1); + expect(blocks[0].type).toBe("heading"); + }); +}); + +describe("parseMarkdownToBlocks — blockquotes", () => { + test("consecutive '>' lines merge into one blockquote block", () => { + const md = "> line one\n> line two\n> line three"; + const blocks = parseMarkdownToBlocks(md); + expect(blocks).toHaveLength(1); + expect(blocks[0].type).toBe("blockquote"); + expect(blocks[0].content).toBe("line one\nline two\nline three"); + }); + + test("blank line between '>' lines starts a new blockquote block", () => { + const md = "> first\n\n> second"; + const blocks = parseMarkdownToBlocks(md); + expect(blocks).toHaveLength(2); + expect(blocks[0].type).toBe("blockquote"); + expect(blocks[0].content).toBe("first"); + expect(blocks[1].type).toBe("blockquote"); + expect(blocks[1].content).toBe("second"); + }); + + test("blockquote followed by paragraph does not absorb the paragraph", () => { + const md = "> quote\nparagraph"; + const blocks = parseMarkdownToBlocks(md); + expect(blocks).toHaveLength(2); + expect(blocks[0].type).toBe("blockquote"); + expect(blocks[0].content).toBe("quote"); + expect(blocks[1].type).toBe("paragraph"); + }); + + test("paragraph followed by blockquote does not merge", () => { + const md = "intro\n> quote"; + const blocks = parseMarkdownToBlocks(md); + expect(blocks).toHaveLength(2); + expect(blocks[0].type).toBe("paragraph"); + expect(blocks[1].type).toBe("blockquote"); + expect(blocks[1].content).toBe("quote"); + }); + + test("single-line blockquote still produces one block", () => { + const md = "> just one"; + const blocks = parseMarkdownToBlocks(md); + expect(blocks).toHaveLength(1); + expect(blocks[0].type).toBe("blockquote"); + expect(blocks[0].content).toBe("just one"); + }); + + test("quoted ordered list does NOT merge — each '> N.' line stays as its own block", () => { + // Regression guard for review comment #7. Merging would flatten the + // markers into run-on inline text; keeping them as separate blockquote + // blocks preserves each line's visual identity. + const md = "> 1. First item\n> 2. Second item\n> 3. Third item"; + const blocks = parseMarkdownToBlocks(md); + expect(blocks).toHaveLength(3); + expect(blocks[0].type).toBe("blockquote"); + expect(blocks[0].content).toBe("1. First item"); + expect(blocks[1].type).toBe("blockquote"); + expect(blocks[1].content).toBe("2. Second item"); + expect(blocks[2].type).toBe("blockquote"); + expect(blocks[2].content).toBe("3. Third item"); + }); + + test("quoted unordered list does NOT merge", () => { + const md = "> - First\n> - Second\n> - Third"; + const blocks = parseMarkdownToBlocks(md); + expect(blocks).toHaveLength(3); + expect(blocks.every(b => b.type === "blockquote")).toBe(true); + expect(blocks.map(b => b.content)).toEqual(["- First", "- Second", "- Third"]); + }); + + test("quoted heading does NOT merge into previous blockquote", () => { + const md = "> intro text\n> # Heading inside quote\n> more text"; + const blocks = parseMarkdownToBlocks(md); + expect(blocks).toHaveLength(3); + expect(blocks[0].content).toBe("intro text"); + expect(blocks[1].content).toBe("# Heading inside quote"); + expect(blocks[2].content).toBe("more text"); + }); + + test("quoted code fence line does NOT merge", () => { + const md = "> some prose\n> ```js\n> more prose"; + const blocks = parseMarkdownToBlocks(md); + expect(blocks).toHaveLength(3); + expect(blocks[0].content).toBe("some prose"); + expect(blocks[1].content).toBe("```js"); + expect(blocks[2].content).toBe("more prose"); + }); + + test("nested blockquote (> >) does NOT merge into the outer quote", () => { + const md = "> outer quote\n> > nested quote\n> back to outer"; + const blocks = parseMarkdownToBlocks(md); + expect(blocks).toHaveLength(3); + expect(blocks[0].content).toBe("outer quote"); + expect(blocks[1].content).toBe("> nested quote"); + expect(blocks[2].content).toBe("back to outer"); + }); + + test("wrapped prose quote still merges (regression guard for the merge fix)", () => { + // This is the case the merge fix was added for — a single logical + // paragraph wrapped across multiple source lines. Must still merge. + const md = "> This is a long quoted paragraph\n> that wraps across several\n> source lines for readability."; + const blocks = parseMarkdownToBlocks(md); + expect(blocks).toHaveLength(1); + expect(blocks[0].type).toBe("blockquote"); + expect(blocks[0].content).toBe( + "This is a long quoted paragraph\nthat wraps across several\nsource lines for readability." + ); + }); + + test("prose quote followed by a quoted list: prose merges, list lines stay separate", () => { + const md = "> intro prose\n> that wraps here\n> 1. first step\n> 2. second step"; + const blocks = parseMarkdownToBlocks(md); + expect(blocks).toHaveLength(3); + expect(blocks[0].content).toBe("intro prose\nthat wraps here"); + expect(blocks[1].content).toBe("1. first step"); + expect(blocks[2].content).toBe("2. second step"); + }); + + test("multi-paragraph blockquote encodes paragraph break as double newline", () => { + // The empty `>` line sits between two quoted paragraphs. We merge all + // three `>` lines into one block, but the blank `>` becomes an empty + // string, leaving a `\n\n` in the content so the renderer can split on + // paragraph breaks and emit two

children. + const md = "> first paragraph\n>\n> second paragraph"; + const blocks = parseMarkdownToBlocks(md); + expect(blocks).toHaveLength(1); + expect(blocks[0].type).toBe("blockquote"); + expect(blocks[0].content).toBe("first paragraph\n\nsecond paragraph"); + expect(blocks[0].content.split(/\n\n+/)).toEqual([ + "first paragraph", + "second paragraph", + ]); + }); +}); + +describe("computeListIndices", () => { + test("all unordered → all null", () => { + const blocks = [li(0, false), li(0, false), li(0, false)]; + expect(computeListIndices(blocks)).toEqual([null, null, null]); + }); + + test("simple ordered run numbers sequentially from orderedStart of first item", () => { + const blocks = [li(0, true, 1), li(0, true, 2), li(0, true, 3)]; + expect(computeListIndices(blocks)).toEqual([1, 2, 3]); + }); + + test("ordered run starting at 5 numbers from 5", () => { + const blocks = [li(0, true, 5), li(0, true, 6)]; + expect(computeListIndices(blocks)).toEqual([5, 6]); + }); + + test("renumbering ignores subsequent orderedStart values (CommonMark)", () => { + // Source markdown `1. / 2. / 99.` should render as 1, 2, 3 + const blocks = [li(0, true, 1), li(0, true, 2), li(0, true, 99)]; + expect(computeListIndices(blocks)).toEqual([1, 2, 3]); + }); + + test("unordered item breaks the ordered streak; next ordered restarts from its orderedStart", () => { + const blocks = [ + li(0, true, 1), + li(0, true, 2), + li(0, false), + li(0, true, 1), + ]; + expect(computeListIndices(blocks)).toEqual([1, 2, null, 1]); + }); + + test("unordered nested children do not break top-level numbering", () => { + // 1. a + // - bullet + // - bullet + // 2. b + const blocks = [ + li(0, true, 1), + li(1, false), + li(1, false), + li(0, true, 2), + ]; + expect(computeListIndices(blocks)).toEqual([1, null, null, 2]); + }); + + test("nested ordered sublists number independently and reset between siblings", () => { + // 1. a + // 1. a.1 + // 2. a.2 + // 2. b + // 1. b.1 + const blocks = [ + li(0, true, 1), + li(1, true, 1), + li(1, true, 2), + li(0, true, 2), + li(1, true, 1), + ]; + expect(computeListIndices(blocks)).toEqual([1, 1, 2, 2, 1]); + }); + + test("ordered sublist after an unordered sub-bullet restarts from its source orderedStart", () => { + // 1. a + // - bullet + // 2. honored as 2 because the source said `2.` + const blocks = [ + li(0, true, 1), + li(1, false), + li(1, true, 2), + ]; + expect(computeListIndices(blocks)).toEqual([1, null, 2]); + }); + + test("mixed sub-bullets between ordered top-level items keep top-level streak alive", () => { + // 1. a + // - sub + // 2. b + // - sub + // 3. c + const blocks = [ + li(0, true, 1), + li(1, false), + li(0, true, 2), + li(1, false), + li(0, true, 3), + ]; + expect(computeListIndices(blocks)).toEqual([1, null, 2, null, 3]); + }); + + test("empty input returns empty array", () => { + expect(computeListIndices([])).toEqual([]); + }); + + test("single ordered item with no orderedStart defaults to 1", () => { + const blocks = [{ ...li(0, true), orderedStart: undefined }]; + expect(computeListIndices(blocks)).toEqual([1]); + }); +}); diff --git a/packages/ui/utils/parser.ts b/packages/ui/utils/parser.ts index 75c30a26..841cc08b 100644 --- a/packages/ui/utils/parser.ts +++ b/packages/ui/utils/parser.ts @@ -131,7 +131,8 @@ export const parseMarkdownToBlocks = (markdown: string): Block[] => { } // List Items (Simple detection) - if (trimmed.match(/^(\*|-|\d+\.)\s/)) { + const listMatch = trimmed.match(/^(\*|-|(\d+)\.)\s/); + if (listMatch) { flush(); // Treat each list item as a separate block for easier annotation // Calculate indentation level from leading whitespace const leadingWhitespace = line.match(/^(\s*)/)?.[1] || ''; @@ -139,8 +140,12 @@ export const parseMarkdownToBlocks = (markdown: string): Block[] => { const spaceCount = leadingWhitespace.replace(/\t/g, ' ').length; const listLevel = Math.floor(spaceCount / 2); + // Distinguish numeric markers (\d+.) from bullet markers (* / -) + const ordered = listMatch[2] !== undefined; + const orderedStart = ordered ? parseInt(listMatch[2]!, 10) : undefined; + // Remove list marker - let content = trimmed.replace(/^(\*|-|\d+\.)\s/, ''); + let content = trimmed.slice(listMatch[0].length); // Check for checkbox syntax: [ ] or [x] or [X] let checked: boolean | undefined = undefined; @@ -156,24 +161,57 @@ export const parseMarkdownToBlocks = (markdown: string): Block[] => { content, level: listLevel, checked, + ordered: ordered || undefined, + orderedStart, order: currentId, startLine: currentLineNum }); continue; } - // Blockquotes + // Blockquotes — consecutive `>` lines merge into one block so wrapped + // paragraph quotes render as a single continuous quote box. A blank line + // breaks the blockquote so the next `>` starts a fresh one. + // + // Exception: if the stripped content starts with a block-level marker + // (list item, heading, code fence, nested blockquote) we do NOT merge. + // Our flat block model can't render a list-inside-a-quote as an actual + // nested list, so merging would flatten the markers into run-on inline + // text. Leaving them as separate blockquote blocks preserves each line's + // visual identity (a stacked-box layout) — imperfect but legible. A + // proper recursive blockquote parser is tracked as a follow-up. if (trimmed.startsWith('>')) { - // Check if previous was blockquote, if so, merge? No, separate for now - flush(); - blocks.push({ - id: `block-${currentId++}`, - type: 'blockquote', - content: trimmed.replace(/^>\s*/, ''), - order: currentId, - startLine: currentLineNum - }); - continue; + flush(); + const stripped = trimmed.replace(/^>\s*/, ''); + // List markers require trailing whitespace to avoid matching inline + // text like "-hyphen" or "1.5 seconds"; headings, code fences, and + // nested blockquote markers don't require it (``` can be followed + // directly by a language tag, # can start a dense heading). + const blockMarkerRe = /^(?:(?:\*|-|\d+\.)\s|#|```|>)/; + const hasBlockMarker = blockMarkerRe.test(stripped); + const prevBlock = blocks.length > 0 ? blocks[blocks.length - 1] : null; + // Don't merge into a previous blockquote whose content itself starts + // with a block marker — otherwise a `> some text` line following a + // `> 1. item` line would get glued onto the list-item block. + const prevIsMarkerQuote = + prevBlock?.type === 'blockquote' && blockMarkerRe.test(prevBlock.content); + if ( + !hasBlockMarker && + !prevIsMarkerQuote && + !prevLineWasBlank && + prevBlock?.type === 'blockquote' + ) { + prevBlock.content += '\n' + stripped; + } else { + blocks.push({ + id: `block-${currentId++}`, + type: 'blockquote', + content: stripped, + order: currentId, + startLine: currentLineNum + }); + } + continue; } // Code blocks (naive) @@ -262,6 +300,49 @@ export const parseMarkdownToBlocks = (markdown: string): Block[] => { return blocks; }; +/** + * Compute the display index for each list item in a contiguous list group. + * + * Returns a parallel array where each entry is either: + * - a positive integer (the numeral to render for an ordered item), or + * - null (the item is unordered, render a bullet symbol). + * + * Semantics: + * - A run of consecutive ordered items at the same level increments + * sequentially. The first item in a run uses its `orderedStart` (the + * number from the source markdown); subsequent items renumber from there + * so `1. / 2. / 5.` renders as 1, 2, 3 (matches CommonMark). + * - An unordered item at level L breaks the ordered streak at L. The next + * ordered item at L restarts from its own `orderedStart`. + * - Visiting a level shallower than the current one truncates deeper-level + * state, so re-entering that depth later starts fresh. Top-level numbering + * continues across nested children of any kind. + */ +export const computeListIndices = (blocks: Block[]): (number | null)[] => { + const counters: number[] = []; + const lastOrderedAtLevel: boolean[] = []; + + return blocks.map(block => { + const lvl = block.level || 0; + // Sibling change at any deeper level resets those levels. + counters.length = lvl + 1; + lastOrderedAtLevel.length = lvl + 1; + + if (!block.ordered) { + lastOrderedAtLevel[lvl] = false; + return null; + } + + if (lastOrderedAtLevel[lvl]) { + counters[lvl] = (counters[lvl] ?? 0) + 1; + } else { + counters[lvl] = block.orderedStart ?? 1; + } + lastOrderedAtLevel[lvl] = true; + return counters[lvl]; + }); +}; + /** Wrap feedback output with the deny preamble for pasting into agent sessions */ export const wrapFeedbackForAgent = (feedback: string): string => planDenyFeedback(feedback); diff --git a/tests/test-fixtures/06-ordered-list-plan.md b/tests/test-fixtures/06-ordered-list-plan.md new file mode 100644 index 00000000..66048b06 --- /dev/null +++ b/tests/test-fixtures/06-ordered-list-plan.md @@ -0,0 +1,65 @@ +# Plan: Slice 6 — API completion + release cleanup + +## Context + +Slices 1–5 landed deterministic cold start, all seven warm-start cases with +git awareness, the live watcher pipeline with single-writer discipline, +graceful shutdown, periodic reconciliation, SIGTERM/SIGINT integration, and +the same-cwd advisory lockfile with `ConcurrentAccessError` as the one +public error class. **226 tests passing, 10 `it.todo` placeholders +remaining.** Those 10 todos are the only gaps left between the library and +the spec: + +- `tree.search()` — 3 todos in `test/unit/query-search.test.ts`; the + implementation in `src/query/list.ts:52` still throws + `NOT_IMPLEMENTED`. +- `tree.hash()` — 3 todos in `test/unit/query-hash.test.ts`; the + implementation in `src/query/hash.ts:5` still throws + `NOT_IMPLEMENTED`. `src/filetree.ts:hash()` also throws. +- `reconciliation diff` — 4 unit todos in + `test/unit/reconcile-diff.test.ts` covering add/update/delete + classification and the "no auto-hash" invariant. + +Plus two coverage items carried over from earlier slices: + +- An explicit empty-directory watcher integration test (deferred from + slice 4 as backend-dependent; slice 6 adds it with per-platform + skips). +- A cold subscribe-failure regression via `vi.mock` of + `src/watcher/parcel.js` (deferred from slice 4; slice 5's + `reconcile-warm-subscribe-failed.test.ts` established the pattern). + +And one release-gating task: flip the Bun CI lane from **advisory** to +**gating** now that slice-4 live watcher + slice-5 hardening have proven +cross-runtime parity. + +After slice 6 lands, `it.todo` should hit zero, every spec section has +live code behind it, and the Bun lane is required for merge. + +## Locked design decisions (user sign-off) + +### 1. `tree.hash()` — freshness-aware, invalidate on update, drop on delete + +The contract is: **`tree.hash(path)` returns the SHA-256 of the +file's current on-disk contents, for a path the library's index +currently believes is a regular file.** The result is cached in +`files.hash`, invalidated whenever reconcile/watcher updates the +row, and dropped whenever the row is deleted. + +### 2. `tree.search()` — AND-filter semantics, basename-substring-ci name, glob-on-cwd-rel-path + +**Query shape**: `{ name?: string; glob?: string }` (already fixed in +`src/types.ts:86`). + +## Verification + +1. `npm run typecheck` clean. +2. `npm run lint` clean. +3. `npm test` — **expected ~251 active passing**, **0 todo**, **0 failing**. +4. `npm run build` produces `dist/` with updated artifacts. +5. `npm run test:pack` still green. +6. `npm run test:bun` green on macOS with the **`continue-on-error` removed**. +7. **Manual sanity — hash lifecycle**: write a file, hash it, verify cache hit on second call, mutate the file, wait for the watcher, re-hash and confirm the digest changes. +8. **Manual sanity — search semantics**: `name`-only, `glob`-only, both (AND), and empty query → `[]`. +9. **Manual sanity — cold subscribe failure**: confirm the cache dir is gone and the lockfile is released after the reject. +10. **Manual sanity — Bun gating**: push a trivial PR with a deliberate Bun-only failure and confirm CI now fails the Bun lane instead of skipping it. Revert before merge. diff --git a/tests/test-fixtures/07-blockquotes.md b/tests/test-fixtures/07-blockquotes.md new file mode 100644 index 00000000..60b977df --- /dev/null +++ b/tests/test-fixtures/07-blockquotes.md @@ -0,0 +1,49 @@ +# Blockquote rendering test + +## Multi-line blockquote (the bug) + +This is a multi-line blockquote. Before the fix, each `>` line rendered as its own blockquote box with its own top/bottom margin, producing visible gaps between every line. After the fix, all consecutive `>` lines should merge into a single continuous quote. + +> `tree.hash(path)` returns the SHA-256 digest of the file as it +> was when the library last observed it. A digest is computed +> on first call from the then-current on-disk bytes, cached in +> `files.hash`, and returned as-is on subsequent calls. The +> cache is invalidated whenever the library **observes** a +> change to the row (watcher or reconcile update) and dropped +> when the row is deleted. The library observes changes via the +> watcher pipeline and reconcile passes, both of which track +> `(size, mtime)` per spec §"Event flow and normalization." A +> content change that does NOT move `(size, mtime)` — e.g. an +> atomic same-length overwrite that preserves mtime, or an +> interval-based edit the watcher drops — is invisible to the +> library and will NOT invalidate the cached hash. + +## Two separate blockquotes (blank line break) + +A blank line between two `>` runs should still produce two distinct blockquote blocks. + +> First quote, line one. +> First quote, line two. + +> Second quote, line one. +> Second quote, line two. + +## Single-line blockquote + +> A standalone one-liner. + +## Blockquote sandwiched between paragraphs + +Some intro text before the quote. + +> The quote itself spans +> two lines and should render +> as one continuous block. + +Some closing text after the quote. + +## Blockquote with inline formatting + +> This quote has **bold**, *italic*, `inline code`, +> and a [link](https://example.com) to verify +> inline markdown still works across merged lines. diff --git a/tests/test-fixtures/08-ordered-edge-cases.md b/tests/test-fixtures/08-ordered-edge-cases.md new file mode 100644 index 00000000..9a4e0a08 --- /dev/null +++ b/tests/test-fixtures/08-ordered-edge-cases.md @@ -0,0 +1,102 @@ +# Ordered list edge cases + +## 1. Ordered task lists — numbers AND checkboxes + +Before the fix: the checkbox branch took precedence and the numeral was dropped, so an ordered task list rendered as an unordered checklist. After the fix: both the numeral and the checkbox render side by side (matching GitHub's `1. [ ]` behavior). + +1. [x] Draft the API contract +2. [x] Land the parser changes +3. [ ] Wire up the diff renderer +4. [ ] Ship the PR +5. [ ] Write the changelog + +### Unordered task list (baseline — should look the same as before) + +- [x] Bullet checkbox one +- [ ] Bullet checkbox two +- [ ] Bullet checkbox three + +### Mixed ordered numbers (no checkboxes, regression guard) + +1. First step +2. Second step +3. Third step + +## 2. Multi-paragraph blockquote — quoted blank line should render as a paragraph break + +Before the fix: `\n\n` inside blockquote content collapsed to a single space, so two quoted paragraphs mashed together into one run-on line. After the fix: the blockquote splits on blank-line paragraph breaks and emits multiple `

` children. + +> This is the first paragraph of a long quote. It discusses +> the contract for `tree.hash(path)` and how the library +> observes file changes via `(size, mtime)`. +> +> This is a second paragraph of the same quote, separated by +> a blank `>` line. It should render as a visually distinct +> paragraph underneath the first one — not mashed into the +> same line. +> +> And here is a third paragraph to really hammer the point +> home. All three should live inside one blockquote box with +> clear paragraph breaks between them. + +Some prose between two blockquotes to confirm the blank-line break between quotes still produces two separate boxes. + +> A completely separate blockquote (blank line above, not a +> `>` continuation). This is one paragraph that spans +> multiple source lines; it should render as one continuous +> paragraph inside its own box. + +> Another completely separate blockquote, also single-paragraph, +> also spanning multiple source lines. + +## 3. Numbered list with nested bullets (regression guard for top-level streak) + +Top-level numbering should continue across nested unordered children. + +1. First ordered step + - nested bullet + - another nested bullet +2. Second ordered step + - nested bullet +3. Third ordered step + +## 4. High numerals (double-digit alignment) + +Tabular numerals + 1.5rem min-width should keep 9 → 10 from jittering. + +1. One +2. Two +3. Three +4. Four +5. Five +6. Six +7. Seven +8. Eight +9. Nine +10. Ten +11. Eleven +12. Twelve + +## 5. Ordered list starting at an arbitrary number + +5. Five (rendered as 5.) +6. Six (rendered as 6.) +7. Seven (rendered as 7.) + +## 6. Single-line blockquote (baseline) + +> Just one line. Should still render inside a blockquote box. + +## Diff-view test (manual, separate run) + +The diff renderer lives in `PlanCleanDiffView` and only appears when a prior +version of the plan is saved. To exercise the diff view specifically: + +1. Serve this fixture once through the plan review flow (`plannotator` plan mode + with a hook invocation), then deny + resubmit a modified copy that changes + one of the numbered items above. +2. Open the diff view via the `+N/-M` badge. +3. Verify that numbered items in the diff render as `1.`, `2.`, ... and NOT as + `•` / `◦` / `▪`. Before this fix the diff view silently flattened all + ordered lists to bullets because `PlanCleanDiffView.SimpleBlockRenderer` + had its own hardcoded marker logic. diff --git a/tests/test-fixtures/09-quoted-list-regression.md b/tests/test-fixtures/09-quoted-list-regression.md new file mode 100644 index 00000000..c9028f9b --- /dev/null +++ b/tests/test-fixtures/09-quoted-list-regression.md @@ -0,0 +1,69 @@ +# Review comment #7: quoted list regression + +This fixture isolates the bug flagged in the latest PR review. Serve it with the +**current** branch state (before any fix) to see the regression, then compare +against what it *should* look like after the fix. + +## The bug: quoted ordered list collapses into one run-on line + +Before this PR, the parser emitted each `>` line as its own blockquote block, so +the three lines below rendered as three stacked quote boxes, each containing the +text "1. First item" / "2. Second item" / "3. Third item". Imperfect but legible. + +After this PR's blockquote-merge fix, the three lines merge into a single +blockquote with content `"1. First item\n2. Second item\n3. Third item"`. The +renderer splits on `\n\n+` (blank-line paragraph breaks) and finds none, so it +emits **one** `

` with the entire string. In HTML, `\n` collapses to whitespace, +so the reader sees one run-on line: + +> 1. First item +> 2. Second item +> 3. Third item + +Expected after fix: three visually distinct lines (either stacked boxes as before, +or — ideally — a proper nested ordered list inside one quote box). + +## Same bug, unordered flavor + +> - First bullet +> - Second bullet +> - Third bullet + +## Same bug, bullet + text + +> - Install the dependency +> - Run the migration +> - Deploy to staging + +## Same bug, numbered steps with longer content + +> 1. Audit the current implementation for any obvious regressions. +> 2. Write a failing test that reproduces the reported issue. +> 3. Land the fix and confirm the test now passes. + +## Control case: multi-line wrapped paragraph (this must stay fixed) + +This is the case the blockquote-merge fix was added for. It should render as one +quote box with properly wrapped prose — NOT as six stacked boxes. + +> This is a long quoted paragraph that spans multiple source lines because the +> author wrapped it at around 80 columns for readability in the markdown source. +> The renderer should treat this as one continuous paragraph with soft wrapping, +> not six stacked blockquote boxes with individual margins between each line. +> The original bug that motivated the blockquote-merge fix was exactly this +> case — the `tree.hash` quote from the Slice 6 plan. Keep it working. + +## Control case: multi-paragraph quote (must stay fixed) + +> First paragraph of a quote that wraps across +> multiple source lines. +> +> Second paragraph of the same quote, separated +> by a blank `>` line. Should render as a visually +> distinct paragraph under the first one. + +## Control case: two separate quotes (must stay as two boxes) + +> A quote. + +> A completely different quote (blank line between, not `>`-continuation).