A cache-key identity bug in the full-screen SVG preview path. Two documents that each referenced ./diagram.svg shared one SnapshotCache entry, so the second document showed the first one's rendered picture. The fix routes both the read and the write site through a single key builder that keys on the resolved absolute identity rather than the authored path.
SnapshotCache.svgKey(source:…) hashes its source argument verbatim, and both detail views passed the authored markdown path. ./diagram.svg is not a unique identity — every document that uses that filename produces the same key.svgKey(for: ResolvedImageSource, colorScheme:, displayScale:) -> String? keys on ResolvedImageSource.cacheKey — absolute file URL, remote URL, or a SHA-256 of data-URI bytes.SnapshotCache at all — they are served as sanitized bytes by PrismDocSchemeHandler and rasterised by WebKit in-page.main fails the same checks. Two builds, lint, and a full macOS unit run were done on this machine.Ready to push
The fix is correct, minimal, and lands exactly on the root cause. Both call sites now build their key through one function, which is the load-bearing property here — keying writes and reads differently would have silently disabled the cache instead of fixing the collision, and the test suite pins both halves (cross-document keys diverge and same-document reads still hit).
Local validation is the authoritative signal on this repo: GitHub Actions is billing-blocked and its checks are Linux-only, so they cannot exercise this code at all. Locally: SwiftLint clean (0 violations / 543 files), iOS and macOS builds both succeed, and the full macOS unit suite reports 4403 passed / 2 failed / 37 skipped — both failures being load-sensitive flakes (WebScrollabilityReportingTests debounce timing, StatePersistenceTests disk write) that pass in isolation and touch nothing in this diff. A first run hit an unrelated WebKit host abort that cascaded ~233 phantom failures; it did not reproduce on the second run, confirming it as environmental rather than a regression.
Four review findings were raised and fixed in the working tree: a missing CHANGELOG entry (11 of the last 12 Fix T- commits on main ship one), three missing test cases, and two stale/misleading doc comments — including the one that made the original bug possible.
95f09e3 Fix T-1866: Relative SVG detail previews collide across documents working-tree Fixes applied during this review (uncommitted) Prism can show an SVG image full screen. Rendering an SVG into a picture is slow, so Prism keeps the finished picture in a memory store called a cache and reuses it the next time you open the same image.
To find things again, a cache needs a label for each item — a key. Prism was using the name written in the markdown file as that label. Markdown lets you write a short, relative name like ./diagram.svg, meaning "the file called diagram.svg sitting next to this document".
That name is only meaningful relative to a document. Two unrelated documents can each have their own diagram.svg next to them, and both would write ./diagram.svg. Prism saw one label, so it saw one item. Open document A's diagram, then open document B's — and B showed you A's picture. Nothing warned you it was the wrong image, and it stayed wrong for the rest of the session.
Prism already had a step that turns a relative name into the real thing: a full file path, a web address, or (for images written directly into the document) the image's own bytes. The cache label is now built from that instead of from the written name. Two diagram.svg files in different folders are different files, so they now get different labels and stop overwriting each other.
./diagram.svg is relative (only meaningful given a starting point); /Users/you/Docs/A/diagram.svg is absolute (meaningful on its own). Only absolute things make safe labels.SnapshotCache is an NSCache-backed store of rasterised SVG and Mermaid snapshots, keyed by a SHA-256 over source + colorScheme + displayScale. The scheme and scale components exist because the same SVG rasterises differently in light vs dark and at 1x vs 2x vs 3x (T-917).
The source component was the defect. svgKey(source:colorScheme:displayScale:) hashes whatever string you hand it, and its doc comment described that string as "the SVG source content string" — but both production callers passed a path: data.imageSource in ImageDetailWindow and request.source in WebImageDetailSheet. Those are the raw, authored markdown references, which for relative paths are not app-unique.
The fix adds an overload rather than changing the existing one:
static func svgKey(for resolved: ResolvedImageSource,
colorScheme: ColorScheme,
displayScale: CGFloat) -> String? {
guard let identity = resolved.cacheKey else { return nil }
return svgKey(source: identity, colorScheme: colorScheme, displayScale: displayScale)
}ResolvedImageSource.cacheKey already existed and already had the right semantics — it is what the raster image path had been keying on all along. It returns url.absoluteString for .localFile and .remote, a SHA-256 of the decoded bytes for .dataURI, and nil for .failed. The SVG path simply was not using it. So this is less "new mechanism" than "bring the outlier into line with the convention the rest of the image pipeline already followed".
String? and callers guard with if let. The rejected alternative — synthesising a fallback key from the raw source — would have quietly reintroduced a narrower version of the same bug.svgKey(source:) means a future caller could still pick the unsafe one. Mitigated here by documentation rather than by type: the string overload now states outright that an authored relative path is not an acceptable argument and names T-1866.NSCache still evicts against the unchanged 256 MB cost limit.This is an identity bug, not a caching bug: the cache behaved exactly as designed, and the key it was given was simply not injective over the domain it was used on. The interesting property is that the correct identity function (ResolvedImageSource.cacheKey) was already present in the codebase and already in use on the sibling raster path in the very same functions — ImageDetailWindow.loadImage guards its raster lookup with cacheKey(for: resolved) three lines above the SVG branch that did not. The SVG path was the outlier, which is why the fix reads as deletion of a special case rather than as new machinery.
Worth naming explicitly because it is the more expensive of the two failure modes and it is invisible: a partial migration that updated the write key but not the read key (or vice versa) yields a cache with a 0% hit rate. Every open re-rasterises through SVGRenderer — a WKWebView round-trip — and nothing is visibly wrong. The collision bug is loud (wrong picture); the miss bug is silent (slow). The commit message calls this out as the reason for centralising on one function, and the test suite encodes it: svgKeyForResolvedSourceEnablesCacheHitForSameDocument stores under a key from one resolution and retrieves under a key from a second, independent resolution, asserting === identity through a real SnapshotCache instance. A key-string equality assertion alone would not have caught a broken round-trip.
.failed → nil → skip caching. Verified benign end to end: SVGSourceLoader.load(_:) throws immediately on .failed, before any set could be reached, so a failed resolution was never cached before this change either. The guards are defensive, not behavioural — and critically, the nil path still falls through to the same catch let error as ImageLoadError branch, so loadState is never left unset.resolvedSource(url:asLocal:) after .standardized normalisation, so they produce one entry rather than two. Pre-existing, preserved.cacheKey hashes the payload, then svgKey hashes a ~70-byte digest string. The second hash is constant-cost regardless of payload size, and the old code hashed a string of the same order of magnitude as the payload, so total work is essentially unchanged.Confined to two full-screen detail surfaces. The in-document SVG path does not participate: PrismDocSchemeHandler serves sanitized SVG bytes over prism-doc:// and WebKit rasterises them in-page, touching SnapshotCache not at all. There is therefore no second keying scheme to keep in step, and no cross-path cache-coherence question to answer — a fact that makes the fix much smaller than the ticket title suggests.
prism/Services/SnapshotCache.swift
Why it matters. This is the whole fix. It converts the cache key from a document-relative, non-unique string into an app-unique identity, and gives both call sites one place to get it from so they cannot drift apart.
What to look at. SnapshotCache.swift:52-73 — svgKey(for:colorScheme:displayScale:)
prism/Views/ImageDetailWindow.swift
Why it matters. These two views ARE the bug's blast radius. An independent grep of every svgKey / snapshotCache.get / snapshotCache.set site confirmed no third production caller, so the migration is complete rather than partial.
What to look at. ImageDetailWindow.swift:383-410 (loadSVGImage) and WebImageDetailSheet.swift:126-147 (loadSVG)
prismTests/SnapshotCacheTests.swift
Why it matters. This test is what distinguishes a real fix from a fix that silently disabled the cache. It stores under a key from one resolution and reads under a key from a second, independent resolution, through a real SnapshotCache.
What to look at. SnapshotCacheTests.swift:211-228 — svgKeyForResolvedSourceEnablesCacheHitForSameDocument
prism/Services/SnapshotCache.swift
Why it matters. The string overload survives and remains callable, so the bug remains reachable by a future caller. Its parameter doc previously said "the SVG source content string" — while both production callers were passing a path. That mismatch is arguably what enabled T-1866 in the first place.
What to look at. SnapshotCache.swift:34-50 — svgKey(source:colorScheme:displayScale:) doc comment
CHANGELOG.md
Why it matters. This is a user-visible correctness bug — the wrong picture, silently — and the repo convention is unambiguous: 11 of the last 12 `Fix T-` commits on main ship an Unreleased/Fixed entry. The branch had none.
What to look at. CHANGELOG.md:23 — new Unreleased/Fixed entry
The existing string-taking function is kept and the new one delegates to it. This preserves the existing Mermaid-key symmetry and leaves the tests that exercise real SVG content strings valid. The cost is that the unsafe overload stays reachable — mitigated in this review by a doc comment that names T-1866 and states outright that an authored relative path is not an acceptable argument.
A .failed resolution has no stable identity. Throwing would push identical try? boilerplate into both call sites for a condition that is not exceptional; a synthesised fallback key (e.g. hashing the raw authored source) would quietly reintroduce a narrower version of the very collision being fixed. Optional plus if let at two sites is the honest encoding of "there is no key for this".
ResolvedImageSource.cacheKey hashes the decoded bytes with no document component, so byte-identical inline SVGs in different documents map to one entry. This is content-addressing and it is correct — rasterisation is a pure function of (bytes, colour scheme, display scale), and all three are in the key. It is also not a behaviour change: the previous key was the full literal data-URI string, which already shared. Documented in the CHANGELOG so it is not later mistaken for a residual collision.
Both call sites already hold a ResolvedImageSource produced by their enclosing load function. Taking it as a parameter keeps ImagePathResolver.resolve at one site per load and keeps SnapshotCache free of any dependency on the resolver.
Sampling recent history, roughly one in four comparable fixes ships a report, generally where the root-cause narrative is involved or several distinct defects were found under one ticket. T-1866 is a single contained root cause with a two-line fix, so the CHANGELOG entry plus the doc comments carry the explanation adequately. The CHANGELOG entry, by contrast, is required by convention and was added.
(inferred — not stated by the author.)| Severity | Area | Finding | Resolution |
|---|---|---|---|
| major | CHANGELOG.md | No Unreleased/Fixed entry, against a strong repo convention: 11 of the last 12 'Fix T-' commits merged to main add one. This is a user-visible correctness bug (wrong image shown, silently), which is exactly the category the changelog exists to record. | Added an entry in the established user-facing prose style. It describes the symptom, the cause, and the fix, and explicitly documents the intended data-URI sharing so it is not later misread as the same bug reopening. |
| minor | prism/Services/SnapshotCache.swift doc comment | svgKey(source:) documents its parameter as 'The SVG source content string', but both production callers were passing an authored path — the mismatch that made T-1866 possible. The overload survives the fix and stays callable, so the trap stays armed for a future caller who reads the doc and concludes a path is fine. | Rewrote the doc to state that source is hashed verbatim and must already be app-unique, that an authored document-relative path is NOT such a string, that this was T-1866, and that any caller holding a ResolvedImageSource must use the new overload. |
| minor | prismTests/SnapshotCacheTests.swift | Only 2 of ResolvedImageSource's 4 cases were exercised (.localFile, .failed). The .dataURI case is named in the new function's own doc comment as an explicit design goal — identical bytes in different documents SHOULD share an entry — with zero coverage; .remote was untested entirely. | Added svgKeyForResolvedSourceHandlesRemoteURLs (distinct remote URLs separate, same URL is stable) and svgKeyForResolvedSourceSharesEntryForIdenticalDataURIs (identical bytes share, differing bytes separate), pinning the sharing behaviour as intentional. |
| minor | prismTests/SnapshotCacheTests.swift | No test pinned that colorScheme and displayScale still fragment the key through the NEW overload. Existing T-917 coverage exercises only the old string overload, and the cross-overload equivalence test fixes scheme and scale at one pair — so a refactor that dropped either parameter from svgKey(for:) would pass the suite. | Added svgKeyForResolvedSourceFragmentsBySchemeAndScale, asserting all four (scheme x scale) combinations produce four distinct keys through the new overload. |
| minor | docs/agent-notes/image-support.md | The Task 16 section documents SnapshotCache.svgKey(source:colorScheme:) reached via ImageBlockView — a view retired in the T-1542 WebKit cutover, and the exact stale signature a future session would copy to reintroduce this bug. Pre-existing, but directly in this change's path, and CLAUDE.md directs that stale notes be fixed rather than left. | Added a 'Superseded' callout at the head of the section: ImageBlockView is gone, in-document SVGs go through PrismDocSchemeHandler and never touch SnapshotCache, and the two detail surfaces must key through svgKey(for:) on the resolved identity. Historical bullets retained below it. |
| minor | Full-suite test run | WebScrollabilityReportingTests ('a sustained trigger burst still reports within the debounce's maximum wait') and StatePersistenceTests ('save overwrites existing state for same session') failed in the full macOS run. The first failed on wall-clock timing (only 1 trigger observed in 0.27s under load); neither touches SnapshotCache or the image path. | Both re-run in isolation: 31/31 pass. Load-sensitive flakes on a shared machine, not regressions from this diff. Left as-is — modifying them would be papering over an environment characteristic, not fixing a bug. |
| minor | Full-suite test run (first attempt) | The first full run reported 234 failures. Analysis of the result bundle showed 233 with no recorded duration (never ran) and every reported failure message reading 'Test crashed with signal abrt' — the signature of one host abort cascading. The crash report shows a WebKit NSException on a Swift concurrency job; the two known crashers (SVGWebViewTests / T-1541, MermaidCSPSpikeTests / T-2219) were verified absent from the bundle, so a third exists. | Re-ran the full suite: the cascade did not reproduce (4403 passed / 2 failed), confirming it as a load-related environmental abort rather than anything introduced here. Not this branch's to fix — the diff contains no WebKit surface. |
| nit | prism/Views/ImageDetailWindow.swift | The private cacheKey(for:) helper is a one-line passthrough to resolved.cacheKey, while WebImageDetailSheet does the identical raster lookup inline. Pre-existing asymmetry, untouched by this branch and still live on the raster path. | Not changed. Removing it is unrelated cleanup on a bugfix branch and would widen the diff without improving the fix. |
| nit | Both call sites | ImageDetailWindow and WebImageDetailSheet now carry near-identical explanatory comments about the keying. | Not changed. The duplicated text is rationale, not logic — the logic is already deduplicated into the single key builder — and keeping it local means each call site explains itself without a jump to SnapshotCache. |
Click to expand.
diff --git a/prism/Services/SnapshotCache.swift b/prism/Services/SnapshotCache.swiftindex a8190e0..eea0424 100644--- a/prism/Services/SnapshotCache.swift+++ b/prism/Services/SnapshotCache.swift@@ -37,8 +37,15 @@ final class SnapshotCache: @unchecked Sendable { /// Display scale is included in the key so snapshots rasterised at /// different pixel densities never collide (T-917). ///+ /// `source` is hashed verbatim, so it must already be a string that+ /// identifies the image uniquely across the whole app. An authored,+ /// document-relative path is NOT such a string — two documents each+ /// referencing `./diagram.svg` produce the same key and collide, which+ /// was T-1866. Callers holding a `ResolvedImageSource` must use+ /// `svgKey(for:colorScheme:displayScale:)` instead of this overload.+ /// /// - Parameters:- /// - source: The SVG source content string.+ /// - source: SVG content, or an already-absolute identity string. /// - colorScheme: The current color scheme (light/dark). /// - displayScale: The destination display's scale factor. /// - Returns: A deterministic cache key string with "svg:" prefix.@@ -49,6 +56,29 @@ final class SnapshotCache: @unchecked Sendable { return "svg:" + hash.compactMap { String(format: "%02x", $0) }.joined() } + /// Generates a cache key for SVG snapshots from a resolved image source,+ /// color scheme, and display scale.+ ///+ /// Keys on `resolved.cacheKey` — the resolved absolute source identity+ /// (file URL, remote URL, or data URI hash) — rather than the raw+ /// markdown-relative path string a caller might otherwise use. Two+ /// different documents that both reference e.g. `./diagram.svg` resolve+ /// to different absolute identities and so no longer collide on one+ /// cache entry (T-1866). Callers at both the write site and the read+ /// site should build the key through this single function so the two+ /// can never disagree.+ ///+ /// - Returns: `nil` when `resolved` is `.failed` and has no stable+ /// identity to key on — callers should skip caching in that case.+ static func svgKey(+ for resolved: ResolvedImageSource,+ colorScheme: ColorScheme,+ displayScale: CGFloat+ ) -> String? {+ guard let identity = resolved.cacheKey else { return nil }+ return svgKey(source: identity, colorScheme: colorScheme, displayScale: displayScale)+ }+ /// Generates a cache key for mermaid snapshots from source, PrismTheme, /// and display scale. ///diff --git a/prism/Views/ImageDetailWindow.swift b/prism/Views/ImageDetailWindow.swiftindex 8ce3098..3ca6f25 100644--- a/prism/Views/ImageDetailWindow.swift+++ b/prism/Views/ImageDetailWindow.swift@@ -383,19 +383,17 @@ struct ImageDetailWindow: View { /// Loads an SVG image by checking the snapshot cache, then falling back to source loading + rendering. private func loadSVGImage(resolved: ResolvedImageSource) async {- // Use the same key format as ImageBlockView for cache consistency.- // Display scale is included so cache entries don't collide across- // displays with different backing scale factors. Pulled from the- // SwiftUI environment so the value tracks the screen this window is- // actually on (not the key window's screen).- let svgKey = SnapshotCache.svgKey(- source: data.imageSource,- colorScheme: colorScheme,- displayScale: displayScale- )+ // Keyed on the resolved absolute identity (not the raw markdown-relative+ // path string), so two documents that both reference e.g. "./diagram.svg"+ // don't collide on one cache entry (T-1866). Display scale is included so+ // cache entries don't collide across displays with different backing+ // scale factors. Pulled from the SwiftUI environment so the value tracks+ // the screen this window is actually on (not the key window's screen).+ // Nil when resolution failed (no stable identity) — caching is skipped.+ let svgKey = SnapshotCache.svgKey(for: resolved, colorScheme: colorScheme, displayScale: displayScale) // Check snapshot cache- if let cached = imageServices.snapshotCache.get(svgKey) {+ if let svgKey, let cached = imageServices.snapshotCache.get(svgKey) { loadState = .loaded(cached) return }@@ -407,7 +405,9 @@ struct ImageDetailWindow: View { colorScheme: colorScheme, displayScale: displayScale )- imageServices.snapshotCache.set(svgKey, image: snapshot)+ if let svgKey {+ imageServices.snapshotCache.set(svgKey, image: snapshot)+ } loadState = .loaded(snapshot) } catch let error as ImageLoadError { loadState = .failed(error.displayMessage)diff --git a/prism/Views/WebImageDetailSheet.swift b/prism/Views/WebImageDetailSheet.swiftindex c2ad367..2b70f45 100644--- a/prism/Views/WebImageDetailSheet.swift+++ b/prism/Views/WebImageDetailSheet.swift@@ -126,12 +126,12 @@ struct WebImageDetailSheet: View { } private func loadSVG(resolved: ResolvedImageSource) async {- let svgKey = SnapshotCache.svgKey(- source: request.source,- colorScheme: colorScheme,- displayScale: displayScale- )- if let cached = imageServices.snapshotCache.get(svgKey) {+ // Keyed on the resolved absolute identity, not the raw markdown-relative+ // path string, so two documents that both reference e.g. "./diagram.svg"+ // don't collide on one cache entry (T-1866). Nil when resolution failed+ // (no stable identity) — caching is skipped in that case.+ let svgKey = SnapshotCache.svgKey(for: resolved, colorScheme: colorScheme, displayScale: displayScale)+ if let svgKey, let cached = imageServices.snapshotCache.get(svgKey) { loadState = .loaded(cached) return }@@ -142,7 +142,9 @@ struct WebImageDetailSheet: View { colorScheme: colorScheme, displayScale: displayScale )- imageServices.snapshotCache.set(svgKey, image: snapshot)+ if let svgKey {+ imageServices.snapshotCache.set(svgKey, image: snapshot)+ } loadState = .loaded(snapshot) } catch let error as ImageLoadError { loadState = .failed(message: error.displayMessage, canGrantAccess: isAccessError(error))diff --git a/prismTests/SnapshotCacheTests.swift b/prismTests/SnapshotCacheTests.swiftindex f7dc6f4..1244f1d 100644--- a/prismTests/SnapshotCacheTests.swift+++ b/prismTests/SnapshotCacheTests.swift@@ -190,6 +190,112 @@ struct SnapshotCacheTests { #expect(Set([light1x, light2x, dark1x, dark2x]).count == 4) } + // MARK: - SVG Key From Resolved Source (T-1866)++ @Test("svgKey(for:) differs for the same relative path resolved against different documents")+ func svgKeyForResolvedSourceDiffersAcrossDocuments() {+ // Two documents that both reference "./diagram.svg" resolve to different+ // absolute file URLs. Regression for T-1866: keying on the raw relative+ // path string made these collide on one cache entry.+ let docA = ResolvedImageSource.localFile(URL(fileURLWithPath: "/Users/x/DocA/diagram.svg"))+ let docB = ResolvedImageSource.localFile(URL(fileURLWithPath: "/Users/x/DocB/diagram.svg"))++ let keyA = SnapshotCache.svgKey(for: docA, colorScheme: .light, displayScale: 2.0)+ let keyB = SnapshotCache.svgKey(for: docB, colorScheme: .light, displayScale: 2.0)++ #expect(keyA != nil)+ #expect(keyB != nil)+ #expect(keyA != keyB)+ }++ @Test("svgKey(for:) produces the same key for the same resolved document, enabling cache hits")+ func svgKeyForResolvedSourceEnablesCacheHitForSameDocument() {+ let resolved = ResolvedImageSource.localFile(URL(fileURLWithPath: "/Users/x/DocA/diagram.svg"))++ let firstLoad = SnapshotCache.svgKey(for: resolved, colorScheme: .light, displayScale: 2.0)+ let secondLoad = SnapshotCache.svgKey(for: resolved, colorScheme: .light, displayScale: 2.0)++ #expect(firstLoad != nil)+ #expect(firstLoad == secondLoad)++ // Prove it end-to-end through the cache: a write keyed by the first+ // resolution is a HIT on a read keyed by a second, independent+ // resolution of the same document — not just equal key strings.+ let cache = SnapshotCache()+ let image = makeTestImage()+ cache.set(firstLoad!, image: image)+ #expect(cache.get(secondLoad!) === image)+ }++ @Test("svgKey(for:) returns nil when resolution failed, so callers skip caching")+ func svgKeyForResolvedSourceIsNilOnFailure() {+ let failed = ResolvedImageSource.failed(.fileNotFound)+ let key = SnapshotCache.svgKey(for: failed, colorScheme: .light, displayScale: 2.0)+ #expect(key == nil)+ }++ @Test("svgKey(for:) matches svgKey(source:) built from the same resolved identity")+ func svgKeyForResolvedSourceMatchesSourceOverload() {+ let url = URL(fileURLWithPath: "/Users/x/DocA/diagram.svg")+ let resolved = ResolvedImageSource.localFile(url)++ let viaResolved = SnapshotCache.svgKey(for: resolved, colorScheme: .dark, displayScale: 3.0)+ let viaSource = SnapshotCache.svgKey(source: url.absoluteString, colorScheme: .dark, displayScale: 3.0)++ #expect(viaResolved == viaSource)+ }++ @Test("svgKey(for:) separates distinct remote URLs and reuses one entry per URL")+ func svgKeyForResolvedSourceHandlesRemoteURLs() {+ let one = ResolvedImageSource.remote(URL(string: "https://a.example/diagram.svg")!)+ let two = ResolvedImageSource.remote(URL(string: "https://b.example/diagram.svg")!)++ let keyOne = SnapshotCache.svgKey(for: one, colorScheme: .light, displayScale: 2.0)+ let keyTwo = SnapshotCache.svgKey(for: two, colorScheme: .light, displayScale: 2.0)+ let keyOneAgain = SnapshotCache.svgKey(for: one, colorScheme: .light, displayScale: 2.0)++ #expect(keyOne != nil)+ #expect(keyOne != keyTwo)+ #expect(keyOne == keyOneAgain)+ }++ @Test("svgKey(for:) shares one entry for byte-identical inline SVGs in different documents")+ func svgKeyForResolvedSourceSharesEntryForIdenticalDataURIs() {+ // Sharing here is correct, not a collision: a data URI carries its own+ // content, so identical bytes render identically no matter which+ // document embedded them. Differing bytes must still separate.+ let svg = Data("<svg xmlns=\"http://www.w3.org/2000/svg\"><rect/></svg>".utf8)+ let other = Data("<svg xmlns=\"http://www.w3.org/2000/svg\"><circle/></svg>".utf8)++ let inDocA = ResolvedImageSource.dataURI(svg, mimeType: "image/svg+xml")+ let inDocB = ResolvedImageSource.dataURI(svg, mimeType: "image/svg+xml")+ let different = ResolvedImageSource.dataURI(other, mimeType: "image/svg+xml")++ let keyA = SnapshotCache.svgKey(for: inDocA, colorScheme: .light, displayScale: 2.0)+ let keyB = SnapshotCache.svgKey(for: inDocB, colorScheme: .light, displayScale: 2.0)+ let keyDifferent = SnapshotCache.svgKey(for: different, colorScheme: .light, displayScale: 2.0)++ #expect(keyA != nil)+ #expect(keyA == keyB)+ #expect(keyA != keyDifferent)+ }++ @Test("svgKey(for:) still fragments by color scheme and display scale")+ func svgKeyForResolvedSourceFragmentsBySchemeAndScale() {+ // Pins T-917 through the new overload specifically: it must carry+ // scheme and scale through, not just the resolved identity.+ let resolved = ResolvedImageSource.localFile(URL(fileURLWithPath: "/Users/x/DocA/diagram.svg"))++ let light1x = SnapshotCache.svgKey(for: resolved, colorScheme: .light, displayScale: 1.0)+ let light2x = SnapshotCache.svgKey(for: resolved, colorScheme: .light, displayScale: 2.0)+ let dark1x = SnapshotCache.svgKey(for: resolved, colorScheme: .dark, displayScale: 1.0)+ let dark2x = SnapshotCache.svgKey(for: resolved, colorScheme: .dark, displayScale: 2.0)++ let keys = [light1x, light2x, dark1x, dark2x].compactMap { $0 }+ #expect(keys.count == 4)+ #expect(Set(keys).count == 4)+ }+ // MARK: - Clear @Test("Clear removes all entries")diff --git a/CHANGELOG.md b/CHANGELOG.mdindex dec5fa0..1b10f3f 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 +- Opening an SVG image full screen now shows that document's image, rather than one belonging to a different document (T-1866). Two documents that each referred to an SVG by the same relative name — `./diagram.svg` sitting next to each file, say — shared a single entry in the store of rendered previews, because that store was keyed on the name written in the markdown instead of on the file the name actually resolves to. Whichever document you opened first won: opening the second document's diagram showed you the first one's picture, with nothing to indicate it was the wrong one, and it kept doing so for the rest of the session. Rendered SVG previews are now keyed on the resolved image itself — a file's full path, a remote address, or the contents of an inline image — so same-named images in different documents can no longer stand in for one another. Reopening the same image in the same document still comes back instantly from the store, as before, and two documents that embed a byte-for-byte identical inline image do still share one rendered preview, which is correct: identical content renders identically. - 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. diff --git a/docs/agent-notes/image-support.md b/docs/agent-notes/image-support.mdindex f40cf65..e914f69 100644--- a/docs/agent-notes/image-support.md+++ b/docs/agent-notes/image-support.md@@ -116,6 +116,8 @@ ## ImageBlockView SVG routing and tap-to-zoom (Task 16) +> **Superseded — read this before the bullets below.** `ImageBlockView` was retired in the T-1542 WebKit cutover. In-document SVGs are now served as sanitized bytes by `PrismDocSchemeHandler` and rasterised by WebKit in-page, with no `SnapshotCache` involvement at all. `SnapshotCache` is used for SVGs only by the two full-screen detail surfaces (`ImageDetailWindow`, `WebImageDetailSheet`), and both build the key through `SnapshotCache.svgKey(for:colorScheme:displayScale:)`, which keys on `ResolvedImageSource.cacheKey` — the resolved absolute identity (file URL, remote URL, or data-URI content hash) — **not** the authored path. Keying on the authored path was T-1866: two documents each referencing `./diagram.svg` collided on one entry and showed each other's snapshot. Build the read key and the write key through that one function; a `nil` return means resolution failed and the caller must skip caching. The bullets below record the original Task 16 design and are kept as history.+ - `ImageBlockView` now accepts `width: ImageDimension?` and `height: ImageDimension?` parameters (from HTML `<img>` attributes). - `TextualBlockView` passes width/height through to `ImageBlockView` instead of discarding with `_, _`. - SVG detection: after `resolveOnce()`, checks `ImagePathResolver.isSVG(source:resolved:)` to route SVG vs raster.Every SVG snapshot key changes with this build, so the first open of any SVG after upgrading re-rasterises. SnapshotCache is an in-memory NSCache with no on-disk persistence, so this costs exactly one render per image per session and nothing needs migrating — but it is worth having consciously accepted rather than discovered.
svgKey(source:colorScheme:displayScale:) is still reachable, and nothing in the type system stops a future caller passing an authored path to it. This review strengthened its documentation, which is proportionate for a bugfix branch. If this class of bug recurs, the durable fix is a type-level one — make the string overload private and have callers reach it only through a resolved identity, or introduce a distinct SnapshotIdentity wrapper type that an authored path cannot be silently coerced into.
Coverage sits entirely at the key-builder level; nothing asserts that ImageDetailWindow.loadSVGImage and WebImageDetailSheet.loadSVG actually call the new overload. This matches the existing convention (mermaidKey and svgKey(source:) have no call-site tests either) and both migrations are two-line mechanical changes, so the risk is low — but this is the same shape as the T-1943 lesson recorded in CLAUDE.md, where a correct component passed all its direct-invocation tests while never being wired up in production.
The first run's abort was not SVGWebViewTests (T-1541) or MermaidCSPSpikeTests (T-2219) — both were confirmed excluded from the result bundle. Something else aborts the host under sequential load. Unrelated to this branch and out of scope here, but it will keep costing full-suite runs their legibility until it is identified, and is worth a ticket.