Pre-push review: T-1759/bugfix-raw-source-recolor-race

diff --git a/prism/ViewModels/RawSourceViewModel.swift b/prism/ViewModels/RawSourceViewModel.swiftindex d3372b4..56db394 100644--- a/prism/ViewModels/RawSourceViewModel.swift+++ b/prism/ViewModels/RawSourceViewModel.swift@@ -72,6 +72,20 @@ final class RawSourceViewModel {     /// Content hash being loaded, used to prevent stale results from overwriting.     private var loadingContentHash: Int? +    /// Identifies the document currently assigned to `lines`.+    ///+    /// Bumped every time a parse writes a document into `lines`, and by+    /// `reset()`. A recolour never bumps it — recolouring restyles the very+    /// lines it snapshotted, so the content identity is unchanged.+    ///+    /// This is what makes parse-vs-recolour ordering safe (T-1759). Cancellation+    /// alone is not enough: `loadContent` can only cancel recolour tasks that+    /// already exist, so a theme change that *starts* after a reload's parse has+    /// begun snapshots the pre-reload lines and, if it finishes last, would+    /// otherwise overwrite the freshly parsed document. Comparing this+    /// generation at write-back time drops that stale result.+    private var linesGeneration: Int = 0+     /// Current parsing task (for cancellation on new content).     private var parseTask: Task<Void, Never>? @@ -177,6 +191,7 @@ final class RawSourceViewModel {                 }                  self.lines = computedLines+                self.linesGeneration &+= 1                 self.maxLineLength = computedLines.reduce(0) { max($0, $1.text.count) }                 self.isLoading = false             }@@ -192,31 +207,61 @@ final class RawSourceViewModel {     ///     /// - Parameter colors: New theme colors to apply.     func recolor(with colors: any ThemeColors) async {-        guard !lines.isEmpty else { return }+        guard let snapshot = makeRecolorSnapshot() else { return }          // Cancel previous recolor if in progress         recolorTask?.cancel() -        let currentLines = lines         let parser = RawSourceHighlightParser()          recolorTask = Task.detached(priority: .userInitiated) { [colors] in-            let recolored = parser.recolor(currentLines, colors: colors)+            let recolored = parser.recolor(snapshot.lines, colors: colors)              await MainActor.run { [weak self] in                 guard let self = self, !Task.isCancelled else { return }-                self.lines = recolored+                self.applyRecolor(recolored, from: snapshot)             }         }          await recolorTask?.value     } +    /// The lines a recolour operates on, tagged with the generation they belong to.+    ///+    /// Recolouring is split into snapshot and apply halves so the ordering+    /// guarantee is expressed in the model rather than left to task scheduling+    /// (T-1759) — and so it can be tested deterministically.+    struct RecolorSnapshot: Sendable {+        let lines: [HighlightedSourceLine]+        let generation: Int+    }++    /// Captures the lines to recolour together with the generation they belong to.+    ///+    /// Returns `nil` when there is nothing to recolour.+    func makeRecolorSnapshot() -> RecolorSnapshot? {+        guard !lines.isEmpty else { return nil }+        return RecolorSnapshot(lines: lines, generation: linesGeneration)+    }++    /// Applies recoloured lines, but only while the snapshot they came from is+    /// still the content on screen.+    ///+    /// A generation mismatch means a parse (or a `reset()`) landed after the+    /// snapshot was taken, so these lines describe a document that is no longer+    /// displayed. Writing them back would resurrect pre-reload content (T-1759).+    func applyRecolor(_ recolored: [HighlightedSourceLine], from snapshot: RecolorSnapshot) {+        guard linesGeneration == snapshot.generation else { return }+        lines = recolored+    }+     /// Resets state when toggling away from raw source view.     func reset() {         parseTask?.cancel()         recolorTask?.cancel()         lines = []+        // Invalidate any recolour snapshot taken before the reset (T-1759).+        linesGeneration &+= 1         isLoading = true         isDocumentTooLarge = false         maxLineLength = 0

diff --git a/prismTests/RawSourceViewModelHighlightingTests.swift b/prismTests/RawSourceViewModelHighlightingTests.swiftindex c9106c5..964c388 100644--- a/prismTests/RawSourceViewModelHighlightingTests.swift+++ b/prismTests/RawSourceViewModelHighlightingTests.swift@@ -141,7 +141,10 @@ struct RawSourceViewModelHighlightingTests {          // The second content should be loaded (first was cancelled or overwritten)         #expect(viewModel.lines.count == 1)-        #expect(viewModel.lines[0].text == "Short second content")+        // `.first` rather than `[0]`: this test races two loads on purpose, and an+        // out-of-range subscript traps the whole test host, turning one failure+        // into a suite-wide cascade.+        #expect(viewModel.lines.first?.text == "Short second content")         #expect(!viewModel.isLoading)     } @@ -216,6 +219,85 @@ struct RawSourceViewModelHighlightingTests {         #expect(viewModel.lines.isEmpty)     } +    // MARK: - Parse vs Recolor Ordering (T-1759)++    // These drive the two halves of `recolor(with:)` — `makeRecolorSnapshot()`+    // and `applyRecolor(_:from:)` — directly, so the interleaving is fixed by+    // the call order rather than by how fast the detached tasks happen to run.+    // A timing-based version of this test would be flaky (T-1541).++    @Test("recolor snapshotted before a reload does not overwrite the new content")+    func recolorDoesNotOverwriteNewerParse() async throws {+        // T-1759: while raw source is visible, a theme change can start *after*+        // a reload's parse has begun. It snapshots the pre-reload lines and, if+        // it finishes last, must not replace the freshly parsed document.+        let viewModel = RawSourceViewModel()+        let lightColors = LightThemeColors()+        let darkColors = DarkThemeColors()++        await viewModel.loadContent("# Old document", colors: lightColors, highlightingEnabled: true)++        // The theme change snapshots whatever is on screen right now.+        let snapshot = try #require(viewModel.makeRecolorSnapshot())+        #expect(snapshot.lines.map(\.text) == ["# Old document"])++        // The reload's parse lands while that recolor is still running off-main.+        await viewModel.loadContent("# New document", colors: lightColors, highlightingEnabled: true)+        #expect(viewModel.lines.map(\.text) == ["# New document"])++        // The stale recolor finishes last. Expected: dropped.+        // Before the fix: it overwrote `lines` with the pre-reload content.+        let recolored = RawSourceHighlightParser().recolor(snapshot.lines, colors: darkColors)+        viewModel.applyRecolor(recolored, from: snapshot)++        #expect(viewModel.lines.map(\.text) == ["# New document"])+    }++    @Test("recolor applies when no reload happened after the snapshot")+    func recolorAppliesWhenGenerationUnchanged() async throws {+        // The T-1759 guard must not block the ordinary theme-change path.+        let viewModel = RawSourceViewModel()+        let lightColors = LightThemeColors()+        let darkColors = DarkThemeColors()++        await viewModel.loadContent("# Header", colors: lightColors, highlightingEnabled: true)++        let snapshot = try #require(viewModel.makeRecolorSnapshot())+        let recolored = RawSourceHighlightParser().recolor(snapshot.lines, colors: darkColors)+        viewModel.applyRecolor(recolored, from: snapshot)++        let applied = try #require(viewModel.lines.first)+        #expect(applied.text == "# Header")+        #expect(applied.attributedText != nil)+        #expect(!applied.tokens.isEmpty)+    }++    @Test("reset invalidates a recolor snapshot taken before it")+    func resetInvalidatesRecolorSnapshot() async throws {+        // A recolor in flight when the view is toggled away must not repopulate+        // `lines` after reset cleared them.+        let viewModel = RawSourceViewModel()+        let lightColors = LightThemeColors()+        let darkColors = DarkThemeColors()++        await viewModel.loadContent("# Header", colors: lightColors, highlightingEnabled: true)++        let snapshot = try #require(viewModel.makeRecolorSnapshot())+        viewModel.reset()++        let recolored = RawSourceHighlightParser().recolor(snapshot.lines, colors: darkColors)+        viewModel.applyRecolor(recolored, from: snapshot)++        #expect(viewModel.lines.isEmpty)+    }++    @Test("makeRecolorSnapshot returns nil when there are no lines")+    func recolorSnapshotNilWhenEmpty() {+        let viewModel = RawSourceViewModel()++        #expect(viewModel.makeRecolorSnapshot() == nil)+    }+     // MARK: - Document Size Threshold Tests (Req 9.4)      @Test("document exceeding 15000 lines disables highlighting")@@ -345,7 +427,9 @@ struct RawSourceViewModelHighlightingTests {          // Final content should be the last one loaded         #expect(viewModel.lines.count == 1)-        #expect(viewModel.lines[0].text == "Third content")+        // See `loadContentCancelsPreviousParse` — `.first` keeps a racy mismatch+        // from trapping the test host.+        #expect(viewModel.lines.first?.text == "Third content")     }      // MARK: - Reset Tests

diff --git a/docs/agent-notes/raw-source-view.md b/docs/agent-notes/raw-source-view.mdindex dc1e0a1..4b00f7d 100644--- a/docs/agent-notes/raw-source-view.md+++ b/docs/agent-notes/raw-source-view.md@@ -57,6 +57,23 @@ SwiftUI's `ScrollView([.vertical, .horizontal])` on macOS centers its content in  `RawSourceViewModel.loadContent` spawns detached tasks. Multiple tasks can overlap when `loadContent` is called rapidly. The `isLoading` state must only be cleared by the *active* task (the one whose content hash matches `loadingContentHash`). Using `defer { self.isLoading = false }` before the freshness guard allows stale tasks to clear loading prematurely. See T-700. +### Recolour Must Validate `linesGeneration`, Not Just Cancellation (T-1759)++`RawSourceViewModel` runs two off-main pipelines that both write `lines`: `loadContent` (parse) and `recolor` (restyle a snapshot). Task cancellation only covers work that *already exists* — `loadContent` cancels the current `recolorTask`, but it cannot cancel a recolour that starts afterwards. A theme change firing after a reload's parse has begun therefore snapshots the pre-reload lines and, if it finishes last, overwrites the freshly parsed document.++The guard is `linesGeneration`: a counter bumped by every parse write-back and by `reset()`, never by a recolour (a recolour restyles the same content, so the identity is unchanged). `recolor(with:)` is split into `makeRecolorSnapshot()` and `applyRecolor(_:from:)`; the apply half drops the result when the generation has moved on.++Two consequences for future edits:++- Do NOT add a code path that assigns `lines` without either bumping `linesGeneration` (new content) or going through `applyRecolor` (restyle). A bare assignment reopens the race.+- Do NOT try to fix this class of bug with a *loading* hash. By the time the racing recolour takes its snapshot, `loadingContentHash` already refers to the new document, so it matches and lets the stale write through. The generation must describe the content currently *assigned* to `lines`, not the one being loaded.++`RawSourceViewModelHighlightingTests` drives the two halves directly so the interleaving is fixed by call order rather than by timing (this repo has a history of flaky timing-threshold tests — T-1541).++### Concurrency Tests in This Area Must Not Subscript `lines` Unguarded++`loadContentCancelsPreviousParse` and `staleParseResultsNotApplied` race two and three overlapping `loadContent` calls via `async let`, whose start order is not guaranteed, and they fail on `main` — reliably in a macOS run, not just occasionally. Their assertions used to read `lines[0]`, which traps when the losing load wins, killing the test *host* so every queued test in the run reports as failed at 0.000s. Measured on 2026-07-26: `main` reported 101 failures, 97 of them at 0.000s; with the subscripts guarded the same run reports only the two honest failures. Use `lines.first?` / `try #require(lines.first)` for any assertion in a test that races loads. (`RawSourceViewModelTests.staleTaskDoesNotClearLoading` races the same way and already does.)+ ### Code Fence Pattern Must Use `\S*` Not `\w*` for Language IDs  `RawSourceHighlightParser.codeFencePattern` uses `\S*` (non-whitespace) for the language identifier capture group. Do NOT change this to `\w*` — language IDs like `c++`, `c#`, `objective-c`, `f#`, and `node.js` contain non-word characters that `\w` would reject. See T-485.

diff --git a/CHANGELOG.md b/CHANGELOG.mdindex c740e3a..578bfe6 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 +- Changing the theme while viewing raw source no longer brings back the previous version of the document (T-1759). If the file changed on disk — or a URL document was refreshed — at the same moment the colours changed, the recolour worked from a snapshot taken before the reload and, finishing last, put the old text back on screen, where it stayed until the next reload or raw/rendered toggle. Recolouring now only applies while the lines it started from are still the ones being shown. - The system **Increase Contrast** accessibility setting applies to the document again (T-1829). Since the WebKit rendering cutover, turning it on changed the app's own interface but left the document body untouched: search highlights and footnote badges stayed translucent and tertiary text stayed low-contrast. The rendered document now uses the same higher-contrast search, footnote, and tertiary colours as the rest of the app, on every theme, and the deliberately-faded "add note" button beside each block is shown at full strength. It follows the setting live — toggling it while a document is open updates immediately, without a reload and without losing your reading position, and the styling survives a WebKit process recovery. Footnote popovers keep the standard palette. - 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. - The saved reading position is no longer corrupted by content the document does not show (T-1851). Hidden content measures as sitting exactly at the top of the window, which beat every genuinely visible block, so the document reported a block the reader could not see as the block being read. That happened with any heading collapsed, and — because the carrier holding a document's YAML frontmatter is hidden the same way — on every document that starts with frontmatter, collapsed or not. Reopening the document, returning from raw source, or recovering from a rendering-process restart then landed on the wrong block or did nothing at all. Content the document does not lay out is now skipped when working out the reading position.