diff --git a/Packages/GanchoKit/Sources/GanchoAppCore/EnrichmentService.swift b/Packages/GanchoKit/Sources/GanchoAppCore/EnrichmentService.swift index 89e5147d..c56e2306 100644 --- a/Packages/GanchoKit/Sources/GanchoAppCore/EnrichmentService.swift +++ b/Packages/GanchoKit/Sources/GanchoAppCore/EnrichmentService.swift @@ -24,9 +24,11 @@ import GanchoKit /// 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. +/// without that optional capability skip vectors. Generated titles similarly require +/// ContentBoundTitleStoring; unsupported stores retain OCR work without unsafe writes. public struct EnrichmentService: Sendable { private let makeEmbedder: @Sendable () -> (any TextEmbedding)? + private let annotator: any ClipAnnotating public init() { self.init(makeEmbedder: { @@ -37,8 +39,12 @@ public struct EnrichmentService: Sendable { }) } - init(makeEmbedder: @escaping @Sendable () -> (any TextEmbedding)?) { + init( + makeEmbedder: @escaping @Sendable () -> (any TextEmbedding)?, + annotator: any ClipAnnotating = TieredClipAnnotator() + ) { self.makeEmbedder = makeEmbedder + self.annotator = annotator } /// Runs the enrichment IO shared by both shells, in the SAME order as the @@ -68,10 +74,12 @@ public struct EnrichmentService: Sendable { } // Tier 1 — Apple Intelligence titles. if writeTitle, case .text(let text)? = content, - let annotation = try? await TieredClipAnnotator().annotate(text) + let titleStore = store as? any ContentBoundTitleStoring, + let annotation = try? await annotator.annotate(text) { let wroteTitle = - (try? await store.updateTitleIfEmpty(id: item.id, title: annotation.title)) == true + (try? await titleStore.updateTitleIfEmptyAndCurrent( + id: item.id, title: annotation.title, expectedText: text)) == true if wroteTitle { await onTitleWritten() } } // Semantic vector (the embedder caches its model after the first call). diff --git a/Packages/GanchoKit/Sources/GanchoKit/ContentBoundTitleStore.swift b/Packages/GanchoKit/Sources/GanchoKit/ContentBoundTitleStore.swift new file mode 100644 index 00000000..efb6b5f8 --- /dev/null +++ b/Packages/GanchoKit/Sources/GanchoKit/ContentBoundTitleStore.swift @@ -0,0 +1,54 @@ +import Foundation +import GRDB + +/// Optional guarded capability for generated titles derived from an earlier body. +/// Retains the legacy ClipEnriching requirements for existing conformers. +public protocol ContentBoundTitleStoring: Sendable { + @discardableResult + func updateTitleIfEmptyAndCurrent( + id: UUID, title: String, expectedText: String + ) async throws -> Bool +} + +extension GRDBClipboardStore { + /// Kinds without a free-text body for a generated title to describe. Masked + /// kinds stay titleable, as capture plans titles for every non-sensitive text + /// clip: the model tier sees sanitized input and the heuristic tier emits + /// fixed titles for them. + static let titleIneligibleKinds: [String] = [ + ClipContentKind.image, .fileReference + ].map(\.rawValue) +} + +extension GRDBClipboardStore: ContentBoundTitleStoring { + public func updateTitleIfEmptyAndCurrent( + id: UUID, title: String, expectedText: String + ) async throws -> Bool { + try Task.checkCancellation() + let excludedKinds = Self.titleIneligibleKinds + let placeholders = excludedKinds.map { _ in "?" }.joined(separator: ", ") + return try await writer.write { db in + try Task.checkCancellation() + let now = Date.now + var arguments: [any DatabaseValueConvertible] = [ + title, now, id.uuidString, expectedText, 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 visible, so + // its current body stays titleable too. + try db.execute( + sql: """ + UPDATE clip SET title = ?, updatedAt = ?, needsUpload = 1 + WHERE clip.id = ? AND clip.title = '' 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/Tests/GanchoAppCoreTests/ClipIngestionCoordinatorTests.swift b/Packages/GanchoKit/Tests/GanchoAppCoreTests/ClipIngestionCoordinatorTests.swift index 02228239..c0bce383 100644 --- a/Packages/GanchoKit/Tests/GanchoAppCoreTests/ClipIngestionCoordinatorTests.swift +++ b/Packages/GanchoKit/Tests/GanchoAppCoreTests/ClipIngestionCoordinatorTests.swift @@ -9,7 +9,7 @@ private enum IngestionStoreError: Error { case unavailable } -private actor IngestionStoreSpy: ClipIngesting, ClipEnriching { +private actor IngestionStoreSpy: ContentBoundTitleStoring, ClipIngesting, ClipEnriching { var returnedItem: ClipItem? var failsInsert = false private(set) var insertedItem: ClipItem? @@ -30,6 +30,11 @@ private actor IngestionStoreSpy: ClipIngesting, ClipEnriching { titleWrites += 1 return true } + func updateTitleIfEmptyAndCurrent( + id: UUID, title: String, expectedText: String + ) async throws -> Bool { + try await updateTitleIfEmpty(id: id, title: title) + } func attachExtractedText(id: UUID, text: String) async throws { extractedTextWrites += 1 } diff --git a/Packages/GanchoKit/Tests/GanchoAppCoreTests/EnrichmentServiceTests.swift b/Packages/GanchoKit/Tests/GanchoAppCoreTests/EnrichmentServiceTests.swift index cc5bfa22..b8970f02 100644 --- a/Packages/GanchoKit/Tests/GanchoAppCoreTests/EnrichmentServiceTests.swift +++ b/Packages/GanchoKit/Tests/GanchoAppCoreTests/EnrichmentServiceTests.swift @@ -12,7 +12,7 @@ import Testing /// degrades to the deterministic `HeuristicAnnotator` (so the title stage still /// succeeds), while the embedder reports no on-device assets (so the embedding /// stage writes nothing); the assertions below are chosen to hold either way. -private actor FakeEnriching: ClipEnriching { +private actor FakeEnriching: ContentBoundTitleStoring, ClipEnriching { private(set) var updateTitleCalls = 0 var allowsGeneratedTitleWrite = true private(set) var attachExtractedTextCalls = 0 @@ -26,6 +26,11 @@ private actor FakeEnriching: ClipEnriching { updateTitleCalls += 1 return allowsGeneratedTitleWrite } + func updateTitleIfEmptyAndCurrent( + id: UUID, title: String, expectedText: String + ) async throws -> Bool { + try await updateTitleIfEmpty(id: id, title: title) + } func attachExtractedText(id: UUID, text: String) async throws { attachExtractedTextCalls += 1 } func updateClipText(id: UUID, text: String) async throws { updateClipTextCalls += 1 } func saveEmbedding(clipID: UUID, vector: [Float]) async throws { saveEmbeddingCalls += 1 } diff --git a/Packages/GanchoKit/Tests/GanchoAppCoreTests/TitleEnrichmentRaceTests.swift b/Packages/GanchoKit/Tests/GanchoAppCoreTests/TitleEnrichmentRaceTests.swift new file mode 100644 index 00000000..87aad50c --- /dev/null +++ b/Packages/GanchoKit/Tests/GanchoAppCoreTests/TitleEnrichmentRaceTests.swift @@ -0,0 +1,88 @@ +import ClipboardCore +import Foundation +import GanchoAI +import GanchoKit +import Testing + +@testable import GanchoAppCore + +private actor PausingTitleAnnotator: ClipAnnotating { + private var pending: CheckedContinuation? + private var waiters: [CheckedContinuation] = [] + private(set) var source: String? + + func annotate(_ text: String) async throws -> ClipAnnotation { + source = text + await withCheckedContinuation { continuation in + pending = continuation + for waiter in waiters { waiter.resume() } + waiters.removeAll() + } + return ClipAnnotation(title: "Original topic", kind: .text) + } + + func waitUntilAnnotationIsPending() async { + if pending != nil { return } + await withCheckedContinuation { waiters.append($0) } + } + + func finish() { + let continuation = pending + pending = nil + continuation?.resume() + } +} + +private actor TitleRaceStore: ClipEnriching, ContentBoundTitleStoring { + private var text = "original" + private(set) var title = "" + private(set) var callbacks = 0 + private(set) var unsafeWrites = 0 + + func updateTitle(id: UUID, title: String) async throws { self.title = title } + func updateTitleIfEmpty(id: UUID, title: String) async throws -> Bool { + unsafeWrites += 1 + guard self.title.isEmpty else { return false } + self.title = title + return true + } + func updateTitleIfEmptyAndCurrent( + id: UUID, title: String, expectedText: String + ) async throws -> Bool { + guard self.title.isEmpty, text == expectedText else { return false } + self.title = title + return true + } + 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 {} + func noteCallback() { callbacks += 1 } +} + +@Suite("Title enrichment body races") +struct TitleEnrichmentRaceTests { + @Test(arguments: [false, true]) + func titleCommitsOnlyForTheStillCurrentBody(changeBody: Bool) async throws { + let store = TitleRaceStore() + let annotator = PausingTitleAnnotator() + let item = ClipItem(preview: "original") + let service = EnrichmentService(makeEmbedder: { nil }, annotator: annotator) + let plan = EnrichmentPlan( + content: .text(hasTitle: false), isSensitive: false, isPro: true, + preferences: IntelligencePreferences(semanticSearch: false)) + let task = Task { + await service.enrich( + item, content: .text("original"), plan: plan, writeTitle: true, store: store, + onTitleWritten: { await store.noteCallback() }) + } + await annotator.waitUntilAnnotationIsPending() + if changeBody { try await store.updateClipText(id: item.id, text: "edited") } + await annotator.finish() + await task.value + + #expect(await annotator.source == "original") + #expect(await store.title == (changeBody ? "" : "Original topic")) + #expect(await store.callbacks == (changeBody ? 0 : 1)) + #expect(await store.unsafeWrites == 0) + } +} diff --git a/Packages/GanchoKit/Tests/GanchoKitTests/ContentBoundTitleTests.swift b/Packages/GanchoKit/Tests/GanchoKitTests/ContentBoundTitleTests.swift new file mode 100644 index 00000000..35a45b60 --- /dev/null +++ b/Packages/GanchoKit/Tests/GanchoKitTests/ContentBoundTitleTests.swift @@ -0,0 +1,112 @@ +import Foundation +import GRDB +import Testing + +@_spi(GanchoInternal) @testable import GanchoKit + +@Suite("Content-bound generated titles") +struct ContentBoundTitleTests { + private func makeStore() throws -> GRDBClipboardStore { + let store = GRDBClipboardStore( + writer: try DatabaseQueue(), + blobs: BlobStore( + directory: FileManager.default.temporaryDirectory + .appendingPathComponent("title-body-\(UUID().uuidString)"))) + try store.migrate() + return store + } + + @Test func changedBodyRejectsTitleWithoutQueueingAnotherUpload() async throws { + let store = try makeStore() + let item = ClipItem(preview: "old", contentHash: "title-race") + try await store.insert(item, content: .text("old")) + try await store.updateClipText(id: item.id, text: "edited") + try await store.markUploaded(id: item.id, systemFields: Data([1])) + let before = try #require(try await store.item(id: item.id)) + + #expect( + !(try await store.updateTitleIfEmptyAndCurrent( + id: item.id, title: "Old topic", expectedText: "old"))) + #expect(try await store.item(id: item.id) == before) + #expect(try await store.pendingUploadIDs().isEmpty) + #expect( + try await store.updateTitleIfEmptyAndCurrent( + id: item.id, title: "Current topic", expectedText: "edited")) + #expect(try await store.item(id: item.id)?.title == "Current topic") + #expect(try await store.pendingUploadIDs() == [item.id]) + } + + @Test func manualTitleStillWinsEvenWithAnIdenticalBody() async throws { + let store = try makeStore() + let item = ClipItem(title: "Manual", preview: "body", contentHash: "manual-race") + try await store.insert(item, content: .text("body")) + #expect( + !(try await store.updateTitleIfEmptyAndCurrent( + id: item.id, title: "Generated", expectedText: "body"))) + #expect(try await store.item(id: item.id)?.title == "Manual") + } + + @Test func missingAndRecreatedRowsRequireMatchingCurrentBody() async throws { + let store = try makeStore() + let item = ClipItem(preview: "old", contentHash: "recreated-title") + #expect( + !(try await store.updateTitleIfEmptyAndCurrent( + id: item.id, title: "Old", expectedText: "old"))) + try await store.insert(item, content: .text("new")) + #expect( + !(try await store.updateTitleIfEmptyAndCurrent( + id: item.id, title: "Old", expectedText: "old"))) + try await store.delete(id: item.id) + try await store.insert(item, content: .text("old")) + #expect( + try await store.updateTitleIfEmptyAndCurrent( + id: item.id, title: "Same topic", expectedText: "old")) + } + + @Test(arguments: [ + "kind = 'jwt'", "kind = 'creditCard'", "kind = 'future-kind'", + "isPinned = 1, expiresAt = '2000-01-01 00:00:00.000'" + ]) + func bodyBackedRowsStayTitleable(mutation: String) async throws { + // Capture plans titles for every non-sensitive text clip, masked kinds + // included, and reads keep a curated expired row visible. + let store = try makeStore() + let item = ClipItem(preview: "body", contentHash: "title-kept-\(mutation)") + try await store.insert(item, content: .text("body")) + try await store.writer.write { db in + try db.execute( + sql: "UPDATE clip SET \(mutation) WHERE id = ?", + arguments: [item.id.uuidString]) + } + #expect( + try await store.updateTitleIfEmptyAndCurrent( + id: item.id, title: "Generated", expectedText: "body")) + } + + @Test func changedEligibilityRejectsAnIdenticalBody() async throws { + let store = try makeStore() + let item = ClipItem(preview: "body", contentHash: "title-eligibility") + try await store.insert(item, content: .text("body")) + let mutations = [ + "isSensitive = 1", "kind = 'image'", "kind = 'fileReference'", "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.updateTitleIfEmptyAndCurrent( + id: item.id, title: "Generated", expectedText: "body")), + "Accepted changed eligibility: \(mutation)") + } + } +} diff --git a/docs/CONTENT_BOUND_TITLES.md b/docs/CONTENT_BOUND_TITLES.md new file mode 100644 index 00000000..36c5248c --- /dev/null +++ b/docs/CONTENT_BOUND_TITLES.md @@ -0,0 +1,38 @@ +# Content-bound generated titles + +An annotation is computed from the captured body. While the annotator awaits its +model, the user may replace that body and leave the title empty. Checking only +`title = ''` prevents overwriting a manual title but still lets the old annotation +label the new body. + +Capture enrichment now requires the optional `ContentBoundTitleStoring` capability +for generated titles. Its GRDB implementation uses one guarded UPDATE: the body +must exactly match the source supplied to the annotator, the title must still be +empty, and the row must remain a visible, unarchived, non-sensitive text-backed +item. Visibility reuses the store's shared read predicate, so an expired row that +retention keeps (pinned, boarded, or a snippet) stays titleable. Only kinds +without a free-text body (images, file references) are excluded. Masked kinds +stay titleable as before: the model tier sees sanitized input and the heuristic +tier emits fixed titles for them. A rejected guarded update changes neither the +revision nor the upload flag. The coordinator's existing follow-up sync enqueue +policy is unchanged. The title-written callback fires only after an accepted +write, so a stale result does not consume the free title taste or announce a +title update. + +The existing `ClipEnriching` requirements and `updateTitleIfEmpty` implementation +remain available for source compatibility. A custom store without the guarded +capability skips generated titles rather than using an unsafe fallback. Production +GRDB supports the capability. OCR, model choice, prompt, input budget, manual-title +editing and enrichment scheduling are unchanged. + +The full body is compared, not the capture hash: curation deliberately retains that +hash for deduplication. Content-equal ABA changes are acceptable when the current +row remains eligible and untitled; the annotation describes that same current +body. Recreated IDs with different text are rejected. A manual title always wins, +even if the body never changed. + +Tests pause an injected annotator with continuations, edit the captured body before +releasing the annotation, then assert both title and callback behavior. An unchanged +body is the positive control. Persistence tests independently verify the database +predicate, upload bookkeeping, manual-title precedence and eligibility changes. +No sleep, model asset download or wall-clock race threshold is required.