From 757e327b6abe8dfb11d68ccd65119482efef56d7 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Tue, 7 Apr 2026 23:32:29 -0700 Subject: [PATCH 1/5] feat(parser): detect ordered list markers and compute display indices MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The block parser collapsed `*`, `-`, and `\d+.` markers into a single `list-item` block type, discarding ordered/unordered status. Add `ordered` + `orderedStart` to Block, capture the numeric marker in the list regex, and introduce `computeListIndices()` — a pure helper that walks a list group and assigns each ordered item a CommonMark-correct display number (sequential renumbering, streak break/restart on unordered items, deeper-level state truncation, top-level numbering preserved across nested children). 21 new unit tests cover both the parser changes and the indexing helper, including the tricky cases: `1./2./99.` renumbers as 1,2,3; sub-bullets between ordered items keep the top-level streak alive; nested ordered sublists number independently and reset between siblings; numeric checkboxes set both `ordered` and `checked`. For provenance purposes, this commit was AI assisted. --- packages/ui/types.ts | 2 + packages/ui/utils/parser.test.ts | 208 ++++++++++++++++++++++++++++++- packages/ui/utils/parser.ts | 54 +++++++- 3 files changed, 261 insertions(+), 3 deletions(-) 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..af187854 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,196 @@ 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("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..bf8b575b 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,6 +161,8 @@ export const parseMarkdownToBlocks = (markdown: string): Block[] => { content, level: listLevel, checked, + ordered: ordered || undefined, + orderedStart, order: currentId, startLine: currentLineNum }); @@ -262,6 +269,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); From a8aab84e219e4561caa5f55c0f3c55d5bfe9888a Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Tue, 7 Apr 2026 23:32:38 -0700 Subject: [PATCH 2/5] feat(ui): render numerals for ordered list items MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Branch the list-item marker span on `block.ordered`: render `${index}.` (with `tabular-nums` and a 1.5rem min-width to keep columns stable across single- and double-digit numerals) when the source marker was numeric, otherwise fall through to the existing `•`/`◦`/`▪` bullet symbols. Indices come from `computeListIndices()` called once per list group; `groupBlocks` is unchanged so mixed nested lists still share a single `data-pinpoint-group="list"` hover wrapper and annotation anchoring is unaffected. Checkbox items still take precedence over numerals. Adds a real-world manual fixture (06-ordered-list-plan.md) whose `## Verification` section exercises a 10-item ordered list, the case that originally surfaced the bug. For provenance purposes, this commit was AI assisted. --- packages/ui/components/Viewer.tsx | 33 ++++++++--- tests/test-fixtures/06-ordered-list-plan.md | 65 +++++++++++++++++++++ 2 files changed, 91 insertions(+), 7 deletions(-) create mode 100644 tests/test-fixtures/06-ordered-list-plan.md diff --git a/packages/ui/components/Viewer.tsx b/packages/ui/components/Viewer.tsx index c00c854e..1414eddd 100644 --- a/packages/ui/components/Viewer.tsx +++ b/packages/ui/components/Viewer.tsx @@ -3,7 +3,7 @@ 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 { AnnotationToolbar } from './AnnotationToolbar'; import { FloatingQuickLabelPicker } from './FloatingQuickLabelPicker'; @@ -530,11 +530,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 +981,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; @@ -1018,6 +1033,10 @@ const BlockRenderer: React.FC<{ ) + ) : block.ordered && orderedIndex != null ? ( + + {orderedIndex}. + ) : ( {(block.level || 0) === 0 ? '•' : (block.level || 0) === 1 ? '◦' : '▪'} 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. From f8911312c94bfe2b8675d44bbd8f5a025b009736 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Tue, 7 Apr 2026 23:35:36 -0700 Subject: [PATCH 3/5] fix(parser): merge consecutive blockquote lines into one block Each `>` line was emitted as its own blockquote block, so the renderer's `my-4` margin produced visible gaps between every line of a multi-line quote (the parser had a literal TODO comment: "Check if previous was blockquote, if so, merge? No, separate for now"). Fix: append to the previous block when it's a blockquote and the prior line wasn't blank, mirroring the list-continuation pattern. A blank line still breaks the quote so two `>` runs separated by a blank line stay distinct. Adds 5 unit tests (merge, blank-line break, paragraph boundaries, single-line) and a manual fixture (07-blockquotes.md) covering the bug case, the blank-line-break case, sandwich-between-paragraphs, and inline markdown across merged lines. For provenance purposes, this commit was AI assisted. --- packages/ui/utils/parser.test.ts | 46 +++++++++++++++++++++++++ packages/ui/utils/parser.ts | 31 +++++++++++------ tests/test-fixtures/07-blockquotes.md | 49 +++++++++++++++++++++++++++ 3 files changed, 115 insertions(+), 11 deletions(-) create mode 100644 tests/test-fixtures/07-blockquotes.md diff --git a/packages/ui/utils/parser.test.ts b/packages/ui/utils/parser.test.ts index af187854..d3868327 100644 --- a/packages/ui/utils/parser.test.ts +++ b/packages/ui/utils/parser.test.ts @@ -381,6 +381,52 @@ describe("parseMarkdownToBlocks — ordered lists", () => { }); }); +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"); + }); +}); + describe("computeListIndices", () => { test("all unordered → all null", () => { const blocks = [li(0, false), li(0, false), li(0, false)]; diff --git a/packages/ui/utils/parser.ts b/packages/ui/utils/parser.ts index bf8b575b..0d6966d5 100644 --- a/packages/ui/utils/parser.ts +++ b/packages/ui/utils/parser.ts @@ -169,18 +169,27 @@ export const parseMarkdownToBlocks = (markdown: string): Block[] => { continue; } - // Blockquotes + // Blockquotes — consecutive `>` lines merge into one block (CommonMark). + // A blank line breaks the blockquote so the next `>` starts a fresh one. 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*/, ''); + if ( + !prevLineWasBlank && + blocks.length > 0 && + blocks[blocks.length - 1].type === 'blockquote' + ) { + blocks[blocks.length - 1].content += '\n' + stripped; + } else { + blocks.push({ + id: `block-${currentId++}`, + type: 'blockquote', + content: stripped, + order: currentId, + startLine: currentLineNum + }); + } + continue; } // Code blocks (naive) 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. From ac77ebbcd7aaab19bf5011a0ad2e67887a2c7f75 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Wed, 8 Apr 2026 06:27:29 -0700 Subject: [PATCH 4/5] =?UTF-8?q?fix(ui):=20address=20code=20review=20?= =?UTF-8?q?=E2=80=94=20diff=20view,=20task=20lists,=20blockquote=20paragra?= =?UTF-8?q?phs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three issues surfaced by PR review #520: 1. **Diff view flattened ordered lists to bullets.** PlanCleanDiffView's SimpleBlockRenderer duplicated Viewer's list-item JSX with hardcoded bullet symbols, so a denied+resubmitted plan with numbered steps showed numerals in the main view but `•` in the diff view — exactly the screen where "which step changed?" matters most. Fixed by threading computeListIndices through MarkdownChunk and sharing the marker rendering via a new ListMarker component used by both renderers, which also removes the root-cause duplication. 2. **Ordered task lists dropped their numbers.** `1. [ ] step` set both `ordered=true` and `checked=false` in the parser, but the renderer's checkbox branch took precedence and the numeral was never shown. GitHub renders `1. [ ]` as numeral + checkbox side by side; we now match that by rendering both glyphs in ListMarker when an ordered task list item appears. 3. **Multi-paragraph blockquotes collapsed.** After the blockquote-merge fix in the previous commit, `> a\n>\n> b` produced content `"a\n\nb"` but the renderer passed it straight to InlineMarkdown, which renders `\n\n` as whitespace — so two quoted paragraphs mashed into one line. Fixed by splitting blockquote content on `/\n\n+/` in both Viewer and PlanCleanDiffView and emitting one `

` child per paragraph. Adds one unit test for the multi-paragraph blockquote content shape and a manual fixture (08-ordered-edge-cases.md) covering ordered task lists, multi-paragraph quotes, nested-bullet counter preservation, double-digit alignment, and start-at-N numbering. The fourth review comment — loose ordered lists with intervening non-list blocks restarting numbering — is deferred. It requires parser-level loose-list detection (CommonMark's indented-continuation rule) and the bug only fires when users rely on lazy `1./1./1.` markers across a break. Tracked as a follow-up. For provenance purposes, this commit was AI assisted. --- packages/ui/components/ListMarker.tsx | 73 +++++++++++++ packages/ui/components/Viewer.tsx | 47 ++++---- .../plan-diff/PlanCleanDiffView.tsx | 76 ++++++------- packages/ui/utils/parser.test.ts | 16 +++ tests/test-fixtures/08-ordered-edge-cases.md | 102 ++++++++++++++++++ 5 files changed, 243 insertions(+), 71 deletions(-) create mode 100644 packages/ui/components/ListMarker.tsx create mode 100644 tests/test-fixtures/08-ordered-edge-cases.md 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 1414eddd..5f53ec88 100644 --- a/packages/ui/components/Viewer.tsx +++ b/packages/ui/components/Viewer.tsx @@ -4,6 +4,7 @@ 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, computeListIndices } from '../utils/parser'; +import { ListMarker } from './ListMarker'; import { AnnotationToolbar } from './AnnotationToolbar'; import { FloatingQuickLabelPicker } from './FloatingQuickLabelPicker'; @@ -994,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 @@ -1017,32 +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.ordered && orderedIndex != null ? ( - - {orderedIndex}. - - ) : ( - - {(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/utils/parser.test.ts b/packages/ui/utils/parser.test.ts index d3868327..73072087 100644 --- a/packages/ui/utils/parser.test.ts +++ b/packages/ui/utils/parser.test.ts @@ -425,6 +425,22 @@ describe("parseMarkdownToBlocks — blockquotes", () => { expect(blocks[0].type).toBe("blockquote"); expect(blocks[0].content).toBe("just one"); }); + + 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", () => { 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. From 78b8558affe7496feeec831812cbdf4d09e466be Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Wed, 8 Apr 2026 06:53:11 -0700 Subject: [PATCH 5/5] fix(parser): don't merge blockquote lines containing block-level markers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-two review flagged a regression: `> 1. foo\n> 2. bar\n> 3. baz` was merging into one blockquote whose content was `"1. foo\n2. bar\n3. baz"`. The renderer split on `\n\n+` (paragraph breaks), found none, and emitted a single `

` — so `\n` collapsed to whitespace in HTML and the user saw `"1. foo 2. bar 3. baz"` as one run-on line. Worse than the pre-PR behavior (which at least kept each line in its own box). Pragmatic fix: don't merge a `>` line whose stripped content starts with a block-level marker (`*`, `-`, `\d+.`, `#`, `` ``` ``, `>`). Those stay as separate blockquote blocks so each marker line is visually distinct (legible, matching pre-PR behavior for quoted lists). Wrapped prose quotes — the original motivating case — still merge correctly because prose lines don't start with markers. Also check the PREVIOUS block's content for markers so a trailing prose line after a `> 1. item` doesn't glue onto the list-item block. 7 new unit tests cover: quoted ordered list stays separate, quoted unordered list stays separate, quoted heading stays separate, quoted code fence stays separate, nested blockquote stays separate, wrapped prose quote still merges (regression guard), and mixed prose+list where prose merges and list lines stay separate. Adds tests/test-fixtures/09-quoted-list-regression.md as a manual repro. Known follow-ups (tracked separately, not in this PR): - Consecutive separate blockquote blocks still get individual `my-4` margins, so a quoted list shows as stacked boxes with gaps between lines. The proper fix is recursive blockquote parsing (render the content as its own Block[] tree with an actual nested list inside the quote). Deferred — requires `children?: Block[]` on Block, parser rework, and exportAnnotations traversal changes. - Clean diff view renumbers ordered lists from the start of each diff chunk when users rely on CommonMark's lazy `1./1./1.` markers. Same power-user population as the earlier deferred loose-list case. - Pure code-hygiene items from the second review (non-list-block handling in computeListIndices, BULLET_BY_LEVEL modulo cycle, extraction, CLAUDE.md Block interface drift, splitBlockquoteParagraphs helper) — batch into a follow-up cleanup. For provenance purposes, this commit was AI assisted. --- packages/ui/utils/parser.test.ts | 71 +++++++++++++++++++ packages/ui/utils/parser.ts | 32 +++++++-- .../09-quoted-list-regression.md | 69 ++++++++++++++++++ 3 files changed, 167 insertions(+), 5 deletions(-) create mode 100644 tests/test-fixtures/09-quoted-list-regression.md diff --git a/packages/ui/utils/parser.test.ts b/packages/ui/utils/parser.test.ts index 73072087..74e055e2 100644 --- a/packages/ui/utils/parser.test.ts +++ b/packages/ui/utils/parser.test.ts @@ -426,6 +426,77 @@ describe("parseMarkdownToBlocks — blockquotes", () => { 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 diff --git a/packages/ui/utils/parser.ts b/packages/ui/utils/parser.ts index 0d6966d5..841cc08b 100644 --- a/packages/ui/utils/parser.ts +++ b/packages/ui/utils/parser.ts @@ -169,17 +169,39 @@ export const parseMarkdownToBlocks = (markdown: string): Block[] => { continue; } - // Blockquotes — consecutive `>` lines merge into one block (CommonMark). - // A blank line breaks the blockquote so the next `>` starts a fresh one. + // 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('>')) { 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 && - blocks.length > 0 && - blocks[blocks.length - 1].type === 'blockquote' + prevBlock?.type === 'blockquote' ) { - blocks[blocks.length - 1].content += '\n' + stripped; + prevBlock.content += '\n' + stripped; } else { blocks.push({ id: `block-${currentId++}`, 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).