fix(markdown): preserve prose around nested inline lists - #239
Open
detail-app[bot] wants to merge 1 commit into
Open
detail-app[bot] wants to merge 1 commit into
detail-app[bot] wants to merge 1 commit into
Conversation
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.
Detail bug report: View on Detail
Bug
Closes #226
InlineListParserrewrites flat<p>paragraphs containing inline list markers into<ol>/<ul>HTML lists, and extracts a sub-list from any list item whose text contains a colon followed by an inline list (e.g.1. Parent: a. Child one b. Child two 2. Sibling).When a list item's post-colon tail contained prose between the colon and the first nested-list marker and/or after the last nested-list marker, that prose was silently dropped. The nested-list consumers (
tryConvertandrenderNestedListsRecursively) iterated onlynestedParse.primaryBlock().entryLabels()and never readnestedParse.leadingText()/nestedParse.trailingText(), even thoughParse.tryParsecaptured both.End-to-end repro via
UnifiedMarkdownService.process:Key points: 1. Setup phase: First gather tools a. Knife b. Spoon 2. Final cleanup<p>Key points:</p><ol><li>Setup phase</li><li>Final cleanup</li></ol><ol><li>Knife</li><li>Spoon</li></ol>—First gather toolsis absent.Steps: 1. Setup: Prepare phase a. Step A b. Step B. Then finish 2. Donelost bothPrepare phase(nested leading) andThen finish(nested trailing).The top-level
Conversion.leadingText/trailingTextwere already surfaced; only the nested level omitted them.Fix
InlineListParser.tryConvertand its recursive helper now emit each nestedParse.leadingText()andParse.trailingText()as sibling<p>elements, mirroring how the top-levelConversionalready surfaces prose. The two duplicated nested-rendering loops were consolidated into a single sharedrenderNestedParse(+buildListElement) helper used by bothtryConvertand the recursive case, so the nestedParseis rendered as: leading<p>→ nested list → deeper nested lists → trailing<p>.MAX_NESTED_DEPTH = 3gating is preserved (the recursion guard is unchanged in effect; only the previously-dropped prose is now emitted).1. Parent: a. Child one b. Child two 2. Sibling) renders identically to before — no spurious paragraphs are introduced when nested leading/trailing prose is empty.new Element("p").text(...), so it remains HTML-escaped and safe.After the fix, the repro input renders as:
<p>Key points:</p><ol><li>Setup phase</li><li>Final cleanup</li></ol><p>First gather tools</p><ol><li>Knife</li><li>Spoon</li></ol>Testing
A committed regression test (
InlineListNestedTextDropReproTest) asserts at theUnifiedMarkdownService.processobservable boundary that:First gather tools) is preserved and precedes the nested list;Prepare phase) and trailing (Then finish) prose are preserved in the correct sibling order around the nested list;<p>paragraphs.Verification performed (all green):
InlineListParserTest(26 cases) pass; full markdown package (InlineListParserTest, the new repro test,MarkdownNormalizerTest,MarkdownAstUtilsTest,MarkdownFenceRenderingTest,EnrichmentPlaceholderizerTest, 110 tests total) passes.MarkdownServiceTest,MarkdownPreprocessingTest, andComprehensiveListFormattingTest(67 tests, exercising list rendering through the publicMarkdownServicefacade) pass — no behavioral regression../gradlew test): 141 classes, 1167 tests, 0 failures, 0 errors.spotlessCheck, andbuild -x testall pass;make lint-ast(ast-grep + Ruby SSOT) clean./api/markdown/renderand/api/markdown/render/structured; both returned HTTP 200 withFirst gather toolspresent as a<p>and the nestedKnife/Spoonlist preserved.<p>/ nested<ol>/ nested trailing<p>render at each nested depth (the probe was run outside the repo and discarded, per repo convention).Automatic Fixes PRs can be configured here.