Pre-push review: T-1669 — inline re-parse drops thematic breaks

diff --git a/prism/Services/WebRendering/InlineHTMLRenderer.swift b/prism/Services/WebRendering/InlineHTMLRenderer.swiftindex 9896536..9df0aa4 100644--- a/prism/Services/WebRendering/InlineHTMLRenderer.swift+++ b/prism/Services/WebRendering/InlineHTMLRenderer.swift@@ -735,6 +735,64 @@ nonisolated struct InlineHTMLRenderer {             unit == 0x20 || unit == 0x09 || unit == 0x0A || unit == 0x0D         } +        // MARK: Block-level thematic breaks produced by the inline re-parse (T-1669)++        /// A block-level thematic break reaching this walker means the block's inline+        /// source text itself re-parses as one: a table cell, heading, list item, or+        /// paragraph whose text is exactly `---`, `***`, or `___` yields a+        /// `ThematicBreak` — block structure, not a `Text` node — so the default descend+        /// (a leaf with no children) used to drop the content entirely. Same class as+        /// T-1640 (list markers reaching this walker) and T-1641 (`@`-directives): any+        /// renderInline call site can hit it. A `ThematicBreak` carries no text of its+        /// own (unlike a list, it has no children to descend into either), so render the+        /// literal source line verbatim as a mapped run rather than reconstructing one+        /// from the node's (nonexistent) properties.+        mutating func visitThematicBreak(_ thematicBreak: ThematicBreak) {+            // The cursor normally arrives at the start of the break's own line: every+            // call site that can produce this node passes a single-block string, so no+            // sibling left the cursor on a preceding line. Where a future text+            // extraction breaks that, `literalThematicBreak` skips the whole gap —+            // whitespace of any kind, since a blank line is two terminators and may+            // hold spaces or tabs besides — so the break degrades to a stray space+            // rather than the silent drop that IS the T-1669 defect. It cannot verify+            // that the line it lands on is the break, so a cursor stranded mid-line+            // renders that line instead; that is the same class of soft failure.+            if cursor < sourceUTF16.count, isWhitespaceUnit(sourceUTF16[cursor]) {+                // One space for the whole gap, matching soft-break handling: the space+                // is layout, appended outside any run so it never enters the source map.+                closeRun()+                html += " "+            }+            // No cursor step of its own: `appendVisible` locates the literal forward+            // from the cursor, and the gap it skipped is whitespace the literal cannot+            // start with, so the first match is the break's own line.+            if let literal = literalThematicBreak(from: cursor) {+                appendVisible(literal)+            }+        }++        /// The literal thematic-break line appearing next in the source at or after+        /// `from`: leading whitespace skipped (as `literalListMarker` does), then+        /// everything up to the next newline, or the end of source. A thematic break+        /// always occupies a whole line by CommonMark's grammar (rule/spacing+        /// characters only), so capturing the raw line is exact — no need to re-derive+        /// the marker character, its repeat count, or interior spacing, unlike+        /// `literalListMarker`, which only needs a list item's leading marker before+        /// descending into the rest of the item.+        private func literalThematicBreak(from: Int) -> String? {+            var index = from+            while index < sourceUTF16.count, isWhitespaceUnit(sourceUTF16[index]) {+                index += 1+            }+            let lineStart = index+            while index < sourceUTF16.count,+                  sourceUTF16[index] != 0x0A, sourceUTF16[index] != 0x0D {+                index += 1+            }+            guard index > lineStart else { return nil }+            return String(decoding: sourceUTF16[lineStart..<index], as: UTF16.self)+        }+         // MARK: Inline wrappers — emphasis, strong, links          mutating func visitEmphasis(_ emphasis: Emphasis) {

diff --git a/prismTests/WebRendering/ThematicBreakInlineTests.swift b/prismTests/WebRendering/ThematicBreakInlineTests.swiftnew file mode 100644index 0000000..4fb8d7a--- /dev/null+++ b/prismTests/WebRendering/ThematicBreakInlineTests.swift@@ -0,0 +1,199 @@+//+//  ThematicBreakInlineTests.swift+//  prismTests+//+//  Regression tests for T-1669: inline content that re-parses as a thematic+//  break renders empty.+//+//  `InlineHTMLRenderer.render` re-parses each block's inline source string+//  with `Document(parsing:)`. A source string that is exactly `---`, `***`,+//  or `___` (optionally with interior spacing) re-parses as a `ThematicBreak`+//  — block structure, not a `Text` node — and the Walker had no visitor for+//  it, so the default descend (a leaf with no children) dropped the content+//  entirely. Same defect class as T-1640 (list markers) and T-1641+//  (`@`-prefixed directives): any renderInline call site can hit it — list+//  items, headings, paragraphs, and table cells.+//+//  Expected: any inline content that re-parses as a thematic break renders+//  the literal source line as visible text, with identity source-run+//  mapping, exactly as authored (`---` stays `---`, not canonicalised).+//++import Foundation+import Testing+@testable import prism++struct ThematicBreakInlineTests {++    private func normalisedText(_ blocks: [MarkdownBlock]) -> String {+        let doc = BlockHTMLEmitterTestSupport.emit(blocks)+        return BlockHTMLEmitterTestSupport.normalisedText(doc.html)+    }++    // MARK: End-to-end (parse + emit) — the exact shape from the ticket++    @Test("Markdown table with a break-shaped body cell renders its text (T-1669)")+    func parsedTableCellExactlyAsterisksRendersText() {+        // Driven from markdown source rather than a hand-built block, so a+        // parser-side surprise in the cell text surfaces here rather than passing+        // unnoticed — and one does: swift-markdown parses with smart punctuation+        // on, so a `---` cell reaches the block model already folded to an em dash+        // and never re-parses as a break. `***` and `___` are not smart-punctuated,+        // so they arrive verbatim and are the spellings that reach the defect from+        // real markdown.+        let source = """+        | Value | Meaning |+        | --- | --- |+        | *** | asterisk spelling |+        | ___ | underscore spelling |+        """+        let blocks = MarkdownBlockParser.parse(source)+        let doc = BlockHTMLEmitterTestSupport.emit(blocks)+        let text = BlockHTMLEmitterTestSupport.normalisedText(doc.html)+        // Expected: the literal `***` renders as text.+        // Actual (bug): an empty cell — the whole node is dropped.+        #expect(text.contains("***"), "rendered text: \(text)")+        #expect(text.contains("___"), "rendered text: \(text)")+        #expect(text.contains("asterisk spelling"), "rendered text: \(text)")+        #expect(text.contains("underscore spelling"), "rendered text: \(text)")+        // A rule found this way is TEXT, never block structure: the cell must not+        // emit an `<hr>`. (A real document-level `---` still does — pinned by+        // WebParityFixtureTests and BlockHTMLEmitterTests.thematicBreak.)+        #expect(!doc.html.contains("<hr"), "emitted html: \(doc.html)")+    }++    @Test("Table row whose only cell is `---` keeps the literal text (T-1669)")+    func tableCellExactlyDashesRendersText() {+        let blocks: [MarkdownBlock] = [+            .table(headers: ["Value"], rows: [["---"]], alignments: [.leading])+        ]+        let text = normalisedText(blocks)+        // Expected: the literal `---` renders as text.+        // Actual (bug): an empty cell — the whole node is dropped.+        #expect(text.contains("---"), "rendered text: \(text)")+    }++    // MARK: Emitter-level content that is exactly a thematic break++    @Test("Paragraph that is exactly `---` keeps its text")+    func paragraphExactlyDashesRendersText() {+        let text = normalisedText([.paragraph(markdown: "---")])+        #expect(text.contains("---"), "rendered text: \(text)")+    }++    @Test("Paragraph that is exactly `***` keeps its text")+    func paragraphExactlyAsterisksRendersText() {+        let text = normalisedText([.paragraph(markdown: "***")])+        #expect(text.contains("***"), "rendered text: \(text)")+    }++    @Test("Paragraph that is exactly `___` keeps its text")+    func paragraphExactlyUnderscoresRendersText() {+        let text = normalisedText([.paragraph(markdown: "___")])+        #expect(text.contains("___"), "rendered text: \(text)")+    }++    @Test("Spaced form `- - -` keeps its text exactly as authored")+    func paragraphSpacedDashesRendersText() {+        // CommonMark allows interior spacing in a thematic break line;+        // the whole-line capture must preserve it verbatim, not canonicalise.+        let text = normalisedText([.paragraph(markdown: "- - -")])+        #expect(text.contains("- - -"), "rendered text: \(text)")+    }++    @Test("List item content that is exactly `---` keeps the text")+    func listItemExactlyDashesRendersText() {+        let items = [ListItem(content: "---", checkbox: nil)]+        let text = normalisedText([.list(ordered: false, start: 1, items: items)])+        #expect(text.contains("---"), "rendered text: \(text)")+    }++    @Test("Heading text that is exactly `---` keeps the text")+    func headingExactlyDashesRendersText() {+        let text = normalisedText([.heading(level: 2, text: "---")])+        #expect(text.contains("---"), "rendered text: \(text)")+    }++    @Test("Table cell text that is exactly `***` keeps the text")+    func tableCellExactlyAsterisksRendersText() {+        let text = normalisedText([.table(headers: ["Value"],+                                          rows: [["***"]],+                                          alignments: [.leading])])+        #expect(text.contains("***"), "rendered text: \(text)")+    }++    // MARK: The defended broken-invariant path — a gap before the break line++    @Test("A blank-line gap before the break still renders the literal line (PR #365)")+    func brokenInvariantBlankLineGapKeepsBreakText() {+        // The visitor's invariant is that the cursor arrives at the start of the+        // break's line. Its defensive branch must skip the WHOLE gap: a blank line is+        // two newlines, and skipping only one leaves the capture starting on a+        // terminator, where it scans nothing and returns nil — dropping the content+        // exactly as the T-1669 defect did, just with a stray space. One space stands+        // in for the gap; the `---` survives.+        let text = normalisedText([.paragraph(markdown: "x\n\n---")])+        #expect(text.contains("---"), "rendered text: \(text)")+        #expect(text.contains("x"), "rendered text: \(text)")+    }++    @Test("A CRLF blank-line gap before the break still renders the literal line")+    func brokenInvariantCRLFGapKeepsBreakText() {+        // Same shape with CRLF terminators: all four units are gap, so the capture+        // must land on the break line rather than mid-pair.+        let text = normalisedText([.paragraph(markdown: "x\r\n\r\n---")])+        #expect(text.contains("---"), "rendered text: \(text)")+    }++    @Test("A whitespace-bearing gap before the break still renders the literal line")+    func brokenInvariantWhitespaceGapKeepsBreakText() {+        // A blank line is blank when it holds only spaces or tabs, so the gap is a run+        // of WHITESPACE, not of terminators. Skipping terminators alone stops on the+        // space, captures it as the "break line", and drops the `---` — the T-1669+        // symptom one blank-line spelling further out (pre-push review).+        let spaced = normalisedText([.paragraph(markdown: "x\n \n---")])+        #expect(spaced.contains("---"), "rendered text: \(spaced)")+        let tabbed = normalisedText([.paragraph(markdown: "x\n\t\n---")])+        #expect(tabbed.contains("---"), "rendered text: \(tabbed)")+    }++    @Test("A gap before the break leaves the source map's runs ordered and in bounds")+    func brokenInvariantGapKeepsSourceMapConsistent() {+        // The space emitted for the gap is layout, not mapped source: it must not+        // become a run, and the runs that do exist must stay ordered, non-overlapping+        // and inside the source, or selection-anchored notes would resolve wrongly.+        let source = "x\n\n---"+        let doc = BlockHTMLEmitter.emit(blocks: [.paragraph(markdown: source)],+                                        footnotes: .empty, settings: RenderSettings())+        let runs = (doc.sourceMap.runs.values.first ?? []).sorted { $0.sourceStart < $1.sourceStart }+        #expect(!runs.isEmpty, "runs: \(runs)")+        var previousEnd = 0+        for run in runs {+            #expect(run.sourceStart >= previousEnd, "runs must be ordered and non-overlapping")+            #expect(run.sourceStart + run.length <= source.utf16.count,+                    "runs must stay inside the source: \(runs)")+            previousEnd = run.sourceStart + run.length+        }+    }++    // MARK: Source-map identity for the thematic-break run++    @Test("Thematic-break run maps identity: total run length equals the content length")+    func thematicBreakRunMapsIdentity() {+        let source = "---"+        let doc = BlockHTMLEmitter.emit(blocks: [.paragraph(markdown: source)],+                                        footnotes: .empty, settings: RenderSettings())+        let runs = (doc.sourceMap.runs.values.first ?? []).sorted { $0.sourceStart < $1.sourceStart }+        // A dropped node yields total 0 ≠ the content length: the runs must+        // tile the whole source string (ordered, non-overlapping, starting+        // at 0) so selection-anchored notes still resolve.+        let total = runs.reduce(0) { $0 + $1.length }+        #expect(total == source.utf16.count, "runs: \(runs)")+        #expect(runs.first?.sourceStart == 0, "runs: \(runs)")+        var previousEnd = 0+        for run in runs {+            #expect(run.sourceStart >= previousEnd, "runs must be ordered and non-overlapping")+            previousEnd = run.sourceStart + run.length+        }+    }+}

diff --git a/docs/agent-notes/webview-rendering-status.md b/docs/agent-notes/webview-rendering-status.mdindex fd8b2a8..a60518b 100644--- a/docs/agent-notes/webview-rendering-status.md+++ b/docs/agent-notes/webview-rendering-status.md@@ -240,6 +240,7 @@ take down unrelated applications' web content. Always PID-verify, never pattern- - **CSS `:has()` does not reliably re-evaluate on a `td` when a child is inserted live** (e.g. a note dot added during the session) — for "hide X when sibling Y appears" driven by live DOM mutation, suppress explicitly in JS, not via `:has()`. - **`InlineHTMLRenderer` run offsets are always relative to the string it is handed** (its source cursor restarts at 0 on every call), and the runs are consumed against `MarkdownBlock.textContent`, which JOINS the sub-spans (cells with `" | "`, rows/items with `"\n"`). A sub-span caller that appended its runs unrebased mis-anchored every selection notes made past the first cell/item — invisible in the emitted HTML, visible only when run offsets are compared against `textContent`. Closed structurally in **T-1941**: `BlockHTMLEmitter.renderInline` takes a **required** `InlineSpan` (`.wholeBlock` / `.subspan(offset:)` / `.unmapped`), is the only place that appends to `context.blockRuns`, and does the rebase itself — so a new caller cannot omit it, only state it wrongly. Offsets come from `MarkdownBlock.tableCellTextOffsets` / `listItemTextOffsets`, which live beside `textContent` and use the same separator constants, so the two sides cannot drift. Anything absent from `textContent` (nested list items, continuation paragraphs, nested blocks, a `<details>` child list, a nested `<details>` summary, the child-less blockquote fallback) is `.unmapped` → selection declined (Decision 8), never mis-anchored; mapping the nested cases would need `textContent` widened to contain them, tracked as **T-2032** — it is a scoped-out limitation, not a defect. `EmittedDocument.badgeSourceStarts` is deliberately NOT rebased: it is keyed by the inline source string and matched against that string's own occurrence scan in `SearchStateFeeder` (T-1853). Regression guard: `WebStructuredSourceMapInvariantTests.parityCorpusRunsMonotonic` sweeps the whole parity fixture corpus asserting runs are monotonic, non-overlapping AND within their block's `textContent` UTF-16 length — the upper bound is what catches a Character-count offset, which stays monotonic and would otherwise pass. - **`InlineHTMLRenderer.Walker.locate` is a naive forward scan, and it is only affordable because failed scans are recorded** (T-1966). The search cannot be BOUNDED in the general case — text legitimately sits far ahead of the cursor whenever the walk skipped source it does not account for (a long image `src`, a long raw-HTML span), so only the pre-badge segment (PR #326) and `claimOccurrences` (T-1992) get bounds. Without a record, a text node whose rendered text does not occur verbatim in the source — the entity/escape family, `A` spelled `&#65;` — scanned to the end of the block, failed, left the cursor where it was, and the next such node re-derived the same scan: `*&#65;* ` x 3200 took 12s. Two exact rejections fix it: `provedAbsent` (a text an unbounded scan proved absent stays absent, capped at 256 entries so the T-2034 class cannot grow it with the block) and a lazily-built `sourceUnits` bitset (a text holding a unit the source does not hold cannot occur in it anywhere; a bitset over the 16-bit domain rather than a `Set`, because the probe runs once per unit of every later text and hashing dominated it). A match at the cursor is tested BEFORE either memo, so the common case — text sitting exactly where the walk expects it — pays for neither. **`provedAbsent` rests on `cursor` never moving backwards**, which is why `locate` takes NO `from:` parameter and reads `cursor` itself: the precondition is structural, not documented. Every mutation of `cursor` is forward, but one of them is only forward *because of the pre-badge bound* — `appendFootnoteBadge` steps to the occurrence's end, and the `appendVisible` before it must stay bounded by `occurrence.sourceStart` or the cursor could overshoot; weakening that bound breaks the memo, not just performance. No rejection changes any output: digests over a 4000-sample generated corpus (html + every run + `badgeSourceStarts`) are byte-identical to `origin/main` at 09cd828, and that corpus is now COMMITTED (`InlineRenderCorpusEquivalenceTests`, seeded SplitMix64, chunk digests pinned, re-blessing protocol in the file header; `PRISM_INLINE_CORPUS_FULL=1` for all 4000). Residual, deliberately open: a distinct-per-node text absent from the source whose every unit is present still costs a scan each — **T-2034**, needs a source index to close. Guards: `InlineSourceMapScanGrowthTests` (G1-G8 growth over the shared `GrowthRatioGuard`, O1-O6 rendered-text-in-order + run invariants, O7 exact pre-fix run coordinates on memo-HIT fixtures, O8 the recording gate — a bounded miss must record nothing or a later unbounded match is silently suppressed).+- **The inline re-parse can hand `InlineHTMLRenderer.Walker` BLOCK nodes, and an unhandled one drops content silently** — three instances so far, all the same shape. `render` re-parses each block's inline source with `Document(parsing:)`, so a source string that satisfies a *block* grammar comes back as block structure rather than a `Text` node, and `MarkupWalker`'s default descend emits nothing for it: content vanishes with no error, no fallback, and no visible trace except an empty cell/item/heading. **T-1640** `1.`/`3)` at the start of an item (→ `OrderedList`), **T-1641** `@Observable` (→ `BlockDirective`; closed by NOT passing `.parseBlockDirectives`, matching `MarkdownBlockParser`), **T-1669** text that is exactly `---`/`***`/`___` (→ `ThematicBreak`). The two structural fixes render the LITERAL source line/marker as a mapped run (`literalListMarker`, `literalThematicBreak`) rather than reconstructing it from the node — a `ThematicBreak` has no text and no children to descend into, so there is nothing to reconstruct from. **The class is not closed**: the Walker still has no `visitHeading`, `visitBlockQuote`, `visitCodeBlock`, `visitHTMLBlock` or `visitTable`, so `# Title` in a table cell drops its `#` (T-1640 shape) and a re-parsed code block or HTML block would drop entirely (T-1669 shape). Before adding a `renderInline` call site, check what its strings can re-parse into; when one of these turns up, add the visitor rather than pre-sanitising the string. The defensive gap-skipping in `visitThematicBreak` is for a broken cursor invariant only — every call site today passes a SINGLE-BLOCK string, so a stranded cursor is unreachable and the branch is deliberately not hardened further (it degrades to a stray space, or to rendering the line it lands on). - **`FootnotePopoverWebPage.reset()` must reload** to actually clear the live page (updating the served-HTML box alone leaves the prior content in the WebContent process). - **Live-WebPage test harness wedges intermittently** (launchservicesd / XPC / "Sandbox restriction"). Stale `prism.app`/`xctest`/`xcodebuild`/`testmanagerd` processes are a cause — `pkill -9` before a run. `livePresentAndReplace` passing while another live test fails means the harness is fine and it's a real assertion. - **`xcodebuild test` hangs** in post-test xcresult finalization (it builds prismUITests). Use `build-for-testing` then `test-without-building` with `NSUnbufferedIO=YES`; the process **exit code is authoritative**. Run targeted classes with `-only-testing:prismTests/<Class>` to avoid the hang.

diff --git a/CHANGELOG.md b/CHANGELOG.mdindex 9938073..69428fc 100644--- a/CHANGELOG.md+++ b/CHANGELOG.md@@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0  ### Fixed +- A block whose text is exactly a thematic break — `---`, `***`, or `___` — no longer renders empty (T-1669). Rendering re-parses a block's text as its own small markdown document, and that string satisfying markdown's rule for a horizontal rule made it parse as one rather than as plain text; nothing then knew how to turn a rule back into visible text, so it vanished. This is the third defect of this shape (after numbered list markers and `@`-prefixed text, T-1640/T-1641): rendering now knows a rule found this way as text too, and shows the characters as written — in a paragraph, a heading, a list item, or a table cell alike. - Arrow keys, Page Up/Down, Space, and the View menu's **Page Down**/**Page Up**/**Scroll to Top**/**Scroll to Bottom** no longer scroll the document behind an open note, footnote, add-note, reply, or document-note modal (T-1099, reopened). This was fixed once before; the WebKit rendering cutover restructured the views that carried the fix and the binding was never rebuilt, so scroll commands kept reaching the document underneath a modal that should have blocked them. The gate is restored on both the iPhone and iPad/Mac layouts, taking effect immediately when a modal is already open and staying live across every presentation and dismissal. - Replying to a document-level note is no longer silently discarded on a document that has only imported notes (T-1865). NotesPanel and SidebarNotesView create replies through a convenience method that used the document's saved user notes as its source of context; on a document where no user note had ever been created — only imported ones — that context was `nil`, so the guard returned early before the reply was ever built, leaving the tap with no visible effect and nothing written to disk. The method now falls back to the document's cached identifier, the same fallback its sibling document-note-creation method already used, so a reply always creates the note container it needs. - The safeguard that stops a broken document from reloading forever now holds when the crashes keep landing mid-load (T-2107). When a document's rendering process stops, the app reloads it, and if the reloads repeatedly fail to bring the document back it gives up after a few attempts and shows a banner offering a manual reload rather than retrying endlessly (T-1943 below). But a reload was counted as having succeeded the moment the page reported in — before it had finished laying out — so a renderer that reliably crashed in that window looked like a fresh failure each time instead of the same one continuing: the count started over on every attempt, and the document reloaded forever, which is exactly the loop the safeguard exists to prevent. A recovery now only counts as successful once the reloaded document has actually settled on screen, so crashes landing in that window accumulate toward the limit and reach the banner. Recoveries that do bring the document back still reset the count, and the banner's reload still restores everything as before.