fix(markdown): render nested chat lists with an indentation stack#5582
Open
BrunooMoniz wants to merge 1 commit into
Open
fix(markdown): render nested chat lists with an indentation stack#5582BrunooMoniz wants to merge 1 commit into
BrunooMoniz wants to merge 1 commit into
Conversation
The chat markdown renderer built lists with regex passes anchored on the absolute start of the line (`^(\d+)\. `, `^(?:- |\* )...`) and grouped only consecutive sentinel lines, with no notion of depth. An indented item such as ` - Sub` never matched, fell through to the paragraph pass, and rendered as literal `- Sub` text while the parent list fragmented. The ordered-nested case was worse: it emitted an `<ol>` inside a `<p>`, which is invalid HTML. Replace the flatten-and-group passes with `_buildLists`, which parses each item's indentation and walks a per-run indentation stack: a deeper item opens a child list inside the still-open parent `<li>` (`<li>...<ul>...</ul></li>`) and a dedent closes levels until the indentation matches an open one. Indentation is compared relatively to the parent on the stack, so inconsistent 2-/3-/4-space and tab widths nest the same way and mixed ordered/unordered nesting produces valid HTML. Task-checkbox items keep their `task-item` class and nest correctly; a small flex-wrap/flex-basis rule drops a sub-list nested under a task item onto its own line instead of sitting inline after the label. Adds nested unordered/ordered/mixed/task/deep cases to tests/test_markdown_rendering_js.py and a mixed-nested sample to the streaming corpus; the existing flat-list, table, code, and autolink contracts are unchanged.
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
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 chat markdown renderer built lists with regex passes that anchored on the absolute start of each line and grouped only consecutive marker lines, with no notion of indentation. As a result, any indented item in a nested list (for example
- Sub) never matched a list pass, fell through to the paragraph pass, and rendered as flat literal text such as- Subwhile the surrounding list fragmented; an interrupted ordered list even restarted its numbering. The ordered-nested case was worse — an indented1. Subwas wrapped as an<ol>inside a<p>, which is invalid HTML. This PR replaces the flatten-and-group passes with a small_buildListshelper that parses each item's indentation and walks a per-run indentation stack: a deeper item opens a child list inside the still-open parent<li>(<li>...<ul>...</ul></li>) and a dedent closes levels until the indentation matches an open one. Indentation is compared relatively to the parent on the stack, so the inconsistent 2-/3-/4-space and tab widths that models emit all nest the same way, and mixed ordered/unordered nesting produces valid HTML. Task checkboxes keep theirtask-itemclass and nest correctly; a smallflex-wrap/flex-basisrule drops a sub-list nested under a task item onto its own line instead of sitting inline after the label.Target branch
dev, notmain. All PRs land indev;mainis curated by the maintainer at each release. If your PR is onmainby accident, click "Edit" on this PR and change the base.Linked Issue
Fixes #5581
Type of Change
Checklist
devdocker compose uporuvicorn app:app) and verified the change works end-to-end. Type-checks and unit tests are not enough.How to Test
Steps a reviewer can reproduce (full app):
docker compose up -d --build, open the chat.dev: the indented items render as flat literal- .../- [ ] ...text, the ordered list restarts at1.after the sub-item, and DevTools shows an<ol>inside a<p>. With this branch: the sub-items render as indented nested sub-lists, ordered numbering stays continuous, nested checkboxes stay checkboxes, and the HTML is valid.What was verified for this change:
dev(before) vs this branch (after) — see the screenshots below.node --check static/js/markdown.js— passes.python -m pytest tests/test_markdown_rendering_js.py— 22 passed (9 new nested-list cases plus the existing flat-list / table / thinking / autolink contracts).node --test tests/streaming/(streaming-invariant + segmenter) — 126 passed; the fuzz streams every corpus sample token-by-token, including the new nested sample.Files changed:
static/js/markdown.js— new_buildListsindentation-stack builder replaces the ordered/task/unordered flatten-and-group passes inmdToHtml.static/style.css—li.task-itemgetsflex-wrap: wrap, plus a rule so a nested<ul>/<ol>under a task item takes its own full-width line and the label keeps its row (reuses the existingtask-item/task-textclasses; no new variables, colors, or fonts).tests/test_markdown_rendering_js.py— nested unordered/ordered/mixed/task/deep cases (and a guard for the flat-list path).tests/streaming/corpus.mjs— a mixed nested-list sample for the streaming-invariant corpus.Visual / UI changes — REQUIRED if you touched anything that renders
Anything that changes what the UI looks like — buttons, icons, padding, colors, fonts, spacing, layout, CSS, HTML, SVG, or any
static/js/module that draws to the DOM — needs all of the following.mdToHtml) andstatic/style.cssin Chromium (desktop, 1280px) and WebKit (mobile, 390px) at the default dark theme, before (currentdev) vs after (this branch).--red,--fg,--bg,--card,--border, etc.) — do not introduce new color values, font sizes, or spacing units.static/index.html) or plain text.Fira Code) for primary UI text. Don't override.Screenshots / clips
Desktop — before (current
dev)Desktop — after (this PR)
Mobile — before (current
dev)Mobile — after (this PR)