Pre-push review: T-1744 loading indicator

diff --git a/prism/Views/DocumentScrollContent.swift b/prism/Views/DocumentScrollContent.swiftindex 7304962..59931ff 100644--- a/prism/Views/DocumentScrollContent.swift+++ b/prism/Views/DocumentScrollContent.swift@@ -41,6 +41,14 @@ struct DocumentScrollContent: View {     /// the session and the theme feed below can route through it (T-1719).     @State private var synchronizer: WebDocumentStateSynchronizer? +    /// Drives the "Preparing document…" overlay (T-1744). A plain `@State` flag rather+    /// than reading `webController?.isLayoutSettled` directly from a `ViewBuilder`: the+    /// earlier attempt at this overlay (T-1681, reverted — see the commit history for+    /// `documentLoadingOverlay`) did that and never appeared on device in either of its+    /// two gated forms, so this flag is instead flipped imperatively by the load task+    /// below, decoupling its visibility from whatever made that read unreliable.+    @State private var isPreparingDocument = false+     /// Routes activated links from their RAW href string, the way the SwiftUI path's     /// `openURL` handler does but without a `URL(string:)` round-trip that would     /// double-escape mixed encoded/raw paths (scheme allowlist applied by the handler,@@ -85,6 +93,18 @@ struct DocumentScrollContent: View {             }         }         .background(context.colors.background)+        // A progress indicator shown while the document HTML is built off the MainActor+        // and WebKit loads + lays it out (T-1744). Emitting + per-block SwiftSoup+        // sanitisation of a large or HTML-heavy document — and WebKit laying out the+        // resulting DOM — takes a moment; without this the responsive-but-blank page+        // looks broken. Applied BEFORE the recovery-abandoned overlay below so that+        // banner always draws on top in the window where both are true. That window is+        // not a single frame: the wait polls at 100ms and the overlay then fades out+        // over 0.2s, so roughly 300ms of overlap in the normal case, and longer if this+        // task was cancelled at that moment (the flag then belongs to its successor).+        // Harmless because of the ordering, not because it is brief.+        .overlay { documentLoadingOverlay }+        .animation(reduceMotion ? nil : .easeInOut(duration: 0.2), value: isPreparingDocument)         // The renderer crashed repeatedly and recovery gave up (T-1943). Without this         // the reader sees the ORIGINAL symptom — a blank document, no error, close and         // reopen the only way out — recorded solely in os_log. The reload here is a@@ -219,6 +239,21 @@ struct DocumentScrollContent: View {             revision: context.session.parseRevision         )) {             guard let webController else { return }+            // Raised at the top of every incarnation of this task that will actually+            // load something (T-1744): a fresh reparse re-keys `WebLoadKey` and starts a+            // NEW task before this one necessarily knows it has been superseded (T-1975 —+            // `Task.detached`'s `.value` ignores the awaiter's cancellation, so the+            // outgoing task can still be suspended inside the emit below). Setting it+            // again here is a harmless no-op if it is already true.+            //+            // `shouldIndicatePreparation` is what pairs the flag with a real load:+            // `loadDocument` is a no-op before the first parse lands, and this task can+            // reach it in that state, so raising the flag unconditionally would put a+            // spinner over a controller that was never told to load (T-1744 review).+            let willLoad = DocumentLoadingIndicatorPolicy.shouldIndicatePreparation(+                parseRevision: context.session.parseRevision+            )+            if willLoad { isPreparingDocument = true }             // The whole sequence — supersede the visible page, emit the HTML off the             // MainActor, navigate, then offer the stored reading position — lives on the             // factory so its ORDER is one testable thing (T-1975). Keep this call site a@@ -229,6 +264,21 @@ struct DocumentScrollContent: View {                 session: context.session,                 settings: context.settings             )+            guard willLoad else { return }+            // Keeps the overlay up for the rest of the window: `loadDocument`'s own+            // await covers issuing the navigation, not the page settling. Every way this+            // returns is enumerated on `DocumentLoadingIndicatorPolicy` — it is bounded,+            // so it cannot outlive its load.+            await DocumentLoadingIndicatorPolicy.waitForPreparation(webController)+            // A cancelled task was superseded by a fresher `.task(id:)` instance (a+            // newer parse revision) that has ALREADY set the flag for its own load —+            // clearing it here would flicker the spinner off mid-load for the revision+            // actually on screen. Only the task that ran to completion, uncancelled, may+            // clear what it set (T-1744; mirrors the announcement-id discipline+            // `beginLoad`/`abandonLoad` use for the same overlap, one layer down).+            if DocumentLoadingIndicatorPolicy.mayClearFlag(wasCancelled: Task.isCancelled) {+                isPreparingDocument = false+            }         }         .onChange(of: context.session.pendingFootnoteId) { _, newId in             if let identifier = newId {@@ -296,6 +346,19 @@ struct DocumentScrollContent: View {      #endif +    /// A progress indicator shown while the document HTML is built off the MainActor and+    /// the web view loads and lays it out (T-1744). Gated on the plain `isPreparingDocument`+    /// flag (see its declaration) rather than on `webController?.isLayoutSettled` directly.+    @ViewBuilder+    private var documentLoadingOverlay: some View {+        if isPreparingDocument {+            ProgressView("Preparing document…")+                .controlSize(.large)+                .frame(maxWidth: .infinity, maxHeight: .infinity)+                .background(context.colors.background)+        }+    }+     /// Banner offering a reload after crash recovery gave up (T-1943). `recoveryAbandoned`     /// is observed state on the controller, so this appears without any push from here.     @ViewBuilder@@ -470,3 +533,146 @@ extension WebContrastMode {         self = contrast == .increased ? .increased : .standard     } }++/// Decision logic and the wait loop behind the "Preparing document…" overlay (T-1744),+/// extracted from the view so the invariants that keep it from stranding the UI are+/// testable against a real `WebDocumentController` without mounting a SwiftUI view.+///+/// **Every exit path from the wait, and the signal that terminates it.** The overlay is+/// only ever as good as this list being exhaustive — an indicator that outlives its load+/// is a worse bug than the blank page it replaced, because it also hides the document.+///+/// | # | Exit | Signal | Who raises it |+/// |---|------|--------|---------------|+/// | 1 | The page laid out — the normal path | `isLayoutSettled` | `markLayoutSettled`, from the page's `layoutSettled` bridge message |+/// | 2 | The renderer crashed repeatedly and recovery gave up | `recoveryAbandoned` | `WebDocumentController.attemptRecovery`, budget spent |+/// | 3 | The document's own navigation failed, with no retry coming | `navigationFailedBeforeSettling` | `WebDocumentController.issueNavigation`, when THIS load's own navigation sequence fails before layout |+/// | 4 | Nothing above arrived within `maximumWait` | the deadline | this loop |+/// | 5 | The load task was cancelled | `Task.isCancelled` | SwiftUI re-keying `.task(id:)`, or the view going away |+///+/// Exit 3 exists because exits 1 and 2 do not cover an ordinary navigation failure:+/// `.navigationFailed` outside a recovery is deliberately classified as a benign bad+/// link and returns without charging the budget (T-1943/T-2107), so `recoveryAbandoned`+/// is unreachable through it and `isLayoutSettled` never rises. Exit 3 reads that case+/// without reclassifying it — see `navigationFailedBeforeSettling`. It is scoped to+/// failures with no retry behind them, so a recovery's own intermediate failures still+/// resolve through exits 1 and 2 rather than dropping the indicator mid-reload.+///+/// Exit 3 is also the one exit that has to know WHOSE navigation failed. Superseding an+/// in-flight load is what makes WebKit fail it, so the stale failure always lands after+/// its successor is under way; read off the page-wide navigation stream it was+/// indistinguishable from the successor's own failure, and ended the successor's wait+/// over a document still loading. The signal is therefore taken from the per-navigation+/// sequence and matched by epoch — see `WebDocumentController.navigationFailedBeforeSettling`.+///+/// Exit 4 is the honest admission that 1–3 are the failures we know about. It is a+/// backstop, not a timeout in the usual sense: `maximumWait` is set far beyond any real+/// load so it cannot fire on a slow one, and when it does fire the overlay simply steps+/// aside and reveals whatever the web view holds. Revealing a blank page is strictly+/// better than covering it with a spinner forever.+///+/// Exit 5 is the one exit that must NOT clear the flag — see `mayClearFlag`. Two+/// `.task(id:)` instances of `DocumentScrollContent`'s load task can be live at once: the+/// outgoing one still suspended inside an uninterruptible off-main emit (`Task.detached`'s+/// `.value` ignores the awaiter's cancellation, same as the `beginLoad`/`abandonLoad`+/// overlap this mirrors, T-1975) while a new one has already started and raised the flag+/// for its own revision. Only the task that finishes UNCANCELLED may lower what it raised.+///+/// Cancellation without a successor — the view unmounting on a raw-source toggle or a+/// document close — reaches the same rule, and there the flag is simply discarded with+/// the view's `@State`. So the rule is safe in both shapes, but only the supersession+/// shape is the one it exists FOR.+enum DocumentLoadingIndicatorPolicy {+    /// How often the wait re-reads the controller. It polls rather than observes: the wait+    /// runs outside any `ViewBuilder`, so SwiftUI's Observation-driven invalidation does+    /// nothing for it, and `navigationFailedBeforeSettling` is computed from+    /// `@ObservationIgnored` storage in any case.+    static let pollInterval: Duration = .milliseconds(100)++    /// The backstop bound for exit 4.+    ///+    /// Deliberately generous. The window this overlay covers — emitting and+    /// SwiftSoup-sanitising a large or HTML-heavy document off the MainActor, then+    /// WebKit laying the resulting DOM out — runs to the better part of a minute at the+    /// 10 MB document limit, and a backstop that fired on a legitimately slow load would+    /// re-create the exact bug being fixed (spinner vanishes, page still blank) on the+    /// documents that need the indicator most. Two minutes leaves that case ample room+    /// while still bounding a stall that none of exits 1–3 caught.+    static let maximumWait: Duration = .seconds(120)++    /// Whether this load will actually issue a navigation, and so whether the indicator+    /// may be raised for it at all (T-1744 review).+    ///+    /// `WebDocumentControllerFactory.loadDocument` returns without calling `beginLoad` or+    /// `load` when nothing has been parsed yet (`guard revision > 0`), and the mount task+    /// that calls it races `DocumentReaderView`'s own parse task with no sequencing+    /// between them — so a `revision == 0` incarnation is reachable, and raising the flag+    /// there would leave it paired with no load. It self-heals when the parse lands and+    /// re-keys the task, but "usually superseded in time" is not something to rest on, and+    /// this gate is cheap.+    ///+    /// It does NOT make "the flag is only ever raised for a load that is actually issued"+    /// an invariant, and the earlier wording here claimed it did. `loadDocument` has a+    /// second early return — `guard !Task.isCancelled` after the off-main emit, which+    /// calls `abandonLoad` and issues nothing — and the flag is already up by then. That+    /// case is safe for a different reason: the same cancellation ends the wait through+    /// exit 5 and `mayClearFlag` then declines, leaving the flag to whichever task+    /// superseded this one (or to the discarded `@State` if the view went away). What this+    /// gate establishes is exactly its own condition: the flag is only raised once+    /// something has been parsed.+    static func shouldIndicatePreparation(parseRevision: UInt64) -> Bool {+        parseRevision > 0+    }++    /// Whether the wait should keep polling. Exits 1–4 of the table above; exit 5+    /// (cancellation) is checked by the loop itself.+    static func shouldContinueWaiting(+        isLayoutSettled: Bool,+        recoveryAbandoned: Bool,+        navigationFailedBeforeSettling: Bool,+        deadlineElapsed: Bool+    ) -> Bool {+        !isLayoutSettled && !recoveryAbandoned && !navigationFailedBeforeSettling && !deadlineElapsed+    }++    /// Whether the task that just finished waiting may clear `isPreparingDocument`.+    static func mayClearFlag(wasCancelled: Bool) -> Bool {+        !wasCancelled+    }++    /// Runs the wait: polls `controller` until one of exits 1–5 fires, then returns.+    ///+    /// Lives here rather than on the view so a test can run the REAL loop against a real+    /// `WebDocumentController` and assert it terminates — the failure being guarded is a+    /// loop that never returns, which no test over `shouldContinueWaiting` alone can see.+    ///+    /// Reads the controller's plain properties directly, deliberately outside any+    /// `ViewBuilder`: the wait needs nothing from SwiftUI's Observation-driven re-render+    /// to see the current value, which is the whole point of moving off the pattern the+    /// reverted T-1681 attempt used.+    ///+    /// `deadline` is injectable so a test can drive exit 4 without waiting out+    /// `maximumWait`. Defaulted to nil and resolved in the body rather than defaulted to+    /// `.now + maximumWait`: a default-argument expression is evaluated in a nonisolated+    /// context, so reading the MainActor-isolated `maximumWait` there is a concurrency+    /// warning — and this repo builds at zero warnings.+    @MainActor+    static func waitForPreparation(+        _ controller: WebDocumentController,+        deadline: ContinuousClock.Instant? = nil+    ) async {+        let deadline = deadline ?? .now + maximumWait+        while !Task.isCancelled+            && shouldContinueWaiting(+                isLayoutSettled: controller.isLayoutSettled,+                recoveryAbandoned: controller.recoveryAbandoned,+                navigationFailedBeforeSettling: controller.navigationFailedBeforeSettling,+                deadlineElapsed: ContinuousClock.now >= deadline+            ) {+            // `try?`: a cancelled sleep throws, and the loop condition above is what+            // acts on that — swallowing it here would spin, checking `Task.isCancelled`+            // is what exits.+            try? await Task.sleep(for: pollInterval)+        }+    }+}

diff --git a/prism/ViewModels/WebDocumentController.swift b/prism/ViewModels/WebDocumentController.swiftindex cc86471..1b62f2d 100644--- a/prism/ViewModels/WebDocumentController.swift+++ b/prism/ViewModels/WebDocumentController.swift@@ -810,15 +810,72 @@ final class WebDocumentController {         startNavigationObservation()         resetForNavigation()         scheduleSnapshotReplay()-        Task { [page] in+        // The previous load's navigation failure is retired here too — implicitly, by+        // the epoch this bumps, rather than by a flag assignment (T-1744 review). See+        // `navigationFailedBeforeSettling`: a stale failure is one whose epoch is no+        // longer current, and the load that superseded it is exactly what makes it so.+        issueNavigation(to: documentURL, describedAs: "Document load failed")+    }++    /// Starts the page navigation for a load or a recovery reload, tagged with the+    /// navigation epoch it belongs to.+    ///+    /// The single place THIS type tells its page to load, which is what makes the epoch a+    /// faithful identity: every navigation this controller issues gets one, and no+    /// navigation gets two. (Other pages elsewhere in the app — `FootnotePopoverWebPage` —+    /// have their own call sites; the structural pin in+    /// `DocumentLoadingIndicatorWiringTests` greps this file only, which is the scope the+    /// invariant needs.) The sequence the load returns is scoped to THAT navigation — unlike+    /// `page.navigations`, which reports every navigation on the page through one stream —+    /// so its failure is the one signal here that arrives already attributed. That is why+    /// `navigationFailedBeforeSettling` is recorded from here and not from+    /// `applyNavigationOutcome` (T-1744 review).+    ///+    /// The task captures `page` and weakens `self` for the same reason the observation+    /// loop does: it must not keep the controller alive across its `await`, or `deinit`+    /// could never run while a navigation was outstanding.+    private func issueNavigation(to documentURL: URL, describedAs label: StaticString) {+        navigationEpoch &+= 1+        let epoch = navigationEpoch+        Task { [weak self, page] in             do {                 for try await _ in page.load(URLRequest(url: documentURL)) {}             } catch {-                Self.logger.error("Document load failed: \(error.localizedDescription)")+                Self.logger.error("\(label, privacy: .public): \(error.localizedDescription)")+                self?.recordNavigationFailure(error, epoch: epoch)             }         }     } +    /// Records a failed navigation for the loading indicator's exit 3, tagged with the+    /// epoch of the navigation that failed (T-1744 review).+    ///+    /// Only ordinary failures are recorded. A termination is the crash-recovery path's+    /// business and is classified identically to the way the observation loop classifies+    /// it, through the shared `classify`, so the two can never disagree about what a+    /// given error means — a termination racing this recorder must not drop the indicator+    /// out from under the reload that is about to fill the page.+    private func recordNavigationFailure(_ error: any Error, epoch: UInt64) {+        // A failure that is not the CURRENT navigation's has nothing to say about the+        // current wait, in EITHER direction (T-1744 review). Reading it is already+        // handled — a stale epoch can never equal the current one — but WRITING it is+        // not: two navigations' failure tasks are unordered, so a superseded load's late+        // failure can land after the successor's own and, unguarded, would overwrite a+        // correctly-raised exit 3 with an epoch that no longer matches. The wait would+        // then fall through to the 120-second backstop over a document that has already+        // failed. `navigationEpoch` is monotone, so dropping a non-current epoch here can+        // never discard a failure that is about to become relevant.+        guard epoch == navigationEpoch else { return }+        guard Self.classify(error) == .navigationFailed else { return }+        // Benign for RECOVERY purposes is not the same as benign for the reader+        // (T-1744) — but a failure a recovery is retrying is neither. That path already+        // ends the wait both ways (the retry reaches `layoutSettled`, or the budget+        // spends and raises `recoveryAbandoned`), so recording it here would drop the+        // indicator mid-recovery and reveal the blank page the reload is about to fill.+        guard !recoveryInFlight, !isLayoutSettled else { return }+        failedNavigationEpoch = epoch+    }+     /// Why a recovery reload is being started.     ///     /// Diagnostics only — the recovery itself is identical in all three cases — but a@@ -863,13 +920,10 @@ final class WebDocumentController {         resetForNavigation()         scheduleSnapshotReplay()         startRecoveryWatchdog()-        Task { [page] in-            do {-                for try await _ in page.load(URLRequest(url: documentURL)) {}-            } catch {-                Self.logger.error("Recovery reload failed: \(error.localizedDescription)")-            }-        }+        // Through the same single issue path as `load`, so a recovery reload owns a+        // navigation epoch of its own: the load it supersedes cannot have its failure+        // charged to the reload, and vice versa (T-1744 review).+        issueNavigation(to: documentURL, describedAs: "Recovery reload failed")     }      // MARK: - Stalled-recovery watchdog (T-1943 review)@@ -1029,6 +1083,49 @@ final class WebDocumentController {     /// `@ObservationIgnored`) so the view can offer a reload.     private(set) var recoveryAbandoned = false +    /// Which navigation this controller most recently ISSUED. Bumped by+    /// `issueNavigation` — the one place `page.load` is called — so every load and every+    /// recovery reload has an identity that its own failure can be matched against.+    ///+    /// Same idiom as `observationEpoch` and the watchdog's `armedGeneration`: capture it+    /// where the work starts, compare it where the work reports back.+    @ObservationIgnored private var navigationEpoch: UInt64 = 0++    /// The epoch of the last navigation that failed before laying out, if any. Set by+    /// `recordNavigationFailure` only for the navigation that is current at the moment+    /// the failure lands, and never cleared — a value that is no longer current is+    /// already meaningless (see `navigationFailedBeforeSettling`). Holding to both halves+    /// of that rule is what makes this monotone: it only ever moves forward, so no+    /// late-arriving failure can walk it backwards over a fresher one.+    @ObservationIgnored private var failedNavigationEpoch: UInt64?++    /// **The CURRENT navigation** failed before the current load ever laid out (T-1744).+    ///+    /// Purely observational: it changes NO recovery decision. `.navigationFailed`+    /// outside a recovery stays deliberately benign — a blocked or bad in-page link is+    /// not a reason to charge the retry budget (T-1943/T-2107), and this flag does not+    /// make it one. What it records is narrower and is the one thing the benign+    /// classification loses: that the failure landed while `isLayoutSettled` was still+    /// false, i.e. the DOCUMENT's own navigation is what failed, not a link off a page+    /// already on screen — nothing the reader could have tapped exists before layout.+    ///+    /// Read only by the "Preparing document…" wait, which otherwise has no signal for+    /// this case and would poll two flags that never move again (`isLayoutSettled`+    /// stays false, `recoveryAbandoned` is reachable only through the crash chain the+    /// benign classification declines to enter).+    ///+    /// Computed from the epoch rather than stored, because "which load does this failure+    /// belong to" is the whole question and a bare `Bool` cannot answer it (T-1744+    /// review). A `load` that supersedes another does NOT cancel the outgoing+    /// navigation, and superseding it is precisely what makes WebKit fail it — so the+    /// stale failure lands AFTER the successor is under way. Set from a plain flag it+    /// read as "the successor's navigation failed, nothing is coming": the wait ended+    /// and the reader was left on a blank page, mid-load, with no indicator. Comparing+    /// epochs makes both halves impossible at once: a stale failure records a stale+    /// epoch that can never match, and a fresh load retires the previous failure simply+    /// by bumping past it, so there is no clear to get stale either.+    var navigationFailedBeforeSettling: Bool { failedNavigationEpoch == navigationEpoch }+     /// How many consecutive recoveries may fail to reach the stability milestone     /// (`layoutSettled`, see `hasReachedStabilityMilestone`) before observation gives     /// up. Without a cap, a WebContent process that cannot be relaunched would have@@ -1167,16 +1264,23 @@ final class WebDocumentController {                 onEvent(event)             }             return .streamEnded-        } catch let error as WebPage.NavigationError {-            // `if case` rather than an exhaustive switch: NavigationError is a-            // non-frozen enum, so a future case must degrade to "not a termination"-            // rather than fail to build.+        } catch {+            return classify(error)+        }+    }++    /// Classifies one thrown navigation error. Shared by the page-wide observation drain+    /// and the per-navigation sequence `issueNavigation` drains, so the two can never+    /// disagree about whether a given error is a crash or an ordinary failure.+    static func classify(_ error: any Error) -> NavigationObservationOutcome {+        // `if case` rather than an exhaustive switch: NavigationError is a non-frozen+        // enum, so a future case must degrade to "not a termination" rather than fail to+        // build.+        if let error = error as? WebPage.NavigationError {             if case .webContentProcessTerminated = error { return .webContentTerminated }             if case .pageClosed = error { return .pageClosed }-            return .navigationFailed-        } catch {-            return .navigationFailed         }+        return .navigationFailed     }      /// Applies one subscription outcome. Returns whether observation should continue.@@ -1197,7 +1301,19 @@ final class WebDocumentController {             // over a blank document, with no retry, no budget charged, and nothing even             // logged: the original bug, reached by the recovery path itself. The             // recovery-in-flight flag is what tells the two apart.-            guard recoveryInFlight else { return true }+            guard recoveryInFlight else {+                // Benign, and left entirely alone. The loading indicator also needs to+                // know about a failure nothing will retry (T-1744), but it deliberately+                // does NOT learn it here: this stream carries every navigation on the+                // page through one subscription, so an outcome arriving on it is not+                // attributable to any particular load — and the one that most often+                // arrives is the OUTGOING load's, failed by the very act of superseding+                // it. Recording it here charged that failure to its successor and+                // dropped the indicator over a document still on its way in (T-1744+                // review). `issueNavigation` records it instead, from the per-navigation+                // sequence, which is attributed by construction.+                return true+            }             return attemptRecovery(reason: .reloadFailed)         case .pageClosed, .streamEnded:             return false@@ -1341,6 +1457,17 @@ final class WebDocumentController {         navigationObservationTask = nil         startNavigationObservation()     }+    /// The epoch of the navigation most recently issued, so a test can hold on to a+    /// SUPERSEDED load's identity and deliver its failure late — the ordering the real+    /// bug needs (a stale failure arriving after its successor is under way) and the one+    /// ordering a test cannot get WebKit to produce on demand (T-1744 review).+    func test_navigationEpoch() -> UInt64 { navigationEpoch }+    /// Delivers a navigation failure exactly as `issueNavigation`'s own task does, tagged+    /// with the epoch it belongs to. Defaults to an ordinary failure, which is the only+    /// class this records.+    func test_recordNavigationFailure(epoch: UInt64, error: any Error = URLError(.cannotFindHost)) {+        recordNavigationFailure(error, epoch: epoch)+    }     #endif } 

diff --git a/prismTests/WebRendering/DocumentLoadingIndicatorWiringTests.swift b/prismTests/WebRendering/DocumentLoadingIndicatorWiringTests.swiftnew file mode 100644index 0000000..d9498ab--- /dev/null+++ b/prismTests/WebRendering/DocumentLoadingIndicatorWiringTests.swift@@ -0,0 +1,731 @@+//+//  DocumentLoadingIndicatorWiringTests.swift+//  prismTests+//+//  T-1744 review: `DocumentLoadingIndicatorPolicyTests` pins the pure decisions and is,+//  by construction, blind to whether anything calls them. That is the exact failure+//  class this codebase has shipped three times — T-1943 (crash recovery fully unit+//  tested, never subscribed), T-1099, and T-1681, which was the EARLIER attempt at THIS+//  VERY overlay and never rendered on device while its tests stayed green. A policy enum+//  that is correct in isolation while the view diverges is the same trap wearing a new+//  hat.+//+//  So this file covers the two things those tests cannot:+//+//  1. THE LOOP TERMINATES. `waitForPreparation` is run for real, against a real+//     `WebDocumentController`, once per exit in the policy's table. The failure being+//     guarded is a wait that never returns — a permanently stuck spinner covering the+//     document, which is worse than the blank page the ticket set out to fix. No test+//     over `shouldContinueWaiting` alone can see it, because a truth table has no loop.+//     Each is bounded by `finishes(within:)`, so a non-terminating wait FAILS on a+//     legible expectation rather than hanging the suite.+//+//     THE BOUNDS ARE DELIBERATELY ENORMOUS — 60s against sub-second real behaviour — and+//     that is not slack, it is the only honest way to bound a wall-clock assertion here.+//     These are `@MainActor` tests polling a MainActor loop, sharing one executor with+//     every other MainActor test in the run; a tight bound measures machine load, not+//     the code. A 10s bound was tried and blew (13-16s) in the full suite while passing+//     in isolation, alongside pre-existing timing tests failing at 17-19s on the same+//     run. Nothing is given up for it: every mutation these tests guard makes the wait+//     NEVER return, so any finite bound separates pass from fail equally well. The one+//     exception is `waitEndsWhenCancelled`, where the mutation DOES eventually return —+//     so it separates the two by three orders of magnitude (a 3600s injected deadline+//     against a 60s bound) rather than by a tight clock. Mutation-verified, not assumed.+//     The suite is `.serialized` for the same reason: these tests should not add their+//     own contention to what they are measuring.+//  2. THE CALL SITE USES IT. Source-structural checks over `DocumentScrollContent.swift`+//     (the `#filePath` pattern `FootnotePresentationHostTests` uses), pinning that the+//     load task raises the flag, awaits the wait, and clears it under `mayClearFlag`.+//  3. EXIT 3 BELONGS TO ONE LOAD. Two loads overlap constantly here (a re-parse landing+//     during the previous one's emit), and superseding a navigation is exactly what makes+//     WebKit fail it — so a stale failure always arrives while its successor is in+//     flight. Three tests fix the attribution from both ends: the successor's wait+//     survives the stale failure, the page-wide stream (which cannot attribute anything)+//     never raises the flag, and the current load's own failure still ends its wait.+//+//  WHAT THIS FILE CANNOT PROVE — stated plainly, because the honest limits are the+//  point of the file:+//+//  - It does not render `DocumentScrollContent`, so it cannot prove a `ProgressView`+//    reaches the screen. That is precisely how T-1681's attempt passed review: its+//    gating read was legal Swift that simply never re-evaluated. What makes this fix+//    different is structural rather than test-provable — the overlay is driven by a+//    plain `@State` flag mutated imperatively, not by a `ViewBuilder` read of an+//    optional `@Observable` — and section 2 pins that structure, not the pixels. Only a+//    UI test or a device check can close the last gap.+//  - Exit 1 raises `layoutSettled` through the controller's own `test_markLayoutSettled`+//    seam rather than by waiting for the real bridge message. The message itself, and+//    the bridge that carries it, are pinned by the message-router tests; what is under+//    test here is that the wait REACTS to the flag, not how the flag arrives.+//  - The deadline (exit 4) is injected, so the test proves the deadline is honoured, not+//    that 120s is the right number. `DocumentLoadingIndicatorPolicyTests` bounds the+//    constant instead.+//++import Foundation+import Testing+import WebKit+@testable import prism++@MainActor+@Suite("Document loading indicator wiring (T-1744)", .serialized)+struct DocumentLoadingIndicatorWiringTests {++    // MARK: - Helpers++    /// A controller whose scheme handler CAN serve the document route, so a real+    /// `controller.load(servableURL)` navigates successfully.+    ///+    /// That matters everywhere a test claims a specific exit fired. Exit 3 is raised from+    /// the load's own navigation sequence (`issueNavigation`), so unlike the page-wide+    /// observation stream it cannot be parked by an injected stub: a load to a URL the+    /// handler rejects raises exit 3 for real, and would end a wait the test meant to end+    /// some other way. Serving the route keeps the failure opt-in — `unservableURL` is+    /// how a test asks for one.+    ///+    /// The served HTML carries no bridge script (`bridgeUserScripts` is empty here), so a+    /// successful load still never posts `ready` or `layoutSettled`: exits 1 and 3 both+    /// stay under the test's control.+    private func makeController() -> WebDocumentController {+        WebDocumentController(+            sessionID: "loading-indicator-wiring",+            parseRevision: 1,+            schemeHandler: PrismDocSchemeHandler(documentHTMLProvider: { "<html><body></body></html>" })+        )+    }++    /// A document URL the handler above serves. Distinct per revision, as production's+    /// `WebDocumentControllerFactory.documentURL` is.+    private func servableURL(revision: UInt64) -> URL {+        URL(string: "prism-doc://document/x?rev=\(revision)")!+    }++    /// A URL the handler rejects outright (unknown route), so the navigation to it fails+    /// as an ordinary provisional-navigation failure — no crash, no retry.+    private func unservableURL(revision: UInt64) -> URL {+        URL(string: "prism-doc://nowhere/x?rev=\(revision)")!+    }++    /// A controller whose navigation observation is parked on a stream that never yields+    /// and never ends, so no crash-recovery decision can fire behind the test's back.+    ///+    /// Only covers the page-wide stream. The load's OWN navigation is not observed+    /// through it (T-1744 review), which is why `makeController` serves the document+    /// route rather than relying on this to keep a real failure out of the way.+    private func makeParkedController() -> WebDocumentController {+        let controller = makeController()+        controller.test_setInjectedNavigationStream { AsyncThrowingStream { _ in } }+        controller.test_rearmNavigationObservation()+        return controller+    }++    /// Polls until `condition` holds or roughly `seconds` have passed. Used only where a+    /// test must let a REAL navigation resolve before asserting.+    private func poll(untilTrue condition: () -> Bool, forUpTo seconds: Double = 5) async {+        let deadline = ContinuousClock.now + .seconds(seconds)+        while ContinuousClock.now < deadline && !condition() {+            try? await Task.sleep(for: .milliseconds(10))+        }+    }++    /// Gives `condition` the full window to become true, for a test that expects it will+    /// not. Identical mechanics to `poll(untilTrue:)`; the name exists because a bare+    /// "poll until true" whose result is then asserted FALSE reads like a mistake at the+    /// call site. Proving an absence has no early exit, so this always burns the window.+    private func waitBriefly(forAbsenceOf condition: () -> Bool, forUpTo seconds: Double = 2) async {+        await poll(untilTrue: condition, forUpTo: seconds)+    }++    /// A navigation stream that fails with `error`, matching the shape of the real+    /// `page.navigations` sequence (which ends when it throws).+    private func stream(+        failingWith error: any Error+    ) -> AsyncThrowingStream<WebPage.NavigationEvent, any Error> {+        AsyncThrowingStream { continuation in+            continuation.finish(throwing: error)+        }+    }++    /// A stream PROVIDER that fails once and then parks forever.+    ///+    /// Not a nicety. `observeNavigations` re-subscribes after a `.navigationFailed`+    /// (that is the classification being tested — a benign failure keeps observing), so+    /// a provider that failed on every subscription would spin the MainActor for the+    /// rest of the process: one `Task.yield()` per iteration, forever, since nothing+    /// releases the controller while the test's page still holds it. That is not a+    /// hypothetical — it starved the whole suite and took the test host down with it,+    /// cascading ~170 phantom failures, while every one of these tests passed in+    /// isolation. Fail once, then go quiet.+    private func failsOnceThenParks(+        with error: any Error+    ) -> WebDocumentController.InjectedNavigationStream {+        let hasFailed = FlagBox()+        return { [self] in+            guard !hasFailed.value else { return AsyncThrowingStream { _ in } }+            hasFailed.value = true+            return stream(failingWith: error)+        }+    }++    /// Runs `work` and reports whether it finished within `seconds`.+    ///+    /// The assertion stays in the caller so a wait that never returns fails on a legible+    /// expectation rather than hanging the suite. `Task.detached` deliberately is NOT+    /// used — the wait must run on the MainActor exactly as it does in production.+    ///+    /// Completion is recorded from INSIDE the task (`Task.value` cannot be polled) and+    /// a wait that overruns is cancelled and drained, so it cannot outlive its test.+    private func finishes(+        within seconds: Double,+        _ work: @escaping @MainActor () async -> Void+    ) async -> Bool {+        let box = CompletionBox()+        let task = Task { @MainActor in+            await work()+            box.finished = true+        }+        let deadline = ContinuousClock.now + .seconds(seconds)+        while ContinuousClock.now < deadline {+            if box.finished { return true }+            try? await Task.sleep(for: .milliseconds(25))+        }+        task.cancel()+        _ = await task.value+        return false+    }++    // MARK: - 1. The wait terminates, once per exit++    @Test("Exit 1: the wait ends when the page lays out")+    func waitEndsWhenLayoutSettles() async {+        let controller = makeParkedController()+        // Raise the signal a moment after the wait starts, so the test proves the LOOP+        // notices a mid-flight transition rather than short-circuiting on a flag that+        // was already true when it was entered.+        Task { @MainActor in+            try? await Task.sleep(for: .milliseconds(150))+            controller.test_markLayoutSettled()+        }++        let ended = await finishes(within: 60) {+            await DocumentLoadingIndicatorPolicy.waitForPreparation(controller)+        }++        #expect(ended, "The wait did not return after the page laid out — the spinner would cover a rendered document.")+        #expect(controller.isLayoutSettled, "Exit 1 is the signal under test; it must actually be the one that fired.")+        #expect(!controller.recoveryAbandoned)+        #expect(!controller.navigationFailedBeforeSettling)+    }++    @Test("Exit 2: the wait ends when crash recovery gives up")+    func waitEndsWhenRecoveryIsAbandoned() async throws {+        // Abandonment is driven through the REAL observation loop over an injected+        // terminating stream, the way `WebContentTerminationWiringTests` drives it, so+        // `recoveryAbandoned` is reached by production code rather than set by the test.+        //+        // The three-step setup is load-bearing in both directions, which is why it is not+        // simply "inject, then load":+        //  - Park BEFORE the load, so no crash-recovery decision fires behind the test's+        //    back while it is being set up.+        //  - Load a SERVABLE URL, or the load's own navigation fails and raises exit 3 —+        //    the wait would end for a reason the test did not intend, intermittently.+        //    Parking cannot prevent that: exit 3 comes from the load's own navigation+        //    sequence, not from the observed stream (T-1744 review).+        //  - Load BEFORE the terminating stream, or `attemptRecovery` finds no+        //    `loadedDocumentURL`, declines to charge the budget (correctly — there is+        //    nothing to reload before the first load), keeps observing, and the loop+        //    re-subscribes to a stream that always throws: a MainActor hot loop that+        //    never abandons.+        let controller = makeController()+        controller.test_setInjectedNavigationStream { AsyncThrowingStream { _ in } }+        controller.test_rearmNavigationObservation()+        controller.load(documentURL: servableURL(revision: 1), parseRevision: 1)+        controller.test_setInjectedNavigationStream { [self] in+            stream(failingWith: WebPage.NavigationError.webContentProcessTerminated)+        }+        controller.test_rearmNavigationObservation()++        let ended = await finishes(within: 60) {+            await DocumentLoadingIndicatorPolicy.waitForPreparation(controller)+        }++        #expect(ended, """+            The wait did not return after recovery was abandoned. The spinner would sit \+            on top of the "This document stopped rendering." banner forever, hiding the \+            one control (Reload) that recovers the document (T-1943).+            """)+        #expect(controller.recoveryAbandoned, "Exit 2 is the signal under test.")+        #expect(!controller.isLayoutSettled, "A page that never rendered must not report a settled layout.")+    }++    @Test("Exit 3: the wait ends when the document's own navigation fails without a crash")+    func waitEndsOnOrdinaryNavigationFailure() async {+        // THE regression test for the first review finding. `.navigationFailed` outside a+        // recovery is deliberately benign — it charges no budget and reaches no+        // abandonment (T-1943/T-2107) — so before this fix `isLayoutSettled` and+        // `recoveryAbandoned` were BOTH false forever and the wait polled until a later+        // reparse happened to supersede it.+        //+        // Driven by a REAL failing navigation rather than an injected stream (T-1744+        // review): exit 3 is now raised from the load's own navigation sequence, so the+        // signal source, the classification and the epoch that attributes it are all+        // production code, and WebKit — not the test — decides what a failed load looks+        // like.+        let controller = makeController()+        controller.load(documentURL: unservableURL(revision: 1), parseRevision: 1)++        let ended = await finishes(within: 60) {+            await DocumentLoadingIndicatorPolicy.waitForPreparation(controller)+        }++        #expect(ended, """+            The wait did not return after an ordinary navigation failure. Neither \+            `isLayoutSettled` nor `recoveryAbandoned` can ever become true for this \+            failure, so the spinner covers a permanently blank document (T-1744 review).+            """)+        #expect(controller.navigationFailedBeforeSettling, "Exit 3 is the signal under test.")+        // The classification itself must be UNCHANGED by this fix: a benign failure is+        // still benign. If this fix had "closed" the gap by charging the failure as a+        // recovery, `recoveryAbandoned` would eventually become true here and every bad+        // in-page link would start reloading the document.+        #expect(!controller.recoveryAbandoned, """+            An ordinary navigation failure must NOT be escalated into a crash recovery — \+            that would make every blocked in-page link reload the document (T-1943/T-2107).+            """)+        // The page-wide stream sees the same failure, and reports it independently of the+        // per-navigation sequence the wait ended on — so poll for it rather than assuming+        // the two land in the same turn.+        await poll(untilTrue: { controller.lastNavigationOutcome != nil })+        #expect(controller.lastNavigationOutcome == .navigationFailed)+    }++    @Test("A failure on the page-wide stream alone does not raise exit 3")+    func pageWideStreamFailureDoesNotRaiseExitThree() async {+        // The invariant the overlap fix rests on, guarded directly (T-1744 review).+        // `page.navigations` carries every navigation on the page through one+        // subscription, so an outcome arriving on it is attributable to nothing in+        // particular — and the failure that most often arrives on it is the OUTGOING+        // load's, failed by the act of superseding it. Raising the flag from there is+        // what charged one load's failure to another. No load has been issued on this+        // controller at all, so the failure below belongs to no load whatsoever.+        let controller = makeController()+        controller.test_setInjectedNavigationStream(failsOnceThenParks(with: URLError(.cannotFindHost)))+        controller.test_rearmNavigationObservation()++        await poll(untilTrue: { controller.lastNavigationOutcome != nil })++        #expect(controller.lastNavigationOutcome == .navigationFailed, "Setup: the loop must have classified it.")+        #expect(!controller.navigationFailedBeforeSettling, """+            The page-wide observation stream must not raise exit 3. It cannot say WHICH \+            navigation failed, so every failure it reports lands on whatever load happens \+            to be current — which is the superseding one (T-1744 review).+            """)+        #expect(!controller.recoveryAbandoned, """+            An ordinary navigation failure must NOT be escalated into a crash recovery — \+            that would make every blocked in-page link reload the document (T-1943/T-2107).+            """)+    }++    @Test("Exit 4: the wait ends at its deadline when no signal ever arrives")+    func waitEndsAtTheDeadline() async {+        // The backstop, and the only exit that covers a stall nobody has named yet. The+        // controller here is parked: no load, no navigation, no settle, no recovery —+        // every other exit is unreachable by construction, so a wait that returns can+        // only have returned on the deadline.+        let controller = makeParkedController()++        let ended = await finishes(within: 60) {+            await DocumentLoadingIndicatorPolicy.waitForPreparation(+                controller, deadline: .now + .milliseconds(300)+            )+        }++        #expect(ended, "The bounded backstop did not fire — the wait is unbounded and the spinner can outlive its load.")+        #expect(!controller.isLayoutSettled)+        #expect(!controller.recoveryAbandoned)+        #expect(!controller.navigationFailedBeforeSettling)+    }++    @Test("Exit 5: the wait ends PROMPTLY when a fresher parse revision supersedes it")+    func waitEndsWhenCancelled() async {+        // The bound is the assertion, not a convenience. Mutation-checked: deleting the+        // loop's `!Task.isCancelled` check left an unbounded version of this test GREEN —+        // the cancelled wait still returned, just two minutes later when its deadline+        // elapsed, having hot-looped the MainActor the whole time (a cancelled+        // `Task.sleep` throws instantly and `try?` swallows it). "Ends eventually" is not+        // what cancellation means here: a superseded load must release the MainActor to+        // the revision the reader is actually waiting on.+        //+        // This is the one test whose mutation terminates on its own, so it is also the+        // one that needs a real gap between the two outcomes rather than a tight clock:+        // the injected deadline sits an hour out against a 60s bound. Cancellation+        // returns in milliseconds; exit 4 cannot be what satisfies this test, and a+        // loaded machine cannot turn the pass into a fail.+        let controller = makeParkedController()+        let box = CompletionBox()++        let task = Task { @MainActor in+            await DocumentLoadingIndicatorPolicy.waitForPreparation(+                controller, deadline: .now + .seconds(3600)+            )+            box.finished = true+            box.wasCancelled = Task.isCancelled+        }+        try? await Task.sleep(for: .milliseconds(150))+        task.cancel()++        // Polled directly rather than through `finishes(within:)`: that helper drains its+        // overrunning task before returning (so a stray wait cannot leak into the next+        // test), and draining THIS one would mean awaiting the full injected hour on the+        // very run where the assertion is meant to fail.+        let bound = ContinuousClock.now + .seconds(60)+        while ContinuousClock.now < bound && !box.finished {+            try? await Task.sleep(for: .milliseconds(25))+        }+        let endedPromptly = box.finished++        #expect(endedPromptly, """+            A cancelled wait did not return within 60s. The superseded load's task keeps \+            polling — and keeps the MainActor busy — while the revision actually on \+            screen is trying to load (T-1744/T-1975).+            """)+        #expect(box.finished, "A cancelled wait must return, or the superseded load's task leaks for the session.")+        #expect(box.wasCancelled, """+            The wait returned but the task does not read as cancelled, so \+            `mayClearFlag` would let a SUPERSEDED load clear the flag its successor \+            raised — the spinner flickers off mid-load for the revision on screen.+            """)+        #expect(!DocumentLoadingIndicatorPolicy.mayClearFlag(wasCancelled: box.wasCancelled))+    }++    @Test("A recovery's own intermediate failure does not trip the indicator's exit")+    func recoveryReloadFailureDoesNotTripExitThree() async {+        // Exit 3 is scoped to "a navigation failed and nothing will retry it". A recovery+        // reload that fails provisionally is the SAME ordinary failure (T-1943), but it+        // IS retried, and the wait already has both of its endings through that path —+        // the retry reaches `layoutSettled`, or the budget spends and raises+        // `recoveryAbandoned`. Recording it would drop the indicator mid-recovery and+        // reveal the blank page the reload is about to fill.+        //+        // Driven through a REAL recovery reload whose navigation really fails, rather+        // than by handing `applyNavigationOutcome` an outcome: that method no longer+        // raises the flag at all (T-1744 review), so a test written against it would+        // assert nothing while looking exactly as it does now.+        // Start from a load that really did fail, so exit 3 is genuinely raised and the+        // recovery has something to retire.+        let controller = makeParkedController()+        controller.load(documentURL: unservableURL(revision: 1), parseRevision: 1)+        await poll(untilTrue: { controller.navigationFailedBeforeSettling })+        #expect(controller.navigationFailedBeforeSettling, "Setup: the first load's failure must be recorded.")++        // A recovery reload is a fresh navigation of its own, so issuing it retires the+        // failure that prompted it — otherwise the wait would exit on the previous load's+        // failure the instant the reload starts.+        controller.handleProcessTermination(documentURL: unservableURL(revision: 2))+        #expect(!controller.navigationFailedBeforeSettling, """+            A recovery reload must retire the failure it is recovering from, or exit 3 \+            fires immediately over the page the reload is about to fill (T-1744).+            """)++        // And its own failure must be swallowed: `recoveryInFlight` is set before the+        // navigation is issued.+        await waitBriefly(forAbsenceOf: { controller.navigationFailedBeforeSettling })+        #expect(!controller.navigationFailedBeforeSettling, """+            A failure that a recovery is retrying must not end the wait — the indicator \+            would step aside for a page the reload is about to replace (T-1744).+            """)+    }++    @Test("A termination reported on the load's own sequence is left to crash recovery")+    func terminationOnTheLoadsOwnSequenceDoesNotTripExitThree() {+        // The load's own navigation sequence throws for a WebContent crash as well as for+        // an ordinary failure, and the crash arrives here BEFORE `recoveryInFlight` is+        // set — the reload that sets it is a consequence of the same event, one hop+        // later. So the `recoveryInFlight` guard cannot be what keeps a crash out; the+        // classification has to, and it is the same `classify` the observation loop uses.+        // Without it a crash would drop the indicator in the instant before recovery+        // reloads the page it is about to fill.+        let controller = makeController()+        controller.load(documentURL: servableURL(revision: 1), parseRevision: 1)++        controller.test_recordNavigationFailure(+            epoch: controller.test_navigationEpoch(),+            error: WebPage.NavigationError.webContentProcessTerminated+        )++        #expect(!controller.navigationFailedBeforeSettling, """+            A WebContent termination must not raise exit 3 — recovery is about to reload \+            the document, and exits 1 and 2 already cover both of its endings (T-1943).+            """)+    }++    @Test("A fresh load retires the previous load's navigation failure")+    func loadClearsThePreviousNavigationFailure() async throws {+        // The other half of exit 3, and the half that fails SILENTLY: if a load did not+        // retire `navigationFailedBeforeSettling`, one failed navigation would latch the+        // flag for the life of the controller and every subsequent load's wait would+        // exit on the FIRST poll — no indicator at all, ever again, with every other+        // test here still green. Same "a user-initiated navigation is fresh evidence"+        // rule that clears `recoveryAbandoned` (T-1943); since T-1744's review it is the+        // navigation epoch moving on that expresses it, not an assignment.+        let controller = makeController()+        controller.load(documentURL: unservableURL(revision: 1), parseRevision: 1)++        await poll(untilTrue: { controller.navigationFailedBeforeSettling })+        #expect(controller.navigationFailedBeforeSettling, "Setup: the failure must have been recorded first.")++        controller.load(documentURL: servableURL(revision: 2), parseRevision: 2)++        #expect(!controller.navigationFailedBeforeSettling, """+            A fresh load must retire the previous load's failure. Leaving it set makes \+            every later wait exit immediately, so the indicator never appears again for \+            the life of the document (T-1744).+            """)+    }++    @Test("A superseded load's late failure does not end its successor's wait")+    func staleNavigationFailureIsNotChargedToTheSuccessor() async {+        // The overlap regression (T-1744 review). `load` does NOT cancel the outgoing+        // load's navigation, and superseding it is exactly what makes WebKit fail it —+        // so the stale failure lands AFTER the successor is already under way. Read off+        // the page-wide stream it was indistinguishable from the successor's own failure:+        // the successor's wait ended, the indicator came down, and the reader was left+        // looking at a blank page while the document they asked for was still loading.+        //+        // The failure is delivered through the seam rather than by racing WebKit, because+        // the ordering IS the bug and a real superseded navigation resolves in+        // milliseconds either side of the supersession. What the seam does not fake is+        // the identity: the epochs come from two real `load` calls, and the recorder it+        // calls is the one `issueNavigation`'s own task calls.+        let controller = makeController()+        controller.load(documentURL: servableURL(revision: 1), parseRevision: 1)+        let supersededEpoch = controller.test_navigationEpoch()++        controller.load(documentURL: servableURL(revision: 2), parseRevision: 2)+        #expect(controller.test_navigationEpoch() != supersededEpoch, """+            Each load must issue a navigation with an identity of its own, or there is \+            nothing to tell a stale failure apart from a fresh one.+            """)++        // The superseded load's navigation now fails, late.+        controller.test_recordNavigationFailure(epoch: supersededEpoch)+        #expect(!controller.navigationFailedBeforeSettling, """+            A superseded load's failure was charged to the load that superseded it. The \+            successor's wait ends on exit 3 while its navigation is still in flight, so \+            the indicator comes down over a document that has not rendered — the T-1744 \+            symptom, restored by its own fix.+            """)++        // ...and the successor's wait is genuinely still running: a deliberately enormous+        // injected deadline means only exit 3 could end it here.+        let endedEarly = await finishes(within: 2) {+            await DocumentLoadingIndicatorPolicy.waitForPreparation(+                controller, deadline: .now + .seconds(3600)+            )+        }+        #expect(!endedEarly, "The indicator must stay up for the load that is actually on screen.")++        // The guard must scope the drop to STALE failures, not disable exit 3: the+        // successor's OWN failure still ends its wait.+        controller.test_recordNavigationFailure(epoch: controller.test_navigationEpoch())+        #expect(controller.navigationFailedBeforeSettling)+        let ended = await finishes(within: 60) {+            await DocumentLoadingIndicatorPolicy.waitForPreparation(controller)+        }+        #expect(ended, """+            Exit 3 no longer fires for the current load's own failure — the identity \+            guard has turned the fix off instead of scoping it.+            """)+    }++    @Test("A superseded load's late failure does not retire the successor's own failure")+    func staleNavigationFailureDoesNotClobberTheCurrentOne() async {+        // The mirror image of the test above, and the half a read-side epoch comparison+        // alone does not cover (T-1744 review). Nothing orders two navigations' failure+        // tasks, so BOTH arrival orders are available: the previous test has the stale+        // failure landing first, this one has it landing second, after the successor's+        // own failure has correctly raised exit 3.+        //+        // With the recorder writing unconditionally, that late write moves+        // `failedNavigationEpoch` BACKWARDS onto an epoch that can no longer match, and+        // the flag flips back to false between two polls of a wait that was about to end.+        // The reader is then held on the spinner until the 120-second backstop, over a+        // document that has already failed — the wait ends, so it is not the original+        // T-1744 hang, but it is two minutes of waiting instead of being told.+        let controller = makeController()+        controller.load(documentURL: servableURL(revision: 1), parseRevision: 1)+        let supersededEpoch = controller.test_navigationEpoch()++        controller.load(documentURL: servableURL(revision: 2), parseRevision: 2)+        let currentEpoch = controller.test_navigationEpoch()+        #expect(currentEpoch != supersededEpoch, "Setup: the two loads must have distinct identities.")++        // The load actually on screen fails on its own account: exit 3, correctly raised.+        controller.test_recordNavigationFailure(epoch: currentEpoch)+        #expect(controller.navigationFailedBeforeSettling, "Setup: the current load's own failure must raise exit 3.")++        // Only now does the superseded load's navigation report its failure.+        controller.test_recordNavigationFailure(epoch: supersededEpoch)++        #expect(controller.navigationFailedBeforeSettling, """+            A superseded load's late failure retired the CURRENT load's failure. Exit 3 \+            is silently withdrawn and the wait falls through to the 120-second backstop, \+            so the reader watches a spinner for two minutes over a document that already \+            failed (T-1744 review).+            """)++        let ended = await finishes(within: 60) {+            await DocumentLoadingIndicatorPolicy.waitForPreparation(controller)+        }+        #expect(ended, """+            The wait did not end on the current load's failure after a stale one landed \+            behind it — the flag is being read at poll time, so a withdrawn exit 3 costs \+            the reader the full deadline.+            """)+    }++    // MARK: - 2. The call site actually uses all of it++    /// A project source file, read from disk relative to this one (the `#filePath`+    /// approach `FootnotePresentationHostTests` uses), so the checks need no bundle+    /// resource wiring.+    private static func projectSource(_ relativePath: String) throws -> String {+        let url = URL(fileURLWithPath: #filePath)+            .deletingLastPathComponent()   // WebRendering+            .deletingLastPathComponent()   // prismTests+            .deletingLastPathComponent()   // repo root+            .appendingPathComponent(relativePath)+        return try String(contentsOf: url, encoding: .utf8)+    }++    private static func scrollContentSource() throws -> String {+        try projectSource("prism/Views/DocumentScrollContent.swift")+    }++    @Test("`issueNavigation` is the only place the page is told to load")+    func pageLoadHasExactlyOneCallSite() throws {+        // Exit 3's whole attribution rests on this: the epoch is a faithful per-navigation+        // identity only because every navigation is issued from `issueNavigation`, which+        // bumps it. A refresh or retry path that called `page.load` directly would get no+        // epoch of its own, so its failure would be recorded against — or silently+        // dropped in favour of — whatever navigation happened to be current. It would+        // compile, and every behavioural test in section 1 would still pass, because they+        // all go through `load`/`handleProcessTermination`. That is the T-1943 shape:+        // wiring that is MISSING rather than wrong, which no direct-invocation test sees.+        //+        // Honest about its reach: this is a source-text tripwire, not a proof. It shows+        // there is no second literal `page.load(` in this file, which is exactly the form+        // the regression would take. It does NOT prove the invariant — a call through a+        // local alias (`let p = page`), a differently-named binding, an extension in+        // another file, or a `page.reload()`-style API that is not spelled `load` would+        // all pass it. It also reads comments, so writing `page.load(` inside a doc+        // comment will fail it; the surrounding prose deliberately says `page.load`+        // without the parenthesis for that reason.+        let source = try Self.projectSource("prism/ViewModels/WebDocumentController.swift")++        let occurrences = source.components(separatedBy: "page.load(").count - 1+        #expect(occurrences == 1, """+            Expected exactly one `page.load(` call site in WebDocumentController, found \+            \(occurrences). A navigation issued outside `issueNavigation` carries no \+            epoch of its own, which breaks the identity the loading indicator's exit 3 \+            is attributed by (T-1744 review). Route it through `issueNavigation`.+            """)++        // ...and that one site is inside `issueNavigation` rather than somewhere else.+        // Bounded by the method that follows it in the file; if these two are ever+        // deliberately reordered, move the bound rather than dropping the check.+        let issue = try #require(source.range(of: "private func issueNavigation("))+        let recorder = try #require(source.range(of: "private func recordNavigationFailure("))+        let call = try #require(source.range(of: "page.load("))+        #expect(issue.upperBound < call.lowerBound && call.upperBound < recorder.lowerBound, """+            The single `page.load(` call site is no longer inside `issueNavigation`, so \+            the epoch bump and the navigation it identifies have come apart (T-1744).+            """)+    }++    @Test("The document body applies the loading overlay")+    func bodyAppliesTheOverlay() throws {+        let source = try Self.scrollContentSource()+        #expect(source.contains(".overlay { documentLoadingOverlay }"), """+            DocumentScrollContent must apply `documentLoadingOverlay`. Without it the \+            flag is maintained perfectly and nothing is ever drawn — T-1681's failure, \+            restored.+            """)+        #expect(source.contains("if isPreparingDocument {"), """+            The overlay must be gated on the plain @State flag. Gating it on \+            `webController?.isLayoutSettled` read from a ViewBuilder is the reverted \+            T-1681 attempt that never appeared on device (T-1744).+            """)+    }++    @Test("The load task raises the flag, waits, and clears it under the supersession rule")+    func loadTaskDrivesTheFlag() throws {+        let source = try Self.scrollContentSource()++        #expect(source.contains("DocumentLoadingIndicatorPolicy.shouldIndicatePreparation("), """+            The load task must gate the flag on `shouldIndicatePreparation`, or a \+            `parseRevision == 0` incarnation raises a spinner for a load \+            `WebDocumentControllerFactory.loadDocument` never issues (T-1744 review).+            """)+        #expect(source.contains("if willLoad { isPreparingDocument = true }"))+        #expect(source.contains("DocumentLoadingIndicatorPolicy.waitForPreparation(webController)"), """+            The load task must await the wait. Without it the flag clears the instant the \+            navigation is ISSUED, long before the page lays out, and the indicator \+            disappears over a still-blank document.+            """)+        #expect(source.contains("DocumentLoadingIndicatorPolicy.mayClearFlag(wasCancelled: Task.isCancelled)"), """+            The clear must be gated on cancellation, or a superseded load erases the flag \+            a fresher one is relying on (T-1744).+            """)+    }++    @Test("The wait is ordered after the load and the clear after the wait")+    func loadTaskOrdersItsStepsCorrectly() throws {+        let source = try Self.scrollContentSource()+        // Order is the whole behaviour here, and the three fragments are unique in the+        // file, so their offsets are a faithful reading of it. Raising the flag after the+        // load, or clearing before the wait, are both compiling no-op-looking edits that+        // silently restore "no indicator at all".+        let raise = try #require(source.range(of: "if willLoad { isPreparingDocument = true }"))+        let load = try #require(source.range(of: "await WebDocumentControllerFactory.loadDocument("))+        let wait = try #require(+            source.range(of: "await DocumentLoadingIndicatorPolicy.waitForPreparation(webController)")+        )+        // The gate, not the assignment: `isPreparingDocument = false` also matches the+        // @State declaration's initial value near the top of the file.+        let clear = try #require(+            source.range(of: "DocumentLoadingIndicatorPolicy.mayClearFlag(wasCancelled: Task.isCancelled)")+        )++        #expect(raise.lowerBound < load.lowerBound, "The flag must be raised BEFORE the off-main emit begins.")+        #expect(load.lowerBound < wait.lowerBound, "The wait covers the window AFTER the navigation is issued.")+        #expect(wait.lowerBound < clear.lowerBound, "The flag may only be cleared once the wait has ended.")+    }++    @Test("The overlay is applied before the recovery banner, so the banner draws on top")+    func overlayOrderKeepsTheBannerOnTop() throws {+        let source = try Self.scrollContentSource()+        let loading = try #require(source.range(of: ".overlay { documentLoadingOverlay }"))+        let banner = try #require(source.range(of: ".overlay(alignment: .top) { recoveryAbandonedBanner }"))+        // SwiftUI stacks overlays in application order (later wins). Reversing these+        // would put the full-bleed spinner over the "stopped rendering" banner in the+        // brief window where both are true — hiding the only control that recovers the+        // document.+        #expect(loading.lowerBound < banner.lowerBound)+    }+}++/// Records a wait task's completion from inside the task, since `Task.value` cannot be+/// polled and the assertions need to run after it has genuinely returned.+@MainActor+private final class CompletionBox {+    var finished = false+    var wasCancelled = false+}++/// Mutable state for a stored (escaping) stream provider, which cannot capture a+/// mutable local the way the inline providers can.+@MainActor+private final class FlagBox {+    var value = false+}

diff --git a/prismTests/WebRendering/DocumentLoadingIndicatorPolicyTests.swift b/prismTests/WebRendering/DocumentLoadingIndicatorPolicyTests.swiftnew file mode 100644index 0000000..0e7c27c--- /dev/null+++ b/prismTests/WebRendering/DocumentLoadingIndicatorPolicyTests.swift@@ -0,0 +1,143 @@+//+//  DocumentLoadingIndicatorPolicyTests.swift+//  prismTests+//+//  T-1744: the "Preparing document…" overlay never appeared on device in either of the+//  two forms tried during T-1681 (gated on `isReady`, then on `isLayoutSettled`, both+//  read directly from a `ViewBuilder` through the `@State private var webController:+//  WebDocumentController?` optional). The fix moves the overlay onto a plain `@State`+//  flag (`DocumentScrollContent.isPreparingDocument`) flipped imperatively by the load+//  task.+//+//  THIS FILE COVERS THE PURE DECISIONS ONLY — the truth table of+//  `shouldContinueWaiting`, the supersession rule of `mayClearFlag`, and the+//  load-pairing rule of `shouldIndicatePreparation`. It is deliberately blind to+//  whether anything calls them, which is the failure class that shipped three times in+//  this codebase (T-1943, T-1099, and T-1681 — the earlier attempt at THIS overlay,+//  which never rendered on device while its tests stayed green).+//+//  `DocumentLoadingIndicatorWiringTests` is the file that pins the wiring: the real+//  wait loop over a real `WebDocumentController` for each exit, and the call site in+//  `DocumentScrollContent`. Neither file is sufficient alone.+//++import Testing+@testable import prism++struct DocumentLoadingIndicatorPolicyTests {++    // MARK: - shouldContinueWaiting++    /// The default arguments spell the "still loading" state, so each test below varies+    /// exactly the one signal it is about.+    private func continueWaiting(+        isLayoutSettled: Bool = false,+        recoveryAbandoned: Bool = false,+        navigationFailedBeforeSettling: Bool = false,+        deadlineElapsed: Bool = false+    ) -> Bool {+        DocumentLoadingIndicatorPolicy.shouldContinueWaiting(+            isLayoutSettled: isLayoutSettled,+            recoveryAbandoned: recoveryAbandoned,+            navigationFailedBeforeSettling: navigationFailedBeforeSettling,+            deadlineElapsed: deadlineElapsed+        )+    }++    @Test("Keeps waiting while no exit signal has arrived")+    func continuesWaitingWhileNoSignalHasArrived() {+        #expect(continueWaiting())+    }++    @Test("Exit 1: stops waiting once the page lays out")+    func stopsWaitingOnceLayoutSettles() {+        #expect(!continueWaiting(isLayoutSettled: true))+    }++    @Test("Exit 2: stops waiting once crash recovery gives up, even though layout never settled")+    func stopsWaitingOnceRecoveryIsAbandoned() {+        // The whole point of the second flag: a WebContent process that never manages to+        // render must not leave the spinner up forever just because `isLayoutSettled`+        // stays false — `recoveryAbandonedBanner` takes over instead.+        #expect(!continueWaiting(recoveryAbandoned: true))+    }++    @Test("Exit 3: stops waiting when the document's own navigation failed before settling")+    func stopsWaitingOnNavigationFailureBeforeSettling() {+        // The gap this closes (T-1744 review): an ordinary, non-crash navigation failure+        // is deliberately classified as a benign bad link and never charges the recovery+        // budget, so `recoveryAbandoned` is UNREACHABLE through it while `isLayoutSettled`+        // stays false forever. Exits 1 and 2 both miss it; without this one the spinner+        // sits over a page that will never load.+        #expect(!continueWaiting(navigationFailedBeforeSettling: true))+    }++    @Test("Exit 4: stops waiting once the bounded deadline elapses")+    func stopsWaitingOnceTheDeadlineElapses() {+        // The backstop for any stall exits 1-3 do not name. An indicator that gives up+        // and reveals the document is strictly better than one that spins forever.+        #expect(!continueWaiting(deadlineElapsed: true))+    }++    @Test("Stops waiting when every signal is set (defensive — should not occur in practice)")+    func stopsWaitingWhenAllFlagsAreSet() {+        #expect(!continueWaiting(+            isLayoutSettled: true,+            recoveryAbandoned: true,+            navigationFailedBeforeSettling: true,+            deadlineElapsed: true+        ))+    }++    // MARK: - shouldIndicatePreparation++    @Test("No indicator before the first parse, because no load will be issued")+    func noIndicatorBeforeTheFirstParse() {+        // `WebDocumentControllerFactory.loadDocument` returns without calling `beginLoad`+        // or `load` when `parseRevision == 0`, and the mount task can reach it in that+        // state — it races `DocumentReaderView`'s parse task with nothing sequencing the+        // two. Raising the flag there would pair it with no load at all, leaving the wait+        // polling a controller nothing ever told to load (T-1744 review). It self-heals+        // when the parse re-keys the task, but "usually superseded in time" is not an+        // invariant; this is.+        #expect(!DocumentLoadingIndicatorPolicy.shouldIndicatePreparation(parseRevision: 0))+    }++    @Test("Indicator raised for any revision that will actually load", arguments: [+        UInt64(1), 2, 17, UInt64.max+    ])+    func indicatorRaisedForRealRevisions(revision: UInt64) {+        #expect(DocumentLoadingIndicatorPolicy.shouldIndicatePreparation(parseRevision: revision))+    }++    // MARK: - mayClearFlag++    @Test("An uncancelled (current) task may clear the flag it raised")+    func uncancelledTaskMayClear() {+        #expect(DocumentLoadingIndicatorPolicy.mayClearFlag(wasCancelled: false))+    }++    @Test("A cancelled (superseded) task must never clear the flag")+    func cancelledTaskMustNotClear() {+        // A cancelled load was superseded by a fresher `.task(id:)` instance that has+        // already raised the flag for its OWN revision (T-1975-style overlap: the+        // outgoing task can still be suspended inside an uninterruptible off-main emit+        // when the successor starts). If the stale task were allowed to clear the flag+        // on its way out, the spinner would flicker off mid-load for the revision+        // actually on screen — the "clears early" failure mode called out for this fix.+        #expect(!DocumentLoadingIndicatorPolicy.mayClearFlag(wasCancelled: true))+    }++    // MARK: - Constants++    @Test("The backstop is generous enough not to fire on a legitimately slow load")+    func backstopLeavesRoomForARealLoad() {+        // A 10 MB document's off-main emit + SwiftSoup pass + WebKit layout runs to the+        // better part of a minute (the window this overlay exists for). A backstop that+        // fired on that would restore the exact bug being fixed — spinner gone, page+        // still blank — on the documents that need the indicator most. Pinned so a+        // future "let's make it snappier" tightening has to argue with this comment.+        #expect(DocumentLoadingIndicatorPolicy.maximumWait >= .seconds(90))+        #expect(DocumentLoadingIndicatorPolicy.pollInterval <= .milliseconds(250))+    }+}

diff --git a/docs/agent-notes/webview-rendering-status.md b/docs/agent-notes/webview-rendering-status.mdindex db25253..8412051 100644--- a/docs/agent-notes/webview-rendering-status.md+++ b/docs/agent-notes/webview-rendering-status.md@@ -241,6 +241,8 @@ take down unrelated applications' web content. Always PID-verify, never pattern- - **`InlineHTMLRenderer` run offsets are always relative to the string it is handed** (its source cursor restarts at 0 on every call), and the runs are consumed against `MarkdownBlock.textContent`, which JOINS the sub-spans (cells with `" | "`, rows/items with `"\n"`). A sub-span caller that appended its runs unrebased mis-anchored every selection notes made past the first cell/item — invisible in the emitted HTML, visible only when run offsets are compared against `textContent`. Closed structurally in **T-1941**: `BlockHTMLEmitter.renderInline` takes a **required** `InlineSpan` (`.wholeBlock` / `.subspan(offset:)` / `.unmapped`), is the only place that appends to `context.blockRuns`, and does the rebase itself — so a new caller cannot omit it, only state it wrongly. Offsets come from `MarkdownBlock.tableCellTextOffsets` / `listItemTextOffsets`, which live beside `textContent` and use the same separator constants, so the two sides cannot drift. Anything absent from `textContent` (nested list items, continuation paragraphs, nested blocks, a `<details>` child list, a nested `<details>` summary, the child-less blockquote fallback) is `.unmapped` → selection declined (Decision 8), never mis-anchored; mapping the nested cases would need `textContent` widened to contain them, tracked as **T-2032** — it is a scoped-out limitation, not a defect. `EmittedDocument.badgeSourceStarts` is deliberately NOT rebased: it is keyed by the inline source string and matched against that string's own occurrence scan in `SearchStateFeeder` (T-1853). Regression guard: `WebStructuredSourceMapInvariantTests.parityCorpusRunsMonotonic` sweeps the whole parity fixture corpus asserting runs are monotonic, non-overlapping AND within their block's `textContent` UTF-16 length — the upper bound is what catches a Character-count offset, which stays monotonic and would otherwise pass. - **`InlineHTMLRenderer.Walker.locate` is a naive forward scan, and it is only affordable because failed scans are recorded** (T-1966). The search cannot be BOUNDED in the general case — text legitimately sits far ahead of the cursor whenever the walk skipped source it does not account for (a long image `src`, a long raw-HTML span), so only the pre-badge segment (PR #326) and `claimOccurrences` (T-1992) get bounds. Without a record, a text node whose rendered text does not occur verbatim in the source — the entity/escape family, `A` spelled `&#65;` — scanned to the end of the block, failed, left the cursor where it was, and the next such node re-derived the same scan: `*&#65;* ` x 3200 took 12s. Two exact rejections fix it: `provedAbsent` (a text an unbounded scan proved absent stays absent, capped at 256 entries so the T-2034 class cannot grow it with the block) and a lazily-built `sourceUnits` bitset (a text holding a unit the source does not hold cannot occur in it anywhere; a bitset over the 16-bit domain rather than a `Set`, because the probe runs once per unit of every later text and hashing dominated it). A match at the cursor is tested BEFORE either memo, so the common case — text sitting exactly where the walk expects it — pays for neither. **`provedAbsent` rests on `cursor` never moving backwards**, which is why `locate` takes NO `from:` parameter and reads `cursor` itself: the precondition is structural, not documented. Every mutation of `cursor` is forward, but one of them is only forward *because of the pre-badge bound* — `appendFootnoteBadge` steps to the occurrence's end, and the `appendVisible` before it must stay bounded by `occurrence.sourceStart` or the cursor could overshoot; weakening that bound breaks the memo, not just performance. No rejection changes any output: digests over a 4000-sample generated corpus (html + every run + `badgeSourceStarts`) are byte-identical to `origin/main` at 09cd828, and that corpus is now COMMITTED (`InlineRenderCorpusEquivalenceTests`, seeded SplitMix64, chunk digests pinned, re-blessing protocol in the file header; `PRISM_INLINE_CORPUS_FULL=1` for all 4000). Residual, deliberately open: a distinct-per-node text absent from the source whose every unit is present still costs a scan each — **T-2034**, needs a source index to close. Guards: `InlineSourceMapScanGrowthTests` (G1-G8 growth over the shared `GrowthRatioGuard`, O1-O6 rendered-text-in-order + run invariants, O7 exact pre-fix run coordinates on memo-HIT fixtures, O8 the recording gate — a bounded miss must record nothing or a later unbounded match is silently suppressed). - **The inline re-parse can hand `InlineHTMLRenderer.Walker` BLOCK nodes, and an unhandled one drops content silently** — three instances so far, all the same shape. `render` re-parses each block's inline source with `Document(parsing:)`, so a source string that satisfies a *block* grammar comes back as block structure rather than a `Text` node, and `MarkupWalker`'s default descend emits nothing for it: content vanishes with no error, no fallback, and no visible trace except an empty cell/item/heading. **T-1640** `1.`/`3)` at the start of an item (→ `OrderedList`), **T-1641** `@Observable` (→ `BlockDirective`; closed by NOT passing `.parseBlockDirectives`, matching `MarkdownBlockParser`), **T-1669** text that is exactly `---`/`***`/`___` (→ `ThematicBreak`). The two structural fixes render the LITERAL source line/marker as a mapped run (`literalListMarker`, `literalThematicBreak`) rather than reconstructing it from the node — a `ThematicBreak` has no text and no children to descend into, so there is nothing to reconstruct from. **The class is not closed**: the Walker still has no `visitHeading`, `visitBlockQuote`, `visitCodeBlock`, `visitHTMLBlock` or `visitTable`, so `# Title` in a table cell drops its `#` (T-1640 shape) and a re-parsed code block or HTML block would drop entirely (T-1669 shape). Before adding a `renderInline` call site, check what its strings can re-parse into; when one of these turns up, add the visitor rather than pre-sanitising the string. The defensive gap-skipping in `visitThematicBreak` is for a broken cursor invariant only — every call site today passes a SINGLE-BLOCK string, so a stranded cursor is unreachable and the branch is deliberately not hardened further (it degrades to a stray space, or to rendering the line it lands on).+- **An injected navigation stream that fails on EVERY subscription hot-loops the MainActor for as long as the controller lives** (T-1744). `WebDocumentController.observeNavigations` re-subscribes after a `.navigationFailed` — that is the deliberate benign-bad-link classification (T-1943/T-2107) — so a `test_setInjectedNavigationStream` provider that throws a non-termination error each time never terminates the loop: it spins at one `Task.yield()` per iteration. Not literally forever — the observation task captures `[weak self]`, `applyOutcome` returns false once the controller is gone, and `deinit` cancels the task — but a test holding the controller keeps it spinning for that test's whole lifetime, and that is long enough: every test in the offending file passed in isolation; run as part of the full suite it starved every later `@MainActor` test, blew their timing bounds, and took the host down — 174 reported failures, ~168 of which never ran. A stream failing with `webContentProcessTerminated` is safer by contrast, but only ONCE a load has been issued (the retry budget spends and `applyNavigationOutcome` returns false, ending the loop); before the first `load` there is no `loadedDocumentURL`, `attemptRecovery` charges nothing and keeps observing, so a terminating stream hot-loops exactly like an ordinary one — hence the three-step setup in `waitEndsWhenRecoveryIsAbandoned`. Fail ONCE then park (`DocumentLoadingIndicatorWiringTests.failsOnceThenParks`). Corollary for any `@MainActor` test that polls: Swift Testing runs these concurrently on the one executor, so a tight timing bound measures machine load, not the code — pick a bound loosely, then mutation-check that it still sits inside the failure mode.+- **`page.navigations` cannot say WHICH navigation failed, and the failure it hands you is usually the one you just superseded** (T-1744 review). It is one subscription for the whole page, and calling `page.load` while a navigation is in flight is exactly what makes WebKit fail the outgoing one — so an outcome read off that stream lands on whichever load is current, which is the load that caused it. Any per-load state derived from it is mis-attributed by default, and the window is not narrow: `WebDocumentController.load` does not cancel the outgoing navigation, and a re-parse landing during the previous load's emit overlaps two loads routinely. The loading indicator's exit 3 (`navigationFailedBeforeSettling`) was built on it and so dropped the spinner over a document that was still loading. **The sequence returned by `page.load(_:)` is scoped to that one navigation** — that is what per-load state belongs on. `issueNavigation` is now the single place **`WebDocumentController`** tells its page to load — other pages have their own call sites (`FootnotePopoverWebPage`), and the structural pin only greps this one file; it stamps a `navigationEpoch`, and the flag is computed as `failedNavigationEpoch == navigationEpoch`, so a stale failure records an epoch that can never match and a fresh load retires the previous failure just by bumping past it — no clear to get stale in the other direction either. `page.navigations` keeps the job it suits: crash recovery, which is page-level and needs no attribution. Corollary for tests: `test_setInjectedNavigationStream` parks only the page-wide stream, so a real `load` to a URL the scheme handler rejects still raises exit 3 for real — give a test that does not want one a servable URL (`PrismDocSchemeHandler(documentHTMLProvider:)`). - **`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 8c650b6..227b1e1 100644--- a/CHANGELOG.md+++ b/CHANGELOG.md@@ -25,6 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Refreshing a document opened from a URL no longer leaves a background download running loose after you close it (T-1805). Tapping Refresh started an untracked download with no way to stop it: closing the document, or navigating to a different one, before the download finished let it keep running, and when it finally completed it could still overwrite that URL's title in Recent Files — even though you had already moved on — or race a second refresh for a stale result. Closing the document, or starting another refresh, now cancels the one in flight, and a refresh that has been superseded or cancelled no longer writes its result anywhere. - Refreshing a document opened from a URL no longer leaves a background download running loose after you close it (T-1805). Tapping Refresh started an untracked download with no way to stop it: closing the document, or navigating to a different one, before the download finished let it keep running, and when it finally completed it could still overwrite that URL's title in Recent Files — even though you had already moved on — or race a second refresh for a stale result. Closing the document, or starting another refresh, now cancels the one in flight, and a refresh that has been superseded or cancelled no longer applies its result once another one has taken over. - The unlock screen no longer spins forever when the App Store returns some but not all of Prism's products (T-1841). Product fetching only reported an error when the whole catalog came back empty; if the store returned the tip products but omitted the unlock product specifically, the purchase button never appeared and the spinner never resolved, with no error message and no way to retry. A fetch that comes back without the unlock product is now treated as a load failure in every case, so it always ends in the existing "Unable to load purchase options" message with a Retry button, never an indefinite spinner. The unlock row in Settings, which sat on the same endless spinner, now reads "Unavailable" and still opens the unlock screen when tapped. Tips are unaffected: a catalog that returned the tips but not the unlock product still fills the tip jar, so an already-unlocked user sees nothing change.+- Opening a large or HTML-heavy document now shows a "Preparing document…" indicator while it loads, instead of an unresponsive-looking blank page (T-1744). Moving the document's HTML build off the main thread (T-1681) kept the app responsive during that work, but left nothing on screen to say the document was still coming — for a large document that window can run to the better part of a minute. An earlier attempt at the indicator, reverted before T-1681 shipped, read the loading state directly from a `ViewBuilder` and never appeared on device in either of its two gated forms; the indicator now drives off a plain flag flipped by the load itself, so its visibility no longer depends on that. It also cannot stay up indefinitely, which for a full-page indicator matters more than the indicator itself — one that never goes away hides the document instead of merely failing to explain it. Every way it ends is accounted for: the page finishes laying out; WebKit's renderer crashes and crash recovery gives up, at which point the indicator steps aside for the existing "This document stopped rendering" banner; the document fails to load for an ordinary, non-crash reason, which reveals the page rather than waiting on a load that is not coming; a second edit lands before the first finishes, and the superseded load's cleanup never erases the flag the fresher one is relying on; and for anything not on that list the indicator gives up by itself after a bound set well past any real load. It is also never shown before there is a document to load at all. - 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.

diff --git a/prism/Localizable.xcstrings b/prism/Localizable.xcstringsindex af181fa..3e9bd00 100644--- a/prism/Localizable.xcstrings+++ b/prism/Localizable.xcstrings@@ -3382,6 +3382,29 @@         }       }     },+    "Preparing document…": {+      "extractionState": "manual",+      "localizations": {+        "en": {+          "stringUnit": {+            "state": "translated",+            "value": "Preparing document…"+          }+        },+        "en-GB": {+          "stringUnit": {+            "state": "translated",+            "value": "Preparing document…"+          }+        },+        "en-US": {+          "stringUnit": {+            "state": "translated",+            "value": "Preparing document…"+          }+        }+      }+    },     "Preview showing current font and theme settings": {       "extractionState": "manual",       "localizations": {

diff --git a/.claude/review.json b/.claude/review.jsonindex 01a8351..df4f010 100644--- a/.claude/review.json+++ b/.claude/review.json@@ -1,180 +1,220 @@ {   "repo": {     "name": "prism",-    "path": "/Users/arjen/projects/personal/prism-worktrees/T-1840",-    "branch": "T-1840/bugfix-clipboard-prose-rewritten-as-mermaid",+    "path": "/Users/arjen/projects/personal/prism-worktrees/T-1744",+    "branch": "T-1744/bugfix-loading-indicator-missing-during-offmain-emit",     "remote": "origin/main"   },-  "title": "Pre-push review: T-1840 clipboard prose rewritten as mermaid",-  "subtitle": "<p>Clipboard paste auto-wraps text that looks like raw mermaid source in a <code>```mermaid</code> fence. Ordinary prose was tripping that check. Four commits over five review rounds narrow it to a three-tier heuristic. This review found — and fixed — a detection regression the five rounds missed: class, ER, and state diagrams whose member block spans lines silently stopped being detected.</p>",+  "title": "Pre-push review: T-1744 loading indicator",+  "subtitle": "<p>A &ldquo;Preparing document&hellip;&rdquo; overlay that tracks the real end of loading &mdash; off-main HTML emit plus WebKit layout &mdash; rather than <code>session.isLoading</code>. Round 6, after five rounds of review. <strong>The engineering underneath is sound; the premise it rests on has never been verified.</strong></p>",   "metrics": [-    {"label": "branch", "value": "T-1840/bugfix-…-as-mermaid"},+    {"label": "branch", "value": "T-1744/&hellip;offmain-emit"},     {"label": "commits", "value": "4 + review fixes"},-    {"label": "files", "value": "3 touched"},-    {"label": "lines", "value": "+574 / -33"},-    {"label": "targeted tests", "value": "115 / 115 pass"},-    {"label": "lint", "value": "0 violations"}+    {"label": "files", "value": "7 touched"},+    {"label": "lines", "value": "+1228 / -17"},+    {"label": "prod vs test", "value": "327 prod / 874 test"},+    {"label": "unit tests", "value": "4516 run, 2 failed (unrelated)"}   ],    "verdict": {-    "label": "Ready to push",-    "tone": "success",-    "detail": "<p><strong>Ready to push, after the fixes applied during this review.</strong> The shipping heuristic is sound and the change is, with one exception, a strict safety improvement: I replicated both <code>origin/main</code>'s and the branch's logic in a standalone harness and ran a 40-case corpus through each — every prose input the branch wraps, main wrapped too, so <em>no new false positive was introduced</em>.</p><p>The exception was real and is now closed. Moving from whole-text to per-line matching silently un-detected <code>classDiagram</code>, <code>erDiagram</code>, and <code>stateDiagram</code> sources whose member block spans lines (<code>class Animal{</code> / <code>+int age</code> / <code>}</code>) — the form used in mermaid's own documentation. <code>\\w\\{[^}]+\\}</code> requires the brace pair to close on one line. The code comment that considered this exact corner reached the wrong conclusion, calling it \"not valid diagram source\"; for the brace shape it is the member block, not a label. No test covered it, which is why five rounds missed it. A member-block-opener pattern in the weak tier restores all three, verified against an 18-case corpus with zero new false positives.</p><p>Two CHANGELOG statements were also factually wrong and are corrected. Everything else is residual scope inherited from <code>main</code>, or follow-up work — recorded below, none of it blocking.</p>"+    "label": "Requires discussion",+    "tone": "warning",+    "detail": "<p>Nothing here is broken. Lint is clean, both platform builds succeed, all 30 new tests pass, and the two suite failures are pre-existing live-WebKit timing tests in files this diff does not touch. The navigation-epoch work is a genuine correctness fix that stands on its own merits.</p><p><strong>What blocks a confident merge is a question, not a defect.</strong> This feature has already shipped dead once (T-1681). The stated reason it shipped dead &mdash; that reading <code>webController?.isLayoutSettled</code> from a <code>ViewBuilder</code> &ldquo;never re-evaluated&rdquo; &mdash; is contradicted by the overlay eight lines below the new one in the same file, which reads <code>webController?.recoveryAbandoned</code> through the same optional, the same <code>@Observable</code> class, the same <code>@ViewBuilder</code>-into-<code>.overlay</code> shape, and works in production today (T-1943). The revert commit says only &ldquo;did not display on-device in either form&rdquo;; the root cause was never found. This PR keeps the <em>identical</em> overlay view at the <em>identical</em> attachment point and changes only the gate. If the cause was anything other than the gate &mdash; compositing against the WKWebView-backed surface, sizing, or the window simply being too short to see &mdash; this ships dead a second time, and 1228 lines with it.</p><p>Five rounds could not see this because no test renders the view, and the review environment cannot either: screen recording and UI-test automation permissions are both unavailable here, so a throwaway XCUITest probe written for this review could not run. <strong>One person opening one large document on a device answers it in thirty seconds.</strong> Do that before merging, and check the two behaviours flagged below while the app is open.</p>"   },    "at_a_glance": [-    "<strong>Verified no new false positives.</strong> A standalone replica of both <code>main</code>'s and the branch's logic over a 40-case corpus: every prose shape the branch wraps, <code>main</code> wrapped too. The branch is a strict subset of <code>main</code>'s wrapping, apart from the multi-line-span corner.",-    "<strong>Fixed: a real detection regression.</strong> Fields-only <code>classDiagram</code> / <code>erDiagram</code> / <code>stateDiagram</code> bodies stopped being detected when matching went per-line. Restored with a member-block-opener pattern plus four regression tests.",-    "<strong>Fixed: two false CHANGELOG claims.</strong> <code>section</code> was listed as applying to pie (it does not — only <code>title</code> does), and \"detection of every real diagram shape is unchanged\" was untrue for period-terminated gantt/journey/pie directive lines.",-    "<strong>Added: coverage for four unpinned branches.</strong> The <code>trimmed.last != \".\"</code> clause, the Timeline and Pie Chart directive pairings, <code>maxDeclarationTokenCount</code>, and the <code>%%</code> comment skip could each be deleted with a green suite. Now pinned.",-    "<strong>Consolidation question answered: follow-up, not this PR.</strong> <code>declaredDiagramType(in:)</code> duplicates only ~6 lines of <code>MermaidTypeParser</code>'s line walk. Its token-count rule is a clipboard heuristic, not mermaid grammar, so it does not belong in the parser — and tightening <code>parse</code> is off the table (two user-visible callers, ~40 assertions).",-    "<strong>No pre-existing test was modified.</strong> Contrary to the working note, the diff against <code>origin/main</code> is a pure append (+348 / −0) — the two lines rewritten in round 4 belonged to a test this branch itself added in round 3.",-    "<strong>Not a performance regression.</strong> Measured: the per-line rewrite is 15–25% <em>faster</em> than <code>main</code> on the 10 MB worst case, because per-line matching caps ICU backtracking. Both still block the main thread for ~11 s at that size — pre-existing, worth a follow-up bound."+    "<strong>The overlay is still unproven.</strong> The wiring tests run the real wait loop and pin the call site structurally, and their header says plainly that they do not render the view. The sibling <code>recoveryAbandonedBanner</code> uses the very pattern this PR declares unreliable and works &mdash; so the diagnosis behind the redesign is unsupported. A device check is warranted before merge.",+    "<strong>The 120s backstop ends in silence.</strong> When it fires the overlay simply disappears, revealing whatever the web view holds &mdash; most likely the blank page T-1744 set out to fix, now after two minutes of spinner, with no message and no Reload. Exit 3 (ordinary navigation failure) does the same. Exit 2 hands off to the existing &ldquo;This document stopped rendering&rdquo; banner; exits 3 and 4 should too.",+    "<strong>Every load gets the overlay, not just the first.</strong> An external file change bumps <code>parseRevision</code>, re-keys the load task, and covers the <em>already-rendered</em> document with an opaque full-page panel &mdash; with a 0.2s fade in and out and no delay-before-show. For a small document that is a visible flash on every save.",+    "<strong>Six prose overclaims found and fixed in this review</strong>, including one stated as an invariant that the code does not establish. The claim audit verified the remaining load-bearing statements as true.",+    "<strong>The epoch work is the durable part.</strong> Routing every navigation through one <code>issueNavigation</code> and attributing failures by monotone epoch is correct and valuable independent of the spinner. The policy enum's three one-line predicates and their 143-line test file are scaffolding by comparison."   ],    "explanation": {-    "beginner": "<h3>What changed</h3><p>Prism lets you paste text straight from the clipboard and read it as a document. As a convenience, if the pasted text <em>looks like</em> a mermaid diagram written without its code fence, Prism adds the fence for you so the diagram renders instead of showing as plain text.</p><p>The trouble was that the \"looks like a diagram\" test was too easy to satisfy. Several mermaid keywords are also everyday English words — <em>graph</em>, <em>pie</em>, <em>journey</em>, <em>timeline</em>, <em>kanban</em>, <em>architecture</em>, <em>block</em>. So a paragraph that merely <em>started</em> with one of those words passed the first check. It then only needed one more clue, and the clues were things ordinary writing contains: a numbered citation like <code>results[3]</code>, a bracket-free aside like <code>the tool(a favorite)</code>, or a sentence beginning with the word \"section\" or \"title\". Paste such a paragraph and Prism would wrap it as a diagram — and since it was not a diagram, you got a broken one.</p><h3>Why it matters</h3><p>You paste some prose and the app silently rewrites it into something unreadable. Nothing is lost, but the feature actively works against you.</p><h3>How it was fixed</h3><p>The clues are now sorted by how likely ordinary writing is to produce them. Arrows and similar operators (<code>--&gt;</code>, <code>-&gt;&gt;</code>) are things prose essentially never writes, so they always count. Brackets, parentheses and the words \"section\"/\"title\" are things prose writes constantly, so they are ignored on any line that ends the way a sentence ends — with a full stop, question mark or exclamation mark.</p><p>One catch: real diagram titles genuinely do ask questions (<code>title Are we on track?</code>). So those keywords get a narrow pass — but only when the text actually opens by <em>declaring</em> a diagram, i.e. the keyword standing alone on its own line (<code>gantt</code>), not a paragraph that merely begins with it.</p><h3>What this review added</h3><p>The rewrite had an unnoticed side effect. Because the check now looks at one line at a time rather than the whole text at once, some diagrams whose shape spans several lines stopped being recognised — specifically class, ER and state diagrams that list their fields across lines. Those are now detected again, and the changelog's description was corrected where it was inaccurate.</p>",+    "beginner": "<h3>What changed</h3><p>When you open a markdown file in Prism, the app has to turn the markdown into HTML and then hand that HTML to a web view to draw. For a big or HTML-heavy file that work can take the better part of a minute. Until now the screen was simply blank for that whole time, which looks like the app has crashed.</p><p>This change puts a spinner and the words &ldquo;Preparing document&hellip;&rdquo; over that blank area until the document is actually ready.</p><h3>Why it matters</h3><p>The app already had a loading flag, but it turned off as soon as the <em>parsing</em> finished &mdash; long before anything appeared on screen. So the indicator vanished while the user was still looking at nothing. The fix tracks the real end of loading instead.</p><h3>Key concepts</h3><ul><li><strong>Overlay</strong> &mdash; a view drawn on top of another. Here it covers the document area while loading.</li><li><strong>The wait loop</strong> &mdash; a small piece of code that checks, ten times a second, whether the document is ready yet, and hides the spinner when it is.</li><li><strong>Exit paths</strong> &mdash; all the different ways loading can end, including the ways it can go wrong. A spinner that never goes away would be worse than the original bug, because it would hide the document too. The code enumerates five of them.</li></ul><h3>The catch</h3><p>This exact feature was written once before and quietly removed, because it never actually appeared on screen and nobody worked out why. This version changes how the spinner is switched on, but not the spinner itself or where it sits. So it is worth someone opening a large file and confirming that they can see it.</p>", -    "intermediate": "<h3>Architecture</h3><p><code>ClipboardService.validateContent</code> runs <code>wrapMermaidIfNeeded</code> on every paste. That function is a conjunction of four gates: no existing code fence, <code>MermaidTypeParser.parse</code> recognises the first word, <code>containsMermaidSyntax</code> finds diagram syntax, and there are 2+ non-empty lines. Only the third gate changed.</p><h3>The three tiers</h3><p><code>containsMermaidSyntax</code> went from one whole-text regex pass over a flat 22-pattern list to a per-line loop over three tiers:</p><ul><li><strong>Strong</strong> (16 patterns) — arrow operators, class/ER relationship operators, pie's <code>\"label\": number</code>. Matched on every line unconditionally, because a sequence-diagram message legitimately ends in sentence punctuation (<code>Alice-&gt;&gt;Bob: Are you there?</code>).</li><li><strong>Weak shape</strong> (9, now 10) — bracket/paren/brace node shapes, edge-label pipes, state markers, <code>-x</code>. Suppressed on any line ending in <code>.</code>, <code>?</code> or <code>!</code>, because each has a prose double.</li><li><strong>Weak directive</strong> (3) — <code>section</code>, <code>title</code>, <code>dateFormat</code>, each paired with the set of diagram types that actually use it. Same suppression, with one exemption.</li></ul><h3>The exemption, and why it needs two conditions</h3><p>Round 2 gated everything on sentence punctuation, which broke genuine <code>title Are we on track?</code> lines. Round 3 exempted <code>?</code>/<code>!</code> for directives — but that reopened the bug, since a paragraph starting \"Timeline mapping is useful\" also parses as Timeline. Round 4 added the second condition: <code>declaredDiagramType(in:)</code> requires the opening line to be a bare <em>declaration</em> (keyword plus at most one direction/modifier token), and the declared type must admit that specific directive. Both conditions are necessary; either alone regresses in one direction.</p><h3>Trade-offs</h3><p>A trailing full stop is never exempt, in any type. That is deliberate — a period-terminated title reads as prose everywhere — and it does narrow detection: <code>gantt</code> + <code>title Project schedule.</code> no longer wraps. The spec's stated preference (\"false positives are worse than false negatives\") supports the call; the CHANGELOG denying it was the problem, now fixed.</p><h3>What this review changed</h3><p>Per-line matching means negated-character-class patterns can no longer span a newline. The author reasoned about this and judged it safe. That holds for bracket, paren and quote labels; it does <em>not</em> hold for <code>\\w\\{[^}]+\\}</code>, because a multi-line brace block is a class/ER/state member block, and a fields-only body carries no other syntax on any line. A member-block-opener pattern in the weak tier restores it.</p>",+    "intermediate": "<h3>Architecture</h3><p>The overlay is driven by a plain <code>@State private var isPreparingDocument</code> on <code>DocumentScrollContent</code>. The view's load task (<code>.task(id: WebLoadKey(hasController:revision:))</code>) sets it at the top of every incarnation that will actually load something, then awaits <code>WebDocumentControllerFactory.loadDocument</code> (off-main HTML emit + navigation), then awaits <code>DocumentLoadingIndicatorPolicy.waitForPreparation</code>, then clears the flag &mdash; but only if the task was not cancelled.</p><p><code>waitForPreparation</code> polls the controller every 100ms until one of four conditions holds: <code>isLayoutSettled</code>, <code>recoveryAbandoned</code>, <code>navigationFailedBeforeSettling</code>, or a 120-second deadline. Cancellation is the fifth exit, checked by the loop's own <code>while</code> condition.</p><h3>The epoch mechanism</h3><p>Exit 3 needed a signal that did not previously exist. An ordinary (non-crash) navigation failure is deliberately classified as a benign bad link and charges no recovery budget (T-1943/T-2107), so neither <code>isLayoutSettled</code> nor <code>recoveryAbandoned</code> would ever become true &mdash; the wait would poll forever.</p><p>The naive fix &mdash; read the failure off <code>page.navigations</code> &mdash; is wrong in a subtle way that took two review rounds to find. That stream carries every navigation on the page through one subscription, and calling <code>page.load</code> while a navigation is in flight is precisely what makes WebKit fail the outgoing one. So the failure that arrives is usually the load you just <em>superseded</em>, arriving after its successor is already under way. Attributing it to the successor dropped the spinner over a document that was still loading &mdash; the original bug, restored by its own fix.</p><p>The final design routes every <code>page.load</code> through a single <code>issueNavigation</code> that stamps a monotone <code>navigationEpoch</code>, and records failures from the sequence <code>page.load(_:)</code> itself returns, which is scoped to that one navigation. <code>navigationFailedBeforeSettling</code> is then simply <code>failedNavigationEpoch == navigationEpoch</code> &mdash; a stale failure records an epoch that can never match, and a fresh load retires the previous one just by bumping past it, so there is no clear to go stale in the other direction either.</p><h3>Trade-offs</h3><ul><li><strong>Polling over observation.</strong> <code>isLayoutSettled</code> and <code>recoveryAbandoned</code> are both <code>@Observable</code>; only the new <code>navigationFailedBeforeSettling</code> is not, because its backing storage was marked <code>@ObservationIgnored</code> in this PR. Making it observable would collapse the whole apparatus into a derived <code>@ViewBuilder</code> condition &mdash; which is exactly the shape the PR rejects.</li><li><strong>Structural tests over rendering tests.</strong> Five tests grep the source of <code>DocumentScrollContent.swift</code> for exact strings. They catch the &ldquo;wiring missing entirely&rdquo; failure class that shipped three times here, at the cost of breaking on any reformat and passing on commented-out code.</li></ul>", -    "expert": "<h3>The failure mode, precisely</h3><p>The bug class is a conjunction whose weakest conjunct does no work. <code>MermaidTypeParser.parse</code> inspects only the first whitespace-delimited token of the first non-comment line, by design — it exists to <em>label</em> source already known to be a diagram, so permissiveness costs it nothing. Reused as an admission gate it admits any paragraph whose first word collides with a keyword, and ten of the 26 keywords are ordinary English. That reduces the whole heuristic to <code>containsMermaidSyntax</code>, which was a flat disjunction over 22 patterns applied to the whole text.</p><h3>Why the punctuation discriminator is the right shape — and its limits</h3><p>The insight is that mermaid syntax lines are terse and unterminated while prose sentences terminate. That is a cheap, language-free discriminator. But it is evaluated on the line's <em>final character</em>, which bounds what it can cover: it fires on terminated sentences only. Hard-wrapped prose, headings, bullet lists, table rows, and sentences closed by a quote or bracket (<code>\"stop[3]!\"</code>) all slip through. I verified each still wraps. All were wrapping on <code>main</code> too, so none is a regression — but the doc comments and CHANGELOG present the gate as <em>the</em> prose/diagram discriminator, which overstates it. The residual class is narrowed, not closed.</p><h3>The regression per-line matching introduced</h3><p>Whole-text matching gave the negated character classes (<code>[^\\]]</code>, <code>[^)]</code>, <code>[^}]</code>, <code>[^\"]</code>) implicit newline-spanning power. The doc comment retires that deliberately, arguing mermaid labels use <code>&lt;br/&gt;</code> and a raw newline inside a bracket label is invalid. Sound for labels — but <code>\\w\\{[^}]+\\}</code> was not matching a label. It was matching the member block:</p><pre>classDiagram\nclass Animal{\n +int age\n +String gender\n}</pre><p>Fields-only bodies carry no arrow, no paren, no quoted number — nothing on any single line. Same for <code>erDiagram CUSTOMER{ … }</code> and composite <code>state First{ … }</code>. A class diagram <em>with methods</em> survives incidentally (<code>+deposit(amount)</code> matches the paren shape), which is exactly the kind of partial survival that hides a regression from spot checks. Verified against both implementations: <code>main</code> wraps all three, the branch wrapped none.</p><p>The fix is <code>#\"\\w[^\\S\\n]*\\{[^\\S\\n]*$\"#</code> in the weak tier — an identifier followed by an unclosed brace at line end. It is in the weak tier for consistency, though the punctuation gate is vacuous for it (a line ending in <code>{</code> never ends in <code>.?!</code>). Prose ending in <code>identifier{</code> is essentially nonexistent, and pasted pseudo-code is excluded because <code>if (x) {</code> has a non-word character before the brace.</p><h3>Residual holes in the new mechanism</h3><p><code>maxDeclarationTokenCount = 2</code> accepts <code>flowchart LR</code>, but also accepts two-word prose openers: <code>Timeline overview</code> / <code>section 4 answers this?</code> wraps, as does <code>Journey mapping</code> / <code>title case matters, right?</code>. The headline test for this exemption uses the three-token <code>Timeline mapping is useful</code>, which passes while its two-token sibling fails. Tightening would mean constraining the second token to a direction or known modifier rather than counting it — a design change, not a fix, so it is recorded as follow-up rather than applied here.</p><h3>Performance</h3><p>Measured, not estimated: on a 10 MB no-match paste the per-line version runs 11.4 s against <code>main</code>'s 15.3 s — 15–25% <em>faster</em>, because per-line ranges cap ICU backtracking on the negated classes that previously scanned the whole buffer. So this is not a regression. It is, however, still an ~11 s synchronous main-thread stall at the 10 MB ceiling, plus three full <code>components(separatedBy:)</code> materialisations, two of which exist to read a single line. Bounding the scan is a ~3-line change worth roughly 490× on that path, and belongs in its own ticket.</p>"+    "expert": "<h3>The premise problem</h3><p>The design rationale, repeated in the view, both test files, the CHANGELOG and the agent notes, is that a <code>ViewBuilder</code> read of <code>webController?.isLayoutSettled</code> &ldquo;never re-evaluated&rdquo; on device. That claim does not survive contact with the file it is written in. <code>recoveryAbandonedBanner</code> at <code>DocumentScrollContent.swift:363</code> is <code>if webController?.recoveryAbandoned == true</code> &mdash; same optional <code>@State</code> controller, same <code>@Observable</code> class, same computed <code>@ViewBuilder</code> fed into <code>.overlay</code>, applied eight lines below the new one. <code>isLayoutSettled</code> (<code>:78</code>) and <code>recoveryAbandoned</code> (<code>:1081</code>) are both plain observable stored properties; neither is <code>@ObservationIgnored</code>. There is no mechanism by which Observation tracks one and not the other.</p><p><code>c2926f0</code> (&ldquo;Remove the non-working loading indicator&rdquo;) records the symptom and no diagnosis. The diff between the reverted <code>documentLoadingOverlay</code> and this one is the gate expression and nothing else &mdash; identical <code>ProgressView</code>, identical <code>.controlSize</code>, identical <code>.frame(maxWidth:maxHeight:)</code>, identical <code>.background</code>, identical <code>.overlay { }</code> attachment point on the same <code>Group</code>. A compositing or sizing cause against the <code>WKWebView</code>-backed <code>WebDocumentView</code> would be untouched by this change.</p><h3>Where exit 1 actually lands</h3><p><code>isLayoutSettled</code> arrives from the <code>layoutSettled</code> bridge message, and <code>prism-bridge.js:294</code> posts it from an unconditional <code>setTimeout(&hellip;, 16)</code> armed alongside <code>ready</code>. Feature scripts can post earlier; nothing posts later, because <code>notifyLayoutSettled</code> debounces to one post. So in the common case <code>isLayoutSettled</code> means &ldquo;DOM parsed, plus 16ms&rdquo; &mdash; not &ldquo;laid out&rdquo; in the sense the exit table's first row implies. The bulk of the window this overlay exists for (the off-main emit) is covered correctly, because the flag is raised before <code>loadDocument</code>. But WebKit's own layout and first paint of a very large DOM happen <em>after</em> that timer, so the overlay may lift a beat before the document is visible on precisely the documents it was built for. Worth watching in the device check.</p><h3>Failure-mode asymmetry</h3><p>Exit 2 hands the reader to <code>recoveryAbandonedBanner</code>: a message and a Reload button. Exits 3 and 4 hand the reader nothing &mdash; the overlay lifts and reveals whatever the web view holds, which for a failed navigation is a blank page. Exit 4 does it after 120 seconds of spinner. The banner already exists, already localises, already offers the one control that recovers the document, and <code>reloadWebDocument()</code> is already in scope. Routing exits 3 and 4 to it would make the failure story uniform and make the backstop's value judgement (&ldquo;revealing a blank page is strictly better than covering it with a spinner forever&rdquo;) unnecessary, because neither outcome would be silent.</p><h3>Proportionality</h3><p>Three of <code>DocumentLoadingIndicatorPolicy</code>'s four members are one-expression predicates (<code>parseRevision &gt; 0</code>, <code>!wasCancelled</code>, four negated conjuncts) that exist to be unit-testable, and <code>DocumentLoadingIndicatorPolicyTests</code> spends 143 lines asserting them in four spellings, including one state its own comment calls impossible. The prose-to-behaviour ratio is roughly 150 lines of comment for 40 lines of logic. Against that, <code>DocumentLoadingIndicatorWiringTests</code> section 1 is genuinely load-bearing: it runs the real loop against a real controller with real failing WebKit navigations, once per exit, and its two overlap tests reproduce an ordering WebKit will not produce on demand. If any of this is trimmed later, trim the policy enum and its test file, keep the epoch attribution and section 1.</p><h3>Edge cases</h3><ul><li><code>loadDocument</code> has a second early return (<code>guard !Task.isCancelled</code> after the emit, calling <code>abandonLoad</code>) that issues no load with the flag already raised. Safe &mdash; the same cancellation ends the wait through exit 5 and <code>mayClearFlag</code> declines &mdash; but the comment claiming an invariant here was wrong and has been corrected.</li><li><code>Task.isCancelled</code> after the wait is also true on view teardown (raw-source toggle, document close), where there is no successor. Safe because <code>@State</code> is discarded with the view identity &mdash; a different argument from the one that was written down, now recorded.</li><li><code>.pageClosed</code> is classified out of exit 3, so that failure rides the full 120 seconds.</li><li><code>ContinuousClock</code> keeps advancing while the process is suspended, so a document backgrounded mid-load can burn the deadline while suspended.</li></ul>"   },    "commits": [-    {"sha": "363e291", "subject": "Fix T-1840: Clipboard prose can be rewritten as a Mermaid diagram", "author": "Arjen Schwarz", "date": "2026-08-15"},-    {"sha": "288ba72", "subject": "Address review: two-tier mermaid syntax detection", "author": "Arjen Schwarz", "date": "2026-08-15"},-    {"sha": "9b92b34", "subject": "Address review: scope the ?/! exemption to directive patterns", "author": "Arjen Schwarz", "date": "2026-08-15"},-    {"sha": "b55d497", "subject": "Address review: correlate the directive exemption with the declared type", "author": "Arjen Schwarz", "date": "2026-08-15"},-    {"sha": "working-tree", "subject": "Fixes applied in this review", "meta": "Member-block-opener pattern + corrected doc comment; two CHANGELOG factual corrections; 8 new tests pinning previously unpinned branches. Uncommitted."}+    {"sha": "c316aca", "subject": "T-1744: Show a loading indicator during off-main emit + WebKit layout", "author": "Arjen Schwarz", "date": "2026-08-17"},+    {"sha": "253c8f5", "subject": "T-1744 review: close both stuck-spinner gaps, pin the wiring", "author": "Arjen Schwarz", "date": "2026-08-17"},+    {"sha": "2ad0403", "subject": "T-1744 review: tag exit 3 with the navigation it belongs to", "author": "Arjen Schwarz", "date": "2026-08-17"},+    {"sha": "7cca4c4", "subject": "T-1744 review: scope the failure record to the current navigation", "author": "Arjen Schwarz", "date": "2026-08-17"},+    {"sha": "working-tree", "subject": "Prose corrections applied in this review", "meta": "6 overclaims fixed across CHANGELOG, agent notes, and two source files. Documentation only; no behaviour change. Uncommitted."}   ],    "important_changes": [     {-      "title": "containsMermaidSyntax: flat disjunction becomes three punctuation-gated tiers",-      "file": "ClipboardService.swift",-      "why": "This is the whole fix. Everything else in the diff supports it. It converts one whole-text regex pass over 22 patterns into a per-line loop that classifies each pattern by how plausibly prose produces it — and that per-line move is also what caused the one regression found in this review.",-      "what": "ClipboardService.swift:320-350 (containsMermaidSyntax), :53-76 (strong), :96-116 (weak shape), :132-139 (weak directive)",-      "takeaway": "When a heuristic over-fires, the useful question is rarely \"which pattern is wrong\" but \"which patterns are evidence and which are merely consistent\". Splitting one flat disjunction into evidence tiers, each with its own admission gate, keeps every pattern's detection power while letting the ambiguous ones be conditioned on context. The alternative — deleting the ambiguous patterns — would have cost detection of pie, gantt, journey and timeline, which have no operator syntax to fall back on.",-      "rationale": "The strong tier is ungated because arrows are unambiguous regardless of how the label ends — a sequence-diagram message routinely ends in sentence punctuation (Alice->>Bob: Are you there?), so gating it would have broken a mainstream diagram type. The weak tiers are gated because each pattern has a documented prose double."+      "title": "DocumentScrollContent: the overlay and the flag that drives it",+      "file": "prism/Views/DocumentScrollContent.swift",+      "why": "This is the whole user-visible feature, and the one part no test in the repo can verify. The overlay body and its attachment point are byte-identical to the version reverted in T-1681 for never appearing on device; only the gate changed. If the T-1681 cause was not the gate, this ships dead again.",+      "what": "DocumentScrollContent.swift:44-50 (flag), :96-104 (attachment), :239-278 (load task), :346-357 (overlay body)",+      "takeaway": "When a fix is a workaround for an undiagnosed failure, the workaround inherits the undiagnosed risk. Changing the trigger while keeping the view and its position identical only helps if the trigger was the problem.",+      "rationale": "The author's stated reason is that the ViewBuilder read never re-evaluated. The sibling recoveryAbandonedBanner in the same file uses that pattern successfully, which leaves the reason unsupported.",+      "rationale_inferred": false     },     {-      "title": "declaredDiagramType(in:): the second condition that keeps the exemption honest",-      "file": "ClipboardService.swift",-      "why": "The narrowest and most easily misread part of the design. It exists because the ?/! exemption added in round 3 reopened the original bug, and it is the only thing separating a punctuated gantt title from a hard-wrapped paragraph that merely starts with a keyword.",-      "what": "ClipboardService.swift:372-385, consulted at :336-339",-      "takeaway": "A permissive parser reused as an admission gate is a recurring bug source. MermaidTypeParser.parse only looks at the first word — correct for labelling source already known to be a diagram, wrong for deciding whether something is one. Rather than tightening the shared parser (which two user-visible callers depend on staying permissive), the strict question gets its own local predicate. Naming the strict variant separately, instead of hardening the lenient one, is usually the safer refactor.",-      "rationale": "Requiring a bare declaration — keyword plus at most one direction/modifier token — is what distinguishes `gantt` from `Timeline mapping is useful`. Pairing each directive with the types that admit it stops `flowchart` + `section Ship it!` claiming an exemption for a directive flowcharts do not have."+      "title": "WebDocumentController: single navigation issue point with a monotone epoch",+      "file": "prism/ViewModels/WebDocumentController.swift",+      "why": "The strongest part of the PR, and correct independent of the spinner. It fixes a real mis-attribution: page.navigations is one subscription for the whole page, and superseding a navigation is exactly what makes WebKit fail it, so a failure read off that stream is usually the outgoing load's, arriving after its successor is under way.",+      "what": "WebDocumentController.swift:820-880 (issueNavigation, recordNavigationFailure), :1089-1130 (epoch storage and the computed flag)",+      "takeaway": "Per-load state belongs on the sequence page.load(_:) returns, which is scoped to one navigation; page-level state belongs on page.navigations. Comparing a captured epoch against the current one makes staleness impossible in both directions with no explicit clear to go stale.",+      "rationale": "Round 3 found that a bare Bool had no per-load identity; round 4 found that even the epoch write could walk backwards without a guard, since two navigations' failure tasks are unordered."     },     {-      "title": "REGRESSION FOUND AND FIXED: multi-line member blocks stopped being detected",-      "file": "ClipboardService.swift",-      "why": "A genuine functional regression that survived five review rounds and 80 tests. Fields-only class, ER and state diagrams — the form in mermaid's own docs — silently stopped auto-wrapping, because per-line matching removed the newline-spanning power \\w\\{[^}]+\\} depended on.",-      "what": "ClipboardService.swift:96-118 (member-block-opener pattern), :309-322 (corrected doc comment); tests at ClipboardServiceTests.swift:795-830",-      "takeaway": "When a refactor narrows a matcher's scope, audit what each pattern was actually matching, not what its name or comment says it matched. The comment here reasoned about the newline-spanning corner and dismissed it as label syntax — true for brackets and quotes, false for braces, where the multi-line form is the member block and the only syntax present. A class diagram with methods survives incidentally, so spot-checking one fixture would not have caught it.",-      "rationale": "Verified by replicating both implementations and diffing outcomes over a corpus: main wraps all three shapes, the branch wrapped none. The restoring pattern was validated against 18 cases covering every existing test fixture with zero new false positives."+      "title": "Exit 3 and exit 4 end in silence",+      "file": "prism/Views/DocumentScrollContent.swift",+      "why": "The overlay's failure story is asymmetric. Exit 2 hands the reader a message and a Reload button; exits 3 and 4 lift the overlay onto a blank page with neither. Exit 4 does it after two minutes of spinner. That is the original T-1744 symptom with a longer preamble.",+      "what": "DocumentScrollContent.swift:541-570 (exit table), :582-591 (maximumWait), :361-388 (the banner that already solves this)",+      "takeaway": "An enumerated exit path is only as good as what the user sees when it fires. Enumerating five ways to stop waiting is not the same as having five acceptable outcomes.",+      "rationale_unknown": true     },     {-      "title": "CHANGELOG: two factual claims corrected",-      "file": "CHANGELOG.md",-      "why": "The entry is the durable user-facing record of a fix with no spec update and no bugfix report, so its accuracy carries more weight than usual here.",-      "what": "CHANGELOG.md:22",-      "takeaway": "A changelog that describes the mechanism rather than the behaviour has to be re-verified against the code every time the mechanism changes across review rounds. Both errors here are round-4 drift: the entry described the design as of round 3.",-      "rationale": "Verified empirically. `pie` + `section Ship it!` does not wrap — `section` maps to gantt/journey/timeline only, and the code's own doc comment says so correctly. And `gantt` + `title Project schedule.` no longer wraps, so \"detection of every real diagram shape is unchanged\" was false for exactly the four types the sentence named."+      "title": "The overlay fires on every reparse, not only the first load",+      "file": "prism/Views/DocumentScrollContent.swift",+      "why": "parseRevision bumps on every successful parse, including the FileChangeObserver reload after an external edit. The load task re-keys and covers the already-rendered document with an opaque full-page panel, with a 0.2s fade each way and no delay-before-show or minimum display time. Editing a document in another editor now flashes the reader's view on every save.",+      "what": "DocumentScrollContent.swift:234-253; DocumentSession.swift:613 (the bump)",+      "takeaway": "A progress indicator for a worst case needs a delay-before-show, or it becomes a flash in the common case. The threshold is conventionally 150-250ms.",+      "rationale": "Inherited from the T-1681 attempt, whose commit message states covering reparse reloads as an explicit goal. Whether that is right for a full-bleed opaque overlay was not revisited.",+      "rationale_inferred": true     },     {-      "title": "Test coverage: four load-bearing branches were deletable with a green suite",-      "file": "ClipboardServiceTests.swift",-      "why": "80 tests, yet the single most load-bearing sub-clause of the fix — `trimmed.last != \".\"` — could be removed without failing anything, because every period-terminated directive test used a prose opener that would return false regardless.",-      "what": "ClipboardServiceTests.swift:832-858 (new pinning tests)",-      "takeaway": "A test that exercises a code path is not the same as a test that pins it. These cases all reached the right answer through a different branch than the one under test, so the suite looked thorough while leaving the clause free. Checking coverage by deleting the branch and re-running is a fast way to find this.",-      "rationale": "Also pinned: the Timeline and Pie Chart directive pairings (never consulted, so a display-name rename would silently disable the exemption), maxDeclarationTokenCount, and the %% comment skip."+      "title": "Five source-text grep tests pin the call site",+      "file": "prismTests/WebRendering/DocumentLoadingIndicatorWiringTests.swift",+      "why": "These exist because no test can render the view, and because missing wiring is the exact failure class that shipped three times in this codebase. They are the right instinct applied at a cost: they assert exact whitespace and local variable names, they pass on commented-out code, and any reformat reds them.",+      "what": "DocumentLoadingIndicatorWiringTests.swift:605-715",+      "takeaway": "A source-text tripwire catches deletion, which type checking cannot. It does not catch disablement, and it makes the formatter a source of test failures. Only pageLoadHasExactlyOneCallSite discloses its own limits.",+      "rationale": "Stated in the file header: a policy enum correct in isolation while the view diverges is the T-1943 trap wearing a new hat."     }   ],    "decisions": [-    {-      "title": "Strong operator patterns are ungated; weak shape and weak directive are both gated on <code>.</code>/<code>?</code>/<code>!</code>.",-      "body": "<p>Arrows, class/ER relationship operators and pie's <code>\"label\": number</code> match on any line. Gating them would break sequence diagrams, whose messages routinely end in sentence punctuation. The ambiguous shapes and directives are suppressed on terminated lines because each has a common prose double.</p>",-      "inferred": false-    },-    {-      "title": "The <code>?</code>/<code>!</code> exemption requires BOTH a real declaration and a type that admits the directive.",-      "body": "<p>Round 3 exempted <code>?</code>/<code>!</code> for directives alone, which reopened the bug — a paragraph opening \"Timeline mapping is useful\" parses as Timeline, a type that genuinely uses <code>section</code>. Round 4 added <code>declaredDiagramType(in:)</code>. Both conditions are load-bearing: the declaration check rejects prose openers, the type pairing rejects <code>flowchart</code> + <code>section</code>.</p>",-      "inferred": false-    },-    {-      "title": "A trailing full stop is never exempt, in any diagram type.",-      "body": "<p>Deliberate, per the doc comment: a period-terminated title reads as prose everywhere. It does narrow detection — <code>gantt</code> + <code>title Project schedule.</code> no longer wraps — which the spec's \"false positives are worse than false negatives\" supports. The CHANGELOG denying it has been corrected.</p>",-      "inferred": false-    },-    {-      "title": "The brace shape gets an explicit member-block-opener pattern rather than a whole-text pass.",-      "body": "<p>Applied during this review. Restoring a whole-text pass for the brace pattern would reintroduce unbounded backtracking on large pastes and split the matching model in two. A line-scoped <code>\\w[^\\S\\n]*\\{[^\\S\\n]*$</code> keeps one model and one cost profile. Placed in the weak tier for consistency, though its punctuation gate is vacuous — a line ending in <code>{</code> never ends in <code>.?!</code>.</p>",-      "inferred": false-    },-    {-      "title": "<code>declaredDiagramType(in:)</code> stays local to ClipboardService; consolidation is a follow-up.",-      "body": "<p>Answering the review question directly. Only ~6 lines duplicate <code>MermaidTypeParser</code>'s walk (split, trim, skip empty, skip <code>%%</code>); keyword extraction and lookup already delegate. The genuinely new part — <code>maxDeclarationTokenCount</code> — is a clipboard false-positive heuristic, not mermaid grammar (<code>pie title Bugs per Module</code> and <code>gitGraph TB:</code> are legal 3+ token openers), so a <code>MermaidTypeParser.parseDeclaration</code> would assert a rule the parser has no authority over. Measured blast radius on tightening <code>parse</code>: two user-visible production callers (<code>BlockHTMLEmitter.swift:515</code>, <code>WebDocumentMessageRouter.swift:141</code>) plus ~40 assertions across six test files, all of which depend on it staying permissive. Consolidating is therefore not right for this PR.</p>",-      "inferred": false-    },-    {-      "title": "The stringly-typed display-name coupling is left standing, and recorded as follow-up.",-      "body": "<p><code>weakMermaidDirectives</code> keys on <code>MermaidTypeParser</code>'s human-readable return values. A rename would silently disable the exemption with no compile error. The compiler-enforced fix is a <code>MermaidDiagramType</code> enum with a <code>displayName</code>, which would also clean up <code>iconName(for:)</code> — but it touches files outside this bugfix. Mitigated here by adding tests that exercise every directive pairing, so a rename now fails the suite.</p>",-      "inferred": false-    }+    {"title": "Drive the overlay from an imperative <code>@State</code> flag rather than an observed controller read.",+     "body": "<p>The contested decision, and the one with no decision-log entry. Its justification is that the observed read &ldquo;never appeared on device&rdquo; in T-1681, but the root cause was never established and the same pattern works for <code>recoveryAbandonedBanner</code> in the same file. Every other cost in this PR &mdash; the poll loop, the 120s backstop, the five-exit table, the cancellation rule, the policy enum &mdash; follows from this one choice. If the device check shows the observed read works, most of it can go.</p><p>Recommend an ADR in <code>specs/webview-rendering/decision_log.md</code> either way: this is precisely the &ldquo;could reasonably have gone another way&rdquo; case the format exists for, and T-1965 set the precedent of adding one from a bugfix PR.</p>",+     "inferred": false},+    {"title": "Attribute navigation failures by epoch from <code>page.load(_:)</code>'s own sequence.",+     "body": "<p>Rounds 3 and 4. <code>page.navigations</code> cannot say which navigation failed, and the one it hands you is usually the load you just superseded &mdash; because superseding it is what makes WebKit fail it. The per-navigation sequence arrives already attributed. A monotone epoch, written only when it is current, makes both staleness directions impossible without an explicit clear. This is sound and should survive regardless of what happens to the spinner.</p>",+     "inferred": false},+    {"title": "Do not reclassify an ordinary navigation failure as a crash.",+     "body": "<p>Exit 3 reads the benign-bad-link case without changing it (T-1943/T-2107). Escalating it would make every blocked in-page link reload the document. The wiring test asserts <code>!recoveryAbandoned</code> on that path specifically to pin the non-escalation.</p>",+     "inferred": false},+    {"title": "120 seconds for the backstop.",+     "body": "<p>Chosen so it cannot fire on a legitimately slow load &mdash; a 10MB document's emit plus layout runs to the better part of a minute, and a backstop firing on that would restore the exact bug on the documents that most need the indicator. The reasoning for the number is sound. What is not addressed is what the user sees when it fires: the overlay lifts onto a blank page with no message. See the important change above.</p>",+     "inferred": false},+    {"title": "Poll at 100ms rather than observe.",+     "body": "<p>The wait runs outside any <code>ViewBuilder</code>, so Observation's invalidation does nothing for it, and <code>navigationFailedBeforeSettling</code> reads <code>@ObservationIgnored</code> storage. Both true &mdash; but the second is true because this PR chose it. <code>WebDocumentStateSynchronizer</code> already uses <code>withObservationTracking</code> for the same &ldquo;react to controller state with no view mounted&rdquo; job, so the alternative was available and in-house.</p>",+     "inferred": true}   ],    "findings": [-    {"severity": "major", "area": "ClipboardService.swift — per-line matching", "finding": "Multi-line member blocks (classDiagram `class Animal{ … }`, erDiagram `CUSTOMER{ … }`, stateDiagram `state First{ … }`) stopped being detected. `\\w\\{[^}]+\\}` requires the brace pair to close on one line, and a fields-only body carries no other syntax. Verified against both implementations: main wraps all three, branch wrapped none. Not covered by any test.", "resolution": "Added `#\"\\w[^\\S\\n]*\\{[^\\S\\n]*$\"#` (identifier + unclosed brace at line end) to the weak shape tier, validated against an 18-case corpus with zero new false positives. Added four regression tests plus a negative test for pasted pseudo-code (`if (x) {`).", "status": "fixed"},--    {"severity": "major", "area": "ClipboardService.swift — doc comment", "finding": "The comment on `containsMermaidSyntax` reasoned about the newline-spanning corner and dismissed it as out of scope: \"a raw newline inside a plain bracket label is not valid diagram source, and any real diagram has other single-line syntax\". True for bracket/paren/quote labels; false for the brace shape, where the multi-line form IS the member block and there is no other syntax. This wrong conclusion is why the regression shipped.", "resolution": "Rewrote the comment to separate the two cases and state explicitly that the brace shape is not such a corner, with a pointer to the pattern that keeps it detected.", "status": "fixed"},--    {"severity": "major", "area": "CHANGELOG.md:22", "finding": "States the exemption covers \"`title` and `section` in gantt, journey, timeline, and pie\". `section` maps to gantt/journey/timeline only — pie is `title`-only. Verified: `pie` + `section Ship it!` does not wrap. The code's own doc comment gets this right; the CHANGELOG dropped the qualifier.", "resolution": "Corrected to \"`title` in gantt, journey, timeline, and pie; `section` in the first three of those; `dateFormat` in gantt alone\".", "status": "fixed"},--    {"severity": "major", "area": "CHANGELOG.md:22", "finding": "Claims \"Detection of every real diagram shape is unchanged, including the types with no arrow operator to fall back on (pie, gantt, journey, timeline)\". False for exactly those types: `gantt`+`title Project schedule.`, `gantt`+`section Phase one.`, `journey`+`title My day.`, `pie`+`title Distribution.` all wrapped on main and no longer do. The narrowing is deliberate (`trimmed.last != \".\"`) but was being denied rather than documented.", "resolution": "Replaced with an explicit statement of the trade-off, and extended the sentence to cover the multi-line member blocks now restored.", "status": "fixed"},--    {"severity": "major", "area": "ClipboardServiceTests.swift — coverage", "finding": "Four load-bearing branches were unpinned: (1) the `trimmed.last != \".\"` clause — deleting it left every test green, because both period-terminated directive tests use prose openers that return false regardless; (2) the Timeline and Pie Chart directive pairings are never consulted, so renaming either display name would silently disable the exemption; (3) `maxDeclarationTokenCount` is only exercised at 3 tokens, never at 1 or 2; (4) the `%%` comment skip in `declaredDiagramType`.", "resolution": "Added three tests pinning all four: every directive pairing including the pie/section negative, period-terminated directives with a valid declaration, and a two-token declaration plus a leading `%%` comment.", "status": "fixed"},--    {"severity": "major", "area": "ClipboardService.swift:35 — maxDeclarationTokenCount", "finding": "A two-word prose opener satisfies the declaration test and claims the exemption: `Timeline overview` / `section 4 answers this?` wraps, as does `Journey mapping` / `title case matters, right?`. Two-word headings are a common shape for pasted prose. The headline test for this exemption uses the three-token `Timeline mapping is useful`, which passes while its two-token sibling fails. Not a regression (main wrapped both), but the new mechanism's own goal is incompletely met.", "resolution": "Not fixed — closing it means constraining the second token to a direction or known modifier rather than counting tokens, which is a design change, not a fix, and this heuristic has already churned across five rounds. Recorded for follow-up.", "status": "skipped"},--    {"severity": "minor", "area": "ClipboardService.swift:53-76 — strong tier framing", "finding": "The strong tier is documented as \"operator shapes that do not occur in prose\", but two members do. `\\w+[^\\S\\n]*---[^\\S\\n]*\\w+` matches the ASCII em-dash convention (`settled---or so we thought`, and the spaced form too), and `o--` matches any word ending in `o` before a double hyphen (`ratio--surprisingly--held`). Both bypass the punctuation gate entirely, so period-terminated prose still wraps. Pre-existing on main, but the tiering decision is new and it is precisely what declares these unambiguous.", "resolution": "Not fixed — tightening these (requiring whitespace or boundaries around `---`, anchoring `o--` with a lookbehind) changes strong-tier matching, which is the highest-risk area to touch after five rounds. Recorded for follow-up alongside the `-x` item.", "status": "skipped"},--    {"severity": "minor", "area": "ClipboardService.swift:98 — the `-x` pattern", "finding": "The doc comment lists \"a hyphenated word containing `-x` (`non-xml`)\" among the prose doubles the gate handles. It does not: the gate only inspects the final character, and a hyphenated word almost never ends a sentence. `Graph databases are useful` / `for non-xml payloads today` wraps, as does a line containing `tar -xzf`. Pre-existing, but the comment claims a mitigation that does not exist.", "resolution": "Not fixed. The clean fix reuses the machinery this branch added — gate `-x`/`--x` on `declaredDiagramType(in:) == \"Sequence Diagram\"`, exactly as directives are gated — but that extends the type-correlation mechanism to a new tier and deserves its own review. Recorded for follow-up.", "status": "skipped"},--    {"severity": "minor", "area": "ClipboardService.swift — gate coverage framing", "finding": "The punctuation gate is presented in doc comments and CHANGELOG as the discriminator between prose and diagram lines, but it covers terminated sentences only. Verified still wrapping: hard-wrapped prose (`Graph theory has broad applications[3] in` / `computer science`), sentences closed by a quote (`Bob asked \"is this results[3]?\"`), ellipsis endings, bulleted lists, and markdown tables under a keyword-led heading. All pre-existing; the residual class is narrowed, not closed.", "resolution": "Not fixed (all pre-existing, none a regression). Recorded here and in the expert explanation so the next reader is not misled by the confident framing.", "status": "skipped"},--    {"severity": "minor", "area": "specs/bugfixes/ + specs/clipboard-mermaid-detection/", "finding": "`specs/bugfixes/clipboard-prose-rewritten-as-mermaid/` exists on disk but is empty and untracked — 103 of 105 sibling directories carry a `report.md`. Separately, `specs/clipboard-mermaid-detection/smolspec.md` still documents a single flat pattern list with no tiering, punctuation gate, or declaration correlation, and smolspec.md:67 / tasks.md:12 say \"keyword + multi-line OR syntax\" where smolspec.md:36 and the code say AND.", "resolution": "Not fixed — writing the bugfix report and updating the spec is author work requiring the decision rationale, not a review edit. Flagged as the highest-value follow-up: with the spec stale and no report, the only durable record of why the heuristic tiers this way is the doc comments. All 8 rows of the spec's Test Cases table still pass.", "status": "skipped"},--    {"severity": "minor", "area": "ClipboardService.swift — stringly-typed coupling", "finding": "`weakMermaidDirectives` keys on `MermaidTypeParser`'s display names (\"Gantt Chart\", \"User Journey\", \"Timeline\", \"Pie Chart\"). A rename compiles clean and silently disables the exemption for that type. This is the third production site keying on those raw strings (`iconName(for:)` and the `\"Diagram\"` sentinel are the others).", "resolution": "Partially mitigated: the new tests exercise every directive pairing, so a rename now fails the suite rather than passing silently. The compiler-enforced fix — a `MermaidDiagramType` enum with a `displayName` — touches files outside this bugfix and is recorded as follow-up.", "status": "fixed"},--    {"severity": "minor", "area": "ClipboardService.swift — main-thread cost", "finding": "Measured, not estimated: 10 MB no-match paste takes 11.4 s on the branch versus 15.3 s on main, so the rewrite is 15-25% FASTER (per-line ranges cap ICU backtracking on the negated classes). But it is still an ~11 s synchronous main-thread stall at the 10 MB ceiling, and `wrapMermaidIfNeeded` makes three full `components(separatedBy:)` passes, two of which exist to read one line.", "resolution": "Not fixed — not a regression, and the effective fix (bounding the scanned prefix and line count, worth ~490x on that path) is a behaviour-affecting optimisation that belongs in its own ticket rather than a bugfix branch already five rounds deep.", "status": "skipped"},--    {"severity": "nit", "area": "ClipboardService.swift:134-138, :99, :336", "finding": "`(?m)` and `^\\s*` in the three directive patterns are vestigial now that matching runs on an already-trimmed single line — and `(?m)` actively signals the whole-text contract this change removed. `#\"--x\"#` is a strict superstring of `#\"-x\"#` and can never be the sole matcher (same for `-->>` vs `-->`). `declaredType.map(directive.diagramTypes.contains) == true` compares a `Bool?` to `Bool` via a curried `Set.contains`, and is loop-invariant.", "resolution": "Not fixed — all cosmetic, all zero-behaviour-change, and touching the core matching arrays for cosmetics after five rounds trades real risk for no user-visible gain. Recorded so a future cleanup pass has the list.", "status": "skipped"},--    {"severity": "nit", "area": "Working note accuracy", "finding": "The task note stated that \"one pre-existing test was deliberately adapted because it asserted the very false positive being closed\". Against `origin/main` the test file is a pure append (+348 / -0, single hunk). The two lines rewritten in commit b55d497 belonged to a test this branch itself added in 9b92b34.", "resolution": "No action needed — recorded because it changes how the diff should be read: no pre-existing contract was renegotiated.", "status": "fixed"}+    {"severity": "blocker", "area": "DocumentScrollContent.swift:44-50, 346-357",+     "finding": "The overlay is still unproven end-to-end. The stated cause of the T-1681 non-display (a ViewBuilder read of an @Observable property not re-evaluating) is contradicted by recoveryAbandonedBanner eight lines below, which uses that exact pattern and works in production. The root cause was never diagnosed, and this PR keeps the identical overlay view at the identical attachment point, changing only the gate. No test renders the view; the wiring test header says so plainly.",+     "resolution": "Cannot be closed from here. A throwaway XCUITest probe was written for this review (paste a 3.6MB HTML-heavy document from the clipboard, assert the 'Preparing document' element appears then disappears); it could not run because this environment lacks UI-test automation permission ('Timed out while enabling automation mode'), and screencapture is likewise blocked ('could not create image from display'). One person opening one large document on a device or simulator settles it. If the overlay does render, that same session should also confirm it does not lift before the document is visible (see the isLayoutSettled finding below).",+     "status": "skipped"},++    {"severity": "major", "area": "DocumentScrollContent.swift:541-591 (exits 3 and 4)",+     "finding": "Exits 3 and 4 leave the reader on a blank page with no message and no control. Exit 2 hands off to the 'This document stopped rendering.' banner with a Reload button; an ordinary navigation failure and the 120-second backstop just lift the overlay onto whatever the web view holds, which for a failed navigation is blank. Exit 4 does it after two minutes of spinner. That is the original T-1744 symptom with a longer preamble, and the backstop's defence ('revealing a blank page is strictly better than covering it with a spinner forever') is only true because the third option was not taken.",+     "resolution": "Not changed &mdash; this is a product decision, not a defect. Recommended: route exits 3 and 4 to the existing recoveryAbandonedBanner. It already exists, already localises, already offers Reload, and reloadWebDocument() is already in scope in this view. That would make the failure story uniform and make the exact value of maximumWait far less consequential.",+     "status": "skipped"},++    {"severity": "major", "area": "DocumentScrollContent.swift:234-253",+     "finding": "The overlay is raised for every load, not only the first. parseRevision bumps on every successful parse, so a FileChangeObserver reload after an external edit re-keys the load task and covers the already-rendered document with an opaque full-page panel. There is no delay-before-show and no minimum display time, and a 0.2s easeInOut is attached in both directions, so a sub-200ms reload of a small document is maximally visible as a flash. Someone editing a document in another editor now flashes the reader's view on every save.",+     "resolution": "Not changed &mdash; the behaviour is inherited from the T-1681 attempt, whose commit message names covering reparse reloads as a goal, so changing it is a design decision rather than a fix. Recommended: raise the flag only once the load has been outstanding ~200ms, or skip the overlay for same-document reparses. Worth reproducing in the device check by editing the open file externally.",+     "status": "skipped"},++    {"severity": "major", "area": "DocumentScrollContent.swift:601-602 (as written before this review)",+     "finding": "A stated invariant the code does not establish: 'usually superseded in time is not an invariant; the flag is only ever raised for a load that is actually issued is, and this is it.' WebDocumentControllerFactory.loadDocument has a second early return that issues no load &mdash; guard !Task.isCancelled after the off-main emit, which calls abandonLoad and returns &mdash; and the flag is already raised by then. shouldIndicatePreparation establishes only its own condition, that something has been parsed. This is the class of overclaim the batch has repeatedly found, and it survived five rounds.",+     "resolution": "Fixed. The comment now states the narrower true condition, names the second early return explicitly, and records why that case is nonetheless safe (the same cancellation ends the wait through exit 5 and mayClearFlag declines).",+     "status": "fixed"},++    {"severity": "minor", "area": "docs/agent-notes/webview-rendering-status.md:245",+     "finding": "'issueNavigation is now the single place page.load is called' is false repo-wide. FootnotePopoverWebPage.swift:178 and prismTests/WebRenderingSpikes/SpikeWebPageHarness.swift:161 both call it. The claim is true only of WebDocumentController's own page, and the structural pin greps that one file, so it cannot catch the discrepancy either. The agent note is the artefact a future session reads, and it was the unqualified one.",+     "resolution": "Fixed. The note now scopes the claim to WebDocumentController, names FootnotePopoverWebPage as the other call site, and says the structural pin greps one file. The controller's own doc comment was scoped the same way.",+     "status": "fixed"},++    {"severity": "minor", "area": "docs/agent-notes/webview-rendering-status.md:244",+     "finding": "Two unsupported claims in the hot-loop note. (1) 'hot-loops the MainActor for the rest of the test process ... because the controller stays alive as long as the test's page holds it' inverts the ownership: the observation task captures [weak self], applyOutcome returns false once the controller is gone, and deinit cancels the task, so the loop ends when the controller is released. (2) 'A stream failing with webContentProcessTerminated is safe by contrast' is false before the first load &mdash; attemptRecovery finds no loadedDocumentURL, charges nothing and keeps observing, so it hot-loops identically. The wiring test's own setup comment says exactly this, so the note contradicted the test it was written to accompany.",+     "resolution": "Fixed. Both claims corrected in place, with the mechanism (weak self, deinit cancel) and the before-first-load exception spelled out, cross-referenced to waitEndsWhenRecoveryIsAbandoned's three-step setup.",+     "status": "fixed"},++    {"severity": "minor", "area": "DocumentScrollContent.swift:548 (exit table row 5)",+     "finding": "Exit 5 was described as 'A fresher parse revision superseded this load'. Task.isCancelled is also true when the view disappears &mdash; the raw-source toggle, a document close, a layout swap &mdash; where there is no successor at all. The safety argument written for mayClearFlag rests entirely on a successor existing; the no-successor case is safe for a different reason (@State is discarded with the view identity), which was not recorded.",+     "resolution": "Fixed. The table row now names both causes, and a paragraph below records the no-successor case and why it is safe.",+     "status": "fixed"},++    {"severity": "minor", "area": "DocumentScrollContent.swift:100-102",+     "finding": "'the (harmless, sub-200ms) window where both could theoretically be true for one frame' understates the overlap. The wait polls at 100ms and the overlay then fades out over 0.2s, so roughly 300ms in the normal case, and longer if this task happened to be cancelled at that moment. The ordering claim itself is correct and is pinned by a test.",+     "resolution": "Fixed. The comment now gives the real figure and states that the overlap is harmless because of the ordering rather than because it is brief.",+     "status": "fixed"},++    {"severity": "minor", "area": "CHANGELOG.md:29",+     "finding": "'It also cannot strand the interface' is stronger than the code supports: a cancelled wait deliberately leaves the flag raised, and .pageClosed is classified out of exit 3 so that failure rides the full 120 seconds. 'Bounded' is defensible; 'cannot strand' is not. A stray blank line before the entry also broke the surrounding bullet run's spacing.",+     "resolution": "Fixed. Reworded to 'cannot stay up indefinitely', which is exactly what the bound gives. Blank line removed.",+     "status": "fixed"},++    {"severity": "minor", "area": "prism/Resources/WebRenderer/prism-bridge.js:294 vs the exit table's row 1",+     "finding": "Exit 1's signal is weaker than its name. layoutSettled is posted from an unconditional setTimeout(..., 16) armed alongside ready, and notifyLayoutSettled debounces to one post, so nothing can post it later. In the common case isLayoutSettled means 'DOM parsed, plus 16ms'. The dominant cost (the off-main emit) is covered correctly because the flag is raised before loadDocument, but WebKit's layout and first paint of a very large DOM happen after that timer &mdash; so the overlay may lift a beat before the document is visible, on exactly the documents it exists for.",+     "resolution": "Not changed &mdash; the signal is pre-existing (webview-rendering), and repointing the overlay at a later milestone is out of scope for this PR. Flagged for the device check: with a multi-megabyte document, watch whether the spinner lifts onto content or onto a blank frame.",+     "status": "skipped"},++    {"severity": "minor", "area": "DocumentLoadingIndicatorWiringTests.swift:646-715",+     "finding": "Four of the five source-text tests do not disclose their limits the way pageLoadHasExactlyOneCallSite does. They pass on commented-out code (a commented .overlay line still satisfies contains), the ordering tests compare string offsets that comments satisfy equally, and they pin exact whitespace and the local name willLoad &mdash; so a SwiftLint autofix, a line-wrap, or a rename reds the suite with no behavioural change.",+     "resolution": "Not changed &mdash; tests are not modified in a pre-push review absent an actual bug, and the technique is defensible for the missing-wiring class. Noted so the cost is a known one.",+     "status": "skipped"},++    {"severity": "minor", "area": "DocumentLoadingIndicatorWiringTests.swift:329-347, :124",+     "finding": "Two test-strength gaps. waitEndsAtTheDeadline would pass against a waitForPreparation that returned immediately &mdash; it asserts only 'finished within 60s' plus three false flags, so it is meaningful only in company with the others. Separately, poll(untilTrue:) defaults to 5s and gates setup expectations in three tests that wait on a real WebKit navigation plus MainActor scheduling; that is the tightest bound in the file and contradicts the header's own argument that a 10s bound blew at 13-16s under full-suite load. It is the likeliest flake here.",+     "resolution": "Not changed (test files). Recommended if touched later: give poll the same generous default the finishes() bounds get, and have waitEndsAtTheDeadline assert the wait did NOT return before the injected deadline.",+     "status": "skipped"},++    {"severity": "minor", "area": "DocumentScrollContent.swift:346-357",+     "finding": "The overlay has no accessibility treatment. The sibling recoveryAbandonedBanner has .accessibilityElement(children: .combine), a label, a hint and a .transition; this one has none, and it does not hide the content beneath it from VoiceOver. A full-page opaque cover that VoiceOver reads straight through is a worse mismatch than a banner doing the same, because the visual reader sees nothing at all.",+     "resolution": "Not changed &mdash; adding accessibility semantics to an overlay whose rendering is itself unverified would be building on sand. Do it in the same pass that confirms it renders: .accessibilityHidden(true) on the covered content, or an accessibilityElement with a label on the overlay, plus a .transition to match the sibling.",+     "status": "skipped"},++    {"severity": "minor", "area": "specs/webview-rendering/decision_log.md",+     "finding": "No decision-log entry for the design choice this PR turns on. 'Drive the overlay from an imperative @State flag rather than an observed controller read' is exactly the 'could reasonably have gone another way' case the project's ADR format exists for &mdash; it has a real alternative, a real cost (poll loop, backstop, five exits, cancellation rule) and a rationale that this review disputes. T-1965 set the precedent of a bugfix PR adding one.",+     "resolution": "Not written. Writing an ADR that presents the choice as settled would be wrong while the premise is disputed. Write it after the device check, recording whichever way that goes.",+     "status": "skipped"},++    {"severity": "nit", "area": "Reuse across the new code",+     "finding": "Four small duplications. projectSource(_:) is the sixth hand-rolled copy of the #filePath-relative source reader (FootnotePresentationHostTests, KeyboardScrollControllerTests, RemoteRefreshFlowTests, PaywallPresenterTests, URLChokepointAdoptionTests). poll(untilTrue:) and waitBriefly(forAbsenceOf:) duplicate WebNavigationPrecedenceHarness.waitUntil(timeout:_:), already re-exported by two other suites. shouldIndicatePreparation re-implements loadDocument's own guard revision > 0 in a second file. finishes(within:) is genuinely new and worth keeping.",+     "resolution": "Not changed. The test-helper duplication predates this PR and deserves one extraction pass of its own rather than a drive-by here.",+     "status": "skipped"}   ],    "double_check": [-    {"title": "The two failures in the full unit sweep were load flakes — confirmed.", "body": "<p>A full <code>prismTests</code> sweep (4308 tests) showed 2 failures: <code>WebScrollabilityReportingTests/A sustained trigger burst still reports within the debounce's maximum wait</code> and <code>WebScrollNavigationTests/visibleBlock is suppressed while a programmatic scroll is in flight</code>. Both are timing/debounce assertions in the WebKit scroll path, which this diff does not touch — it changes only <code>ClipboardService</code>, a leaf with no production callers beyond <code>validateContent</code>. That sweep ran concurrently with two builds on the same machine. Re-running both classes in isolation: <strong>25/25 pass</strong>. Load flakes, not a real failure — but they are timing-sensitive enough to fail under parallel load, which is worth knowing when reading CI output.</p>"},--    {"title": "Both builds carry two pre-existing warnings.", "body": "<p><code>make build-ios</code> and <code>make build-macos</code> both succeed but emit <code>main actor-isolated conformance of 'ImageDimension' to 'Equatable' cannot be used in nonisolated context; this is an error in the Swift 6 language mode</code>. <code>ImageDimension</code> lives in <code>prism/Models/MarkdownBlock.swift</code>, untouched by this diff, so these are inherited from <code>main</code> — but the project's pre-push bar is zero warnings, so they will need their own ticket.</p>"},--    {"title": "The fixes applied in this review are uncommitted.", "body": "<p>Three files are modified in the working tree and not yet committed: the member-block-opener pattern and corrected doc comment in <code>ClipboardService.swift</code>, two factual corrections in <code>CHANGELOG.md</code>, and 8 new tests in <code>ClipboardServiceTests.swift</code>. Targeted suite is 115/115 green and SwiftLint reports 0 violations across 528 files, but they need committing before the push.</p>"},--    {"title": "The residual false-positive class is real and undocumented.", "body": "<p>Unpunctuated prose still wraps: hard-wrapped paragraphs, headings, bullet lists, table rows, and sentences closed by a quote. <code>ClipboardServiceTests.swift:723</code> asserts <code>containsMermaidSyntax(\"Results[3] improved significantly\") == true</code> — that is the bug's own shape minus its full stop, now pinned as an invariant that will fight the next tightening. Worth a comment on that assertion saying it records a known residual rather than a desired contract.</p>"}+    {"title": "Does the spinner appear at all?",+     "body": "<p>The one thing that matters and the one thing nothing here proves. Open a multi-megabyte, HTML-heavy markdown file on a device or simulator. If the spinner does not appear, this PR has repeated T-1681 and the cause is in the view, not the gate &mdash; look at compositing against the <code>WKWebView</code>-backed <code>WebDocumentView</code> before anything else.</p>"},+    {"title": "Does it lift onto content, or onto a blank frame?",+     "body": "<p><code>layoutSettled</code> is, in the common case, a 16ms timer armed alongside <code>ready</code>. On a very large document the overlay may lift before WebKit has painted. If it does, the fix is partial on exactly the documents it was built for.</p>"},+    {"title": "What does an external edit look like?",+     "body": "<p>With the document open, edit and save it in another editor. Every reparse re-raises the flag, so watch for a full-page opaque flash over content the reader was reading. A delay-before-show would remove it.</p>"},+    {"title": "What does a failed load look like?",+     "body": "<p>Exits 3 and 4 lift the overlay onto a blank page with no message. Worth seeing once before deciding it is acceptable, given the <code>recoveryAbandonedBanner</code> is sitting right there.</p>"},+    {"title": "Two suite failures, attributed but not fully controlled.",+     "body": "<p><code>WebScrollabilityReportingTests.reportArrivesWithinMaxWaitDuringTriggerBurst</code> and <code>WebScrollNavigationTests.visibleBlockSuppressedDuringProgrammaticScroll</code> failed in the full run (4516 tests, 2 failures). Both are wall-clock-bound live-WebKit tests in suites this diff does not touch, and a stray <code>prism.app</code> instance holding a 3.6MB document was competing for CPU during that run. Three consecutive attempts at a control run wedged before executing anything ('test runner hung before establishing connection', zero tests recorded) &mdash; the harness flake the agent notes describe. Worth noting that <code>check-test-results.sh</code> reported <code>total=1 passed=0 failed=1</code> for those empty runs, so the guard is not a reliable oracle for a wedged host either (cf. T-2224). Attribution is strong on the evidence but not closed by a green control run; re-run those two classes on a quiet machine to confirm.</p>"}   ],    "files": [-    {"path": "prism/Services/ClipboardService.swift", "badge": "Modified", "stat": "+225 / -33", "diff_file": "diff-ClipboardService.swift.txt"},-    {"path": "prismTests/ClipboardServiceTests.swift", "badge": "Modified", "stat": "+348 / -0", "diff_file": "diff-ClipboardServiceTests.swift.txt"},-    {"path": "CHANGELOG.md", "badge": "Modified", "stat": "+1 / -0", "diff_file": "diff-CHANGELOG.md.txt"}+    {"path": "prism/Views/DocumentScrollContent.swift", "badge": "Modified", "stat": "+186 / -0 (+ review edits)", "diff_file": "diff-prism_Views_DocumentScrollContent.swift.txt"},+    {"path": "prism/ViewModels/WebDocumentController.swift", "badge": "Modified", "stat": "+141 / -17 (+ review edits)", "diff_file": "diff-prism_ViewModels_WebDocumentController.swift.txt"},+    {"path": "prismTests/WebRendering/DocumentLoadingIndicatorWiringTests.swift", "badge": "Added", "stat": "+731", "diff_file": "diff-prismTests_WebRendering_DocumentLoadingIndicatorWiringTests.swift.txt"},+    {"path": "prismTests/WebRendering/DocumentLoadingIndicatorPolicyTests.swift", "badge": "Added", "stat": "+143", "diff_file": "diff-prismTests_WebRendering_DocumentLoadingIndicatorPolicyTests.swift.txt"},+    {"path": "docs/agent-notes/webview-rendering-status.md", "badge": "Modified", "stat": "+2 (+ review edits)", "diff_file": "diff-docs_agent-notes_webview-rendering-status.md.txt"},+    {"path": "CHANGELOG.md", "badge": "Modified", "stat": "+2 (+ review edits)", "diff_file": "diff-CHANGELOG.md.txt"},+    {"path": "prism/Localizable.xcstrings", "badge": "Modified", "stat": "+23", "diff_file": "diff-prism_Localizable.xcstrings.txt"},+    {"path": "(working tree) review corrections", "badge": "Uncommitted", "stat": "4 files", "diff_file": "diff-working-tree.txt"}   ],    "publish_metadata": {-    "title": "Pre-push review: T-1840 clipboard prose rewritten as mermaid",+    "title": "Pre-push review: T-1744 loading indicator",     "repoUrl": "https://github.com/ArjenSchwarz/prism",-    "branch": "T-1840/bugfix-clipboard-prose-rewritten-as-mermaid",-    "severity": "suggestions",-    "summary": "The three-tier heuristic is sound and introduces no new false positives — verified by replicating both main's and the branch's logic over a 40-case corpus. But moving to per-line matching silently un-detected class, ER and state diagrams whose member block spans lines, and two CHANGELOG claims were factually wrong; all three are fixed here, with 8 tests pinning branches that were previously deletable with a green suite. Ready to push once committed."+    "branch": "T-1744/bugfix-loading-indicator-missing-during-offmain-emit",+    "severity": "needs-changes",+    "summary": "The overlay is still unproven end-to-end after six review rounds: the stated reason the T-1681 attempt never rendered is contradicted by the sibling overlay eight lines below it, which uses the same pattern and works. A device check is warranted before merge. Six prose overclaims were found and fixed, including one stated as an invariant; the navigation-epoch attribution work underneath is sound and worth keeping either way."   } }diff --git a/CHANGELOG.md b/CHANGELOG.mdindex 1c10a13..227b1e1 100644--- a/CHANGELOG.md+++ b/CHANGELOG.md@@ -25,8 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Refreshing a document opened from a URL no longer leaves a background download running loose after you close it (T-1805). Tapping Refresh started an untracked download with no way to stop it: closing the document, or navigating to a different one, before the download finished let it keep running, and when it finally completed it could still overwrite that URL's title in Recent Files — even though you had already moved on — or race a second refresh for a stale result. Closing the document, or starting another refresh, now cancels the one in flight, and a refresh that has been superseded or cancelled no longer writes its result anywhere. - Refreshing a document opened from a URL no longer leaves a background download running loose after you close it (T-1805). Tapping Refresh started an untracked download with no way to stop it: closing the document, or navigating to a different one, before the download finished let it keep running, and when it finally completed it could still overwrite that URL's title in Recent Files — even though you had already moved on — or race a second refresh for a stale result. Closing the document, or starting another refresh, now cancels the one in flight, and a refresh that has been superseded or cancelled no longer applies its result once another one has taken over. - The unlock screen no longer spins forever when the App Store returns some but not all of Prism's products (T-1841). Product fetching only reported an error when the whole catalog came back empty; if the store returned the tip products but omitted the unlock product specifically, the purchase button never appeared and the spinner never resolved, with no error message and no way to retry. A fetch that comes back without the unlock product is now treated as a load failure in every case, so it always ends in the existing "Unable to load purchase options" message with a Retry button, never an indefinite spinner. The unlock row in Settings, which sat on the same endless spinner, now reads "Unavailable" and still opens the unlock screen when tapped. Tips are unaffected: a catalog that returned the tips but not the unlock product still fills the tip jar, so an already-unlocked user sees nothing change.--- Opening a large or HTML-heavy document now shows a "Preparing document…" indicator while it loads, instead of an unresponsive-looking blank page (T-1744). Moving the document's HTML build off the main thread (T-1681) kept the app responsive during that work, but left nothing on screen to say the document was still coming — for a large document that window can run to the better part of a minute. An earlier attempt at the indicator, reverted before T-1681 shipped, read the loading state directly from a `ViewBuilder` and never appeared on device in either of its two gated forms; the indicator now drives off a plain flag flipped by the load itself, so its visibility no longer depends on that. It also cannot strand the interface, which for a full-page indicator matters more than the indicator itself — one that never goes away hides the document instead of merely failing to explain it. Every way it ends is accounted for: the page finishes laying out; WebKit's renderer crashes and crash recovery gives up, at which point the indicator steps aside for the existing "This document stopped rendering" banner; the document fails to load for an ordinary, non-crash reason, which reveals the page rather than waiting on a load that is not coming; a second edit lands before the first finishes, and the superseded load's cleanup never erases the flag the fresher one is relying on; and for anything not on that list the indicator gives up by itself after a bound set well past any real load. It is also never shown before there is a document to load at all.+- Opening a large or HTML-heavy document now shows a "Preparing document…" indicator while it loads, instead of an unresponsive-looking blank page (T-1744). Moving the document's HTML build off the main thread (T-1681) kept the app responsive during that work, but left nothing on screen to say the document was still coming — for a large document that window can run to the better part of a minute. An earlier attempt at the indicator, reverted before T-1681 shipped, read the loading state directly from a `ViewBuilder` and never appeared on device in either of its two gated forms; the indicator now drives off a plain flag flipped by the load itself, so its visibility no longer depends on that. It also cannot stay up indefinitely, which for a full-page indicator matters more than the indicator itself — one that never goes away hides the document instead of merely failing to explain it. Every way it ends is accounted for: the page finishes laying out; WebKit's renderer crashes and crash recovery gives up, at which point the indicator steps aside for the existing "This document stopped rendering" banner; the document fails to load for an ordinary, non-crash reason, which reveals the page rather than waiting on a load that is not coming; a second edit lands before the first finishes, and the superseded load's cleanup never erases the flag the fresher one is relying on; and for anything not on that list the indicator gives up by itself after a bound set well past any real load. It is also never shown before there is a document to load at all. - 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.diff --git a/docs/agent-notes/webview-rendering-status.md b/docs/agent-notes/webview-rendering-status.mdindex 58bceb0..8412051 100644--- a/docs/agent-notes/webview-rendering-status.md+++ b/docs/agent-notes/webview-rendering-status.md@@ -241,8 +241,8 @@ take down unrelated applications' web content. Always PID-verify, never pattern- - **`InlineHTMLRenderer` run offsets are always relative to the string it is handed** (its source cursor restarts at 0 on every call), and the runs are consumed against `MarkdownBlock.textContent`, which JOINS the sub-spans (cells with `" | "`, rows/items with `"\n"`). A sub-span caller that appended its runs unrebased mis-anchored every selection notes made past the first cell/item — invisible in the emitted HTML, visible only when run offsets are compared against `textContent`. Closed structurally in **T-1941**: `BlockHTMLEmitter.renderInline` takes a **required** `InlineSpan` (`.wholeBlock` / `.subspan(offset:)` / `.unmapped`), is the only place that appends to `context.blockRuns`, and does the rebase itself — so a new caller cannot omit it, only state it wrongly. Offsets come from `MarkdownBlock.tableCellTextOffsets` / `listItemTextOffsets`, which live beside `textContent` and use the same separator constants, so the two sides cannot drift. Anything absent from `textContent` (nested list items, continuation paragraphs, nested blocks, a `<details>` child list, a nested `<details>` summary, the child-less blockquote fallback) is `.unmapped` → selection declined (Decision 8), never mis-anchored; mapping the nested cases would need `textContent` widened to contain them, tracked as **T-2032** — it is a scoped-out limitation, not a defect. `EmittedDocument.badgeSourceStarts` is deliberately NOT rebased: it is keyed by the inline source string and matched against that string's own occurrence scan in `SearchStateFeeder` (T-1853). Regression guard: `WebStructuredSourceMapInvariantTests.parityCorpusRunsMonotonic` sweeps the whole parity fixture corpus asserting runs are monotonic, non-overlapping AND within their block's `textContent` UTF-16 length — the upper bound is what catches a Character-count offset, which stays monotonic and would otherwise pass. - **`InlineHTMLRenderer.Walker.locate` is a naive forward scan, and it is only affordable because failed scans are recorded** (T-1966). The search cannot be BOUNDED in the general case — text legitimately sits far ahead of the cursor whenever the walk skipped source it does not account for (a long image `src`, a long raw-HTML span), so only the pre-badge segment (PR #326) and `claimOccurrences` (T-1992) get bounds. Without a record, a text node whose rendered text does not occur verbatim in the source — the entity/escape family, `A` spelled `&#65;` — scanned to the end of the block, failed, left the cursor where it was, and the next such node re-derived the same scan: `*&#65;* ` x 3200 took 12s. Two exact rejections fix it: `provedAbsent` (a text an unbounded scan proved absent stays absent, capped at 256 entries so the T-2034 class cannot grow it with the block) and a lazily-built `sourceUnits` bitset (a text holding a unit the source does not hold cannot occur in it anywhere; a bitset over the 16-bit domain rather than a `Set`, because the probe runs once per unit of every later text and hashing dominated it). A match at the cursor is tested BEFORE either memo, so the common case — text sitting exactly where the walk expects it — pays for neither. **`provedAbsent` rests on `cursor` never moving backwards**, which is why `locate` takes NO `from:` parameter and reads `cursor` itself: the precondition is structural, not documented. Every mutation of `cursor` is forward, but one of them is only forward *because of the pre-badge bound* — `appendFootnoteBadge` steps to the occurrence's end, and the `appendVisible` before it must stay bounded by `occurrence.sourceStart` or the cursor could overshoot; weakening that bound breaks the memo, not just performance. No rejection changes any output: digests over a 4000-sample generated corpus (html + every run + `badgeSourceStarts`) are byte-identical to `origin/main` at 09cd828, and that corpus is now COMMITTED (`InlineRenderCorpusEquivalenceTests`, seeded SplitMix64, chunk digests pinned, re-blessing protocol in the file header; `PRISM_INLINE_CORPUS_FULL=1` for all 4000). Residual, deliberately open: a distinct-per-node text absent from the source whose every unit is present still costs a scan each — **T-2034**, needs a source index to close. Guards: `InlineSourceMapScanGrowthTests` (G1-G8 growth over the shared `GrowthRatioGuard`, O1-O6 rendered-text-in-order + run invariants, O7 exact pre-fix run coordinates on memo-HIT fixtures, O8 the recording gate — a bounded miss must record nothing or a later unbounded match is silently suppressed). - **The inline re-parse can hand `InlineHTMLRenderer.Walker` BLOCK nodes, and an unhandled one drops content silently** — three instances so far, all the same shape. `render` re-parses each block's inline source with `Document(parsing:)`, so a source string that satisfies a *block* grammar comes back as block structure rather than a `Text` node, and `MarkupWalker`'s default descend emits nothing for it: content vanishes with no error, no fallback, and no visible trace except an empty cell/item/heading. **T-1640** `1.`/`3)` at the start of an item (→ `OrderedList`), **T-1641** `@Observable` (→ `BlockDirective`; closed by NOT passing `.parseBlockDirectives`, matching `MarkdownBlockParser`), **T-1669** text that is exactly `---`/`***`/`___` (→ `ThematicBreak`). The two structural fixes render the LITERAL source line/marker as a mapped run (`literalListMarker`, `literalThematicBreak`) rather than reconstructing it from the node — a `ThematicBreak` has no text and no children to descend into, so there is nothing to reconstruct from. **The class is not closed**: the Walker still has no `visitHeading`, `visitBlockQuote`, `visitCodeBlock`, `visitHTMLBlock` or `visitTable`, so `# Title` in a table cell drops its `#` (T-1640 shape) and a re-parsed code block or HTML block would drop entirely (T-1669 shape). Before adding a `renderInline` call site, check what its strings can re-parse into; when one of these turns up, add the visitor rather than pre-sanitising the string. The defensive gap-skipping in `visitThematicBreak` is for a broken cursor invariant only — every call site today passes a SINGLE-BLOCK string, so a stranded cursor is unreachable and the branch is deliberately not hardened further (it degrades to a stray space, or to rendering the line it lands on).-- **An injected navigation stream that fails on EVERY subscription hot-loops the MainActor for the rest of the test process** (T-1744). `WebDocumentController.observeNavigations` re-subscribes after a `.navigationFailed` — that is the deliberate benign-bad-link classification (T-1943/T-2107) — so a `test_setInjectedNavigationStream` provider that throws a non-termination error each time never terminates the loop: it spins at one `Task.yield()` per iteration, forever, because the controller stays alive as long as the test's page holds it. Every test in the offending file passed in isolation; run as part of the full suite it starved every later `@MainActor` test, blew their timing bounds, and took the host down — 174 reported failures, ~168 of which never ran. A stream failing with `webContentProcessTerminated` is safe by contrast (the retry budget spends and `applyNavigationOutcome` returns false, ending the loop); an ordinary error is not. Fail ONCE then park (`DocumentLoadingIndicatorWiringTests.failsOnceThenParks`). Corollary for any `@MainActor` test that polls: Swift Testing runs these concurrently on the one executor, so a tight timing bound measures machine load, not the code — pick a bound loosely, then mutation-check that it still sits inside the failure mode.-- **`page.navigations` cannot say WHICH navigation failed, and the failure it hands you is usually the one you just superseded** (T-1744 review). It is one subscription for the whole page, and calling `page.load` while a navigation is in flight is exactly what makes WebKit fail the outgoing one — so an outcome read off that stream lands on whichever load is current, which is the load that caused it. Any per-load state derived from it is mis-attributed by default, and the window is not narrow: `WebDocumentController.load` does not cancel the outgoing navigation, and a re-parse landing during the previous load's emit overlaps two loads routinely. The loading indicator's exit 3 (`navigationFailedBeforeSettling`) was built on it and so dropped the spinner over a document that was still loading. **The sequence returned by `page.load(_:)` is scoped to that one navigation** — that is what per-load state belongs on. `issueNavigation` is now the single place `page.load` is called; it stamps a `navigationEpoch`, and the flag is computed as `failedNavigationEpoch == navigationEpoch`, so a stale failure records an epoch that can never match and a fresh load retires the previous failure just by bumping past it — no clear to get stale in the other direction either. `page.navigations` keeps the job it suits: crash recovery, which is page-level and needs no attribution. Corollary for tests: `test_setInjectedNavigationStream` parks only the page-wide stream, so a real `load` to a URL the scheme handler rejects still raises exit 3 for real — give a test that does not want one a servable URL (`PrismDocSchemeHandler(documentHTMLProvider:)`).+- **An injected navigation stream that fails on EVERY subscription hot-loops the MainActor for as long as the controller lives** (T-1744). `WebDocumentController.observeNavigations` re-subscribes after a `.navigationFailed` — that is the deliberate benign-bad-link classification (T-1943/T-2107) — so a `test_setInjectedNavigationStream` provider that throws a non-termination error each time never terminates the loop: it spins at one `Task.yield()` per iteration. Not literally forever — the observation task captures `[weak self]`, `applyOutcome` returns false once the controller is gone, and `deinit` cancels the task — but a test holding the controller keeps it spinning for that test's whole lifetime, and that is long enough: every test in the offending file passed in isolation; run as part of the full suite it starved every later `@MainActor` test, blew their timing bounds, and took the host down — 174 reported failures, ~168 of which never ran. A stream failing with `webContentProcessTerminated` is safer by contrast, but only ONCE a load has been issued (the retry budget spends and `applyNavigationOutcome` returns false, ending the loop); before the first `load` there is no `loadedDocumentURL`, `attemptRecovery` charges nothing and keeps observing, so a terminating stream hot-loops exactly like an ordinary one — hence the three-step setup in `waitEndsWhenRecoveryIsAbandoned`. Fail ONCE then park (`DocumentLoadingIndicatorWiringTests.failsOnceThenParks`). Corollary for any `@MainActor` test that polls: Swift Testing runs these concurrently on the one executor, so a tight timing bound measures machine load, not the code — pick a bound loosely, then mutation-check that it still sits inside the failure mode.+- **`page.navigations` cannot say WHICH navigation failed, and the failure it hands you is usually the one you just superseded** (T-1744 review). It is one subscription for the whole page, and calling `page.load` while a navigation is in flight is exactly what makes WebKit fail the outgoing one — so an outcome read off that stream lands on whichever load is current, which is the load that caused it. Any per-load state derived from it is mis-attributed by default, and the window is not narrow: `WebDocumentController.load` does not cancel the outgoing navigation, and a re-parse landing during the previous load's emit overlaps two loads routinely. The loading indicator's exit 3 (`navigationFailedBeforeSettling`) was built on it and so dropped the spinner over a document that was still loading. **The sequence returned by `page.load(_:)` is scoped to that one navigation** — that is what per-load state belongs on. `issueNavigation` is now the single place **`WebDocumentController`** tells its page to load — other pages have their own call sites (`FootnotePopoverWebPage`), and the structural pin only greps this one file; it stamps a `navigationEpoch`, and the flag is computed as `failedNavigationEpoch == navigationEpoch`, so a stale failure records an epoch that can never match and a fresh load retires the previous failure just by bumping past it — no clear to get stale in the other direction either. `page.navigations` keeps the job it suits: crash recovery, which is page-level and needs no attribution. Corollary for tests: `test_setInjectedNavigationStream` parks only the page-wide stream, so a real `load` to a URL the scheme handler rejects still raises exit 3 for real — give a test that does not want one a servable URL (`PrismDocSchemeHandler(documentHTMLProvider:)`). - **`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/prism/ViewModels/WebDocumentController.swift b/prism/ViewModels/WebDocumentController.swiftindex 8567f98..1b62f2d 100644--- a/prism/ViewModels/WebDocumentController.swift+++ b/prism/ViewModels/WebDocumentController.swift@@ -820,9 +820,12 @@ final class WebDocumentController {     /// Starts the page navigation for a load or a recovery reload, tagged with the     /// navigation epoch it belongs to.     ///-    /// The single place `page.load` is called, which is what makes the epoch a faithful-    /// identity: every navigation this controller issues gets one, and no navigation gets-    /// two. The sequence `page.load` returns is scoped to THAT navigation — unlike+    /// The single place THIS type tells its page to load, which is what makes the epoch a+    /// faithful identity: every navigation this controller issues gets one, and no+    /// navigation gets two. (Other pages elsewhere in the app — `FootnotePopoverWebPage` —+    /// have their own call sites; the structural pin in+    /// `DocumentLoadingIndicatorWiringTests` greps this file only, which is the scope the+    /// invariant needs.) The sequence the load returns is scoped to THAT navigation — unlike     /// `page.navigations`, which reports every navigation on the page through one stream —     /// so its failure is the one signal here that arrives already attributed. That is why     /// `navigationFailedBeforeSettling` is recorded from here and not fromdiff --git a/prism/Views/DocumentScrollContent.swift b/prism/Views/DocumentScrollContent.swiftindex 508b1fc..59931ff 100644--- a/prism/Views/DocumentScrollContent.swift+++ b/prism/Views/DocumentScrollContent.swift@@ -98,8 +98,11 @@ struct DocumentScrollContent: View {         // sanitisation of a large or HTML-heavy document — and WebKit laying out the         // resulting DOM — takes a moment; without this the responsive-but-blank page         // looks broken. Applied BEFORE the recovery-abandoned overlay below so that-        // banner always draws on top in the (harmless, sub-200ms) window where both-        // could theoretically be true for one frame.+        // banner always draws on top in the window where both are true. That window is+        // not a single frame: the wait polls at 100ms and the overlay then fades out+        // over 0.2s, so roughly 300ms of overlap in the normal case, and longer if this+        // task was cancelled at that moment (the flag then belongs to its successor).+        // Harmless because of the ordering, not because it is brief.         .overlay { documentLoadingOverlay }         .animation(reduceMotion ? nil : .easeInOut(duration: 0.2), value: isPreparingDocument)         // The renderer crashed repeatedly and recovery gave up (T-1943). Without this@@ -545,7 +548,7 @@ extension WebContrastMode { /// | 2 | The renderer crashed repeatedly and recovery gave up | `recoveryAbandoned` | `WebDocumentController.attemptRecovery`, budget spent | /// | 3 | The document's own navigation failed, with no retry coming | `navigationFailedBeforeSettling` | `WebDocumentController.issueNavigation`, when THIS load's own navigation sequence fails before layout | /// | 4 | Nothing above arrived within `maximumWait` | the deadline | this loop |-/// | 5 | A fresher parse revision superseded this load | `Task.isCancelled` | SwiftUI re-keying `.task(id:)` |+/// | 5 | The load task was cancelled | `Task.isCancelled` | SwiftUI re-keying `.task(id:)`, or the view going away | /// /// Exit 3 exists because exits 1 and 2 do not cover an ordinary navigation failure: /// `.navigationFailed` outside a recovery is deliberately classified as a benign bad@@ -574,9 +577,16 @@ extension WebContrastMode { /// `.value` ignores the awaiter's cancellation, same as the `beginLoad`/`abandonLoad` /// overlap this mirrors, T-1975) while a new one has already started and raised the flag /// for its own revision. Only the task that finishes UNCANCELLED may lower what it raised.+///+/// Cancellation without a successor — the view unmounting on a raw-source toggle or a+/// document close — reaches the same rule, and there the flag is simply discarded with+/// the view's `@State`. So the rule is safe in both shapes, but only the supersession+/// shape is the one it exists FOR. enum DocumentLoadingIndicatorPolicy {-    /// How often the wait re-reads the controller. The controller's flags are plain-    /// properties, so this polls rather than observes (see `waitForPreparation`).+    /// How often the wait re-reads the controller. It polls rather than observes: the wait+    /// runs outside any `ViewBuilder`, so SwiftUI's Observation-driven invalidation does+    /// nothing for it, and `navigationFailedBeforeSettling` is computed from+    /// `@ObservationIgnored` storage in any case.     static let pollInterval: Duration = .milliseconds(100)      /// The backstop bound for exit 4.@@ -598,8 +608,18 @@ enum DocumentLoadingIndicatorPolicy {     /// that calls it races `DocumentReaderView`'s own parse task with no sequencing     /// between them — so a `revision == 0` incarnation is reachable, and raising the flag     /// there would leave it paired with no load. It self-heals when the parse lands and-    /// re-keys the task, but "usually superseded in time" is not an invariant; "the flag-    /// is only ever raised for a load that is actually issued" is, and this is it.+    /// re-keys the task, but "usually superseded in time" is not something to rest on, and+    /// this gate is cheap.+    ///+    /// It does NOT make "the flag is only ever raised for a load that is actually issued"+    /// an invariant, and the earlier wording here claimed it did. `loadDocument` has a+    /// second early return — `guard !Task.isCancelled` after the off-main emit, which+    /// calls `abandonLoad` and issues nothing — and the flag is already up by then. That+    /// case is safe for a different reason: the same cancellation ends the wait through+    /// exit 5 and `mayClearFlag` then declines, leaving the flag to whichever task+    /// superseded this one (or to the discarded `@State` if the view went away). What this+    /// gate establishes is exactly its own condition: the flag is only raised once+    /// something has been parsed.     static func shouldIndicatePreparation(parseRevision: UInt64) -> Bool {         parseRevision > 0     }