fix(ui): render numerals for ordered list items#520
Merged
backnotprop merged 5 commits intomainfrom Apr 8, 2026
Merged
Conversation
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.
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.
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.
backnotprop
force-pushed
the
fix/mkdown-numbers
branch
from
69e23ad to
f891131
Compare
…graphs 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 `<p>` 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.
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 `<p>` — 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, <ListGroup> extraction, CLAUDE.md Block interface drift, splitBlockquoteParagraphs helper) — batch into a follow-up cleanup. For provenance purposes, this commit was AI assisted.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The branch name promised numbered-list rendering, but the prior commit (d850b78) only fixed list-continuation merging. Ordered lists like
1.…10.were still rendered as bullet points because the data model had no concept of "ordered" — the parser collapsed*,-, and\d+.into a singlelist-itemblock type and the renderer hardcoded•/◦/▪by indent level.This PR fixes the actual numbering bug:
packages/ui/utils/parser.ts): captures the numeric marker via a second capture group; setsordered+orderedStarton list-item blocks. Switched from a secondreplace()toslice(listMatch[0].length)to avoid coupling to capture-group ordering.Blocktype (packages/ui/types.ts): two optional fields,ordered?: booleanandorderedStart?: number. Optional so every existing fixture is byte-identical.computeListIndices()(new pure helper inparser.ts): walks a list group and assigns each ordered item a CommonMark-correct display number. Handles sequential renumbering (1./2./99.→ 1,2,3), streak break/restart on unordered items, deeper-level state truncation when siblings change, and top-level numbering survival across nested children.packages/ui/components/Viewer.tsx): branches the list-item marker span onblock.orderedand renders${index}.withtabular-nums+ a 1.5rem min-width so 9 → 10 doesn't jitter.groupBlocksis untouched — mixed nested lists still share a singledata-pinpoint-group="list"hover wrapper, so annotation anchoring is unaffected. Checkbox items still take precedence over numerals.parseMarkdownToBlocksandcomputeListIndicescovering the tricky cases — sub-bullets between ordered items keeping the top-level streak alive, nested ordered sublists numbering independently, sourceorderedStarthonored on restart, numeric checkboxes setting bothorderedandchecked,1.5 secondnot matching,### 1. Foostaying a heading.tests/test-fixtures/06-ordered-list-plan.md— the real-world Slice 6 plan whose 10-item## Verificationsection originally surfaced the bug.Test plan
bun test packages/ui/utils/parser.test.ts— 43 pass, 0 fail (22 existing + 21 new)bun test— only failure is a pre-existing pi-extension module-resolution error unrelated to this branchbun run --cwd apps/review build && bun run build:hook— bundles rebuilt01..06) viabun run apps/hook/server/index.ts annotate <file>; ordered lists render with numerals, bullets unchanged, no regressions in continuation merging / hard line breaks / real-world plansselect-none)Out of scope
PlanCleanDiffView) renders diff lines without going throughBlockRenderer, so ordered-list changes still show bullets in diff view. Pre-existing limitation, not a new regression — worth a follow-up.apps/review) uses a different renderer that doesn't shareViewer.tsx.- 1. nested(where1.is content of an unordered item) aren't detected because parsing is line-level.For provenance purposes, this PR was AI assisted.