A one-line indexing swap in ImportedNotesProcessor.buildAnchorLookups() that closes a producer/consumer asymmetry: CommentBlockExtractor has emitted nested dotted anchor IDs since T-1144, but the lookup that resolves them only ever indexed top-level items. Reviewed against origin/main, PR #371.
CommentBlockExtractor.extractListItemComments has recursed into nested sublists and emitted dotted anchors like {blockId}-item-0-item-0 since T-1144. buildAnchorLookups never learned to read them, so the pipeline produced IDs its own consumer could not resolve.contextQuote, sectionHeading, sectionId and headingPath all came back empty, leaving a blank quote and no heading in the notes pane.NotesExporter (T-1125) and RelocationEngine (specs/bugfixes/nested-item-inline-notes/). This closes the last inconsistent consumer.anchor=-override note naming a nested item was accepted by CommentBlockExtractor.isValidSubBlockId (which already used allListItemIds()) and then failed to resolve in buildAnchorLookups. Same one-line fix.MarkdownBlockParser.parse → CommentBlockExtractor.extract → NotesManager.loadImportedNotes — rather than calling the private lookup directly, and asserts all four previously-empty fields.MermaidCSPSpikeTests aborted with signal abrt, killing the host; 189 tests never ran and are recorded as failures with no duration.Ready to push
Approve. The fix is the smallest correct change: it replaces a hand-rolled top-level-only loop with block.allListItemIds(), the recursive enumerator that eight other production consumers of list-item sub-IDs already use. Three independent review agents (reuse, quality/correctness, spec/docs) found no blocking and no major issues.
The three properties that make this safe were each verified rather than assumed. Backward compatibility: listItemId(at:) and collectItemIds both delegate to the same nestedListItemId helper, so top-level IDs are byte-identical and every stored anchor still resolves — the change is strictly additive. No ID collisions: block IDs are 16 lowercase hex characters and can never contain -item-, so a nested ID can never alias another block's top-level ID. No lost fallback: the empty-AnchorContext branch stays reachable for its legitimate case — an anchor whose block no longer exists after a reload — and is still covered by the orphan tests.
Validated locally on macOS, which is the stronger signal here — GitHub Actions is billing-blocked and its checks are Linux-only. SwiftLint --strict: clean across 541 files, re-run after the review edits. Unit suite: green. Both failure clusters were run to ground rather than waved off — a pre-existing MermaidCSPSpikeTests crash (signal abrt) that fakes 189 extra failures by killing the host mid-run, and 15 live-WebKit timeouts caused by a sibling agent's concurrent suite, which pass in 1.2s instead of 37s when run alone. Neither is attributable to this branch.
bbce47f T-1871: Fix imported nested-list notes losing anchor context a927e7f T-1871: Pre-push review fixes (docs and comment accuracy) Prism lets you write notes into a markdown document as specially-formatted quote blocks. When you open that document, Prism reads those notes back out and attaches each one to the thing it was written about — a paragraph, a table row, or an item in a bulleted list.
To attach a note, Prism needs a name for every possible target. A list item's name looks like a3f0…-item-2, meaning "the third item of that list". An item nested underneath another item gets a longer name: a3f0…-item-0-item-1.
Prism had two pieces of code that disagreed. The piece that wrote down the names knew about nested items and produced the long names. The piece that looked names up only knew the short ones. So when a note pointed at a nested item, the lookup found nothing.
The note itself was fine — it stayed attached to the right item. What went missing was everything around it: the quoted snippet of the item's text, and which section of the document it lived in. In the notes pane the note showed up with a blank quote and no heading, so you could not tell what it referred to.
One loop was swapped for a different one. The old loop walked only the top-level items. The new one, allListItemIds(), walks nested items too — and it was already the standard choice everywhere else in the app. Both produce exactly the same names for top-level items, so nothing that already worked changed.
The imported-notes pipeline runs in three stages: MarkdownBlockParser.parse produces [MarkdownBlock]; CommentBlockExtractor.extract pulls [!COMMENT …] blockquotes out and returns cleaned blocks plus [ImportedNoteData], each carrying an anchorBlockId; NotesManager.loadImportedNotes hands both to ImportedNotesProcessor, which builds lookup tables and resolves per-note context.
buildAnchorLookups built three dictionaries — blockById, listItemText, tableRowText. Sub-block IDs map back to the parent block, because DocumentStructure only indexes real block IDs for section membership; that indirection is deliberate (T-236) and unchanged.
List items were enumerated with for index in 0..<items.count { block.listItemId(at: index) } — top-level only. The extractor on the other side of the pipeline recurses through parentItemPath + [itemIndex] and has since T-1144. So the two ends of the same pipeline used different notions of "the set of list-item IDs".
The consequence surfaces in resolveAnchorContext, which is a three-way branch: sub-block hit → context from the parent block; whole-block hit → context from extractQuote; neither → an all-empty AnchorContext. A nested anchor missed both dictionaries and fell into the third branch. Critically it did not fail loudly — grouping is keyed on data.anchorBlockId independently of the lookup, so the note was stored correctly and only its metadata was hollow.
for entry in block.allListItemIds() { blockById[entry.id] = block; listItemText[entry.id] = entry.text }.
allListItemIds() → collectItemIds → nestedListItemId(itemPath: []) for top-level items, the same helper listItemId(at:) delegates to. Identical strings, so persisted anchors are unaffected.entry.text is item.content — the item's own leading paragraph, excluding nested children and the checkbox marker. Same property the old code read, and the same value the native note path stores, so imported and native context quotes for one item now agree.if case .list wrapper is gone because allListItemIds() self-guards and returns []. This makes the loop read identically to the allTableRowIds() loop below it.Prism encodes sub-block identity as a string path — {blockId}(-item-{n})+ — with MarkdownBlock.nestedListItemId as the documented single source of truth (T-1144). That gives a canonical format, but format canonicalisation does not imply enumeration canonicalisation. Two APIs enumerate the same space with different reach: listItemId(at:) (index-addressed, top-level, correct for creation where the caller supplies a flat index) and allListItemIds() (recursive, correct for building any lookup). Nothing in the type system distinguishes them — both hand back String IDs of the same shape — so choosing the narrow one at a lookup site is a silent, type-checking bug.
That is precisely why this is the third occurrence: NotesExporter.collectNotes (T-1125), RelocationEngine (specs/bugfixes/nested-item-inline-notes/), and now ImportedNotesProcessor. The project anticipated it — specs/table-row-notes/design.md defines a "Pattern Parity Audit" and decision_log.md:371 mandates a mechanical audit of both call sites — but an audit is a point-in-time artefact, not an invariant. After this change the only remaining production listItemId(at:) caller is NotesManager.createNoteForListItem, where index-addressing is genuinely correct and whose public wrapper has no production callers at all.
The bug's severity came from where it degraded. Grouping (ImportedNotesProcessor line 141) keys on data.anchorBlockId directly, so storage was always correct; only the derived context was empty. The result was a defect with no exception, no log line, and no structural symptom — visible only as a blank quote in the notes pane, which reads as "the author wrote no context" rather than "the app failed to resolve it". This is the same shape as T-785 (table rows), whose gotcha note describes the identical empty-contextQuote/nil-metadata signature.
The path encoding is unambiguous for a non-obvious reason worth stating: block IDs are 16 lowercase hex characters (BlockIDStore.computeId), so no block ID can contain the substring -item-. For a nested ID {hex}-item-0-item-1 to collide with another block's top-level ID {hex2}-item-N, you would need hex2 == "{hex}-item-0", which the hex alphabet forbids. Within one block, indices are integers, so -item-0-item-1 cannot be re-parsed as top-level index 0-item-1. The -item-/-row- namespaces stay disjoint, preserving the listItemText[id] ?? tableRowText[id] coalesce.
ordered/start, matching CommentBlockExtractor.listItemId exactly. content excludes the checkbox marker.contextQuote == "" but still get correct section metadata — strictly better than the old all-empty result, and identical to top-level behaviour.hasActiveAnchoredNotes, MarkdownSection.noteCount)..details or blockquote children, where allListItemIds() returns [] — but the extractor produces no anchors there either, so the two ends still agree.blockById/listItemText are keyed by content-derived block ID and are last-wins. Nested items therefore now inherit the same duplicate-content ambiguity top-level items have always had: two identical nested items under duplicate parents collapse to one entry. This is a net improvement (empty context → possibly-wrong-occurrence context, matching top-level behaviour) and not a regression, but the open T-2085/T-2086/T-2087/T-2088 occurrence-identity cluster must account for nested items when it reaches this surface.
Fully implemented: nested list-item anchor resolution for imported notes, at any depth, for ordered and unordered lists and checklists; the anchor=-override path for nested items comes along for free. Partially covered: tests pin depth 2 with one note; depth ≥ 3 and multiple notes at differing depths go through the same dictionary and are exercised indirectly by the relocation and exporter suites. Out of scope: duplicate-content occurrence identity (the T-2085–T-2088 cluster).
prism/Services/ImportedNotesProcessor.swift
Why it matters. This is the whole fix. The old loop bounded itself to 0..<items.count, so any anchor produced by the extractor's recursion was unresolvable — the note kept its correct storage key but lost contextQuote, sectionHeading, sectionId and headingPath to the empty-AnchorContext branch.
What to look at. prism/Services/ImportedNotesProcessor.swift:253-270 (buildAnchorLookups)
prism/Services/ImportedNotesProcessor.swift
Why it matters. The diff removes a pattern-match guard, which normally warrants scrutiny. Here it is safe and improves symmetry: allListItemIds() self-guards on `case .list` and returns [] for every other variant, exactly as allTableRowIds() does on the following line.
What to look at. prism/Services/ImportedNotesProcessor.swift:262-269
prismTests/NotesManagerImportedNotesTests.swift
Why it matters. buildAnchorLookups is private, so a direct-invocation test is impossible — and would have been the weaker test anyway. This one parses real markdown, extracts real comment blockquotes, and loads through NotesManager, so it pins the producer/consumer agreement that actually broke.
What to look at. prismTests/NotesManagerImportedNotesTests.swift:872-928
prism/Services/CommentBlockExtractor.swift
Why it matters. An unfiled second symptom of the same defect. isValidSubBlockId already validated overrides via allListItemIds(), so a note explicitly naming a nested item was accepted at extraction and then failed to resolve at lookup — accepted by one gate, dropped by the next.
What to look at. prism/Services/CommentBlockExtractor.swift:216 (validation) vs ImportedNotesProcessor.swift:262 (resolution)
The alternative — keeping listItemId(at:) and adding a nested branch — would have reimplemented collectItemIds at a third call site and risked drifting from nestedListItemId, the documented single source of truth for the format (T-1144). Adopting the existing recursive enumerator keeps one implementation.
Unchanged from T-236/T-785 and load-bearing: resolveAnchorContext uses the block only for structure.sectionHeading/sectionId/headingPath(forBlockId:), and MarkdownSectionBuilder indexes the parent block for section membership. Mapping a sub-ID to its parent is required for section metadata to resolve at all.
Superficially similar (both build blockById plus a sub-block index) but semantically different: RelocationEngine keys entries by parent ID for later fuzzy matching and resolves duplicate block IDs first-wins, whereas buildAnchorLookups keys text by sub-block ID for direct lookup and is last-wins. Extracting a shared helper would silently change duplicate-block resolution in one of them.
Consistent with current practice, not an omission: 17 of the last 18 bugfix commits on main shipped without one. The exception (T-2140) was a 24-file, 2273-insertion change. Sibling reports are root-cause write-ups for multi-file investigations; a one-line indexing swap does not warrant one. The knowledge that is worth keeping went into docs/agent-notes/imported-notes.md instead, beside the T-236 and T-785 bullets it belongs with.
specs/table-row-notes/design.md:70 still lists "Import context | NotesManager.swift | listItemId(at:)", now doubly stale (the code moved to ImportedNotesProcessor in T-1217, and the pattern changed here). Shipped specs are historical records in this project; corrections belong in docs/agent-notes/, which is where the T-1871 bullet went.
| Severity | Area | Finding | Resolution |
|---|---|---|---|
| minor | docs/agent-notes/imported-notes.md | The Gotchas list tracks this exact function's history (T-236 list-item headingPath, T-785 table-row indexing) and its T-785 bullet describes the identical empty-contextQuote/nil-metadata failure mode — but nothing recorded that list-item indexing was top-level-only. A future session would re-investigate the same asymmetry. | Added a T-1871 bullet beside the T-236/T-785 entries, naming the producer/consumer asymmetry, the two prior occurrences (NotesExporter T-1125, RelocationEngine), and the detection heuristic: any site enumerating listItemId(at:) to build a lookup has this bug. |
| minor | CHANGELOG.md | The entry said context was lost "when the document is reloaded or re-imported", understating the scope. Imported notes are rebuilt from the document on every open, so the context was missing the very first time a document was opened. The entry also listed the empty internal fields rather than the visible symptom, unlike its neighbours. | Rewritten to lead with what the user sees (a blank quote and no heading in the notes pane), to state that it happened on every open rather than only on reload, and to drop internal field names in favour of user-facing language. |
| nit | ImportedNotesProcessor.resolveAnchorContext | The comment at line 292 still described list-item IDs as {blockId}-item-N. The disjointness claim it supports remains true, but the format description was made stale by this very diff. | Updated to {blockId}-item-N, or {blockId}-item-N-item-M… once nested, matching the wording of the buildAnchorLookups doc comment above it. |
| nit | specs/bugfixes/ | An empty specs/bugfixes/imported-nested-list-notes-lose-anchor/ directory was left behind in the worktree. Invisible to git (empty directories are untracked) but confusing to anyone browsing the folder, since every sibling contains a report. | Removed. |
| nit | prismTests/NotesManagerImportedNotesTests.swift | #expect(note.sectionId != nil) is weaker than its neighbours; asserting equality with structure.sectionId(forBlockId: listBlock.id) would pin the value the way the headingPath assertion does. | Skipped. The skill forbids editing test files except to fix an actual bug, and the sectionHeading and headingPath assertions on the adjacent lines already pin the resolution to the correct parent block, so the nil-check is belt-and-braces rather than a coverage gap. |
| nit | Test coverage | No test drives depth >= 3, or two notes at different nesting levels, through the processor and asserts each gets its own item's contextQuote. | Skipped. ExportNestedListImportTests already covers three-level nesting and multiple comments at differing depths up to anchorBlockId, collectItemIds recursion is exercised by the relocation and exporter suites, and the lookup itself is a plain dictionary — the marginal value does not justify a slower suite. |
| nit | prism/Services/NotesManager.swift:709-735 | createNoteForListItem's public wrapper has zero production callers — all ten call sites are in prismTests. Production list-item note creation goes through WebDocumentMessageRouter instead. | Skipped as out of scope. Not the same bug class (index-addressed creation, where listItemId(at:) is correct), and dead-code removal does not belong in a one-line bugfix. Worth a separate cleanup ticket. |
| nit | CHANGELOG.md [Unreleased] | The Unreleased block contains duplicated near-identical entries for T-1812 and T-1840. | Skipped — pre-existing on origin/main, not introduced by this branch. Flagged so it is not misattributed here. |
Click to expand.
diff --git a/prism/Services/ImportedNotesProcessor.swift b/prism/Services/ImportedNotesProcessor.swiftindex 6b6015d..7d4b880 100644--- a/prism/Services/ImportedNotesProcessor.swift+++ b/prism/Services/ImportedNotesProcessor.swift@@ -247,22 +247,21 @@ enum ImportedNotesProcessor { /// Builds lookup dictionaries for blocks, list items, and table rows. ///- /// Indexes each block by its ID, each top-level list item by its item sub-ID,- /// and each table row by its row sub-ID (T-785). Sub-block entries map back to- /// the parent block so `resolveAnchorContext` can derive section metadata.+ /// Indexes each block by its ID, each list item (including nested items, T-1871)+ /// by its dotted sub-ID, and each table row by its row sub-ID (T-785). Sub-block+ /// entries map back to the parent block so `resolveAnchorContext` can derive+ /// section metadata. private static func buildAnchorLookups(from blocks: [MarkdownBlock]) -> AnchorLookups { var blockById: [String: MarkdownBlock] = [:] var listItemText: [String: String] = [:] var tableRowText: [String: String] = [:] for block in blocks { blockById[block.id] = block- if case .list(_, _, let items) = block {- for index in 0..<items.count {- if let itemId = block.listItemId(at: index) {- blockById[itemId] = block- listItemText[itemId] = items[index].content- }- }+ // allListItemIds() recurses into nested sublists, unlike listItemId(at:)+ // which only addresses top-level items (T-1871).+ for entry in block.allListItemIds() {+ blockById[entry.id] = block+ listItemText[entry.id] = entry.text } for row in block.allTableRowIds() { blockById[row.id] = block@@ -290,8 +289,9 @@ enum ImportedNotesProcessor { ) -> AnchorContext { let anchorBlock = blockById[anchorBlockId] - // List-item IDs ({blockId}-item-N) and table-row IDs ({blockId}-row-N) are- // distinct by format, so anchorBlockId can only match one dictionary.+ // List-item IDs ({blockId}-item-N, or {blockId}-item-N-item-M… once nested)+ // and table-row IDs ({blockId}-row-N) are distinct by format, so+ // anchorBlockId can only match one dictionary. if let subBlockText = listItemText[anchorBlockId] ?? tableRowText[anchorBlockId] { // Sub-block (list item or table row): derive headingPath from parent block (T-236, T-785) guard let block = anchorBlock else {diff --git a/prismTests/NotesManagerImportedNotesTests.swift b/prismTests/NotesManagerImportedNotesTests.swiftindex 4f29286..97bbbcf 100644--- a/prismTests/NotesManagerImportedNotesTests.swift+++ b/prismTests/NotesManagerImportedNotesTests.swift@@ -866,4 +866,64 @@ struct NotesManagerImportedNotesTests { "Distinct notes should not be de-duplicated, got \(notes.count)" ) }++ // MARK: - T-1871: Nested List-Item Notes Lose Anchor Context++ @Test("Imported note anchored to a nested list item gets non-empty anchor context")+ @MainActor+ func importedNestedListItemNoteGetsAnchorContext() {+ let store = MockNotesStore()+ let manager = NotesManager.makeForTesting(store: store)++ // Real imported-notes payload: a comment blockquote nested two list+ // levels deep, matching the exported format (ExportNestedListImportTests).+ let source = """+ ## Section A++ - [x] First item+ - Sub-item++ > [!COMMENT Bob id=abc123 ts=2026-03-20T10:00:00Z]+ > Note on nested item+ """++ let blocks = MarkdownBlockParser.parse(source)+ let extraction = CommentBlockExtractor.extract(from: blocks)+ #expect(extraction.importedNotes.count == 1, "Precondition: exactly one imported note extracted")++ guard let listBlock = extraction.blocks.first(where: { block in+ guard case .list = block else { return false }+ return true+ }) else {+ Issue.record("Expected a list block")+ return+ }++ // Dotted id for the nested "Sub-item" — see MarkdownBlock.allListItemIds().+ let expectedNestedId = "\(listBlock.id)-item-0-item-0"+ #expect(+ extraction.importedNotes.first?.anchorBlockId == expectedNestedId,+ "Precondition: note anchors to the nested item"+ )++ let structure = makeStructure(from: extraction.blocks)+ manager.loadImportedNotes(extraction.importedNotes, blocks: extraction.blocks, structure: structure)++ guard let note = manager.importedNotes[expectedNestedId]?.first else {+ Issue.record("Expected imported note stored under the nested item id")+ return+ }++ // Before the fix, `buildAnchorLookups` only indexed top-level list-item+ // IDs via `listItemId(at:)`, so `resolveAnchorContext` never found the+ // nested item and fell through to the empty-context branch:+ // contextQuote == "", sectionHeading/sectionId/headingPath == nil.+ #expect(+ note.contextQuote == "Sub-item",+ "Nested item note should get the nested item's own text as context, got '\(note.contextQuote)'"+ )+ #expect(note.sectionHeading == "Section A")+ #expect(note.sectionId != nil)+ #expect(note.headingPath == ["Section A"])+ } }diff --git a/docs/agent-notes/imported-notes.md b/docs/agent-notes/imported-notes.mdindex a456c2f..6542037 100644--- a/docs/agent-notes/imported-notes.md+++ b/docs/agent-notes/imported-notes.md@@ -29,6 +29,7 @@ Imported notes are **reconstructed from the document on every load**. They are n - The toggle test `toggleImportedNoteStatusUpdatesStateAndCache` requires document context (via `loadNotes(for:blocks:)`) because the cache is now document-scoped - **List-item headingPath (T-236)**: For list-item notes, `anchorBlockId` is a composite ID like `{blockId}-item-{index}` which doesn't exist in `DocumentStructure`. The `blockById` lookup maps these item IDs to the parent list block, so `headingPath` must be derived from `block.id` (the parent), not `data.anchorBlockId` (the item). This is handled by `resolveAnchorContext()`. Before this fix, headingPath was always nil for list-item imported notes, causing them to match any duplicate list under a different heading. - **Table-row context in imports (T-785)**: `loadImportedNotes` must index table-row sub-IDs (e.g., `{blockId}-row-0`, `{blockId}-row-header`) alongside list-item sub-IDs. The `buildAnchorLookups` static method builds `blockById`, `listItemText`, and `tableRowText` dictionaries. `resolveAnchorContext` checks `tableRowText` and derives section metadata from the parent table block, parallel to list items. Without this, imported table-row notes get empty `contextQuote` and nil section/heading metadata.+- **Nested list-item anchors must be indexed too (T-1871)**: `buildAnchorLookups` indexed list items with `block.listItemId(at:)`, which only addresses top-level items (`0..<items.count`). `CommentBlockExtractor.extractListItemComments` has recursed into nested sublists since T-1144, so it *produces* dotted anchors like `{blockId}-item-0-item-0` that the lookup never *consumed* — a producer/consumer asymmetry, not a missing feature. Any imported note on a nested item fell through to the empty `AnchorContext` branch (empty `contextQuote`, nil `sectionHeading`/`sectionId`/`headingPath`) even though the note was stored under the right ID. Fixed by swapping to `block.allListItemIds()`, which recurses and is what every other consumer of list-item sub-IDs already uses. The identical bug was fixed the same way in `NotesExporter` (T-1125) and `RelocationEngine` (see `specs/bugfixes/nested-item-inline-notes/`) — if you find another site enumerating `listItemId(at:)` to build a lookup, it has this bug. Note `allListItemIds()` returns `[]` for non-list blocks, so the call needs no `case .list` guard. - **blockStartOffsets must skip comment blockquotes (T-394)**: When building `blockStartOffsets` from `ExportSourceMapper.map`, entries with `isCommentBlock: true` must be filtered out. `ExportSourceMapper.mapListItems` appends comment blockquote entries before the list-item entry, both sharing the same `blockId`. Using "first occurrence wins" without filtering picks the blockquote's offset (deeper into the item) instead of the item's actual start, causing `applyTaggedRanges` to produce negative offsets. - **blockStartOffsets stores all occurrences per blockId (T-424)**: `blockStartOffsets` is `[String: [Int]]`, not `[String: Int]`, because duplicate content blocks share the same content-based ID. `applyTaggedRanges` selects the correct block occurrence by finding the largest block start offset that doesn't exceed the tag's document-level offset. Without this, notes on the second (or later) occurrence of a duplicate block get wrong block-local offsets. - **Within-batch deduplication (T-441)**: `loadImportedNotes` tracks `seenHashes` (a `Set<String>`) during the import loop. This prevents duplicate `[!COMMENT]` blocks or repeated `id=` values in the same file from creating duplicate imported notes. The `existingNoteHashes` set only catches duplicates against persisted user notes; `seenHashes` catches duplicates within the current import batch.diff --git a/CHANGELOG.md b/CHANGELOG.mdindex 2814e59..e2ecf43 100644--- a/CHANGELOG.md+++ b/CHANGELOG.md@@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- A note imported from a document, anchored to a nested list item (a sub-item under a top-level list item), now shows its quoted text and the section it belongs to (T-1871). In the notes pane it appeared with a blank quote and no heading, and it could not be told apart from any other nested-item note in the document. The note itself was always attached to the correct item — only the surrounding context was missing, and it was missing every time the document was opened, not just on a reload. Imported notes are rebuilt from the document on each open, and that rebuild only recognised top-level list-item identifiers, so a nested item's identifier was never matched and the context came back empty. Nested identifiers are now recognised the same way every other part of the app that addresses list items already recognises them, so an imported note on a nested item gets the same context as one on a top-level item. - A `prism://open?url=…` link now opens the address it names, even when that address mixes already-escaped and unescaped characters (T-2140). `…/my%20file and more.md` was fetched as `…/my%2520file%20and%20more.md` — a different resource, with no error shown — because the address had already been unescaped one layer by the time it was read, and was then escaped a second time in full. Investigating it surfaced a second fault of the same kind, live on every markdown link and image in every document: the escaping used for addresses turned an escaped `%2F` back into a real `/`, splitting one path segment into two. That silently broke any address that identifies something by an escaped path — a GitLab project URL, for instance, which 404s once `group%2Fproj` becomes `group/proj`. Escaped slashes and escaped ampersands now survive every route into the app: typed and pasted addresses, deep links, document links and images, `mailto:` links, and the GitHub blob-to-raw rewrite. One narrow side effect of reworking that rewrite: a GitHub address written with a doubled slash in it (`github.com//owner/repo/blob/…`) is no longer recognised as a file address, so it now reports an unsupported content type instead of opening. - In a document opened from a URL, an image or link whose query or anchor was already partly escaped no longer resolves to a corrupted address (T-1663). `/images/logo.png?token=a%20b c` was resolved as `?token=a%2520b%20c`, so the image failed to load and the link opened the wrong page: the already-escaped `%20` was escaped a second time because the whole query was treated as though none of it had been escaped yet. This affected both site-root addresses (starting with `/`) and the far more common document-relative form (`images/logo.png?token=a%20b c`); both are fixed. Query and anchor are now escaped the same way absolute URLs already were (T-1624) — an existing escape is left alone and only genuinely unescaped characters are encoded, so an escaped `%26` stays a literal character instead of decoding into a parameter separator and requesting a different resource.The first make test-quick reported total=4440 passed=4211 failed=190. Reading the result bundle rather than the console: 189 of those have no recorded duration — they never ran. The single real failure is MermaidCSPSpikeTests/corpusSourceMatchesParser on the graph argument, with the message "Test crashed with signal abrt." That crash took the host down and left the remaining 189 queued.
That file is untouched by this branch (last modified in the T-1542 cutover) and exercises MermaidTypeParser, which has no code path to ImportedNotesProcessor. Worth confirming separately whether this crasher is a new arrival on main — it is unrelated to this PR either way, but if it is recent it deserves its own ticket, and while it stands every full make test-quick on this repo will report a four-figure failure count that is almost entirely fictional.
The re-run skipping the crasher still reported passed=5496 failed=15. All 15 were live WebKit harness tests (WebNotesBehaviourTests, WebMediaBehaviourTests, WebScrollabilityReportingTests, WebContentTerminationWiringTests) and every one burned 26–39 seconds before failing — a timeout signature, not an assertion signature.
pgrep showed a concurrent full prismTests run in a sibling worktree (T-1805), competing for WebContent processes. Re-running those four suites alone: ** TEST SUCCEEDED **, 118 passed, 0 failed — with the same tests completing in ~1.2 seconds instead of 37. A 30× swing in duration is the contention proving itself.
Recorded here because WebNotesBehaviourTests is notes-adjacent and would otherwise look implicating; it exercises the WebKit rendering bridge, which this change never touches.
blockById and listItemText are keyed by content-derived block ID and are last-wins. Nested items therefore inherit the ambiguity top-level items already had: two identical nested items under duplicate parents collapse to a single entry, and a note may resolve context from the wrong occurrence.
This is not a regression — it is strictly better than the empty context those notes got before, and identical to long-standing top-level behaviour. But the open T-2085 / T-2086 / T-2087 / T-2088 occurrence-identity cluster now has a wider surface to cover, and should account for nested items when it touches this code.
An item with two sibling nested sublists produces colliding child IDs — but identically in both collectItemIds and CommentBlockExtractor.extractListItemComments, since both thread the same parentPath + [index]. Producer and consumer still agree, so notes resolve consistently. Not introduced by this change and not worth fixing here, but it is a real limit of the path encoding.