PR #320 — BlockHTMLEmitter.renderInline rebases footnote-split segment runs into the coordinate space its consumers actually index, so a note taken on text after a footnote badge quotes the text you selected. Reviewed against the merge base e9d2fed; origin/main advanced to 6e2a112 mid-review, so a rebase with one CHANGELOG conflict is required before merge.
InlineHTMLRenderer restarts its source cursor at 0 for every string handed to it. renderInline splits a block's source around resolvable [^id] tokens and rendered each segment separately, then appended the returned runs verbatim — so in before[^1] after the run for after claimed sourceStart 1 instead of 11.[0,6] and [1,5] — overlapping. specs/webview-rendering/design.md:175 already asserts runs are "non-overlapping and ordered in source space", so the code was wrong and the spec was right. No spec change needed.lastEnd, which advances past each consumed token. Unresolvable references continue without advancing lastEnd, so their literal text stays inside the following segment — now pinned by a test.renderListMarkup (item content), tableHTML (header/cell) and emitDetails' recursive nested summary — all of which hand renderInline a substring with recordRuns: true and record the result into the enclosing block's runs. Pre-existing, correctly left out of scope, tracked as T-1941. The PR's original comment claimed the runs were rebased into "the FULL block source", which is not true on any of them; that comment is corrected here.String.Index.utf16Offset(in:) is not an O(n) scan from the string start — the stdlib's UTF-16 breadcrumbs make it a bounded ≤64-unit lookup (0.92 µs on a 1M-unit string, flat in length). The new calls add 1–4% to a loop the regex scan already dominates by ~30×. renderInline stays linear.Fixtures/Parity/footnotes.md contained only trailing references, so no fixture could produce text after a badge; and no parity suite read sourceMap, so adding the shape alone would have asserted nothing. The interior-reference paragraph and the generic ordered/non-overlapping invariant are now both wired, and verified red pre-fix.InlineHTMLRenderer.Walker has no visitCodeBlock, so the text is emitted as nothing. word[^1] more silently drops more from the rendered document. Not introduced here and distinct from T-1941; needs its own ticket.InlineHTMLRenderer.render already takes a footnotes: parameter whose doc comment claims it renders resolvable refs as badge chrome — but the Walker never reads it (dead stored property) and the emitter passes .empty. Moving badge substitution into the Walker gives one source cursor, retiring the split entirely: it would fix T-1941, the indentation content loss, and emphasis-spanning-a-badge together.Ready to push
The fix is correct, minimal, and restores a contract the spec already documented rather than inventing a new one. I verified it empirically: reverting only BlockHTMLEmitter.swift to the base turns every one of the eight footnote source-map tests red, and restoring it turns them green — these are genuine regression tests, not tests written around the implementation.
No blocker or major issue survived review. Four review agents (reuse, quality, efficiency, spec/docs/tests) raised one major and thirteen minor/nit items; the substantive ones are fixed in the working tree, the rest are out of scope (tracked as T-1941) or follow existing file conventions. make lint is clean, both platform builds succeed, and 20+ neighbouring test classes pass.
Two things to handle at push time, neither a defect in the change: origin/main moved to 6e2a112 during the review and the branch needs a rebase (one trivial adjacent-line CHANGELOG conflict), and the review turned up a separate pre-existing content-loss bug in the same function that deserves its own ticket.
1eceb36 Fix T-1876: Rebase footnote-split source-map runs into block coordinates working-tree Fixes applied in this review When you select some words in a document and add a note, Prism has to remember which words you picked. It does that by storing a start position and a length — like saying "characters 11 through 16".
Those positions are worked out by the code that turns markdown into the HTML you see on screen. The problem was in how it handles footnotes. A paragraph like:
before[^1] aftergets chopped into pieces around the [^1] marker, so the marker can be replaced with a little numbered badge. Each piece is then converted to HTML separately — and here is the catch: the converter always counts from zero for whatever piece it is given. So the piece containing after reported "I start at position 1" (its position inside its own piece) rather than "I start at position 11" (its position in the whole paragraph).
The result: selecting after and adding a note quoted efore instead. The fix adds four lines that shift each piece's positions by where that piece actually begins in the full paragraph.
A note that quotes the wrong words is worse than no note — you would only find out later, when re-reading. And the wrong position was saved, so it also travelled into two other features: note relocation (finding your note again after the document is edited) and inline-note export (writing your notes back into a markdown file). Anywhere a footnote appeared mid-paragraph, all three were subtly wrong.
Prism's WebKit rendering path keeps native code as the source of truth: BlockHTMLEmitter emits HTML from parsed MarkdownBlocks, and InlineHTMLRenderer wraps mappable text runs in <span data-prism-run> while recording a DocumentSourceMap. The map ships to the page as an inert <div hidden> data island. When you select text, prism-notes.js walks up to the enclosing run element and computes run.sourceStart + offsetWithinRun, then posts that back over the bridge for native code to turn into a NoteTextRange.
That whole chain rests on one assumption: every Run.sourceStart is an offset into the same string. renderInline broke it. To substitute footnote badges it splits the inline source on resolvable [^id] tokens and calls InlineHTMLRenderer.render once per text segment — and that renderer builds sourceUTF16 = Array(source.utf16) and starts its cursor at 0 for whatever string it receives. Segment runs were appended unshifted, so every run after the first badge was in segment-local coordinates.
The fix is a pure value transform applied at the boundary where the coordinate space changes:
private static func rebased(_ runs: [DocumentSourceMap.Run], by offset: Int) -> [DocumentSourceMap.Run]Both call sites capture lastEnd.utf16Offset(in: source) before slicing the segment. Because lastEnd is advanced to match.range.upperBound after each consumed badge, the offset automatically accounts for the skipped [^id] token text and composes across any number of badges — no running counter to keep in sync.
The guard offset != 0 else { return runs } fast path is not just a micro-optimisation: it makes the leading-segment case provably identical to the pre-fix behaviour, which is what keeps the change's blast radius to "segments after a badge".
InlineHTMLRenderer. Adding a baseOffset parameter would fix every caller at once, but it pushes coordinate-space bookkeeping into a renderer that has no business knowing about it, and renderInline is its only caller in the target. Rebasing at the boundary keeps the renderer honest about its own contract: "offsets are relative to what you gave me."recordRuns: false would have honoured that with a one-word change — but it also declines selections that anchor correctly today. For renderInline the rebase is strictly better. For renderListMarkup/tableHTML (same defect, item 1+ and cell 1+) the same argument applies, which is why they are deferred to T-1941 rather than declined; that choice is now recorded as Quick Decision Q1.The pre-fix failure mode is sharper than "wrong offset". For before[^1] after the emitted runs were {start: 0, length: 6} and {start: 1, length: 5} — overlapping, which directly violates the invariant at specs/webview-rendering/design.md:175. That matters because resolveSelectionRange in prism-notes.js takes the covering range across endpoints:
var lo = Math.min(startEndpoint.sourceOffset, endEndpoint.sourceOffset);
var hi = Math.max(startEndpoint.sourceOffset, endEndpoint.sourceOffset);With overlapping runs the min/max collapses onto a span that is not merely shifted but structurally wrong, so a multi-run selection spanning a badge produced a range unrelated to either endpoint. The single-run case degraded more gently (a pure shift), which is why the reported symptom — after quoting efore — reads like a simple off-by-N.
Correctness of the rebase turns on lastEnd being the only cursor. It is advanced solely at lastEnd = match.range.upperBound, inside the guard let definition success path. So:
continues without advancing, leaving its literal [^missing] text inside the segment that terminates at the next resolvable token — and since segmentStart is recomputed from lastEnd each iteration, that segment's offset is still correct. The commit message asserted this; it was untested, and now is (unresolvedThenResolvedFootnoteComposes).lastEnd, so the tail rebase carries the full accumulated offset. Both shapes are now pinned.utf16Offset(in:) is the right unit: the renderer's own cursor is a UTF-16 index into Array(source.utf16), and NoteTextRange conversion is UTF-16 throughout. A Character-based distance would drift on astral scalars, which astralBeforeFootnoteKeepsUTF16Coordinates exists to catch.The change is confined to a nonisolated, pure function on the off-MainActor emit path introduced by T-1681. rebased is a static transform over a trivial POD (Run is three Ints), so it inherits that isolation for free and adds no synchronisation surface — notably it sits outside the Mutex that serialises SwiftSoup's unsynchronised static pools.
Block identity and DOM ids are untouched: run IDs still come from the shared context.nextRunID allocator and the emitted HTML is byte-identical, so BlockIdentityStabilityTests and the parity corpus are unaffected — only the data island's numbers change. Existing notes anchored in footnote-bearing blocks were stored with the wrong range and are not retroactively repaired; nothing in the design promises that, but it is worth knowing.
The comment now names the real precondition: the rebase restores block coordinates only when renderInline was handed the block's own source. renderListMarkup passes item.content and tableHTML passes header/cell substrings, both with recordRuns: true, so those runs remain item-/cell-local. The natural acceptance criterion for T-1941 is pointing expectRunsOrderedNonOverlapping (currently private in WebStructuredSelectionTests.swift:56) at the full samples/ corpus — it will fail today on exactly those blocks.
*em [^1] tail*): each half is re-parsed independently, so the asterisks become literal text inside runs. Not a regression from this PR — it is inherent to the split-then-render strategy — but it is the one case where splitting changes what the runs mean. Uncovered; worth pinning if the splitting strategy is ever revisited.MarkdownBlock.textContent for .blockquote joins children with \n\n, so paragraph 0's runs index a prefix of textContent. The rebase is correct there, but it composes with a second, separately-documented fact. Uncovered.utf16Offset(in:) uses the stdlib's UTF-16 breadcrumb index (one entry per 64 units), so it is a bounded lookup, not a scan — 0.92 µs on a 1M-unit non-ASCII string, flat in length. Across m = 10…5000 references per block the new calls add 1–4% to a loop the matches(of:) regex scan dominates by ~30×. No quadratic behaviour.recordRuns: false paths need nothing: no runs are recorded, so there is nothing to rebase, and WebStructuredSelectionTests already covers that such selections are declined.BlockHTMLEmitter.swift
Why it matters. This is the entire fix. Every consumer of DocumentSourceMap (selection notes, note relocation, inline-note export) does run.sourceStart + offsetWithinRun against the block's source; segments after a badge were reporting segment-local offsets, so the resulting ranges pointed at the wrong text and — worse — overlapped, breaking the design's own ordering invariant.
What to look at. prism/Services/WebRendering/BlockHTMLEmitter.swift:702-766 (renderInline + the new private rebased(_:by:))
BlockHTMLEmitter.swift
Why it matters. The comment shipped in the commit promised more than the code delivers. renderListMarkup passes item.content, tableHTML passes header/cell substrings, and emitDetails recurses into renderInline(model.summary) for NESTED <details> — all with recordRuns: true, all landing in the enclosing block's runs with sub-span-local offsets. A future reader would have taken the comment as a guarantee and stopped looking. The nested-<details> path was missed by my own first correction and caught on the second pass.
What to look at. prism/Services/WebRendering/BlockHTMLEmitter.swift:719-729
DocumentSourceMapTests.swift
Why it matters. The PR's unresolvedFootnoteReferenceKeepsBlockCoordinates used "text[^missing] tail" with no resolvable reference, so it never entered the multi-segment path at all — it asserted the plain-render case under a name that promised otherwise. Badge-at-offset-0 and adjacent-badges are the two shapes where a segment is empty, i.e. where the lastEnd bookkeeping is load-bearing; adjacent badges is the exact shape at samples/footnotes.md:33.
footnotes.md
Why it matters. Fixtures/Parity/footnotes.md contained only trailing references, so no fixture in the corpus ever produced text after a badge — which is why this reached a release. Adding the shape alone buys nothing, though: no parity suite reads sourceMap. The generic ordered/non-overlapping invariant already existed and would have caught the bug (pre-fix runs were [0,6] then [1,5] — non-monotonic); it was simply never pointed at a footnote fixture.
What to look at. prismTests/WebRendering/Fixtures/Parity/footnotes.md:5-6 and prismTests/WebRendering/WebStructuredSelectionTests.swift:140-150 (parityFootnoteFixture)
decision_log.md
Why it matters. The PR knowingly leaves an identical defect in two sibling functions. That is the right call for a bugfix, but before this review the relationship existed nowhere in the repo — not in the code, the decision log, or the agent notes — so the next person to hit a mis-anchored note in a table cell would have re-derived the whole investigation.
What to look at. specs/webview-rendering/decision_log.md (Quick Decision Q1), docs/agent-notes/webview-rendering-status.md (Gotchas), BlockHTMLEmitter.swift:725-727
InlineHTMLRenderer.render takes no start/base offset — its Walker starts cursor at 0 and hard-codes sourceUTF16 = Array(source.utf16) over the string it is handed. Adding a parameter would fix every caller at once, including the two T-1941 ones. It was not done: renderInline is the renderer's only caller in the target, and the renderer's current contract ("offsets are relative to what you gave me") is clean and worth keeping. The cost is that each slicing caller must remember to rebase — which is exactly the trap this bug was.
segmentStart = lastEnd.utf16Offset(in: source) is captured before each slice, and lastEnd is only ever advanced to match.range.upperBound on the resolvable-reference path. This makes the rebase self-composing across any number of badges and automatically correct for consumed-but-unrendered [^id] source, with no state to keep in sync. Stated in the commit message.
Returns the runs untouched for the leading segment. Beyond avoiding an allocation, this makes the pre-badge case provably byte-identical to the pre-fix behaviour, confining the change's blast radius to segments that follow a badge.
(inferred — not stated by the author.)The alternative was switching those callers to recordRuns: false (safe-by-decline, per Decision 8), which would stop the mis-anchoring immediately. Rejected because it also declines selections in item 0 / cell 0 that anchor correctly today, and the proper rebase for those callers needs each item's/cell's offset in the block source — a separate change. Recorded as Quick Decision Q1 during this review.
The reuse review noted the helper operates purely on Run values and would sit naturally next to the model at prism/Models/DocumentSourceMap.swift:33. Left private: there is one call site, and Models/ currently keeps DocumentSourceMap as a bare Codable value type with no behaviour. Worth revisiting when T-1941 adds the second and third callers.
Roughly 6 of the last 23 bugfix commits on main ship a report.md; nothing in CLAUDE.md or any spec mandates one. The commit message already carries the investigation narrative, and the T-1941 relationship — the one thing a report would uniquely have housed — is now in the decision log and agent notes instead.
expectRunsOrderedNonOverlapping is a generic invariant that any block should satisfy, so the tempting move is to run it across every parity fixture and every samples/ document. That fails today — list-item and table-cell blocks violate it because of T-1941. Scoping it to Parity/footnotes.md gives real regression protection now (verified red pre-fix) without wiring a suite that has to be immediately disabled.
| Severity | Area | Finding | Resolution |
|---|---|---|---|
| major | Fixtures/Parity/footnotes.md — corpus gap | The parity fixture for footnotes contained only trailing references, so no fixture ever produced text after a badge. Compounding it, no parity suite reads sourceMap at all — so simply adding the shape would buy no regression protection. | Added a paragraph with an interior reference followed by trailing prose, AND wired the existing generic ordered/non-overlapping invariant to that fixture as WebStructuredSourceMapInvariantTests.parityFootnoteFixture. Verified red against the pre-fix emitter, green after. Corpus-wide sweep deliberately deferred — list/table fixtures violate the same invariant under T-1941, so that is recorded as T-1941's acceptance criterion. |
| minor | BlockHTMLEmitter.swift:729/743 — pre-existing content loss in the same function | Because each footnote-split segment is block-parsed independently, a segment starting with 4+ spaces or a tab becomes an indented code block, and InlineHTMLRenderer.Walker has no visitCodeBlock — so the text is emitted as nothing. 'word[^1] more' silently drops 'more' from the rendered document. Verified directly against swift-markdown: Document(parsing: " after") yields a CodeBlock. Pre-existing, not introduced by this PR, and distinct from T-1941 (this one loses content rather than mis-anchoring it). | Not fixed here — out of scope for a bugfix PR and it needs its own ticket. Recorded in the agent-notes gotcha alongside T-1941, together with the structural fix that would resolve both: move badge substitution into InlineHTMLRenderer.Walker (whose footnotes stored property already exists at line 72 and is never read, while its doc comment at lines 43-44 claims it renders badge chrome) so there is one source cursor and no splitting. That single change would fix T-1941, this indentation bug, and emphasis-spanning-a-badge together. |
| minor | BlockHTMLEmitter.swift:719-729 — comment overstates the guarantee | The comment said segment runs are rebased 'by the segment's UTF-16 start in the FULL block source'. That holds only when renderInline is handed the block's own source. renderListMarkup (line 413, item.content), tableHTML (lines 524/531, header/cell) and emitDetails' recursive nested-summary path (line 668 reached via line 677) all pass substrings with recordRuns: true, and their runs land in the enclosing block's map. | Reworded to 'the WHOLE string passed to renderInline', naming the safe callers (including 'top-level details summary' specifically) and all three unsafe ones with the T-1941 reference. Decision-log Q1 and the agent-notes gotcha corrected to match — the nested-<details> surface was missing from all three on the first pass. |
| minor | DocumentSourceMapTests.swift — unresolved-reference test did not exercise the split path | unresolvedFootnoteReferenceKeepsBlockCoordinates used "text[^missing] tail" against footnoteData(["1"]) — no resolvable reference, so the emitter never entered the multi-segment loop. It was equivalent to the plain-render case despite its name, and the commit message's explicit claim ('the loop continues without advancing lastEnd, so their text stays inside the following segment') had no coverage. | Kept the original test and added unresolvedThenResolvedFootnoteComposes with "a[^missing] b[^1] c", asserting the first run spans "a[^missing] b" and the tail run starts at "c"'s offset in the full source. Verified red against the pre-fix emitter. |
| minor | DocumentSourceMapTests.swift — empty-segment boundary shapes uncovered | Badge at offset 0 ("[^1] after") and adjacent badges with no text between ("a[^1][^2] b" — the exact shape at samples/footnotes.md:33) were untested. Both are the cases where a segment is empty and skipped, so the tail rebase is the only thing keeping the map correct and a desynchronised lastEnd would go unnoticed. | Added footnoteAtStartKeepsTailInBlockCoordinates and adjacentFootnotesKeepTailInBlockCoordinates. Both verified red against the pre-fix emitter, green after. |
| minor | specs/webview-rendering/decision_log.md — undocumented scope decision | The PR knowingly leaves the identical defect in renderListMarkup and tableHTML rather than switching them to safe-by-decline (recordRuns: false), which is what the design's own Decision 8 would suggest. That is a real fork with a real consequence — list-item and table-cell selections after item 0 / cell 0 are mis-anchored rather than declined — and it was recorded nowhere. | Added Quick Decision Q1 (the log had no Quick Decisions table; one was created under the header per the documented format). |
| minor | docs/agent-notes/webview-rendering-status.md — missing gotcha | Nothing in the repo recorded that InlineHTMLRenderer restarts its run cursor at 0 for every string it is handed, so any caller rendering a sub-span must rebase. This is genuinely non-derivable from the code — the emitted HTML is identical either way, and the defect only surfaces when run offsets are compared against block.textContent. | Added a Gotchas entry stating the renderer contract, the symptom, and the T-1941 pointer for the two still-broken callers. CLAUDE.md deliberately left alone — its Markdown Rendering section describes InlineHTMLRenderer at the right altitude, and a per-caller rule there would duplicate the agent note. |
| nit | CHANGELOG.md:21 — wording | 'keeps its offsets in the paragraph's own coordinates' understates the fix (headings, blockquote first children and details summaries are equally fixed) while implicitly over-promising to a reader whose footnote is in a list item. Separately, 'a wrong source range that also affected note relocation and inline-note export' presents two downstream consequences as if separately verified; neither has a direct test. | Changed to 'the block's own coordinates', reworded the consequence as 'which the note then carried into relocation and inline-note export', and appended a sentence noting list items and table cells are tracked separately. |
| nit | Test helper duplication across two files | The diff adds an identical 9-line footnoteData(_:) helper to both DocumentSourceMapTests.swift:142 and WebSelectionNoteTests.swift:225, re-inlines the ordered/non-overlapping assertion loop (a reusable expectRunsOrderedNonOverlapping already exists, private, at WebStructuredSelectionTests.swift:56), and rebuilds the DocumentSession + DocumentLayoutCoordinator + WebDocumentMessageRouter triple twice more. | Skipped. Four existing test files (WebFootnotePopoverTests, WebSearchBridgeTests, BlockHTMLEmitterTests, FootnoteSearchTests) each hand-roll their own FootnoteData construction, and the router triple is already built three times in WebSelectionNoteTests — the diff follows the target's established convention rather than diverging from it. Promoting the shared helpers is a test-support refactor that should not ride on a bugfix; it is the natural companion to T-1941, which will need the invariant helper cross-file anyway. |
| nit | rebased(_:by:) placement and allocation | Two suggestions: move the helper onto DocumentSourceMap.Run as shifted(by:) next to the model, and avoid the per-segment map allocation via in-place mutation of context.blockRuns. | Both skipped. Placement: one call site, and Models/DocumentSourceMap.swift is currently a behaviour-free Codable value type — revisit when T-1941 adds callers. Allocation: Run is 24 bytes of POD and a segment yields 1-3 runs, next to a full swift-markdown AST parse plus a Mutex-serialised SwiftSoup pass per segment; in-place mutation would trade a provably pure helper for index bookkeeping in the exact function whose bookkeeping bug this PR exists to fix. |
| nit | segmentStart computed when recordRuns is false | segmentStart and tailStart are computed unconditionally even though they are only read inside the if recordRuns branch, so nested blocks, blockquote non-first children and table cells pay for them needlessly. | Skipped. The measured cost is ~1 µs per call against a loop the matches(of:) regex scan dominates by ~30x, and hoisting the assignment above the branch is what keeps the explanatory comment block adjacent to the code it describes. |
| nit | Coverage still open after this PR | Three shapes remain untested: emphasis spanning a badge (*em [^1] tail* — each half is re-parsed independently so the asterisks become literal run text; inherent to the split strategy, not a regression); a blockquote first paragraph containing a footnote (composes the rebase with .blockquote's \n\n-joined textContent); and characterisation of today's wrong behaviour for footnotes inside list items and table cells. | Skipped for this PR. The first two are pre-existing gaps unrelated to the reported bug; the third belongs to T-1941, which should reference renderListMarkup:413 and tableHTML:524,531 by line and adopt the full-corpus invariant sweep as its acceptance criterion. |
| nit | Pre-existing iOS build warning | make build-ios emits 'main actor-isolated conformance of ImageDimension to Equatable cannot be used in nonisolated context; this is an error in the Swift 6 language mode'. CLAUDE.md's pre-push bar is zero warnings on both platforms. | Not attributable to this branch and left alone. ImageDimension is declared at prism/Models/MarkdownBlock.swift:289, added in 28f8fa2 (2026-03-18), and appears nowhere in this diff; the production change here is a private static helper plus comments. Worth a separate ticket alongside the earlier nonisolated/Sendable warning sweep. make build-macos is clean. |
| minor | Branch is behind origin/main | origin/main advanced from e9d2fed to 6e2a112 (PR #318, T-1851 collapsed-heading scroll persistence) during this review. git merge-tree reports a conflict: both branches add an entry at the top of CHANGELOG.md's ### Fixed section. | Not resolved here — rebasing is the pusher's call, not the reviewer's. The conflict is a trivial adjacent-line add/add; keep both entries. #318 touches prism-scroll.js, docs/agent-notes/scroll-persistence.md and a new scroll test class, none of which this branch goes near, so there is no semantic interaction with the source-map change. |
| nit | DocumentSourceMapTests.swift:185 / WebSelectionNoteTests.swift:266,295 — test coupling | #expect(expectedStart == 11) asserted NSString.range(of:) arithmetic rather than production behaviour, and hardcoded a constant that must track the fixture string. Separately, the live tests select runs by position (runIndex: 1 / 2), coupling them to the emitter's run count. | The redundant arithmetic assertion was deleted — the two following lines already pin the semantics. The positional run indices were left: selecting by textContent would be marginally more robust, but it was explicitly verified that none of the new assertions can pass silently (a nil resolve falls to -1 and fails the offset comparison; a nil domID fails its own expect; a nil sourceSpan shortens the compactMapped array and breaks the == comparison), so the failure mode is a confusing red, not a false green. |
Click to expand.
diff --git a/prism/Services/WebRendering/BlockHTMLEmitter.swift b/prism/Services/WebRendering/BlockHTMLEmitter.swiftindex 72aa56a..796aa58 100644--- a/prism/Services/WebRendering/BlockHTMLEmitter.swift+++ b/prism/Services/WebRendering/BlockHTMLEmitter.swift@@ -716,28 +716,57 @@ nonisolated enum BlockHTMLEmitter { for match in source.matches(of: FootnoteData.referencePattern) { let identifier = String(match.1) guard let definition = footnotes.definition(for: identifier) else { continue }+ // The inline renderer maps runs against the string it is handed, so a segment's+ // offsets are segment-local. They are rebased by the segment's UTF-16 start in+ // the WHOLE string passed to renderInline — which skips every preceding [^id]+ // token — because the source map's consumers (selection notes, relocation,+ // inline-note export) index into that string, not into a segment (T-1876/T-1673).+ // Note this restores block coordinates only when renderInline was handed the+ // block's own source (paragraph, heading, blockquote first child, top-level+ // details summary). renderListMarkup (item content), tableHTML (header/cell)+ // and emitDetails' recursive NESTED summary all hand it a substring and record+ // the result into the enclosing block's runs, so those stay sub-span-local —+ // tracked as T-1941.+ let segmentStart = lastEnd.utf16Offset(in: source) let segment = String(source[lastEnd..<match.range.lowerBound]) if !segment.isEmpty { let result = InlineHTMLRenderer.render( source: segment, footnotes: .empty, runIDAllocator: &context.nextRunID )- if recordRuns { context.blockRuns.append(contentsOf: result.runs) }+ if recordRuns {+ context.blockRuns.append(contentsOf: rebased(result.runs, by: segmentStart))+ } html += result.html } html += footnoteBadge(identifier: identifier, displayNumber: definition.displayNumber) lastEnd = match.range.upperBound }+ let tailStart = lastEnd.utf16Offset(in: source) let tail = String(source[lastEnd...]) if !tail.isEmpty { let result = InlineHTMLRenderer.render( source: tail, footnotes: .empty, runIDAllocator: &context.nextRunID )- if recordRuns { context.blockRuns.append(contentsOf: result.runs) }+ if recordRuns {+ context.blockRuns.append(contentsOf: rebased(result.runs, by: tailStart))+ } html += result.html } return html } + /// Shifts segment-local run offsets into the block's full source coordinate space.+ private static func rebased(+ _ runs: [DocumentSourceMap.Run], by offset: Int+ ) -> [DocumentSourceMap.Run] {+ guard offset != 0 else { return runs }+ return runs.map {+ DocumentSourceMap.Run(+ runID: $0.runID, sourceStart: $0.sourceStart + offset, length: $0.length+ )+ }+ }+ /// An inert footnote badge: pill chrome with an open action (Req 7.1/10.2). The /// badge text is the display number, not document-derived prose. Tapping dispatches /// linkActivated for `prism://footnote/{id}` via the bridge.diff --git a/prismTests/WebRendering/DocumentSourceMapTests.swift b/prismTests/WebRendering/DocumentSourceMapTests.swiftindex 30973f6..f47f478 100644--- a/prismTests/WebRendering/DocumentSourceMapTests.swift+++ b/prismTests/WebRendering/DocumentSourceMapTests.swift@@ -136,6 +136,148 @@ struct DocumentSourceMapInvariantTests { #expect(!Self.runWrappersInside(para.html, container: "prism-footnote-badge")) } + // MARK: - Footnote-split segments stay in full-block coordinates (T-1876 / T-1673)++ /// A `FootnoteData` with one definition per supplied identifier, numbered in order.+ private func footnoteData(_ identifiers: [String]) -> FootnoteData {+ var definitions: [String: FootnoteDefinition] = [:]+ for (index, identifier) in identifiers.enumerated() {+ definitions[identifier] = FootnoteDefinition(+ identifier: identifier, displayNumber: index + 1, content: "def \(identifier)"+ )+ }+ return FootnoteData(definitions: definitions, referenceOrder: identifiers)+ }++ /// The source substring a run covers, in the block's full source coordinate space.+ private func sourceSpan(_ run: DocumentSourceMap.Run, in source: String) -> String? {+ let units = Array(source.utf16)+ let end = run.sourceStart + run.length+ guard run.sourceStart >= 0, end <= units.count, run.length > 0 else { return nil }+ return String(decoding: units[run.sourceStart..<end], as: UTF16.self)+ }++ @Test("Runs around a resolved footnote stay ordered, non-overlapping and block-relative")+ func footnoteRunsStayInBlockCoordinates() {+ // T-1673/T-1876: the emitter splits inline source around resolvable [^id] tokens+ // and renders each segment independently. Each segment's runs must be rebased by+ // the segment's UTF-16 start in the FULL block source (including the skipped+ // footnote token), or a selection after the badge anchors near the block start.+ let source = "before[^1] after"+ let runs = runs(for: .paragraph(markdown: source), footnotes: footnoteData(["1"]))+ #expect(runs.count >= 2, "text either side of the badge should map to separate runs")++ // Ordered and non-overlapping in full-block source space.+ 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, "run must stay in bounds")+ previousEnd = run.sourceStart + run.length+ }++ // The pre-badge run covers "before" at offset 0.+ #expect(runs.first?.sourceStart == 0)+ #expect(sourceSpan(runs[0], in: source) == "before")++ // The post-badge run starts at "after"'s offset in the FULL block source (11),+ // not at its offset within the trailing segment (1).+ let expectedStart = (source as NSString).range(of: "after").location+ #expect(runs.last?.sourceStart == expectedStart)+ #expect(sourceSpan(runs[runs.count - 1], in: source) == "after")+ }++ @Test("Runs after multiple resolved footnotes stay in full-block source coordinates")+ func multipleFootnoteRunsStayInBlockCoordinates() {+ let source = "alpha[^a] beta[^b] gamma"+ let runs = runs(for: .paragraph(markdown: source), footnotes: footnoteData(["a", "b"]))+ #expect(runs.count >= 3, "three text segments should map to three runs")++ var previousEnd = 0+ for run in runs {+ #expect(run.sourceStart >= previousEnd, "runs must be ordered and non-overlapping")+ previousEnd = run.sourceStart + run.length+ }+ // Each run's mapped span is the text it actually renders.+ let spans = runs.compactMap { sourceSpan($0, in: source) }+ #expect(spans == ["alpha", "beta", "gamma"])+ #expect(runs.last?.sourceStart == (source as NSString).range(of: "gamma").location)+ }++ @Test("An unresolved footnote reference leaves the run spans block-relative")+ func unresolvedFootnoteReferenceKeepsBlockCoordinates() {+ // [^missing] has no definition, so the emitter renders no badge and the whole+ // source stays one segment — offsets must still be block-relative.+ let source = "text[^missing] tail"+ let runs = runs(for: .paragraph(markdown: source), footnotes: footnoteData(["1"]))+ #expect(runs.first?.sourceStart == 0)+ var previousEnd = 0+ for run in runs {+ #expect(run.sourceStart >= previousEnd)+ #expect(run.sourceStart + run.length <= source.utf16.count)+ previousEnd = run.sourceStart + run.length+ }+ }++ @Test("An unresolved reference before a resolved one keeps its literal text in the segment")+ func unresolvedThenResolvedFootnoteComposes() {+ // The emitter's loop `continue`s on an unresolvable reference without advancing+ // lastEnd, so [^missing]'s literal text must stay inside the segment that ends at+ // the NEXT resolvable token — and the tail rebase must still land on "c".+ let source = "a[^missing] b[^1] c"+ let runs = runs(for: .paragraph(markdown: source), footnotes: footnoteData(["1"]))+ #expect(runs.first?.sourceStart == 0)+ #expect(sourceSpan(runs[0], in: source) == "a[^missing] b")+ #expect(runs.last?.sourceStart == (source as NSString).range(of: "c").location)+ #expect(sourceSpan(runs[runs.count - 1], in: source) == "c")+ }++ @Test("A footnote at the very start leaves the tail run in block coordinates")+ func footnoteAtStartKeepsTailInBlockCoordinates() {+ // The pre-badge segment is empty and skipped, so the tail rebase is the only thing+ // keeping the map correct.+ let source = "[^1] after"+ let runs = runs(for: .paragraph(markdown: source), footnotes: footnoteData(["1"]))+ #expect(runs.last?.sourceStart == (source as NSString).range(of: "after").location)+ #expect(sourceSpan(runs[runs.count - 1], in: source) == "after")+ }++ @Test("Adjacent footnotes with no text between keep the tail run in block coordinates")+ func adjacentFootnotesKeepTailInBlockCoordinates() {+ // The inter-badge segment is empty; skipping it must not desynchronise lastEnd.+ let source = "a[^1][^2] b"+ let runs = runs(for: .paragraph(markdown: source), footnotes: footnoteData(["1", "2"]))+ #expect(sourceSpan(runs[0], in: source) == "a")+ #expect(runs.last?.sourceStart == (source as NSString).range(of: "b").location)+ #expect(sourceSpan(runs[runs.count - 1], in: source) == "b")+ }++ @Test("Astral text before a footnote keeps the tail run in UTF-16 block coordinates")+ func astralBeforeFootnoteKeepsUTF16Coordinates() {+ // The rebase offset must be UTF-16, not Character count: 😀 is two UTF-16 units.+ let source = "a😀b[^1] tail"+ let runs = runs(for: .paragraph(markdown: source), footnotes: footnoteData(["1"]))+ let expectedStart = (source as NSString).range(of: "tail").location+ #expect(runs.last?.sourceStart == expectedStart)+ #expect(sourceSpan(runs[runs.count - 1], in: source) == "tail")+ }++ @MainActor+ @Test("A selection after a footnote badge converts to the selected text (T-1876)")+ func selectionAfterFootnoteConvertsToSelectedText() {+ // End-to-end of the reported bug: the run offsets the source map hands JS are fed+ // back through the router's UTF-16 -> NoteTextRange conversion against the block's+ // textContent. The quoted text must be the selected text, not text near the start.+ let source = "before[^1] after"+ let block = MarkdownBlock.paragraph(markdown: source)+ let runs = runs(for: block, footnotes: footnoteData(["1"]))+ let tail = runs[runs.count - 1]+ let textRange = WebDocumentMessageRouter.noteTextRange(+ in: block.textContent,+ utf16Range: .init(start: tail.sourceStart, length: tail.length)+ )+ #expect(textRange?.selectedText == "after")+ }+ @Test("UTF-16 coordinate space: astral chars count as surrogate pairs") func utf16Coordinates() { // One astral scalar = 2 UTF-16 units; the run length must reflect that.diff --git a/prismTests/WebRendering/WebSelectionNoteTests.swift b/prismTests/WebRendering/WebSelectionNoteTests.swiftindex 382bc16..6f0846e 100644--- a/prismTests/WebRendering/WebSelectionNoteTests.swift+++ b/prismTests/WebRendering/WebSelectionNoteTests.swift@@ -219,6 +219,98 @@ struct WebSelectionNoteTests { #expect(length == ("plain *bold* tail" as NSString).length) } + // MARK: - Selections after resolved footnote badges (T-1876)++ /// One definition per identifier, numbered in reference order.+ private func footnoteData(_ identifiers: [String]) -> FootnoteData {+ var definitions: [String: FootnoteDefinition] = [:]+ for (index, identifier) in identifiers.enumerated() {+ definitions[identifier] = FootnoteDefinition(+ identifier: identifier, displayNumber: index + 1, content: "def \(identifier)"+ )+ }+ return FootnoteData(definitions: definitions, referenceOrder: identifiers)+ }++ /// Selects the whole text of the run at `runIndex` inside `domID` and resolves it.+ private func resolveWholeRun(+ _ harness: WebDocumentLiveHarness, domID: String, runIndex: Int+ ) async throws -> [String: Any]? {+ let json = try await harness.evalString(+ """+ var runs = document.querySelectorAll('#\(domID) [data-prism-run]');+ if (runs.length <= \(runIndex)) { return null; }+ var node = runs[\(runIndex)].firstChild;+ var range = document.createRange();+ range.setStart(node, 0);+ range.setEnd(node, node.length);+ var r = window.__prismBridge.resolveSelectionRange(range);+ return r ? JSON.stringify(r) : null;+ """+ )+ return json.flatMap {+ try? JSONSerialization.jsonObject(with: Data($0.utf8)) as? [String: Any]+ }+ }++ @Test("A live selection after a resolved footnote badge resolves to the selected text")+ func liveSelectionAfterFootnoteResolvesSelectedText() async throws {+ // T-1876: the source map's footnote-split segments must be rebased into full-block+ // coordinates, otherwise the range resolved for text after the badge points near+ // the block start and the note quotes the wrong words.+ let source = "before[^1] after"+ let para = MarkdownBlock.paragraph(markdown: source)+ let harness = try await WebDocumentLiveHarness.make(+ blocks: [para], footnotes: footnoteData(["1"]), featureScripts: Self.notesScripts+ )+ let resolved = try await resolveWholeRun(harness, domID: domID(para), runIndex: 1)+ #expect(resolved?["domID"] as? String == domID(para))+ let start = (resolved?["start"] as? NSNumber)?.intValue ?? -1+ let length = (resolved?["length"] as? NSNumber)?.intValue ?? -1+ #expect(start == (source as NSString).range(of: "after").location)+ #expect(length == 5)++ // The native create path quotes the selected text, not text near the block start.+ let session = DocumentSession(clipboardContent: "x")+ session.parsedBlocks = [para]+ let coordinator = DocumentLayoutCoordinator()+ let router = WebDocumentMessageRouter(+ session: session, coordinator: coordinator,+ notesManager: NotesManager.makeForTesting(store: MockNotesStore())+ )+ router.createSelectionNote(+ blockID: domID(para), range: .init(start: start, length: length)+ )+ #expect(coordinator.addNoteTextRange?.selectedText == "after")+ }++ @Test("A live selection after two resolved footnote badges resolves to the selected text")+ func liveSelectionAfterMultipleFootnotesResolvesSelectedText() async throws {+ let source = "alpha[^a] beta[^b] gamma"+ let para = MarkdownBlock.paragraph(markdown: source)+ let harness = try await WebDocumentLiveHarness.make(+ blocks: [para], footnotes: footnoteData(["a", "b"]), featureScripts: Self.notesScripts+ )+ // Run 2 is "gamma", after both badges.+ let resolved = try await resolveWholeRun(harness, domID: domID(para), runIndex: 2)+ let start = (resolved?["start"] as? NSNumber)?.intValue ?? -1+ let length = (resolved?["length"] as? NSNumber)?.intValue ?? -1+ #expect(start == (source as NSString).range(of: "gamma").location)+ #expect(length == 5)++ let session = DocumentSession(clipboardContent: "x")+ session.parsedBlocks = [para]+ let coordinator = DocumentLayoutCoordinator()+ let router = WebDocumentMessageRouter(+ session: session, coordinator: coordinator,+ notesManager: NotesManager.makeForTesting(store: MockNotesStore())+ )+ router.createSelectionNote(+ blockID: domID(para), range: .init(start: start, length: length)+ )+ #expect(coordinator.addNoteTextRange?.selectedText == "gamma")+ }+ @Test("A cross-block selection is reported as declined (crossBlock), not a range") func liveCrossBlockDeclined() async throws { let first = MarkdownBlock.paragraph(markdown: "first block")diff --git a/prismTests/WebRendering/WebStructuredSelectionTests.swift b/prismTests/WebRendering/WebStructuredSelectionTests.swiftindex 2ecc42a..f750579 100644--- a/prismTests/WebRendering/WebStructuredSelectionTests.swift+++ b/prismTests/WebRendering/WebStructuredSelectionTests.swift@@ -135,6 +135,18 @@ struct WebStructuredSourceMapInvariantTests { let map = sourceMap(markdown) expectRunsOrderedNonOverlapping(map, "representative table") }++ @Test("Parity footnote fixture: runs are ordered and non-overlapping (T-1876)")+ func parityFootnoteFixture() throws {+ // This generic invariant is what would have caught T-1876: pre-fix, a paragraph+ // with an interior reference emitted runs [0,6] then [1,5] — non-monotonic. The+ // fixture is pointed at deliberately rather than sweeping the whole corpus, since+ // list-item and table-cell fixtures still violate the invariant under T-1941.+ let markdown = try ParityFixtureSupport.load("Parity/footnotes.md")+ let map = sourceMap(markdown)+ expectRunsOrderedNonOverlapping(map, "parity footnotes fixture")+ #expect(!map.runs.isEmpty, "the footnote fixture's paragraphs should map runs")+ } } // MARK: - Safe-decline selection behaviour (Decision 8)diff --git a/prismTests/WebRendering/Fixtures/Parity/footnotes.md b/prismTests/WebRendering/Fixtures/Parity/footnotes.mdindex 73b3df7..1815568 100644--- a/prismTests/WebRendering/Fixtures/Parity/footnotes.md+++ b/prismTests/WebRendering/Fixtures/Parity/footnotes.md@@ -2,5 +2,9 @@ A paragraph that references a footnote.[^note] Another paragraph with a second reference.[^second] +A paragraph with an interior reference[^interior] followed by trailing prose, so the+corpus contains text after a badge (T-1876).+ [^note]: The first footnote definition content. [^second]: The second footnote definition content.+[^interior]: The interior footnote definition content.diff --git a/specs/webview-rendering/decision_log.md b/specs/webview-rendering/decision_log.mdindex 8eab5c4..08c1289 100644--- a/specs/webview-rendering/decision_log.md+++ b/specs/webview-rendering/decision_log.md@@ -1,5 +1,11 @@ # Decision Log: WebView Document Rendering +## Quick Decisions++| ID | Date | Decision | Rationale |+|----|------|----------|-----------|+| Q1 | 2026-07-26 | Rebase only `renderInline`'s footnote-split segments (T-1876); leave the other sub-span callers that record runs — `renderListMarkup` (item content), `tableHTML` (header/cell) and `emitDetails`' recursive nested summary — for T-1941 rather than switching them to safe-by-decline (`recordRuns: false`) | Declining would regress selection notes that anchor correctly today in item 0 / cell 0 / the top-level summary; rebasing those callers needs each sub-span's offset in the block source, which is a separate change |+ ## Decision 1: Full replacement, both platforms, one phased spec **Date**: 2026-06-12diff --git a/docs/agent-notes/webview-rendering-status.md b/docs/agent-notes/webview-rendering-status.mdindex 4e497a2..bfc829d 100644--- a/docs/agent-notes/webview-rendering-status.md+++ b/docs/agent-notes/webview-rendering-status.md@@ -89,6 +89,7 @@ Resolved this session: **build warnings** (now zero, `f8b9480`/`3594bc0`) and ** - **Horizontal rubber-band in the WebView is NOT fixed by CSS `overflow-x: hidden`** (that only stops content overflow) — the scroll-view elasticity needs `.scrollBounceBehavior(.basedOnSize, axes: .horizontal)` on the SwiftUI `WebView`. - **`.fileImporter` / pickers present reliably at LAYOUT level, not from the nested DocumentScrollContent** — drive folder pickers from the layout (coordinator-flag) or from a sheet's own context. - **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). Any `BlockHTMLEmitter` caller that renders a *sub-span* of a block with `recordRuns: true` must rebase the returned runs by that sub-span's UTF-16 start, or the block's `DocumentSourceMap` runs overlap and selection notes anchor to the wrong text. The defect is invisible in the emitted HTML — it only surfaces when run offsets are compared against `block.textContent`. `renderInline`'s footnote-split segments are rebased (T-1876); `renderListMarkup` (item content), `tableHTML` (header/cell) and `emitDetails`' recursive **nested** summary still record sub-span-local offsets into the enclosing block's runs — **T-1941**. Related, same root cause: because each footnote-split segment is *block*-parsed independently, a segment starting with 4+ spaces becomes an indented code block and `InlineHTMLRenderer.Walker` has no `visitCodeBlock`, so `word[^1] more` silently drops `more`. The structural fix for all of it is to move badge substitution into the Walker (its `footnotes` property is already stored but never read) so there is one source cursor. - **`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 c52d7e6..0db49db 100644--- a/CHANGELOG.md+++ b/CHANGELOG.md@@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Adding a note to text selected after a footnote badge now quotes the text you actually selected (T-1876). In a paragraph like `before[^1] after`, selecting `after` quoted words from near the start of the paragraph instead, and saving stored a wrong source range, which the note then carried into relocation and inline-note export. The rendered document's text-to-source map now keeps its offsets in the block's own coordinates across any number of footnote badges. Footnotes inside list items and table cells are tracked separately. - Opening or reloading a large or HTML-heavy document no longer freezes the UI (T-1681). Since the WebKit rendering cutover, the full document HTML — including a SwiftSoup sanitisation pass per raw-HTML block and per inline HTML run — was built synchronously on the main thread on every serve, so a big or markup-dense document blocked the app while it rendered, and re-built the HTML on every reload. The build now runs off the main thread and its result is cached per parse: the UI stays responsive, and reloads (external file change, URL refresh, WebContent-process recovery, the iOS folder-access retry) reuse the cached HTML instead of re-emitting. Very HTML-dense documents can still take a noticeable moment to appear; making that incremental is tracked separately. - Search matches are highlighted in the rendered document again (T-1680). Since the WebKit rendering cutover, searching counted matches and navigated natively but the page never showed a highlight — the native→web search-state feed was never connected. Matches now light up as you type, the current match gets its distinct emphasis and scrolls into view when navigating (including when a result is picked from the iPhone search overlay), footnote badges whose content matches are marked, and dismissing search clears the highlights. - Navigation and display state reach the rendered document again (T-1719). The rendering-engine cutover left core behaviours attached to a retired scroll surface, so they silently stopped running: tapping a table-of-contents entry, a note, or a search result now scrolls the document again; the iPhone bottom toolbar hides when scrolling down and returns when scrolling up; table display-mode choices and expanded/collapsed `<details>` sections now survive a WebKit process recovery, the raw/rendered toggle, and the image-access re-fetch exactly as left (including collapsing a section that was open by default; reloading changed file content still re-seeds them from the document, as designed); and the macOS View-menu scroll commands (Page Up/Down, Top, Bottom) work on the rendered document. A new `WebDocumentStateSynchronizer` owns keeping the page in sync with native state independent of any view being mounted, backed by production-assembly regression suites (`specs/bugfixes/webkit-state-integration/report.md`).Notes already saved against a footnote-bearing block were stored with the wrong NoteTextRange. This fix corrects the map going forward; it does not migrate stored anchors. Nothing in the design promises that, and relocation may re-seat some of them on next load, but a user who reports "my old note still quotes the wrong words" is not seeing a regression of this fix.
Three artefacts in this branch (the code comment, decision-log Q1, the agent-notes gotcha) point at T-1941 for renderListMarkup and tableHTML. Confirm that ticket exists and carries the specifics: the two call sites by line (BlockHTMLEmitter.swift:413, :524/:531), and the suggested acceptance criterion of running expectRunsOrderedNonOverlapping across the full samples/ corpus.
This repo's CI does not build or run tests, so a green check means nothing about correctness. Validation for this review was local: make lint (0 violations), make build-macos and make build-ios (both succeeded), and targeted -only-testing runs across 20 classes spanning the emitter, footnote, source-map, selection, notes, router, export, parity, scheme-handler and security suites. make test-quick was deliberately not used — its documented pre-existing crash cascade has its exit code masked by the xcbeautify pipe.
Every footnote source-map test in this branch was run against a working tree with only BlockHTMLEmitter.swift reverted to origin/main. All seven failed (EXIT=65), including the three added during this review, and all passed once the fix was restored. The tests genuinely encode the bug rather than the implementation.
origin/main is now 6e2a112; this branch's merge base is e9d2fed. Rebase and resolve the CHANGELOG add/add conflict by keeping both entries, then re-run the targeted suites — the incoming commit only touches prism-scroll.js and scroll tests, so no interaction with the source map is expected, but the branch has not been tested against that tip.
word[^1] more drops more from the rendered document today, on main, independent of this PR. It is recorded in the agent notes but has no ticket. File one, and consider whether it and T-1941 should be folded into a single "move badge substitution into InlineHTMLRenderer.Walker" change rather than fixed separately.