Repository navigation
fix(launch): paint first Sessions snapshot within seconds of warm hydration - #10
Conversation
…es behind the scan Three compounding causes kept 'Preparing session index' up for the whole validation scan despite a 3s cache hydrate: navigation rebuilds were ordered after refreshNow RETURNS (which awaits the entire scan), so the hydrated corpus never reached the snapshot store; each scan batch then cancel-restarted the lineage resolve, starving the first forest; and the scan saturated the cooperative pool at userInitiated, starving whatever resolve survived. Rebuilds now ride the store's own publishes (throttled 500ms), the forest is stale-while-revalidate (resolves are never cancelled; the newest completed forest paints and the fresh one swaps in silently), warm revalidation runs at utility priority, and the resolve hops to GCD so a saturated pool cannot delay it. TRIFOLA_LAUNCH_METRICS now also stamps the first published snapshot. Measured on the real corpus: hydrated 8,036 entries in 3.3s, first sessions snapshot at 4.5s (previously ~36-40s, gated on scan completion).
📝 WalkthroughWalkthroughSessions snapshot generation now supports stale-while-revalidate lineage rendering. Completed lineage results remain available while newer resolutions run, warm scans use utility priority, session changes trigger throttled navigation refreshes, and first-snapshot row counts are reported through launch metrics. ChangesSessions lineage snapshot flow
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant SessionStore
participant AppServices
participant NavigationSnapshotStore
participant LineageResolutionTask
SessionStore->>AppServices: publish sessions
AppServices->>NavigationSnapshotStore: throttle and refresh snapshots
NavigationSnapshotStore->>LineageResolutionTask: reuse or start lineage resolve
NavigationSnapshotStore-->>AppServices: publish stale or cached snapshot
LineageResolutionTask-->>NavigationSnapshotStore: return completed forest
NavigationSnapshotStore-->>AppServices: publish fresh snapshot
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/Trifola/NavigationSnapshotStore.swift`:
- Around line 394-414: Update lineageResolveCompleted so scheduleSessions() is
invoked even when resolution is nil, preserving the existing cleanup and
successful-resolution memoization while removing the early return that skips
retry scheduling after a failed resolve.
- Around line 473-481: Update the staleResolution branch in the
resolution-selection flow so the forest is rendered with the session snapshot it
was resolved from, rather than the current inputs.sessions. Propagate that
matching snapshot through projectLineage, sourceCount, and Flat-mode
resolution.key(for:) lookups; if no matching snapshot is available, wait for the
current resolution instead of combining stale forest data with live session
membership.
- Around line 436-449: The detached Task’s GCD hop prevents cancellation from
reaching SessionLineage.resolveWithIndexCancellable, so update the task creation
around lineageInFlight?.task to keep resolution on a cancellable Swift task or
explicitly propagate cancellation into the dispatched work item. Preserve the
existing user-initiated priority and ensure deinit-triggered task cancellation
can abort the in-flight resolve.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b7adb9ab-99fa-4029-a7a8-1a7f21f2abb6
📒 Files selected for processing (6)
CHANGELOG.mdSources/Trifola/AppServices.swiftSources/Trifola/NavigationSnapshotStore.swiftSources/TrifolaKit/Stores.swiftTests/TrifolaKitTests/NavigationSnapshotShapeTests.swiftdocs/ARCHITECTURE.md
| /// A lineage resolve finished. Record it as the freshest forest, memoize | ||
| /// when it still matches the live inputs, and re-schedule so a snapshot | ||
| /// that painted a stale forest swaps to this one silently. When inputs | ||
| /// moved on mid-resolve, the re-schedule starts the follow-up resolve. | ||
| private func lineageResolveCompleted( | ||
| source: LineageSource, | ||
| task: Task<SessionLineageResolution?, Never>, | ||
| resolution: SessionLineageResolution? | ||
| ) { | ||
| if lineageInFlight?.task == task { lineageInFlight = nil } | ||
| guard let resolution else { return } | ||
| lastLineageResolution = resolution | ||
| if let inputs, LineageSource( | ||
| sessionsRevision: inputs.sessionsRevision, | ||
| evidenceRevision: inputs.lineageEvidenceRevision, | ||
| includeHeuristics: inputs.showHeuristicLinks) == source { | ||
| lineageMemo = (source: source, resolution: resolution) | ||
| } | ||
| scheduleSessions() | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
A failed resolve never retries itself.
When resolution is nil (the try? in the detached task swallowed a throw), lineageInFlight is cleared but scheduleSessions() is never called — the early return at Line 404 skips it. Nothing else will re-arm a resolve attempt until some unrelated external event (a session/filter change) happens to call rebuild()/updateSessionFilter() again. On the very first launch, if this first-ever resolve fails and the corpus is otherwise idle, Sessions can stay stuck exactly the way this PR is fixing.
🐛 Proposed fix: always re-schedule, even on failure
private func lineageResolveCompleted(
source: LineageSource,
task: Task<SessionLineageResolution?, Never>,
resolution: SessionLineageResolution?
) {
if lineageInFlight?.task == task { lineageInFlight = nil }
- guard let resolution else { return }
- lastLineageResolution = resolution
- if let inputs, LineageSource(
- sessionsRevision: inputs.sessionsRevision,
- evidenceRevision: inputs.lineageEvidenceRevision,
- includeHeuristics: inputs.showHeuristicLinks) == source {
- lineageMemo = (source: source, resolution: resolution)
+ if let resolution {
+ lastLineageResolution = resolution
+ if let inputs, LineageSource(
+ sessionsRevision: inputs.sessionsRevision,
+ evidenceRevision: inputs.lineageEvidenceRevision,
+ includeHeuristics: inputs.showHeuristicLinks) == source {
+ lineageMemo = (source: source, resolution: resolution)
+ }
}
scheduleSessions()
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// A lineage resolve finished. Record it as the freshest forest, memoize | |
| /// when it still matches the live inputs, and re-schedule so a snapshot | |
| /// that painted a stale forest swaps to this one silently. When inputs | |
| /// moved on mid-resolve, the re-schedule starts the follow-up resolve. | |
| private func lineageResolveCompleted( | |
| source: LineageSource, | |
| task: Task<SessionLineageResolution?, Never>, | |
| resolution: SessionLineageResolution? | |
| ) { | |
| if lineageInFlight?.task == task { lineageInFlight = nil } | |
| guard let resolution else { return } | |
| lastLineageResolution = resolution | |
| if let inputs, LineageSource( | |
| sessionsRevision: inputs.sessionsRevision, | |
| evidenceRevision: inputs.lineageEvidenceRevision, | |
| includeHeuristics: inputs.showHeuristicLinks) == source { | |
| lineageMemo = (source: source, resolution: resolution) | |
| } | |
| scheduleSessions() | |
| } | |
| /// A lineage resolve finished. Record it as the freshest forest, memoize | |
| /// when it still matches the live inputs, and re-schedule so a snapshot | |
| /// that painted a stale forest swaps to this one silently. When inputs | |
| /// moved on mid-resolve, the re-schedule starts the follow-up resolve. | |
| private func lineageResolveCompleted( | |
| source: LineageSource, | |
| task: Task<SessionLineageResolution?, Never>, | |
| resolution: SessionLineageResolution? | |
| ) { | |
| if lineageInFlight?.task == task { lineageInFlight = nil } | |
| if let resolution { | |
| lastLineageResolution = resolution | |
| if let inputs, LineageSource( | |
| sessionsRevision: inputs.sessionsRevision, | |
| evidenceRevision: inputs.lineageEvidenceRevision, | |
| includeHeuristics: inputs.showHeuristicLinks) == source { | |
| lineageMemo = (source: source, resolution: resolution) | |
| } | |
| } | |
| scheduleSessions() | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Sources/Trifola/NavigationSnapshotStore.swift` around lines 394 - 414, Update
lineageResolveCompleted so scheduleSessions() is invoked even when resolution is
nil, preserving the existing cleanup and successful-resolution memoization while
removing the early return that skips retry scheduling after a failed resolve.
| let task = Task.detached(priority: .userInitiated) { | ||
| try? SessionLineage.resolveWithIndexCancellable( | ||
| sessions: inputs.sessions, | ||
| evidence: inputs.lineageEvidence, | ||
| includeHeuristicLinks: inputs.showHeuristicLinks) | ||
| // Hop to GCD: the cooperative pool can be saturated by a | ||
| // corpus scan, and the first snapshot must not starve behind | ||
| // it. GCD over-commits, so this resolve always gets a thread. | ||
| await withCheckedContinuation { continuation in | ||
| DispatchQueue.global(qos: .userInitiated).async { | ||
| continuation.resume( | ||
| returning: try? SessionLineage.resolveWithIndexCancellable( | ||
| sessions: inputs.sessions, | ||
| evidence: inputs.lineageEvidence, | ||
| includeHeuristicLinks: inputs.showHeuristicLinks)) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -B3 -A20 'func resolveWithIndexCancellable|func buildResolution' Sources/TrifolaKit/SessionLineage.swiftRepository: ss251/trifola
Length of output: 1650
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== NavigationSnapshotStore relevant methods ==\n'
ast-grep outline Sources/Trifola/NavigationSnapshotStore.swift --view expanded | sed -n '1,260p'
printf '\n== Focused slice around the detached task ==\n'
sed -n '400,480p' Sources/Trifola/NavigationSnapshotStore.swift
printf '\n== SessionLineage cancellation checks ==\n'
sed -n '405,470p' Sources/TrifolaKit/SessionLineage.swiftRepository: ss251/trifola
Length of output: 8441
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Search for lineageInFlight cancellation and task lifecycle ==\n'
rg -n -B3 -A10 'lineageInFlight|task.cancel\(|Task\.detached|withCheckedContinuation|DispatchQueue\.global' Sources/Trifola/NavigationSnapshotStore.swift Sources/TrifolaKit/SessionLineage.swiftRepository: ss251/trifola
Length of output: 15989
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- NavigationSnapshotStore.swift: task lifecycle ---'
rg -n -B3 -A12 'lineageInFlight|task.cancel\(|Task\.detached|withCheckedContinuation|DispatchQueue\.global' Sources/Trifola/NavigationSnapshotStore.swift
echo
echo '--- NavigationSnapshotStore.swift: around detached task ---'
sed -n '420,470p' Sources/Trifola/NavigationSnapshotStore.swift
echo
echo '--- SessionLineage.swift: cancellation plumbing ---'
sed -n '393,460p' Sources/TrifolaKit/SessionLineage.swiftRepository: ss251/trifola
Length of output: 15076
🌐 Web query:
Swift concurrency DispatchQueue.async Task.checkCancellation current task context cancellation does not propagate through GCD documentation
💡 Result:
Swift task cancellation does not automatically propagate into work dispatched via Grand Central Dispatch (GCD) [1][2]. Because Swift concurrency and GCD are separate systems with different execution models, there is no inherent linkage between a Swift Task's cancellation state and the execution of a block submitted to a DispatchQueue [3]. Key points regarding this behavior include: 1. System Separation: Structured concurrency relies on the Swift runtime to track parent-child task relationships and automatically propagate cancellation [4][5][6]. GCD operates independently of these task trees and does not monitor or respect the cancellation status of the surrounding Swift Task [1][3]. 2. Cooperative Cancellation: Cancellation in Swift is cooperative [2][6]. Functions such as Task.checkCancellation only function when called within an asynchronous Swift context where a task handle exists [7][8]. Since a block inside DispatchQueue.async executes on a different execution context (typically a system-managed thread pool not associated with a specific Swift Task), calling Task.checkCancellation inside it will generally not behave as expected or will fail to observe the parent task's state [2]. 3. Handling Cancellation Across Systems: If you need to stop work submitted to GCD based on a Task being cancelled, you must manually bridge the two systems [1][2]. Common approaches include: - Using a DispatchWorkItem: You can create a DispatchWorkItem and call its cancel method manually when the parent task is cancelled [9]. - Cancellation Handlers: Use withTaskCancellationHandler(operation:onCancel:) to perform custom logic, such as calling cancel on a DispatchWorkItem or a URLSessionTask, when the Swift Task is cancelled [7][2]. In summary, because GCD tasks are not part of the Swift structured concurrency tree, they are "invisible" to automatic propagation [1]. You must explicitly coordinate cancellation if you are mixing the two concurrency models [2].
Citations:
- 1: https://forums.swift.org/t/dispatchqueue-asyncafter-deadline-in-structured-concurrency/69006
- 2: https://forums.swift.org/t/is-task-cancellation-totally-cooperative/83100
- 3: https://stackoverflow.com/questions/76387403/wrap-async-task-into-dispatchworkitem-in-swift-to-make-it-cancellable
- 4: https://developer.apple.com/videos/play/wwdc2023/10170/
- 5: https://developer.apple.com/videos/play/wwdc2023/10170/?time=1176
- 6: https://github.com/swiftlang/swift-evolution/blob/main/proposals/0304-structured-concurrency.md
- 7: https://developer.apple.com/documentation/swift/task?changes=_3
- 8: https://github.com/apple/swift/blob/main/stdlib/public/Concurrency/TaskCancellation.swift
- 9: https://www.linkedin.com/posts/rishbaranwal_iosdevelopment-swift-gcd-activity-7312448151645294592-zKyo
Cancellation doesn’t cross the GCD hop. resolveWithIndexCancellable()’s Task.checkCancellation() calls won’t see lineageInFlight?.task.cancel() once the work is submitted to DispatchQueue.global, so deinit can’t stop an in-flight resolve. If teardown should be able to abort this path, keep it on a Swift task or bridge cancellation into the dispatched work item.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Sources/Trifola/NavigationSnapshotStore.swift` around lines 436 - 449, The
detached Task’s GCD hop prevents cancellation from reaching
SessionLineage.resolveWithIndexCancellable, so update the task creation around
lineageInFlight?.task to keep resolution on a cancellable Swift task or
explicitly propagate cancellation into the dispatched work item. Preserve the
existing user-initiated priority and ensure deinit-triggered task cancellation
can abort the in-flight resolve.
| } else if let staleResolution { | ||
| // Serve the newest completed forest NOW; the in-flight resolve | ||
| // re-schedules a silent swap when it lands. | ||
| resolution = staleResolution | ||
| } else if let resolutionTask { | ||
| // First-ever paint with nothing to show — the only case that | ||
| // waits on a resolve. | ||
| guard let completed = await resolutionTask.value else { return } | ||
| resolution = completed |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Stale forest can transiently disagree with the live session count / Flat-mode membership.
When staleResolution is served (Line 476), projectLineage is called with the OLD forest but the CURRENT inputs.sessions/inputs.sessions.count (used for sourceCount at Line 555 and for Flat-mode's resolution.key(for: summary) lookups at Lines 900-909). During an active scan this can briefly show a session-count header that doesn't match the rendered rows, and Flat mode can silently drop newly-discovered sessions the stale forest doesn't know about yet, until the fresh resolve lands and swaps in. This self-corrects once the follow-up resolve completes, so it's low severity, but worth confirming it matches the "session counts... preserved" claim in the PR description for a live, still-scanning corpus (vs. the static benchmark corpus used for validation).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Sources/Trifola/NavigationSnapshotStore.swift` around lines 473 - 481, Update
the staleResolution branch in the resolution-selection flow so the forest is
rendered with the session snapshot it was resolved from, rather than the current
inputs.sessions. Propagate that matching snapshot through projectLineage,
sourceCount, and Flat-mode resolution.key(for:) lookups; if no matching snapshot
is available, wait for the current resolution instead of combining stale forest
data with live session membership.
1 participant
Intent
Fix the trifola macOS app warm-launch stall: the Sessions view sat in 'Preparing session index' for 30+ seconds on every launch even though the on-disk session cache hydrates in ~3s, because navigation snapshot rebuilds were sequenced after the full validation scan completed, the lineage resolve was cancel-restarted by every scan batch, and the scan saturated the cooperative thread pool. The first Sessions snapshot must paint within a few seconds of hydration (stale-while-revalidate: never cancel an in-flight resolve, serve the newest completed forest immediately and swap the fresh one in silently, run background revalidation at lower priority), with no change to the resolved lineage or session counts
What Changed
refreshNowvalidation scan returns, so the first snapshot no longer waits on the whole scan.NavigationSnapshotStore: an in-flight lineage resolve is never cancelled when the source revision bumps; the newest completed forest is served immediately and the fresh resolve swaps in silently on completion vialineageResolveCompleted.DispatchQueue.global(qos: .userInitiated)continuation, and drop warm-launch revalidation scans to.utilitypriority (cold start stays.userInitiated); adds aTRIFOLA_LAUNCH_METRICSfirst-snapshot stderr marker, shape-test coverage, and CHANGELOG/architecture doc updates.Risk Assessment
✅ Low: Well-bounded, well-commented change that satisfies the intent's required stale-while-revalidate behavior with preserved final-state correctness (single-flight resolve, convergence, no dropped/double first-paint verified); no error- or warning-level defects found, only two informational design tradeoffs.
Testing
Baseline
swift buildcompiles the whole package including the warnings-as-errorsTrifolaapp target, andswift testruns the full suite green (788 tests / 143 suites) — covering the warm-launch hydration test (8192 sessions publish with identical IDs and exact token totals) and the new stale-while-revalidate NavigationSnapshotShapeTests, confirming no change to counts or lineage. Because the actual timing behavior lives in the executable (untestable)NavigationSnapshotStore+AppServices, I produced the decisive evidence by running the real app pipeline via its own--benchmark-nav-liveharness against pre-primed synthetic warm corpora withTRIFOLA_LAUNCH_METRICS=1, timestamping the launch timeline. The first Sessions snapshot (the exact moment the "Preparing session index…" overlay clears) painted at +3.71s on 6k sessions and +5.61s on 12k — ~1s after cache hydration and 3–6s before "scan reconciled" — and tracked hydration rather than scan duration as the corpus grew, directly demonstrating the snapshot no longer starves behind the scan. No visual UI screenshot was captured because the change is purely about WHEN the (visually unchanged) Sessions list appears; the timestamped launch-metric transcript is the precise product-level artifact for that, matching how the author measured it in the commit. All checks passed; no failures or unfixable setup issues. Transient synthetic corpus/cache were removed and the worktree is clean.Evidence: Warm-launch timeline — 6,000 sessions (first snapshot at +3.71s, before scan reconciled at +6.79s)
[+ 2934ms] [launch-metric] cache hydrated: 6000 entries in 2.36s [+ 3711ms] [launch-metric] first sessions snapshot: 400 rows [+ 6789ms] [launch-metric] scan reconciled: 6000 entries at +6.69sEvidence: Warm-launch timeline — 12,000 sessions (first snapshot at +5.61s, ~6s before scan reconciled at +11.68s)
[+ 4449ms] [launch-metric] cache hydrated: 12000 entries in 3.75s [+ 5609ms] [launch-metric] first sessions snapshot: 400 rows [+ 11681ms] [launch-metric] scan reconciled: 12000 entries at +11.60sEvidence: Consolidated evidence write-up (before/after decoupling table + invariants)
Evidence: Reproduction harness — synthetic corpus generator + timestamping launch runner
Evidence: Warm hydration test evidence line (counts/lineage unchanged)
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
Sources/Trifola/NavigationSnapshotStore.swift:440- The lineage resolve now runs insideDispatchQueue.global(qos:).asyncwrapped bywithCheckedContinuation(NavigationSnapshotStore.swift:440-448). On a GCD thread there is no current Swift task, soTask.isCancelledinsideSessionLineage.resolveWithIndexCancellableis always false — its cancellation checks are inert, anddeinit'slineageInFlight?.task.cancel()(line 213) no longer stops the running resolve (the continuation still resumes when GCD finishes). This is intentional per the intent's 'never cancel an in-flight resolve', and store lifetime ≈ app lifetime so the leftover work is negligible; noting it because the deadcancel()call and the now-misleading...Cancellablename are a latent trap for future maintainers who assume teardown halts the resolve.Sources/Trifola/NavigationSnapshotStore.swift:473- Stale-while-revalidate serves a resolution computed from an older session set (NavigationSnapshotStore.swift:473), but in.flatmodeprojectLineagemaps the CURRENTinputs.sessionsthrough the staleresolution.key(for:)(line 900), which returns nil for any summary absent from the stale resolution's key map (SessionLineage.swift:350-352). Newly-scanned sessions can therefore be transiently missing from the flat list until the fresh resolve swaps in, while the headersourceCountreflects the current count. This self-heals within one resolve and is inherent to the stale-while-revalidate design the intent explicitly requested (.lineageis the default mode); recording it as an accepted transient tradeoff, not a defect.✅ **Test** - passed
✅ No issues found.
swift build— full package including theTrifolaexecutable target compiles under-warnings-as-errorsswift test— full suite passes: 788 tests across 143 suitesswift test --filter 'NavigationSnapshotShapeTests|WarmLaunchHydrationTests'— stale-while-revalidate source contracts + 8192-session warm hydration (identical IDs, exact token totals, presentation=liveRefreshing)Primed a warm cache headlessly:Trifola --selfcheckwith CLAUDE_CONFIG_DIR/TRIFOLA_SESSION_INDEX_CACHE pointed at a synthetic 6k then 12k session corpusLive warm-launch timeline:.build/debug/Trifola --benchmark-nav-livewithTRIFOLA_LAUNCH_METRICS=1(+ NSArgumentDomain args forcing default Sessions filters), timestamping each[launch-metric]stderr line — captured at 6,000 and 12,000 sessions✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.
Summary by CodeRabbit
Bug Fixes
Documentation