From 39e94ffe41641051e3ee26ec3f0799cec13f7830 Mon Sep 17 00:00:00 2001 From: clem Date: Sat, 28 Mar 2026 01:57:11 +0800 Subject: [PATCH 1/8] docs: add design spec for renderNodes plain text fast path (issue #145) Co-Authored-By: Claude Opus 4.6 (1M context) --- .../2026-03-28-plain-text-fast-path-design.md | 139 ++++++++++++++++++ 1 file changed, 139 insertions(+) create mode 100644 docs/superpowers/specs/2026-03-28-plain-text-fast-path-design.md diff --git a/docs/superpowers/specs/2026-03-28-plain-text-fast-path-design.md b/docs/superpowers/specs/2026-03-28-plain-text-fast-path-design.md new file mode 100644 index 0000000..f4d02be --- /dev/null +++ b/docs/superpowers/specs/2026-03-28-plain-text-fast-path-design.md @@ -0,0 +1,139 @@ +# Plain Text Fast Path in renderNodes + +**Issue:** [#145](https://github.com/rohal12/spindle/issues/145) +**Date:** 2026-03-28 +**Status:** Approved + +## Problem + +After the #143 whitespace-only fast path (1,673ms → 288ms), 54% of remaining click time is spent in `innerHTML` parsing plain UI text that contains no markdown syntax. Text like `"ALMA"`, `"▸ Crew"`, `"Activate"` passes through `micromark → innerHTML → DOM walk → Preact vnodes` and produces the same text it started with. + +The bottleneck is `HtmlNodeRenderer` (render.tsx:106) calling `renderNodes` for every HTML element's children. In a `[nobr]` passage with 23 `{for}` cards, this triggers ~655 `innerHTML` calls on plain text. + +## Solution + +Add a plain text fast path in `renderNodes` between the combined-string construction and the `markdownToHtml` call. When the text portions of the combined string contain no markdown-triggering characters, split on `` placeholders and build vnodes directly — bypassing both micromark and `innerHTML`. + +## Detection + +After building the combined string (render.tsx line 304), strip placeholder spans and test the remaining text: + +```typescript +const MARKDOWN_SYNTAX_RE = /[*_`#|~\[>\\\-]|!\[|\d+\./; +const BLANK_LINE_RE = /\n\s*\n/; +``` + +Characters covered: + +- `*_` — emphasis/strong +- `` ` `` — code spans +- `#` — ATX headings +- `|` — GFM tables +- `~` — GFM strikethrough +- `[` — links/images +- `>` — blockquotes +- `\` — escape sequences +- `-` — list items, thematic breaks (`---`) +- `![` — images (`![alt](url)`) — bare `!` excluded to avoid false positives on UI text like `"Activate!"` +- `\d+\.` — ordered list items (`1.`) — note: also matches version-like strings e.g. `"v2.3"`, a harmless false positive +- `\n\s*\n` — paragraph breaks (blank lines) + +This is broadly correct for all `renderNodes` callers. Any false positive (text contains `-` but not as a list item) harmlessly falls through to the existing micromark path. + +## Direct Vnode Construction + +New helper function `buildPlainTextVnodes`: + +1. Split combined string on `` placeholders using a regex +2. Interleave text strings with pre-rendered components from the `components` array +3. With `nobr` (the hot path): return `<>{children}` +4. Without `nobr`: wrap in `

` to match what micromark would produce + +~15 lines, no DOM allocation. + +## Insertion Point + +In `renderNodes`, after line 304 (combined string built), before line 307 (`markdownToHtml` call): + +```typescript +// ... existing combined string construction (lines 287-304) ... + +// Fast path: skip micromark + innerHTML when text has no markdown syntax. +const textOnly = combined.replace(/<\/span>/g, ''); +if (!MARKDOWN_SYNTAX_RE.test(textOnly) && !BLANK_LINE_RE.test(textOnly)) { + return buildPlainTextVnodes(combined, components, options?.nobr); +} + +// Run combined text through markdown (existing code) +const html = markdownToHtml(combined); +return htmlToPreact(html, components, options?.nobr); +``` + +The existing whitespace-only fast path (lines 279-285) stays as-is. It handles the case where there's no text at all (pure macro/HTML content). This new path handles the next tier: text exists but contains no markdown syntax. + +## What Doesn't Change + +- `htmlToPreact`, `convertDomNode`, `HtmlNodeRenderer`, `markdownToHtml` — untouched +- The whitespace-only fast path from #143 — stays as-is +- Any text with markdown syntax characters — uses the existing pipeline +- Public API — no signature changes + +## Test Strategy + +### Correctness tests + +Verify the fast path produces identical output to the existing pipeline: + +- Plain text in HTML element children (nobr) +- Plain text with variable placeholders +- Unicode symbols (not markdown syntax) +- Multiple HTML elements with plain text children +- Text with colons, commas, slashes (non-markdown punctuation) +- Numbers and version strings +- `{for}` loops with HTML + plain text bodies + +### Fallthrough tests + +Verify text with markdown syntax still uses the full pipeline: + +- Emphasis (`*`, `_`) +- Code spans (`` ` ``) +- Headings (`#`) +- Links (`[`) +- GFM strikethrough (`~`) +- Backslash escapes (`\`) +- List items (`-`, `1.`) +- Images (`![`) +- Blockquotes (`>`) +- GFM tables (`|`) +- Blank lines (paragraph breaks) + +Note: bare `!` (e.g. `"Activate!"`) should NOT trigger fallthrough — only `![` does. + +### Non-nobr tests + +Verify `

` wrapping when nobr is false. + +### Benchmark test + +ALMA-like UI passage with nested HTML + 20 `{for}` cards, measuring ms/render. + +## Expected Impact + +| Version | Click duration | Bottleneck | +| -------------------------- | -------------- | ------------------------------------------ | +| 0.43.2 (before #143) | 1,673 ms | `htmlToPreact` in `{for}` loops (89%) | +| 0.43.3 (after #143) | 297 ms | `htmlToPreact` from HtmlNodeRenderer (61%) | +| 0.43.3 + game-side CSS fix | 288 ms | `set innerHTML` from plain text (54%) | +| **This fix (projected)** | **~80-100 ms** | Preact diffing + layout | + +## Source Locations + +All in `src/markup/render.tsx`: + +| Function | Line | Role | +| ------------------ | -------------- | ----------------------------------------------------------------------------- | +| `htmlToPreact` | 38 | Creates temp div, sets innerHTML, strips `

`, walks DOM → vnodes | +| `HtmlNodeRenderer` | 90 | Renders `

`, ``, etc. — calls `renderNodes` for children (line 106) | +| `renderNodes` | 268 | Entry point — builds combined string, calls `markdownToHtml` + `htmlToPreact` | +| `markdownToHtml` | markdown.ts:12 | Runs micromark with CommonMark + GFM tables + strikethrough | From 512cab195a8ca2596ce37477066ea70bf451ea0c Mon Sep 17 00:00:00 2001 From: clem Date: Sat, 28 Mar 2026 01:59:58 +0800 Subject: [PATCH 2/8] docs: add implementation plan for renderNodes plain text fast path (#145) Co-Authored-By: Claude Opus 4.6 (1M context) --- .../plans/2026-03-28-plain-text-fast-path.md | 522 ++++++++++++++++++ 1 file changed, 522 insertions(+) create mode 100644 docs/superpowers/plans/2026-03-28-plain-text-fast-path.md diff --git a/docs/superpowers/plans/2026-03-28-plain-text-fast-path.md b/docs/superpowers/plans/2026-03-28-plain-text-fast-path.md new file mode 100644 index 0000000..7719a0e --- /dev/null +++ b/docs/superpowers/plans/2026-03-28-plain-text-fast-path.md @@ -0,0 +1,522 @@ +# Plain Text Fast Path Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Skip micromark + innerHTML for plain text without markdown syntax in `renderNodes`, eliminating ~54% of remaining click time (288ms → ~80-100ms). + +**Architecture:** Add a detection regex and `buildPlainTextVnodes` helper in `src/markup/render.tsx`. After the combined string is built, strip placeholders, test for markdown syntax characters. If none found, split on placeholders and build vnodes directly. Falls through to existing pipeline on any match. + +**Tech Stack:** Preact, TypeScript, Vitest (happy-dom) + +--- + +### Task 1: Write failing tests for plain text fast path correctness + +**Files:** + +- Create: `test/dom/render-plain-text.test.tsx` + +These tests exercise `renderNodes` directly through tokenize → buildAST → renderNodes. They should pass both before and after the implementation (the fast path must produce identical output to the existing pipeline). + +- [ ] **Step 1: Create the test file with correctness tests** + +```tsx +// @vitest-environment happy-dom +import { describe, it, expect, beforeEach } from 'vitest'; +import { render } from 'preact'; +import { tokenize } from '../../src/markup/tokenizer'; +import { buildAST } from '../../src/markup/ast'; +import { renderNodes } from '../../src/markup/render'; +import { useStoryStore } from '../../src/store'; +import type { StoryData, Passage } from '../../src/parser'; + +function makePassage(pid: number, name: string, content: string): Passage { + return { pid, name, tags: [], metadata: {}, content }; +} + +function makeStoryData(passages: Passage[], startNode = 1): StoryData { + const byName = new Map(passages.map((p) => [p.name, p])); + const byId = new Map(passages.map((p) => [p.pid, p])); + return { + name: 'Test', + startNode, + ifid: 'test', + format: 'spindle', + formatVersion: '0.1.0', + passages: byName, + passagesById: byId, + userCSS: '', + userScript: '', + }; +} + +function renderMarkup( + markup: string, + options?: { nobr?: boolean }, +): HTMLElement { + const tokens = tokenize(markup); + const ast = buildAST(tokens); + const container = document.createElement('div'); + render(<>{renderNodes(ast, options)}, container); + return container; +} + +describe('plain text fast path (issue #145)', () => { + beforeEach(() => { + const store = useStoryStore.getState(); + store.init(makeStoryData([makePassage(1, 'Start', 'Start')])); + }); + + describe('correctly renders plain text without markdown pipeline', () => { + it('plain text in nobr mode', () => { + const el = renderMarkup('Hello World', { nobr: true }); + expect(el.textContent).toBe('Hello World'); + }); + + it('plain text with variable placeholder', () => { + useStoryStore.getState().setVariable('name', 'ALMA'); + const el = renderMarkup('Name: {$name}', { nobr: true }); + expect(el.textContent).toBe('Name: ALMA'); + }); + + it('Unicode symbols (not markdown syntax)', () => { + const el = renderMarkup('▸ Menu ✕ Close', { nobr: true }); + expect(el.textContent).toBe('▸ Menu ✕ Close'); + }); + + it('emoji content', () => { + const el = renderMarkup('🔒 Locked', { nobr: true }); + expect(el.textContent).toBe('🔒 Locked'); + }); + + it('text with colons, commas, slashes (non-markdown punctuation)', () => { + const el = renderMarkup('Requires: Level 2, Policy / Tier 3', { + nobr: true, + }); + expect(el.textContent).toBe('Requires: Level 2, Policy / Tier 3'); + }); + + it('text with parentheses and equals', () => { + const el = renderMarkup('Speed (km/h) = 100', { nobr: true }); + expect(el.textContent).toBe('Speed (km/h) = 100'); + }); + + it('bare exclamation mark is not markdown', () => { + const el = renderMarkup('Activate!', { nobr: true }); + expect(el.textContent).toBe('Activate!'); + }); + + it('multiple variables interleaved with text', () => { + useStoryStore.getState().setVariable('a', 'Alpha'); + useStoryStore.getState().setVariable('b', 'Beta'); + const el = renderMarkup('{$a} and {$b}', { nobr: true }); + expect(el.textContent).toBe('Alpha and Beta'); + }); + + it('HTML element children with plain text', () => { + const el = renderMarkup( + '
Hello
', + { nobr: true }, + ); + expect(el.querySelector('.label')!.textContent).toBe('Hello'); + }); + }); + + describe('non-nobr wraps in

', () => { + it('plain text without nobr gets

wrapper', () => { + const el = renderMarkup('Just some text'); + const p = el.querySelector('p'); + expect(p).not.toBeNull(); + expect(p!.textContent).toBe('Just some text'); + }); + + it('plain text with variable without nobr', () => { + useStoryStore.getState().setVariable('name', 'ALMA'); + const el = renderMarkup('Hello {$name}'); + const p = el.querySelector('p'); + expect(p).not.toBeNull(); + expect(p!.textContent).toBe('Hello ALMA'); + }); + }); + + describe('falls through to markdown for syntax-bearing text', () => { + it('emphasis with asterisks', () => { + const el = renderMarkup('This is *important* text'); + expect(el.querySelector('em')!.textContent).toBe('important'); + }); + + it('emphasis with underscores', () => { + const el = renderMarkup('This is _emphasized_ text'); + expect(el.querySelector('em')!.textContent).toBe('emphasized'); + }); + + it('inline code', () => { + const el = renderMarkup('Use `console.log` here'); + expect(el.querySelector('code')!.textContent).toBe('console.log'); + }); + + it('heading', () => { + const el = renderMarkup('# Title'); + expect(el.querySelector('h1')!.textContent).toBe('Title'); + }); + + it('link syntax', () => { + const el = renderMarkup('See [docs](http://example.com)'); + const link = el.querySelector('a'); + expect(link).not.toBeNull(); + expect(link!.textContent).toBe('docs'); + }); + + it('GFM strikethrough', () => { + const el = renderMarkup('This is ~~deleted~~ text'); + expect(el.querySelector('del')!.textContent).toBe('deleted'); + }); + + it('backslash escape', () => { + const el = renderMarkup('Price\\: free'); + expect(el.textContent).toContain('Price'); + }); + + it('unordered list item', () => { + const el = renderMarkup('- Item 1\n- Item 2'); + expect(el.querySelector('ul')).not.toBeNull(); + }); + + it('ordered list item', () => { + const el = renderMarkup('1. First\n2. Second'); + expect(el.querySelector('ol')).not.toBeNull(); + }); + + it('image syntax', () => { + const el = renderMarkup('![alt](http://example.com/img.png)'); + expect(el.querySelector('img')).not.toBeNull(); + }); + + it('blockquote', () => { + const el = renderMarkup('> quoted text'); + expect(el.querySelector('blockquote')).not.toBeNull(); + }); + + it('GFM table', () => { + const el = renderMarkup('| A | B |\n| --- | --- |\n| 1 | 2 |'); + expect(el.querySelector('table')).not.toBeNull(); + }); + + it('blank lines create paragraphs', () => { + const el = renderMarkup('Para 1\n\nPara 2'); + expect(el.querySelectorAll('p').length).toBeGreaterThanOrEqual(2); + }); + }); +}); +``` + +- [ ] **Step 2: Run tests to verify they pass (baseline)** + +These tests verify existing behavior — they should all pass before any implementation changes. + +Run: `npx vitest run test/dom/render-plain-text.test.tsx` +Expected: All tests PASS + +- [ ] **Step 3: Commit** + +```bash +git add test/dom/render-plain-text.test.tsx +git commit -m "test: add correctness baseline tests for renderNodes plain text fast path (#145)" +``` + +--- + +### Task 2: Implement the plain text fast path + +**Files:** + +- Modify: `src/markup/render.tsx:268-315` + +- [ ] **Step 1: Add the detection regex constants and `buildPlainTextVnodes` helper** + +Add these above the `renderNodes` function (before line 268), after the `getVariableTextValue` function: + +```tsx +/** + * Characters/patterns that trigger CommonMark or GFM transformations. + * Any match → fall through to the full micromark pipeline. + * False positives (e.g. `-` used as text, not list) just use the slower path. + */ +const MARKDOWN_SYNTAX_RE = /[*_`#|~\[>\\\-]|!\[|\d+\./; +const BLANK_LINE_RE = /\n\s*\n/; +const PLACEHOLDER_SPLIT_RE = /(<\/span>)/; +const PLACEHOLDER_IDX_RE = /^<\/span>$/; +const PLACEHOLDER_STRIP_RE = /<\/span>/g; + +/** + * Build Preact vnodes from a combined string that contains only plain text + * and placeholders. No micromark, no innerHTML. + */ +function buildPlainTextVnodes( + combined: string, + components: preact.ComponentChildren[], + nobr?: boolean, +): preact.ComponentChildren { + const parts = combined.split(PLACEHOLDER_SPLIT_RE); + const children: preact.ComponentChildren[] = []; + for (const part of parts) { + const m = PLACEHOLDER_IDX_RE.exec(part); + if (m) { + children.push(components[parseInt(m[1]!, 10)]); + } else if (part) { + children.push(part); + } + } + return nobr ? <>{children} : h('p', null, ...children); +} +``` + +- [ ] **Step 2: Insert the fast path check in `renderNodes`** + +In `renderNodes`, after the combined string loop (after the `for` loop ending at line 308), and before the `markdownToHtml` call (line 311), add: + +Replace this block in `renderNodes`: + +```tsx +// Run combined text through markdown +const html = markdownToHtml(combined, { inline: options?.inline }); + +// Convert HTML to Preact VNodes, replacing placeholders with components +return htmlToPreact(html, components, options?.nobr); +``` + +With: + +```tsx +// Fast path: skip micromark + innerHTML when text has no markdown syntax. +// This eliminates ~655 innerHTML calls on plain UI text like "ALMA", +// "▸ Crew", "Activate" that pass through the full pipeline only to +// produce the same text they started with (issue #145). +const textOnly = combined.replace(PLACEHOLDER_STRIP_RE, ''); +if (!MARKDOWN_SYNTAX_RE.test(textOnly) && !BLANK_LINE_RE.test(textOnly)) { + return buildPlainTextVnodes(combined, components, options?.nobr); +} + +// Run combined text through markdown +const html = markdownToHtml(combined, { inline: options?.inline }); + +// Convert HTML to Preact VNodes, replacing placeholders with components +return htmlToPreact(html, components, options?.nobr); +``` + +- [ ] **Step 3: Run the type checker** + +Run: `npx tsc --noEmit` +Expected: No errors + +- [ ] **Step 4: Run all tests** + +Run: `npx vitest run` +Expected: All tests PASS (both new and existing) + +- [ ] **Step 5: Commit** + +```bash +git add src/markup/render.tsx +git commit -m "fix: skip micromark + innerHTML for plain text without markdown syntax (#145)" +``` + +--- + +### Task 3: Add benchmark test + +**Files:** + +- Create: `test/dom/render-plain-text-bench.test.tsx` + +This is a separate file so it can be run independently. It measures the performance impact of the fast path with an ALMA-like UI structure. + +- [ ] **Step 1: Create the benchmark test** + +```tsx +// @vitest-environment happy-dom +import { describe, it, expect, beforeEach } from 'vitest'; +import { render } from 'preact'; +import { Passage } from '../../src/components/Passage'; +import { useStoryStore } from '../../src/store'; +import type { StoryData, Passage as PassageData } from '../../src/parser'; + +function makePassage( + pid: number, + name: string, + content: string, + tags: string[] = [], +): PassageData { + return { pid, name, tags, metadata: {}, content }; +} + +function makeStoryData(passages: PassageData[], startNode = 1): StoryData { + const byName = new Map(passages.map((p) => [p.name, p])); + const byId = new Map(passages.map((p) => [p.pid, p])); + return { + name: 'Test', + startNode, + ifid: 'test', + format: 'spindle', + formatVersion: '0.1.0', + passages: byName, + passagesById: byId, + userCSS: '', + userScript: '', + }; +} + +describe('renderNodes plain-text fast path benchmark', () => { + beforeEach(() => { + const store = useStoryStore.getState(); + store.init(makeStoryData([makePassage(1, 'Start', 'Start')])); + }); + + it('ALMA-like UI passage with nested HTML + plain text labels', () => { + useStoryStore.getState().setVariable( + 'items', + Array.from({ length: 20 }, (_, i) => ({ + id: `item-${i}`, + name: `Research ${i}`, + status: i % 3 === 0 ? 'active' : 'locked', + effects: `+${i} bonus`, + })), + ); + + const content = [ + '

', + '
', + ' ALMA', + ' ✕', + '
', + '
', + '
Home
', + '
Crew
', + '
Colony
', + '
▸ Research
', + '
▸ Policies
', + '
Construction
', + '
Settings
', + '
', + '
', + ' Research Tree', + ' {for @node of $items}', + '
', + '
', + ' ', + ' {@node.name}', + '
', + '
', + ' {@node.effects}', + ' {if @node.status == "active"}', + ' ', + ' {/if}', + '
', + '
', + ' {/for}', + '
', + '
', + ].join('\n'); + + const passage = makePassage(1, 'Test', content, ['nobr']); + const storyData = makeStoryData([passage]); + useStoryStore.getState().init(storyData); + useStoryStore.getState().show(1); + + const container = document.createElement('div'); + + // Warm up + render(, container); + container.innerHTML = ''; + + // Benchmark + const iterations = 50; + const start = performance.now(); + for (let i = 0; i < iterations; i++) { + render(, container); + container.innerHTML = ''; + } + const elapsed = performance.now() - start; + const perRender = elapsed / iterations; + + // Verify correctness + render(, container); + expect(container.querySelectorAll('.card').length).toBe(20); + expect(container.querySelector('.header-id')!.textContent).toBe('ALMA'); + expect(container.querySelectorAll('.nav-item').length).toBe(7); + + console.log( + `ALMA-like UI: ${perRender.toFixed(2)}ms/render (${iterations} iterations, ${elapsed.toFixed(0)}ms total)`, + ); + }); + + it('worst case: many small HTML elements with short text labels', () => { + const items = Array.from({ length: 100 }, (_, i) => `Label ${i}`); + useStoryStore.getState().setVariable('labels', items); + + const content = + '{for @label of $labels}
{@label}
{/for}'; + + const passage = makePassage(1, 'Test', content, ['nobr']); + const storyData = makeStoryData([passage]); + useStoryStore.getState().init(storyData); + useStoryStore.getState().show(1); + + const container = document.createElement('div'); + + // Warm up + render(, container); + container.innerHTML = ''; + + const iterations = 20; + const start = performance.now(); + for (let i = 0; i < iterations; i++) { + render(, container); + container.innerHTML = ''; + } + const elapsed = performance.now() - start; + const perRender = elapsed / iterations; + + render(, container); + expect(container.querySelectorAll('.cell').length).toBe(100); + + console.log( + `100 HTML elements with text: ${perRender.toFixed(2)}ms/render (${iterations} iterations, ${elapsed.toFixed(0)}ms total)`, + ); + }); +}); +``` + +- [ ] **Step 2: Run the benchmark** + +Run: `npx vitest run test/dom/render-plain-text-bench.test.tsx` +Expected: PASS, console output shows ms/render timings + +- [ ] **Step 3: Commit** + +```bash +git add test/dom/render-plain-text-bench.test.tsx +git commit -m "test: add benchmark for renderNodes plain text fast path (#145)" +``` + +--- + +### Task 4: Final verification + +**Files:** None (verification only) + +- [ ] **Step 1: Run full test suite** + +Run: `npx vitest run` +Expected: All tests PASS + +- [ ] **Step 2: Run type checker** + +Run: `npx tsc --noEmit` +Expected: No errors + +- [ ] **Step 3: Verify the fast path is actually being hit** + +Add a temporary `console.log` in `buildPlainTextVnodes` and run the ALMA benchmark to confirm the fast path fires. Then remove the console.log. + +Alternatively, check that the benchmark shows measurable improvement compared to a baseline (comment out the fast path check, run benchmark, restore, run again, compare). From 935924b66ec63a8da6d0b74ab8d7b67adddd102f Mon Sep 17 00:00:00 2001 From: clem Date: Sat, 28 Mar 2026 09:43:09 +0800 Subject: [PATCH 3/8] test: add correctness baseline tests for renderNodes plain text fast path (#145) Co-Authored-By: Claude Sonnet 4.6 --- test/dom/render-plain-text.test.tsx | 187 ++++++++++++++++++++++++++++ 1 file changed, 187 insertions(+) create mode 100644 test/dom/render-plain-text.test.tsx diff --git a/test/dom/render-plain-text.test.tsx b/test/dom/render-plain-text.test.tsx new file mode 100644 index 0000000..04b7db2 --- /dev/null +++ b/test/dom/render-plain-text.test.tsx @@ -0,0 +1,187 @@ +// @vitest-environment happy-dom +import { describe, it, expect, beforeEach } from 'vitest'; +import { render } from 'preact'; +import { tokenize } from '../../src/markup/tokenizer'; +import { buildAST } from '../../src/markup/ast'; +import { renderNodes } from '../../src/markup/render'; +import { useStoryStore } from '../../src/store'; +import type { StoryData, Passage } from '../../src/parser'; + +function makePassage(pid: number, name: string, content: string): Passage { + return { pid, name, tags: [], metadata: {}, content }; +} + +function makeStoryData(passages: Passage[], startNode = 1): StoryData { + const byName = new Map(passages.map((p) => [p.name, p])); + const byId = new Map(passages.map((p) => [p.pid, p])); + return { + name: 'Test', + startNode, + ifid: 'test', + format: 'spindle', + formatVersion: '0.1.0', + passages: byName, + passagesById: byId, + userCSS: '', + userScript: '', + }; +} + +function renderMarkup( + markup: string, + options?: { nobr?: boolean }, +): HTMLElement { + const tokens = tokenize(markup); + const ast = buildAST(tokens); + const container = document.createElement('div'); + render(<>{renderNodes(ast, options)}, container); + return container; +} + +describe('plain text fast path (issue #145)', () => { + beforeEach(() => { + const store = useStoryStore.getState(); + store.init(makeStoryData([makePassage(1, 'Start', 'Start')])); + }); + + describe('correctly renders plain text without markdown pipeline', () => { + it('plain text in nobr mode', () => { + const el = renderMarkup('Hello World', { nobr: true }); + expect(el.textContent).toBe('Hello World'); + }); + + it('plain text with variable placeholder', () => { + useStoryStore.getState().setVariable('name', 'ALMA'); + const el = renderMarkup('Name: {$name}', { nobr: true }); + expect(el.textContent).toBe('Name: ALMA'); + }); + + it('Unicode symbols (not markdown syntax)', () => { + const el = renderMarkup('▸ Menu ✕ Close', { nobr: true }); + expect(el.textContent).toBe('▸ Menu ✕ Close'); + }); + + it('emoji content', () => { + const el = renderMarkup('🔒 Locked', { nobr: true }); + expect(el.textContent).toBe('🔒 Locked'); + }); + + it('text with colons, commas, slashes (non-markdown punctuation)', () => { + const el = renderMarkup('Requires: Level 2, Policy / Tier 3', { + nobr: true, + }); + expect(el.textContent).toBe('Requires: Level 2, Policy / Tier 3'); + }); + + it('text with parentheses and equals', () => { + const el = renderMarkup('Speed (km/h) = 100', { nobr: true }); + expect(el.textContent).toBe('Speed (km/h) = 100'); + }); + + it('bare exclamation mark is not markdown', () => { + const el = renderMarkup('Activate!', { nobr: true }); + expect(el.textContent).toBe('Activate!'); + }); + + it('multiple variables interleaved with text', () => { + useStoryStore.getState().setVariable('a', 'Alpha'); + useStoryStore.getState().setVariable('b', 'Beta'); + const el = renderMarkup('{$a} and {$b}', { nobr: true }); + expect(el.textContent).toBe('Alpha and Beta'); + }); + + it('HTML element children with plain text', () => { + const el = renderMarkup( + '
Hello
', + { nobr: true }, + ); + expect(el.querySelector('.label')!.textContent).toBe('Hello'); + }); + }); + + describe('non-nobr wraps in

', () => { + it('plain text without nobr gets

wrapper', () => { + const el = renderMarkup('Just some text'); + const p = el.querySelector('p'); + expect(p).not.toBeNull(); + expect(p!.textContent).toBe('Just some text'); + }); + + it('plain text with variable without nobr', () => { + useStoryStore.getState().setVariable('name', 'ALMA'); + const el = renderMarkup('Hello {$name}'); + const p = el.querySelector('p'); + expect(p).not.toBeNull(); + expect(p!.textContent).toBe('Hello ALMA'); + }); + }); + + describe('falls through to markdown for syntax-bearing text', () => { + it('emphasis with asterisks', () => { + const el = renderMarkup('This is *important* text'); + expect(el.querySelector('em')!.textContent).toBe('important'); + }); + + it('emphasis with underscores', () => { + const el = renderMarkup('This is _emphasized_ text'); + expect(el.querySelector('em')!.textContent).toBe('emphasized'); + }); + + it('inline code', () => { + const el = renderMarkup('Use `console.log` here'); + expect(el.querySelector('code')!.textContent).toBe('console.log'); + }); + + it('heading', () => { + const el = renderMarkup('# Title'); + expect(el.querySelector('h1')!.textContent).toBe('Title'); + }); + + it('link syntax', () => { + const el = renderMarkup('See [docs](http://example.com)'); + const link = el.querySelector('a'); + expect(link).not.toBeNull(); + expect(link!.textContent).toBe('docs'); + }); + + it('GFM strikethrough', () => { + const el = renderMarkup('This is ~~deleted~~ text'); + expect(el.querySelector('del')!.textContent).toBe('deleted'); + }); + + it('backslash escape', () => { + const el = renderMarkup('Price\\: free'); + expect(el.textContent).toContain('Price'); + }); + + it('unordered list item', () => { + const el = renderMarkup('- Item 1\n- Item 2'); + expect(el.querySelector('ul')).not.toBeNull(); + }); + + it('ordered list item', () => { + const el = renderMarkup('1. First\n2. Second'); + expect(el.querySelector('ol')).not.toBeNull(); + }); + + it('image syntax', () => { + const el = renderMarkup('![alt](http://example.com/img.png)'); + expect(el.querySelector('img')).not.toBeNull(); + }); + + it('blockquote', () => { + const el = renderMarkup('> quoted text'); + expect(el.querySelector('blockquote')).not.toBeNull(); + }); + + it('GFM table', () => { + const el = renderMarkup('| A | B |\n| --- | --- |\n| 1 | 2 |'); + expect(el.querySelector('table')).not.toBeNull(); + }); + + it('blank lines create paragraphs', () => { + const el = renderMarkup('Para 1\n\nPara 2'); + expect(el.querySelectorAll('p').length).toBeGreaterThanOrEqual(2); + }); + }); +}); From 57d59741aab9bdb50da3a7a83d03c1b6a9e381ba Mon Sep 17 00:00:00 2001 From: clem Date: Sat, 28 Mar 2026 09:56:39 +0800 Subject: [PATCH 4/8] fix: disable block-level markdown inside inline HTML elements Inline elements like , , should not produce block-level markdown constructs (lists, headings, blockquotes). Added INLINE_ELEMENTS set and inline option to markdownToHtml to disable these constructs. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/markup/markdown.ts | 21 ++++++++++- src/markup/render.tsx | 54 ++++++++++++++++++++++++--- test/unit/html-nesting.test.tsx | 65 +++++++++++++++++++++++++++++++++ 3 files changed, 133 insertions(+), 7 deletions(-) diff --git a/src/markup/markdown.ts b/src/markup/markdown.ts index ec3690e..862693f 100644 --- a/src/markup/markdown.ts +++ b/src/markup/markdown.ts @@ -8,14 +8,31 @@ import { /** * Parse a text string as CommonMark markdown and return an HTML string. * Includes GFM table and strikethrough extensions. + * + * When `inline` is true, block-level constructs (lists, headings, blockquotes, + * thematic breaks) are disabled. This is used when rendering content inside + * inline HTML elements like `` where block-level output is invalid. */ -export function markdownToHtml(text: string): string { +export function markdownToHtml( + text: string, + options?: { inline?: boolean }, +): string { + const disabled: string[] = ['codeIndented']; + if (options?.inline) { + disabled.push( + 'list', + 'headingAtx', + 'setextUnderline', + 'thematicBreak', + 'blockQuote', + ); + } return micromark(text, { allowDangerousHtml: true, extensions: [ gfmTable(), gfmStrikethrough(), - { disable: { null: ['codeIndented'] } }, + { disable: { null: disabled } }, ], htmlExtensions: [gfmTableHtml(), gfmStrikethroughHtml()], }); diff --git a/src/markup/render.tsx b/src/markup/render.tsx index 6078490..cf213eb 100644 --- a/src/markup/render.tsx +++ b/src/markup/render.tsx @@ -87,6 +87,43 @@ function convertDomNode( return null; } +/** Inline elements where block-level markdown (lists, headings) is invalid. */ +const INLINE_ELEMENTS = new Set([ + 'a', + 'abbr', + 'b', + 'bdi', + 'bdo', + 'br', + 'cite', + 'code', + 'data', + 'dfn', + 'em', + 'i', + 'kbd', + 'label', + 'mark', + 'meter', + 'output', + 'progress', + 'q', + 'rp', + 'rt', + 'ruby', + 's', + 'samp', + 'small', + 'span', + 'strong', + 'sub', + 'sup', + 'time', + 'u', + 'var', + 'wbr', +]); + function HtmlNodeRenderer({ node }: { node: HtmlNode }) { const resolve = useInterpolate(); const nobr = useContext(NobrContext); @@ -97,13 +134,16 @@ function HtmlNodeRenderer({ node }: { node: HtmlNode }) { attrs[k] = resolve(v) ?? v; } const isSvgRoot = node.tag.toLowerCase() === 'svg'; - // Inside SVG, skip markdown processing — markdown wraps content in

tags - // which break the SVG namespace and produce zero-dimension elements. + const isInline = INLINE_ELEMENTS.has(node.tag.toLowerCase()); + // Inside SVG, skip markdown processing entirely — markdown wraps content + // in

tags which break the SVG namespace. + // Inside inline elements, disable block-level markdown (lists, headings, + // blockquotes) since those produce invalid HTML inside inline containers. const children = node.children.length > 0 ? inSvg || isSvgRoot ? renderInlineNodes(node.children) - : renderNodes(node.children, { nobr, locals }) + : renderNodes(node.children, { nobr, locals, inline: isInline }) : undefined; const element = h(node.tag, attrs, children); return isSvgRoot ? ( @@ -267,7 +307,11 @@ function getVariableTextValue( */ export function renderNodes( nodes: ASTNode[], - options?: { nobr?: boolean; locals?: Record }, + options?: { + nobr?: boolean; + locals?: Record; + inline?: boolean; + }, ): preact.ComponentChildren { if (nodes.length === 0) return null; @@ -304,7 +348,7 @@ export function renderNodes( } // Run combined text through markdown - const html = markdownToHtml(combined); + const html = markdownToHtml(combined, { inline: options?.inline }); // Convert HTML to Preact VNodes, replacing placeholders with components return htmlToPreact(html, components, options?.nobr); diff --git a/test/unit/html-nesting.test.tsx b/test/unit/html-nesting.test.tsx index 6eaa11d..c754875 100644 --- a/test/unit/html-nesting.test.tsx +++ b/test/unit/html-nesting.test.tsx @@ -307,3 +307,68 @@ describe('issue #61: deeply nested HTML with {include}', () => { }); }); }); + +describe('inline HTML elements should not produce block-level markdown', () => { + beforeEach(() => { + const storyData = makeStoryData([makePassage(1, 'Start', 'Start')]); + useStoryStore.getState().init(storyData); + }); + + it('does not interpret "+" inside as a list marker', () => { + // Simulates what RelDimBar produces after spindle-lsp formatting: + // \n +\n 0.95\n + const markup = '\n+\n0.95\n'; + const container = document.createElement('div'); + const ast = buildAST(tokenize(markup)); + render(<>{renderNodes(ast)}, container); + + // Must NOT contain a