From 5f0a1e11817965b65ef685e32cfffab23e3bb94e Mon Sep 17 00:00:00 2001 From: Johnny IV Young Ospino Date: Wed, 7 Oct 2026 12:44:22 -0500 Subject: [PATCH 1/3] fix(embeddings): bind background writes to current text --- .../EmbeddingRefreshService.swift | 11 +- .../GanchoAppCore/EnrichmentService.swift | 35 ++++-- .../ContentBoundEmbeddingStore.swift | 48 ++++++++ .../EmbeddingRefreshServiceTests.swift | 9 +- .../EmbeddingWriteRaceTests.swift | 116 ++++++++++++++++++ .../ContentBoundEmbeddingTests.swift | 95 ++++++++++++++ docs/CONTENT_BOUND_EMBEDDINGS.md | 44 +++++++ 7 files changed, 342 insertions(+), 16 deletions(-) create mode 100644 Packages/GanchoKit/Sources/GanchoKit/ContentBoundEmbeddingStore.swift create mode 100644 Packages/GanchoKit/Tests/GanchoAppCoreTests/EmbeddingWriteRaceTests.swift create mode 100644 Packages/GanchoKit/Tests/GanchoKitTests/ContentBoundEmbeddingTests.swift create mode 100644 docs/CONTENT_BOUND_EMBEDDINGS.md diff --git a/Packages/GanchoKit/Sources/GanchoAppCore/EmbeddingRefreshService.swift b/Packages/GanchoKit/Sources/GanchoAppCore/EmbeddingRefreshService.swift index 0d746e25..dd34defa 100644 --- a/Packages/GanchoKit/Sources/GanchoAppCore/EmbeddingRefreshService.swift +++ b/Packages/GanchoKit/Sources/GanchoAppCore/EmbeddingRefreshService.swift @@ -68,10 +68,14 @@ public struct EmbeddingRefreshService: Sendable { } /// Runs the refresh until nothing is stale, the task is cancelled, or the - /// environment turns hostile. Returns how many clips were re-embedded. + /// environment turns hostile. Returns how many vectors were actually committed. + /// Sources without ContentBoundEmbeddingStoring skip the pass rather than use + /// the retained, unconditional legacy write requirement. @discardableResult public func run(store: any EmbeddingRefreshSource) async -> Int { - guard let embedder = makeEmbedder() else { return 0 } + guard let embeddingStore = store as? any ContentBoundEmbeddingStoring, + let embedder = makeEmbedder() + else { return 0 } var refreshed = 0 while !Task.isCancelled, isEnvironmentSuitable() { guard let batch = try? await store.staleEmbeddingClipIDs(limit: Self.batchSize), @@ -82,7 +86,8 @@ public struct EmbeddingRefreshService: Sendable { if Task.isCancelled || !isEnvironmentSuitable() { return refreshed } guard case .text(let text)? = try? await store.content(for: id), let vector = try? embedder.vector(for: String(text.prefix(Self.inputLimit))), - (try? await store.saveEmbedding(clipID: id, vector: vector)) != nil + (try? await embeddingStore.saveEmbeddingIfCurrent( + clipID: id, vector: vector, expectedText: text)) == true else { continue } refreshed += 1 progressed = true diff --git a/Packages/GanchoKit/Sources/GanchoAppCore/EnrichmentService.swift b/Packages/GanchoKit/Sources/GanchoAppCore/EnrichmentService.swift index 02352704..89e5147d 100644 --- a/Packages/GanchoKit/Sources/GanchoAppCore/EnrichmentService.swift +++ b/Packages/GanchoKit/Sources/GanchoAppCore/EnrichmentService.swift @@ -20,17 +20,26 @@ import GanchoKit /// `refreshRecents()`. The caller passes that as `onTitleWritten`, which runs on /// the caller's actor immediately after the write, in the original position. /// -/// Stateless and `Sendable`: it constructs `ImageTextExtractor` / -/// `TieredClipAnnotator` / `ContextualSentenceEmbedder` per call exactly as the -/// inlined code did (the process-lifetime-instance optimization is deliberately -/// NOT taken here — this is a pure move). Each per-call helper is created and -/// consumed inside its own `if` with no `await` between construction and last -/// use, so no non-`Sendable` value crosses an actor boundary and the type needs no -/// `@MainActor` isolation. On CI the annotator degrades to the heuristic and the -/// embedder reports `hasAvailableAssets == false`, so those stages yield nothing -/// there; the plan/`writeTitle` gating remains unit-testable. +/// Sendable configuration constructs each non-Sendable helper only for its own +/// stage, without suspending during model computation. The injected embedder +/// factory gives race tests deterministic vectors without requiring model assets. +/// Background vector writes require ContentBoundEmbeddingStoring; legacy stores +/// without that optional capability still receive title/OCR work but skip vectors. public struct EnrichmentService: Sendable { - public init() {} + private let makeEmbedder: @Sendable () -> (any TextEmbedding)? + + public init() { + self.init(makeEmbedder: { + guard let embedder = ContextualSentenceEmbedder(), embedder.hasAvailableAssets else { + return nil + } + return embedder + }) + } + + init(makeEmbedder: @escaping @Sendable () -> (any TextEmbedding)?) { + self.makeEmbedder = makeEmbedder + } /// Runs the enrichment IO shared by both shells, in the SAME order as the /// inlined code: OCR → title → embedding. @@ -67,10 +76,12 @@ public struct EnrichmentService: Sendable { } // Semantic vector (the embedder caches its model after the first call). if plan.runs(.embedding), case .text(let text)? = content, - let embedder = ContextualSentenceEmbedder(), embedder.hasAvailableAssets, + let embeddingStore = store as? any ContentBoundEmbeddingStoring, + let embedder = makeEmbedder(), let vector = try? embedder.vector(for: String(text.prefix(1_000))) { - _ = try? await store.saveEmbedding(clipID: item.id, vector: vector) + _ = try? await embeddingStore.saveEmbeddingIfCurrent( + clipID: item.id, vector: vector, expectedText: text) } } diff --git a/Packages/GanchoKit/Sources/GanchoKit/ContentBoundEmbeddingStore.swift b/Packages/GanchoKit/Sources/GanchoKit/ContentBoundEmbeddingStore.swift new file mode 100644 index 00000000..bbad00a0 --- /dev/null +++ b/Packages/GanchoKit/Sources/GanchoKit/ContentBoundEmbeddingStore.swift @@ -0,0 +1,48 @@ +import Foundation +import GRDB + +/// Optional capability for background work derived from an earlier text snapshot. +/// The legacy ClipEnriching surface stays source-compatible; asynchronous producers +/// require this capability rather than falling back to an unconditional vector write. +public protocol ContentBoundEmbeddingStoring: Sendable { + /// Stores a vector only while the exact source body is still eligible and current. + /// Title/keyword-only edits do not invalidate a body-derived vector. Returning to + /// the same body is safe: the vector depends on that body, not its edit history. + @discardableResult + func saveEmbeddingIfCurrent( + clipID: UUID, vector: [Float], expectedText: String + ) async throws -> Bool +} + +extension GRDBClipboardStore: ContentBoundEmbeddingStoring { + public func saveEmbeddingIfCurrent( + clipID: UUID, vector: [Float], expectedText: String + ) async throws -> Bool { + try Task.checkCancellation() + let data = vector.withUnsafeBufferPointer { Data(buffer: $0) } + let excludedKinds = ClipContentKind.allCases.filter { + $0.prefersMaskedPreview || $0 == .image || $0 == .fileReference + }.map(\.rawValue) + let placeholders = excludedKinds.map { _ in "?" }.joined(separator: ", ") + return try await writer.write { db in + try Task.checkCancellation() + var arguments: [any DatabaseValueConvertible] = [ + vector.count, data, EmbeddingModelInfo.currentVersion, clipID.uuidString, + expectedText, Date.now, "public.file-url" + ] + arguments.append(contentsOf: excludedKinds) + try db.execute( + sql: """ + INSERT OR REPLACE INTO clip_embedding (clipID, dimension, vector, modelVersion) + SELECT id, ?, ?, ? FROM clip + WHERE id = ? AND contentText = ? AND contentBlobHash IS NULL + AND isSensitive = 0 AND isArchived = 0 + AND (expiresAt IS NULL OR expiresAt > ?) + AND (contentTypeIdentifier IS NULL OR contentTypeIdentifier != ?) + AND kind NOT IN (\(placeholders)) + """, + arguments: StatementArguments(arguments)) + return db.changesCount == 1 + } + } +} diff --git a/Packages/GanchoKit/Tests/GanchoAppCoreTests/EmbeddingRefreshServiceTests.swift b/Packages/GanchoKit/Tests/GanchoAppCoreTests/EmbeddingRefreshServiceTests.swift index 354ecffe..a8374cfe 100644 --- a/Packages/GanchoKit/Tests/GanchoAppCoreTests/EmbeddingRefreshServiceTests.swift +++ b/Packages/GanchoKit/Tests/GanchoAppCoreTests/EmbeddingRefreshServiceTests.swift @@ -8,7 +8,7 @@ import Testing /// Scriptable refresh source: `stale` drains as saves land, exactly like the /// real store (a refreshed row stops being stale), so batch-loop convergence /// is exercised honestly. -private actor FakeRefreshStore: EmbeddingRefreshSource { +private actor FakeRefreshStore: EmbeddingRefreshSource, ContentBoundEmbeddingStoring { private var stale: [UUID] private let texts: [UUID: String] private(set) var saved: [UUID: [Float]] = [:] @@ -28,6 +28,13 @@ private actor FakeRefreshStore: EmbeddingRefreshSource { saved[clipID] = vector stale.removeAll { $0 == clipID } } + func saveEmbeddingIfCurrent( + clipID: UUID, vector: [Float], expectedText: String + ) async throws -> Bool { + guard texts[clipID] == expectedText else { return false } + try await saveEmbedding(clipID: clipID, vector: vector) + return true + } } /// Deterministic embedder — no model assets, no Accelerate. diff --git a/Packages/GanchoKit/Tests/GanchoAppCoreTests/EmbeddingWriteRaceTests.swift b/Packages/GanchoKit/Tests/GanchoAppCoreTests/EmbeddingWriteRaceTests.swift new file mode 100644 index 00000000..0e8e5bf9 --- /dev/null +++ b/Packages/GanchoKit/Tests/GanchoAppCoreTests/EmbeddingWriteRaceTests.swift @@ -0,0 +1,116 @@ +import ClipboardCore +import Foundation +import GanchoAI +import GanchoKit +import Testing + +@testable import GanchoAppCore + +private struct RaceEmbedder: TextEmbedding { + let dimension = 2 + func vector(for text: String) throws -> [Float] { [1, 0] } +} + +/// Holds the exact await between computation and guarded persistence. No timing +/// assumptions or on-device model assets are involved in either caller test. +private actor EmbeddingRaceStore: ClipEnriching, EmbeddingRefreshSource, + ContentBoundEmbeddingStoring +{ + let id = UUID() + private var text = "original" + private var pendingWrite: CheckedContinuation? + private var writeWaiters: [CheckedContinuation] = [] + private(set) var saved = false + private(set) var unsafeWrites = 0 + private(set) var observedText: String? + + func content(for id: UUID) async throws -> ClipContent? { .text(text) } + func staleEmbeddingClipIDs(limit: Int) async throws -> [UUID] { saved ? [] : [id] } + func updateTitle(id: UUID, title: String) async throws {} + func updateTitleIfEmpty(id: UUID, title: String) async throws -> Bool { false } + func attachExtractedText(id: UUID, text: String) async throws {} + func updateClipText(id: UUID, text: String) async throws { self.text = text } + func saveEmbedding(clipID: UUID, vector: [Float]) async throws { + unsafeWrites += 1 + await pauseWrite() + saved = true + } + + func saveEmbeddingIfCurrent( + clipID: UUID, vector: [Float], expectedText: String + ) async throws -> Bool { + observedText = expectedText + await pauseWrite() + guard expectedText == text else { return false } + saved = true + return true + } + + private func pauseWrite() async { + await withCheckedContinuation { continuation in + pendingWrite = continuation + for waiter in writeWaiters { waiter.resume() } + writeWaiters.removeAll() + } + } + + func waitUntilWriteIsPending() async { + if pendingWrite != nil { return } + await withCheckedContinuation { writeWaiters.append($0) } + } + + func resumeWrite(withCurrentText text: String) { + self.text = text + let continuation = pendingWrite + pendingWrite = nil + continuation?.resume() + } +} + +@Suite("Background embedding write races") +struct EmbeddingWriteRaceTests { + @Test func captureEnrichmentRejectsAChangedBody() async { + let store = EmbeddingRaceStore() + let item = ClipItem(id: store.id, preview: "original") + let plan = EnrichmentPlan( + content: .text(hasTitle: true), isSensitive: false, isPro: true, + preferences: IntelligencePreferences()) + let service = EnrichmentService(makeEmbedder: { RaceEmbedder() }) + let task = Task { + await service.enrich( + item, content: .text("original"), plan: plan, writeTitle: false, store: store, + onTitleWritten: {}) + } + await store.waitUntilWriteIsPending() + await store.resumeWrite(withCurrentText: "edited") + await task.value + #expect(await store.observedText == "original") + #expect(await store.unsafeWrites == 0) + #expect(await store.saved == false) + } + + @Test func refreshRejectsChangedBodyWithoutReportingProgressOrSpinning() async { + let store = EmbeddingRaceStore() + let service = EmbeddingRefreshService( + makeEmbedder: { RaceEmbedder() }, isEnvironmentSuitable: { true }) + let task = Task { await service.run(store: store) } + await store.waitUntilWriteIsPending() + await store.resumeWrite(withCurrentText: "edited") + #expect(await task.value == 0) + #expect(await store.observedText == "original") + #expect(await store.unsafeWrites == 0) + #expect(await store.saved == false) + } + + @Test func refreshAcceptsUnchangedBodyAsPositiveControl() async { + let store = EmbeddingRaceStore() + let service = EmbeddingRefreshService( + makeEmbedder: { RaceEmbedder() }, isEnvironmentSuitable: { true }) + let task = Task { await service.run(store: store) } + await store.waitUntilWriteIsPending() + await store.resumeWrite(withCurrentText: "original") + #expect(await task.value == 1) + #expect(await store.saved) + #expect(await store.unsafeWrites == 0) + } +} diff --git a/Packages/GanchoKit/Tests/GanchoKitTests/ContentBoundEmbeddingTests.swift b/Packages/GanchoKit/Tests/GanchoKitTests/ContentBoundEmbeddingTests.swift new file mode 100644 index 00000000..252aaec9 --- /dev/null +++ b/Packages/GanchoKit/Tests/GanchoKitTests/ContentBoundEmbeddingTests.swift @@ -0,0 +1,95 @@ +import Foundation +import GRDB +import Testing + +@_spi(GanchoInternal) @testable import GanchoKit + +@Suite("Content-bound embedding persistence") +struct ContentBoundEmbeddingTests { + private func makeStore() throws -> GRDBClipboardStore { + let store = GRDBClipboardStore( + writer: try DatabaseQueue(), + blobs: BlobStore( + directory: FileManager.default.temporaryDirectory + .appendingPathComponent("embedding-body-\(UUID().uuidString)"))) + try store.migrate() + return store + } + + @Test func anEditCannotBeFollowedByAnOldVector() async throws { + let store = try makeStore() + let item = ClipItem(preview: "old", contentHash: "body-race") + try await store.insert(item, content: .text("old")) + try await store.updateClipText(id: item.id, text: "new") + + #expect( + !(try await store.saveEmbeddingIfCurrent( + clipID: item.id, vector: [1, 0], expectedText: "old"))) + #expect(try await store.semanticSearch(queryVector: [1, 0]).isEmpty) + #expect( + try await store.saveEmbeddingIfCurrent( + clipID: item.id, vector: [0, 1], expectedText: "new")) + #expect(try await store.semanticSearch(queryVector: [0, 1]).map(\.id) == [item.id]) + } + + @Test func titleAndContentEqualABAChangesKeepValidBodyEvidence() async throws { + let store = try makeStore() + let item = ClipItem(preview: "body", contentHash: "body-aba") + try await store.insert(item, content: .text("body")) + try await store.updateTitle(id: item.id, title: "New title") + try await store.updateClipText(id: item.id, text: "temporary") + try await store.updateClipText(id: item.id, text: "body") + #expect( + try await store.saveEmbeddingIfCurrent( + clipID: item.id, vector: [1, 0], expectedText: "body")) + } + + @Test func deletedAndRecreatedIdentityRequiresCurrentBodyEvidence() async throws { + let store = try makeStore() + let item = ClipItem(preview: "old", contentHash: "recreated-body") + try await store.insert(item, content: .text("old")) + try await store.delete(id: item.id) + #expect( + !(try await store.saveEmbeddingIfCurrent( + clipID: item.id, vector: [1, 0], expectedText: "old"))) + try await store.insert(item, content: .text("new")) + #expect( + !(try await store.saveEmbeddingIfCurrent( + clipID: item.id, vector: [1, 0], expectedText: "old"))) + #expect( + try await store.saveEmbeddingIfCurrent( + clipID: item.id, vector: [0, 1], expectedText: "new")) + try await store.delete(id: item.id) + try await store.insert(item, content: .text("old")) + #expect( + try await store.saveEmbeddingIfCurrent( + clipID: item.id, vector: [1, 0], expectedText: "old")) + } + + @Test func changedPrivacyAndExpiryRejectEvenAnIdenticalBody() async throws { + let store = try makeStore() + let item = ClipItem(preview: "body", contentHash: "protected-body") + try await store.insert(item, content: .text("body")) + let mutations = [ + "isSensitive = 1", "kind = 'jwt'", "isArchived = 1", + "expiresAt = '2000-01-01 00:00:00.000'", "contentTypeIdentifier = 'public.file-url'", + ] + for mutation in mutations { + try await store.writer.write { db in + try db.execute( + sql: """ + UPDATE clip SET isSensitive = 0, kind = 'text', isArchived = 0, + expiresAt = NULL, contentTypeIdentifier = NULL WHERE id = ? + """, + arguments: [item.id.uuidString]) + try db.execute( + sql: "UPDATE clip SET \(mutation) WHERE id = ?", + arguments: [item.id.uuidString]) + } + #expect( + !(try await store.saveEmbeddingIfCurrent( + clipID: item.id, vector: [1, 0], expectedText: "body")), + "Accepted changed eligibility: \(mutation)") + } + } +} diff --git a/docs/CONTENT_BOUND_EMBEDDINGS.md b/docs/CONTENT_BOUND_EMBEDDINGS.md new file mode 100644 index 00000000..24cfe3df --- /dev/null +++ b/docs/CONTENT_BOUND_EMBEDDINGS.md @@ -0,0 +1,44 @@ +# Content-bound background embeddings + +Capture enrichment and model-version refresh compute a vector from text read before +an asynchronous persistence call. A text edit can occur between those operations. +Deleting the old vector during the edit is insufficient: the older producer must +not restore it afterward. + +`ContentBoundEmbeddingStoring` is the optional guarded-write capability. Gancho's +GRDB store implements it with one `INSERT ... SELECT` statement in a write +transaction. It compares the complete source text using SQLite's exact text +comparison and requires a live, visible, non-sensitive text-backed row. A changed, +deleted, expired, archived, masked or binary row causes a no-op, reported as false. +No unguarded read/write gap, model execution or asynchronous wait occurs inside the +transaction. The source body is never logged or persisted as extra metadata. + +Both background producers use this capability. Refresh counts only accepted writes +as progress and stops after a zero-progress batch. A future pass can retry eligible +work; this correction does not introduce a new reindexing scheduler. + +## Compatibility and identity + +The existing `ClipEnriching` and `EmbeddingRefreshSource` requirements and their +unconditional write methods remain available. Existing conformers still compile. +Background producers skip embedding work for a store that lacks the guarded +capability, rather than fall back to an unsafe write. Production uses the conforming +GRDB store; title and OCR behavior is unchanged. + +Vectors depend on body text and the current model version, not on title, keyword or +revision timestamps. Title-only changes are therefore allowed. Content-equal ABA +changes, including a deleted/recreated identity with the same eligible body, can +accept the vector because it describes the current body. Reuse of the same identity +with different text or changed privacy eligibility is rejected. + +The input hash is unsuitable for this comparison: curation deliberately preserves +that hash so copying the original still deduplicates. Comparing only the first +1,000 characters would also discard evidence the producer had, so the guarded write +compares the full body even though current model computation truncates its input. + +## Verification + +Store tests cover changed bodies, deletion and identity reuse, title/content-equal +changes, and altered eligibility. Caller tests suspend at the write boundary with +continuations, change the body, then resume; they use no sleeps or model downloads. +An unchanged-body control proves a successful refresh can still make progress. From 919acb3689997370e3b693b36e12844ea8792ec0 Mon Sep 17 00:00:00 2001 From: Johnny IV Young Ospino Date: Wed, 7 Oct 2026 16:36:40 -0500 Subject: [PATCH 2/3] style(tests): remove rejected trailing comma --- .../Tests/GanchoKitTests/ContentBoundEmbeddingTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Packages/GanchoKit/Tests/GanchoKitTests/ContentBoundEmbeddingTests.swift b/Packages/GanchoKit/Tests/GanchoKitTests/ContentBoundEmbeddingTests.swift index 252aaec9..e594ec4d 100644 --- a/Packages/GanchoKit/Tests/GanchoKitTests/ContentBoundEmbeddingTests.swift +++ b/Packages/GanchoKit/Tests/GanchoKitTests/ContentBoundEmbeddingTests.swift @@ -72,7 +72,7 @@ struct ContentBoundEmbeddingTests { try await store.insert(item, content: .text("body")) let mutations = [ "isSensitive = 1", "kind = 'jwt'", "isArchived = 1", - "expiresAt = '2000-01-01 00:00:00.000'", "contentTypeIdentifier = 'public.file-url'", + "expiresAt = '2000-01-01 00:00:00.000'", "contentTypeIdentifier = 'public.file-url'" ] for mutation in mutations { try await store.writer.write { db in From 8e10d543cec6ac1d8584eef59795e8d839f6f6d3 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 04:44:38 +0000 Subject: [PATCH 3/3] fix(embeddings): match the read visibility rule and keep refresh moving - Accept a guarded vector write for any row the store still shows, using the shared unexpired predicate, so an expired pinned, boarded or snippet row stays indexable as it was before - Share the ineligible-kind list between the guarded write and the stale refresh queue, so rows the write always rejects cannot fill a batch and stop the refresh for every row behind them - Bound the write-race tests with a time limit so a regression fails instead of hanging, and cover both rules --- .../ContentBoundEmbeddingStore.swift | 28 +++++++++++++------ .../Sources/GanchoKit/SemanticSearch.swift | 14 ++++++++-- .../EmbeddingWriteRaceTests.swift | 2 +- .../ContentBoundEmbeddingTests.swift | 19 +++++++++++++ .../DatabaseMigrationTests.swift | 10 +++++-- .../GanchoKitTests/SemanticSearchTests.swift | 14 ++++++++++ docs/CONTENT_BOUND_EMBEDDINGS.md | 10 +++++-- 7 files changed, 79 insertions(+), 18 deletions(-) diff --git a/Packages/GanchoKit/Sources/GanchoKit/ContentBoundEmbeddingStore.swift b/Packages/GanchoKit/Sources/GanchoKit/ContentBoundEmbeddingStore.swift index bbad00a0..e7bd2a20 100644 --- a/Packages/GanchoKit/Sources/GanchoKit/ContentBoundEmbeddingStore.swift +++ b/Packages/GanchoKit/Sources/GanchoKit/ContentBoundEmbeddingStore.swift @@ -14,15 +14,22 @@ public protocol ContentBoundEmbeddingStoring: Sendable { ) async throws -> Bool } +extension GRDBClipboardStore { + /// Kinds background embedding never indexes: every masked-preview kind and the + /// payloads without a free-text body. Shared by the guarded write and the stale + /// refresh queue, so the queue never offers a row the write must reject. + static let embeddingIneligibleKinds: [String] = ClipContentKind.allCases.filter { + $0.prefersMaskedPreview || $0 == .image || $0 == .fileReference + }.map(\.rawValue) +} + extension GRDBClipboardStore: ContentBoundEmbeddingStoring { public func saveEmbeddingIfCurrent( clipID: UUID, vector: [Float], expectedText: String ) async throws -> Bool { try Task.checkCancellation() let data = vector.withUnsafeBufferPointer { Data(buffer: $0) } - let excludedKinds = ClipContentKind.allCases.filter { - $0.prefersMaskedPreview || $0 == .image || $0 == .fileReference - }.map(\.rawValue) + let excludedKinds = Self.embeddingIneligibleKinds let placeholders = excludedKinds.map { _ in "?" }.joined(separator: ", ") return try await writer.write { db in try Task.checkCancellation() @@ -31,15 +38,18 @@ extension GRDBClipboardStore: ContentBoundEmbeddingStoring { expectedText, Date.now, "public.file-url" ] arguments.append(contentsOf: excludedKinds) + // Visibility uses the shared read predicate: an expired row that + // retention keeps (pinned, boarded, or a snippet) stays searchable, + // so its current body must stay indexable too. try db.execute( sql: """ INSERT OR REPLACE INTO clip_embedding (clipID, dimension, vector, modelVersion) - SELECT id, ?, ?, ? FROM clip - WHERE id = ? AND contentText = ? AND contentBlobHash IS NULL - AND isSensitive = 0 AND isArchived = 0 - AND (expiresAt IS NULL OR expiresAt > ?) - AND (contentTypeIdentifier IS NULL OR contentTypeIdentifier != ?) - AND kind NOT IN (\(placeholders)) + SELECT clip.id, ?, ?, ? FROM clip + WHERE clip.id = ? AND clip.contentText = ? AND clip.contentBlobHash IS NULL + AND clip.isSensitive = 0 AND clip.isArchived = 0 + AND \(Self.unexpiredPredicate) + AND (clip.contentTypeIdentifier IS NULL OR clip.contentTypeIdentifier != ?) + AND clip.kind NOT IN (\(placeholders)) """, arguments: StatementArguments(arguments)) return db.changesCount == 1 diff --git a/Packages/GanchoKit/Sources/GanchoKit/SemanticSearch.swift b/Packages/GanchoKit/Sources/GanchoKit/SemanticSearch.swift index 15028799..f69afdad 100644 --- a/Packages/GanchoKit/Sources/GanchoKit/SemanticSearch.swift +++ b/Packages/GanchoKit/Sources/GanchoKit/SemanticSearch.swift @@ -38,21 +38,29 @@ extension GRDBClipboardStore { /// spend battery on rows no query can return. The sensitive exclusion is /// belt-and-suspenders — capture never embeds sensitive clips, but this /// query must not feed one to the re-embed loop even if some future path - /// flips `isSensitive` after a vector exists. + /// flips `isSensitive` after a vector exists. Kinds the guarded write + /// always rejects are excluded too: a permanently rejected row would stay + /// at the head of every batch and stall the pass for the rows behind it. public func staleEmbeddingClipIDs(limit: Int) async throws -> [UUID] { // SQLite treats a negative LIMIT as "no limit" — the exact unbounded // read this API exists to prevent. guard limit > 0 else { return [] } + let excludedKinds = Self.embeddingIneligibleKinds + let placeholders = excludedKinds.map { _ in "?" }.joined(separator: ", ") return try await writer.read { db in - try String.fetchAll( + var arguments: [any DatabaseValueConvertible] = [EmbeddingModelInfo.currentVersion] + arguments.append(contentsOf: excludedKinds) + arguments.append(limit) + return try String.fetchAll( db, sql: """ SELECT e.clipID FROM clip_embedding e JOIN clip c ON c.id = e.clipID WHERE e.modelVersion < ? AND c.isArchived = 0 AND c.isSensitive = 0 + AND c.kind NOT IN (\(placeholders)) LIMIT ? """, - arguments: [EmbeddingModelInfo.currentVersion, limit] + arguments: StatementArguments(arguments) ).compactMap(UUID.init(uuidString:)) } } diff --git a/Packages/GanchoKit/Tests/GanchoAppCoreTests/EmbeddingWriteRaceTests.swift b/Packages/GanchoKit/Tests/GanchoAppCoreTests/EmbeddingWriteRaceTests.swift index 0e8e5bf9..5a093b71 100644 --- a/Packages/GanchoKit/Tests/GanchoAppCoreTests/EmbeddingWriteRaceTests.swift +++ b/Packages/GanchoKit/Tests/GanchoAppCoreTests/EmbeddingWriteRaceTests.swift @@ -67,7 +67,7 @@ private actor EmbeddingRaceStore: ClipEnriching, EmbeddingRefreshSource, } } -@Suite("Background embedding write races") +@Suite("Background embedding write races", .timeLimit(.minutes(1))) struct EmbeddingWriteRaceTests { @Test func captureEnrichmentRejectsAChangedBody() async { let store = EmbeddingRaceStore() diff --git a/Packages/GanchoKit/Tests/GanchoKitTests/ContentBoundEmbeddingTests.swift b/Packages/GanchoKit/Tests/GanchoKitTests/ContentBoundEmbeddingTests.swift index e594ec4d..8a635c32 100644 --- a/Packages/GanchoKit/Tests/GanchoKitTests/ContentBoundEmbeddingTests.swift +++ b/Packages/GanchoKit/Tests/GanchoKitTests/ContentBoundEmbeddingTests.swift @@ -66,6 +66,25 @@ struct ContentBoundEmbeddingTests { clipID: item.id, vector: [1, 0], expectedText: "old")) } + @Test func expiredRowsThatRetentionKeepsStayIndexable() async throws { + // Reads keep a pinned, boarded, or snippet row visible past its expiry, + // so the guarded write must accept that row's current body as main did. + let store = try makeStore() + let item = ClipItem(preview: "body", contentHash: "curated-expired") + try await store.insert(item, content: .text("body")) + try await store.writer.write { db in + try db.execute( + sql: """ + UPDATE clip SET isPinned = 1, expiresAt = '2000-01-01 00:00:00.000' + WHERE id = ? + """, + arguments: [item.id.uuidString]) + } + #expect( + try await store.saveEmbeddingIfCurrent( + clipID: item.id, vector: [1, 0], expectedText: "body")) + } + @Test func changedPrivacyAndExpiryRejectEvenAnIdenticalBody() async throws { let store = try makeStore() let item = ClipItem(preview: "body", contentHash: "protected-body") diff --git a/Packages/GanchoKit/Tests/GanchoKitTests/DatabaseMigrationTests.swift b/Packages/GanchoKit/Tests/GanchoKitTests/DatabaseMigrationTests.swift index 2d306a40..1ff3426a 100644 --- a/Packages/GanchoKit/Tests/GanchoKitTests/DatabaseMigrationTests.swift +++ b/Packages/GanchoKit/Tests/GanchoKitTests/DatabaseMigrationTests.swift @@ -160,17 +160,23 @@ struct DatabaseMigrationTests { // rows the planner takes any index, and `< 99` matches every row, which // on real data makes it prefer a scan. let store = try await makeAnalyzedStore() + let excludedKinds = GRDBClipboardStore.embeddingIneligibleKinds + let placeholders = excludedKinds.map { _ in "?" }.joined(separator: ", ") let plan = try await store.writer.read { db in - try String.fetchAll( + var arguments: [any DatabaseValueConvertible] = [EmbeddingModelInfo.currentVersion] + arguments.append(contentsOf: excludedKinds) + arguments.append(16) + return try String.fetchAll( db, sql: """ EXPLAIN QUERY PLAN SELECT e.clipID FROM clip_embedding e JOIN clip c ON c.id = e.clipID WHERE e.modelVersion < ? AND c.isArchived = 0 AND c.isSensitive = 0 + AND c.kind NOT IN (\(placeholders)) LIMIT ? """, - arguments: [EmbeddingModelInfo.currentVersion, 16], + arguments: StatementArguments(arguments), adapter: ColumnMapping(["detail": "detail"])) } diff --git a/Packages/GanchoKit/Tests/GanchoKitTests/SemanticSearchTests.swift b/Packages/GanchoKit/Tests/GanchoKitTests/SemanticSearchTests.swift index 41fd843a..4cb5de8a 100644 --- a/Packages/GanchoKit/Tests/GanchoKitTests/SemanticSearchTests.swift +++ b/Packages/GanchoKit/Tests/GanchoKitTests/SemanticSearchTests.swift @@ -164,4 +164,18 @@ struct SemanticSearchTests { } #expect(try await store.staleEmbeddingClipIDs(limit: 10).isEmpty) } + + @Test("Kinds the guarded write rejects never stall the stale-refresh queue") + func staleSkipsIneligibleKinds() async throws { + // A non-sensitive JWT can carry a vector from an earlier capture path. + // Left in the queue, it would be rejected forever and hold its batch slot. + let store = try makeStore() + let item = ClipItem(kind: .jwt, preview: "x", contentHash: "h") + try await store.insert(item, content: .text("x")) + try await store.saveEmbedding(clipID: item.id, vector: [1, 0]) + try await store.writer.write { db in + try db.execute(sql: "UPDATE clip_embedding SET modelVersion = modelVersion - 1") + } + #expect(try await store.staleEmbeddingClipIDs(limit: 10).isEmpty) + } } diff --git a/docs/CONTENT_BOUND_EMBEDDINGS.md b/docs/CONTENT_BOUND_EMBEDDINGS.md index 24cfe3df..75d0949c 100644 --- a/docs/CONTENT_BOUND_EMBEDDINGS.md +++ b/docs/CONTENT_BOUND_EMBEDDINGS.md @@ -8,13 +8,17 @@ not restore it afterward. `ContentBoundEmbeddingStoring` is the optional guarded-write capability. Gancho's GRDB store implements it with one `INSERT ... SELECT` statement in a write transaction. It compares the complete source text using SQLite's exact text -comparison and requires a live, visible, non-sensitive text-backed row. A changed, -deleted, expired, archived, masked or binary row causes a no-op, reported as false. +comparison and requires a live, visible, non-sensitive text-backed row. Visibility +uses the same expiry predicate as reads, so a pinned, boarded or snippet row that +retention keeps past its expiry stays indexable. A changed, deleted, hidden expired, +archived, masked or binary row causes a no-op, reported as false. No unguarded read/write gap, model execution or asynchronous wait occurs inside the transaction. The source body is never logged or persisted as extra metadata. Both background producers use this capability. Refresh counts only accepted writes -as progress and stops after a zero-progress batch. A future pass can retry eligible +as progress and stops after a zero-progress batch. The stale-vector queue excludes +the kinds the guarded write always rejects, so such a row cannot occupy a batch +forever and stall the rows behind it. A future pass can retry eligible work; this correction does not introduce a new reindexing scheduler. ## Compatibility and identity