P0: Fix flaky SQLite WAL livenessTest — replace Date() comparison with monotonic clock, remove CI skip

[totalslacker]

Summary

SQLiteStorageConcurrencyTests.livenessTest() was disabled on CI runners via @Test(.disabled(if: ProcessInfo.processInfo.environment["CI"] != nil)) because its writeEnd < readEnd assertion uses Date() timestamps that lack the resolution to distinguish ticks on fast release-mode CI runners. This issue removes the skip and replaces the timing primitive with a monotonic, sub-microsecond clock so the assertion is both correct and reliable on CI. Per the project engineering policy (.claude/CLAUDE.md), the CI skip is itself a policy violation. This is P0 — it blocks all merges until resolved.

Requirements

• Replace Date() timestamp captures in livenessTest() with a monotonic, sub-microsecond clock. Preferred: ContinuousClock / SuspendingClock instants. Acceptable: clock_gettime_nsec_np(CLOCK_UPTIME_RAW) / mach_absolute_time for nanosecond resolution.
• Compare instants using the chosen clock's native Instant type (e.g. < on ContinuousClock.Instant) — not bridged through TimeInterval or Double.
• Remove the .disabled(if: CI) annotation. The test must run unconditionally, including on CI.
• livenessTest() must pass ≥20 consecutive runs locally in both debug and release configurations before the fix is considered complete.
• If after the clock fix the assertion is still flaky, do not skip again — root-cause the real ordering race in the WAL writer/reader split.
• No new failing tests, anywhere, for any reason. This includes:
  • No new tests that fail locally (debug or release) on a clean checkout.
  • No new intermittently-failing ("flaky") tests — flaky counts as failing under project policy.
  • No new .disabled(if: ...), XCTSkipIf(...), XCTSkipUnless(...), #if !CI, @Test(.disabled(...)), or equivalent annotations on tests that previously ran.
  • No deleting assertions, weakening them to tautologies, or making any test a no-op.
  • No raising a threshold or budget without measurement evidence (median of ≥11 runs on a quiet machine) recorded in a comment next to the new value.
• The full test suite after the fix must be at least as healthy as main was before this PR, with exactly one fewer skipped/failing test (the one this issue fixes).
• If making livenessTest() pass requires introducing fragility elsewhere, stop and escalate to a human comment instead of merging.

Scope

In scope:

• Tests/SwitchcraftTests/SQLiteStorageConcurrencyTests.swift — clock fix and CI annotation removal in livenessTest().

Out of scope:

• Changes to performanceAssertionTest() (separate issue, different root cause).
• Changes to safariUnfuckerRegressionTest() (currently passing, leave alone).
• Changes to the SQLite storage actor split itself.

Acceptance Criteria

• livenessTest() no longer uses Date() for the ordering assertion.
• .disabled(if: CI) annotation removed from livenessTest().
• Test passes ≥20/20 consecutive runs locally in both debug and release.
• CI green on the fix branch (both swift test and swift test -c release jobs).
• No new test skips, XCTSkip, #if !CI, or equivalent introduced anywhere.
• No other test newly fails or newly becomes flaky as a result of this PR.
• Validate stage verifies ≥10 consecutive passing runs and confirms no other test regressed.

Prior Art / Context

• Root cause: Tests/SwitchcraftTests/SQLiteStorageConcurrencyTests.swift:87–116 — Date() timestamps are microsecond-class; under release-mode CI the seeded FTS scan completes fast enough that both timestamps can land in the same observed tick, causing the strict writeEnd < readEnd comparison to tie.
• The underlying assertion (writer must finish before slow reader on the WAL split) is correct and must be preserved.
• CI skip was introduced in commit f43e29e4 (2026-05-06) as a workaround.
• See also: issue T5CoreMLEmbedder CPU fallback path never recovers — 1,038/1,038 fail in production, dead weight #93 thread for prior discussion of why .disabled(if: CI) is not an acceptable resolution on this project.

Risks / Dependencies

• If after the clock fix the assertion is still non-deterministic, that indicates a real ordering race in the WAL writer/reader split — a deeper bug requiring separate root-cause analysis. The correct response is escalation, not another skip.
• Release-mode inlining/optimization may affect observed ordering of timestamp captures; the chosen monotonic clock must be used with appropriate memory ordering to avoid compiler reordering.

Engineering Policy Reminder

Per .claude/CLAUDE.md: failing tests never merge. "Flaky" is a diagnosis, not a resolution. Moving a bug to a different test is not a fix. This issue exists because the prior fix (skip on CI) was the wrong move — do not repeat it.

[totalslacker]

Hard constraint for the agent picking up this issue

Do not introduce any new failing tests, anywhere, for any reason.

This includes:

• ❌ No new tests that fail locally on a clean checkout (debug or release).
• ❌ No new tests that fail intermittently — "flaky" tests count as failing tests under the project policy in .claude/CLAUDE.md.
• ❌ No new .disabled(if: ...), XCTSkipIf(...), XCTSkipUnless(...), #if !CI, @Test(.disabled(...)), or any other skip annotation on tests that previously ran.
• ❌ No "fixing" the existing failure by deleting the assertion, weakening it to a tautology, or making the test a no-op.
• ❌ No raising of a budget/threshold without measurement evidence (e.g. median of 11+ runs on a quiet machine) recorded in a comment next to the new value.

The Validate stage on this issue must verify with a fresh swift test (debug) and swift test -c release that:

1. The test this issue targets passes ≥10 consecutive runs.
2. No other test newly fails — i.e. the full suite is at least as healthy as main was before this PR, with one fewer failing/skipped test (the one you just fixed).
3. CI on the PR is green on both jobs (swift test (macOS) and swift test -c release (macOS)).

If you find a way to make this issue's target test pass only by introducing fragility somewhere else, that's not a fix — it's moving the bug. Stop and escalate to a human comment instead of merging.

(See also: issue #93 thread comment about why .disabled(if: CI) and "skip flaky on CI" are not acceptable resolutions on this project.)

[totalslacker]

🏭 Fabrik — stage: Specify
branch: fabrik/issue-95 | commit: d814646 | main: d814646 | 2026-05-07 21:14 UTC

No stage-Specify.md exists yet. The issue body is well-structured and the new comment adds explicit hard constraints — not a question — that need to be incorporated into the spec. I'll restructure into the standard format and add the new constraints.

[totalslacker]

🏭 Fabrik — stage: Research
branch: fabrik/issue-95 | commit: e83d644 | main: e83d644 | 2026-05-07 21:20 UTC

Now I have all the information needed. Let me write up the research findings.

Research Findings

Relevant Code

• Tests/SwitchcraftTests/SQLiteStorageConcurrencyTests.swift (sole file in scope): Contains livenessTest() (lines 80–116), the test to fix. The fix is entirely local to this file.

  • Line 80–86: @Test decorator with .disabled(if: ProcessInfo.processInfo.environment["CI"] != nil, ...) — remove this annotation.
  • Line 106: let writeEnd = Date() — replace with ContinuousClock.now.
  • Line 110: let readEnd = Date() — replace with ContinuousClock.now.
  • Line 115: #expect(writeEnd < readEnd, ...) — no change needed; ContinuousClock.Instant conforms to Comparable so < works natively.
  • Lines 47–60: measureMedian helper also uses Date(), but is only called by performanceAssertionTest() (out of scope per spec). Do not change.
  • Lines 222–227: safariUnfuckerRegressionTest() uses Date() for wall-time measurement, but it's out of scope and already passing. Do not change.

• Sources/SwitchcraftCore/Search/SearchDeadlineContext.swift: Defines ContinuousClock.Instant usage pattern for this codebase — ContinuousClock.now captures instants, ContinuousClock.Instant is compared directly with < and >=. This is the established pattern to follow.

• Sources/SwitchcraftSQLite/SQLiteStorage.swift (lines 534–552): configureSearchDeadline uses ContinuousClock.now and ContinuousClock.Instant arithmetic — confirms the type is available project-wide.

• Sources/SwitchcraftSQLite/SQLiteReaderActor.swift (lines 209–220): configureProgressHandler uses ContinuousClock.now — further confirmation of the pattern.

• Sources/SwitchcraftSQLite/SQLiteWriterActor.swift: Owns the READWRITE | FULLMUTEX connection; upsertDocument runs on this actor, serially but independently from the reader actor.

• Sources/SwitchcraftSQLite/SQLiteReaderActor.swift: Owns the READONLY | FULLMUTEX connection; searchFullText runs here. Under WAL, the reader and writer connections proceed concurrently at the SQLite file level.

Architecture Notes

The WAL concurrency architecture (ADR 019) is the foundation of why livenessTest() should work:

• SQLiteStorage is a facade over SQLiteWriterActor + SQLiteReaderActor, each holding an independent SQLite connection.
• Under WAL, SQLite permits one writer + N concurrent readers at the file level. The reader holds a snapshot at the start of its read; the writer can commit new pages into the WAL concurrently.
• livenessTest() exploits this: it starts a slow FTS scan (searchFullText over 5,000 docs) on the reader actor, then immediately issues a write (upsertDocument) on the writer actor. Because they are on separate actors and separate connections, the write completes without waiting for the read to finish. writeEnd < readEnd confirms this.

The root cause of the original flakiness is purely timing resolution: Date() has ~1µs wall-clock resolution, and under release-mode CI the write can complete so fast that both writeEnd and readEnd land in the same 1µs tick, causing the strict < to fail on a tie. Replacing with ContinuousClock.now gives nanosecond-class resolution (backed by mach_absolute_time on Darwin), making ties statistically impossible when the FTS scan over 5,000 docs takes multiple milliseconds.

Relevant ADRs

• ADR 019 (019-sqlite-writer-reader-split.md): Directly relevant — defines the writer/reader actor split whose correctness livenessTest() asserts. The underlying WAL concurrency model is sound; the assertion must be preserved. No conflict with this fix.
• ADR 020 (020-search-timeout-and-cancellation.md): Relevant as context — ContinuousClock.Instant is the established type for time captures in this layer (SearchDeadlineContext, progress handler). The fix aligns with this convention. No conflict.
• ADR 012 (012-performance-regression-thresholds.md): Not directly relevant — the fix touches test timing capture, not any production performance threshold.

Constraints

1. Platform availability: ContinuousClock requires Swift 5.7+. The package targets macOS 13, iOS 16, visionOS 1 — all shipped with Swift 5.7+. ContinuousClock is available on all supported platforms with no additional import.

2. No new import needed: ContinuousClock is in the Swift standard library. The test file already imports Foundation and Testing.

3. Native comparison: ContinuousClock.Instant conforms to Comparable. The < in #expect(writeEnd < readEnd, ...) works directly without bridging through TimeInterval or Double. The spec explicitly requires this — no bridging.

4. Compiler reordering: In Swift, await is a suspension point and acts as a memory ordering barrier. writeEnd is captured immediately after await storage.upsertDocument(...) returns and readEnd immediately after _ = try await slowRead. The Swift compiler cannot reorder these captures past the await boundaries. No special withUnsafeContinuation or memory fence is needed.

5. The .disabled(if:) on performanceAssertionTest() (lines 136–141) must not be touched — it is explicitly out of scope. Only the annotation on livenessTest() is removed.

6. Task.yield() × 8: The test uses this to give the cooperative scheduler time to begin the reader scan before the write is issued. This is valid Swift Concurrency idiom. ContinuousClock does not affect this; the yield loop stays unchanged.

7. Flakiness residual risk: If after the clock change the test is still non-deterministic, that indicates a real ordering race (the reader actor hadn't started the FTS scan yet when the write completed, so the timestamps happened to be in the wrong order). The spec is explicit: do not re-skip in that case; escalate to a human. However, with 5,000-document FTS scans taking tens to hundreds of milliseconds and a single-upsert write taking microseconds to low milliseconds, a residual race is unlikely.

Technical Questions

No unresolved technical questions. The fix is mechanically clear, the required API (ContinuousClock.now) is already used extensively in adjacent code, and the scope is limited to 4 lines in a single file:

1. Remove the .disabled(if: CI) attribute from the @Test decorator on livenessTest().
2. Replace let writeEnd = Date() with let writeEnd = ContinuousClock.now.
3. Replace let readEnd = Date() with let readEnd = ContinuousClock.now.
4. Update the doc comment on livenessTest() to remove the "Disabled on CI runners" rationale paragraph (it's no longer accurate).

The #expect(writeEnd < readEnd, ...) comparison needs no change — ContinuousClock.Instant already supports < natively.

Risks

• Residual flakiness: Theoretically, if the 8× Task.yield() isn't enough to get the reader actor started before the write completes, the test could still fail occasionally. In practice with 5,000 docs and cooperative scheduling on Apple silicon, the reader will be well into the FTS scan before a single upsert finishes. If it happens, per spec: escalate rather than re-skip.
• performanceAssertionTest() still has its own CI skip: This is out of scope per spec and should not be touched. There is no risk of regressing it.
• measureMedian uses Date(): This helper is only called by the out-of-scope performanceAssertionTest(). Leaving it alone is correct.

Documentation Impact

No documentation impact. This is an internal test-only fix. No public API, CLI commands, config fields, or user-facing behavior changes.

---

Used 20/50 turns, 0k input / 6k output tokens.