From ec0feff2e03756074f5c093930004eede2b6c2e1 Mon Sep 17 00:00:00 2001 From: Gordon Beeming Date: Wed, 10 Jun 2026 08:17:52 +1000 Subject: [PATCH 1/2] Paginate the grid and split click from double-click The panel capped results at 400 rows, so on a 6000-shot index the grid only reached back about two weeks. It now pages in older screenshots as you scroll, using a (captured_at, id) keyset cursor so the live indexer adding new shots mid-scroll can't dupe or skip rows. Single click now selects a thumbnail instead of copying; double click copies and closes, matching Enter. Also adds the bug and new-feature issue forms, issue config, and FUNDING.yml carried over from the ide repo. Co-authored-by: Claude Co-authored-by: GitButler --- .github/FUNDING.yml | 3 + .github/ISSUE_TEMPLATE/bug.yml | 118 +++++++++++ .github/ISSUE_TEMPLATE/config.yml | 1 + .github/ISSUE_TEMPLATE/new-feature.yml | 189 ++++++++++++++++++ Sources/Vista/PanelContentView.swift | 19 +- Sources/Vista/SearchViewModel.swift | 42 +++- Sources/VistaCore/ScreenshotStore.swift | 48 ++++- .../VistaCoreTests/ScreenshotStoreTests.swift | 59 ++++++ 8 files changed, 471 insertions(+), 8 deletions(-) create mode 100644 .github/FUNDING.yml create mode 100644 .github/ISSUE_TEMPLATE/bug.yml create mode 100644 .github/ISSUE_TEMPLATE/config.yml create mode 100644 .github/ISSUE_TEMPLATE/new-feature.yml diff --git a/.github/FUNDING.yml b/.github/FUNDING.yml new file mode 100644 index 0000000..37d8be3 --- /dev/null +++ b/.github/FUNDING.yml @@ -0,0 +1,3 @@ +github: [GordonBeeming] +patreon: GordonBeeming +buy_me_a_coffee: gordonbeeming diff --git a/.github/ISSUE_TEMPLATE/bug.yml b/.github/ISSUE_TEMPLATE/bug.yml new file mode 100644 index 0000000..bf091c6 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/bug.yml @@ -0,0 +1,118 @@ +name: Bug report +description: Report something that is not working as expected. +title: "[Bug]: " +labels: + - bug +body: + - type: markdown + attributes: + value: | + Use this form for defects, regressions, and unexpected behavior. + + If you are proposing new end-user value rather than reporting broken behavior, use the New feature form instead. + - type: textarea + id: summary + attributes: + label: Summary + description: What is broken? + placeholder: ... + validations: + required: true + - type: textarea + id: current-behavior + attributes: + label: Current behavior + description: What happens now? + placeholder: ... + validations: + required: true + - type: textarea + id: expected-behavior + attributes: + label: Expected behavior + description: What should happen instead? + placeholder: ... + validations: + required: true + - type: textarea + id: steps-to-reproduce + attributes: + label: Steps to reproduce + description: Give the smallest reliable reproduction steps. + placeholder: | + 1. + 2. + 3. + validations: + required: true + - type: dropdown + id: reproducibility + attributes: + label: Reproducibility + options: + - Always + - Often + - Sometimes + - Once + - Unknown + validations: + required: true + - type: dropdown + id: impact + attributes: + label: Impact + description: How much does this affect users? + options: + - Blocks core workflow + - Major workflow degradation + - Minor workflow degradation + - Cosmetic or polish issue + - Unknown + validations: + required: true + - type: textarea + id: environment + attributes: + label: Environment + description: App version, OS, architecture, browser/webview details, workspace shape, or other context. + placeholder: | + - App version: + - OS: + - Architecture: + - Workspace/project: + validations: + required: false + - type: textarea + id: affected-area + attributes: + label: Affected area + description: Product area, workflow, command, screen, integration, or subsystem involved. + placeholder: ... + validations: + required: false + - type: textarea + id: evidence + attributes: + label: Evidence + description: Screenshots, logs, stack traces, diagnostics, or links that support the report. + placeholder: Paste relevant evidence here. + validations: + required: false + - type: textarea + id: workaround + attributes: + label: Workaround + description: Is there a known workaround? + placeholder: ... + validations: + required: false + - type: textarea + id: validation + attributes: + label: Fix validation + description: How should we confirm this is fixed? + placeholder: | + - [ ] The reproduction steps no longer fail. + - [ ] Regression coverage exists for the broken behavior. + validations: + required: false diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml new file mode 100644 index 0000000..0086358 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -0,0 +1 @@ +blank_issues_enabled: true diff --git a/.github/ISSUE_TEMPLATE/new-feature.yml b/.github/ISSUE_TEMPLATE/new-feature.yml new file mode 100644 index 0000000..97b9150 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/new-feature.yml @@ -0,0 +1,189 @@ +name: New feature +description: Capture a feature idea with the same planning fields used by the new-feature workflow. +title: "[Feature]: " +labels: + - enhancement +body: + - type: markdown + attributes: + value: | + Use this form for deliverable product or application features that create end-user value. + + Keep implementation tasks at the "what work is needed" level. Avoid file names, code symbols, and exact implementation steps in task checklists because the code can change before the PBI is worked. + - type: textarea + id: summary + attributes: + label: Summary + description: What should the user be able to do when this ships? + placeholder: Users can... + validations: + required: true + - type: textarea + id: user-value + attributes: + label: User value + description: Why does this matter to the end user, operator, or product? + placeholder: This matters because... + validations: + required: true + - type: textarea + id: scope + attributes: + label: Scope + description: What is in scope, out of scope, and assumed? + value: | + ## In scope + + - + + ## Out of scope + + - + + ## Assumptions + + - + validations: + required: true + - type: textarea + id: acceptance-criteria + attributes: + label: Acceptance criteria + description: Observable outcomes that prove this feature is done. + placeholder: | + - [ ] ... + - [ ] ... + validations: + required: true + - type: textarea + id: implementation-tasks + attributes: + label: Implementation tasks + description: Describe what work is needed, not how code should be changed. Do not include exact files, paths, or code symbols here. + placeholder: | + - [ ] Product behavior is defined and exposed to users. + - [ ] Relevant data/state is captured and persisted. + - [ ] User experience covers empty, loading, success, and error states. + - [ ] Validation coverage exists for the expected behavior. + validations: + required: true + - type: dropdown + id: effort + attributes: + label: Effort + description: Planning estimate for total implementation effort. + options: + - XS + - S + - M + - L + - XL + validations: + required: true + - type: dropdown + id: complexity + attributes: + label: Complexity + description: 1 is straightforward, 5 is uncertain, cross-cutting, or high-risk. + options: + - 1/5 + - 2/5 + - 3/5 + - 4/5 + - 5/5 + validations: + required: true + - type: dropdown + id: foreignness + attributes: + label: Foreignness + description: 1 matches existing patterns, 5 introduces unfamiliar architecture, tooling, or product behavior. + options: + - 1/5 + - 2/5 + - 3/5 + - 4/5 + - 5/5 + validations: + required: true + - type: dropdown + id: confidence + attributes: + label: Confidence + description: Confidence in the estimates and proposed shape. + options: + - Low + - Medium + - High + validations: + required: true + - type: input + id: existing-files + attributes: + label: Existing files likely touched + description: Estimate only. Use a range if uncertain. + placeholder: "Example: 3-6" + validations: + required: false + - type: input + id: new-files + attributes: + label: New files likely added + description: Estimate only. Use a range if uncertain. + placeholder: "Example: 1-3" + validations: + required: false + - type: dropdown + id: rough-loc + attributes: + label: Rough LOC + description: Broad code-change size estimate. + options: + - 0-100 + - 100-250 + - 250-600 + - 600-1,500 + - 1,500-4,000 + - 4,000+ + validations: + required: true + - type: textarea + id: dependencies + attributes: + label: Dependencies + description: New or existing runtime, build, service, API, infrastructure, data, auth, or vendor dependencies. + placeholder: | + - Required: + - Optional: + - Avoidable: + validations: + required: false + - type: textarea + id: risks-decisions + attributes: + label: Risks and decisions + description: Data migration, permissions, privacy, accessibility, performance, compatibility, UX ambiguity, rollout, or maintenance risks. + placeholder: | + - Risk: + - Decision: + validations: + required: false + - type: textarea + id: validation-plan + attributes: + label: Validation plan + description: Tests, type checks, builds, smoke tests, visual checks, and manual acceptance checks. + placeholder: | + - [ ] ... + - [ ] ... + validations: + required: true + - type: textarea + id: likely-touch-points + attributes: + label: Likely touch points + description: Planning-time hints only. These are not implementation instructions and may drift before the PBI is worked. + placeholder: | + - Existing area, API, schema, workflow, or pattern that informed the estimate. + validations: + required: false diff --git a/Sources/Vista/PanelContentView.swift b/Sources/Vista/PanelContentView.swift index 2c482de..b752ddf 100644 --- a/Sources/Vista/PanelContentView.swift +++ b/Sources/Vista/PanelContentView.swift @@ -173,10 +173,27 @@ struct PanelContentView: View { thumbnails: thumbnails, thumbSize: thumbCacheSize ) - .onTapGesture { + // Double-click before single so the single + // handler doesn't also fire on a double: double + // copies + dismisses (same as Enter), single just + // moves the selection and leaves the panel up. + .onTapGesture(count: 2) { model.selectedIndex = index runPrimary() } + .onTapGesture(count: 1) { + model.selectedIndex = index + } + // Infinite scroll: when the trailing cell scrolls + // into view, page in the next batch. LazyVGrid only + // realizes (and fires onAppear for) cells near the + // viewport, so this fires once the user nears the + // bottom of what's loaded. + .onAppear { + if record.id == model.results.last?.id { + model.loadMore() + } + } } } .padding(spacing) diff --git a/Sources/Vista/SearchViewModel.swift b/Sources/Vista/SearchViewModel.swift index 6bbb114..e0d0c0d 100644 --- a/Sources/Vista/SearchViewModel.swift +++ b/Sources/Vista/SearchViewModel.swift @@ -24,6 +24,19 @@ public final class SearchViewModel { /// update so the UI can rely on it being valid when results change. public var selectedIndex: Int = 0 + /// True while a `loadMore()` page fetch is in flight. Guards against the + /// grid's trailing-cell `onAppear` firing a second fetch before the + /// first appends. + public private(set) var isLoadingMore: Bool = false + + /// False once a page comes back short of `pageSize` — we've reached the + /// end of the index and further `loadMore()` calls are no-ops. + public private(set) var canLoadMore: Bool = true + + /// Rows fetched per page. The grid pages in more as the user scrolls + /// rather than loading the whole index up front. + private let pageSize = 200 + private let store: ScreenshotStore private var debounceTask: Task? @@ -80,11 +93,38 @@ public final class SearchViewModel { private func runQuery(_ text: String) { do { let query = QueryParser.parse(text) - self.results = try store.search(query, limit: 400) + let page = try store.search(query, limit: pageSize) + self.results = page self.selectedIndex = 0 + // A full page means there may be more behind it; a short page + // (or empty) means we've hit the end of the index. + self.canLoadMore = page.count == pageSize + self.isLoadingMore = false } catch { NSLog("vista: search failed: \(error)") self.results = [] + self.canLoadMore = false + self.isLoadingMore = false + } + } + + /// Appends the next page of results below the last row currently shown. + /// Driven by the grid's trailing-cell `onAppear` for infinite scroll. + /// Selection is left untouched so paging in older rows never yanks the + /// highlight away from where the user is. + public func loadMore() { + guard canLoadMore, !isLoadingMore, let last = results.last else { return } + isLoadingMore = true + defer { isLoadingMore = false } + do { + let query = QueryParser.parse(queryText) + let cursor = ScreenshotStore.Cursor(capturedAt: last.capturedAt.timeIntervalSince1970, id: last.id) + let page = try store.search(query, limit: pageSize, after: cursor) + self.results.append(contentsOf: page) + self.canLoadMore = page.count == pageSize + } catch { + NSLog("vista: loadMore failed: \(error)") + self.canLoadMore = false } } } diff --git a/Sources/VistaCore/ScreenshotStore.swift b/Sources/VistaCore/ScreenshotStore.swift index c99e031..f8fec2a 100644 --- a/Sources/VistaCore/ScreenshotStore.swift +++ b/Sources/VistaCore/ScreenshotStore.swift @@ -318,15 +318,41 @@ public final class ScreenshotStore: @unchecked Sendable { } } + /// Keyset pagination cursor — the (captured_at, id) of the last row a + /// caller has already seen. Paginating on this compound key instead of + /// SQL OFFSET keeps the result window stable while the indexer inserts + /// newer rows mid-scroll: OFFSET would shift every window and dupe/skip + /// rows, whereas a keyset always continues strictly below a fixed point. + /// `id` breaks ties when two screenshots share a captured_at timestamp. + public struct Cursor: Sendable { + public let capturedAt: Double + public let id: Int64 + public init(capturedAt: Double, id: Int64) { + self.capturedAt = capturedAt + self.id = id + } + } + /// Returns recently-captured records, newest first. Used as the default - /// view when no query is typed. - public func recent(limit: Int = 200) throws -> [ScreenshotRecord] { + /// view when no query is typed. Pass `after` to fetch the next page below + /// a cursor the caller already holds. + public func recent(limit: Int = 200, after cursor: Cursor? = nil) throws -> [ScreenshotRecord] { try queue.sync { var stmt: OpaquePointer? - let sql = "SELECT id, path, name, captured_at, mtime, size, width, height, ocr_text, pinned, pinned_at FROM screenshots ORDER BY captured_at DESC LIMIT ?;" + var sql = "SELECT id, path, name, captured_at, mtime, size, width, height, ocr_text, pinned, pinned_at FROM screenshots" + if cursor != nil { + sql += " WHERE (captured_at < ? OR (captured_at = ? AND id < ?))" + } + sql += " ORDER BY captured_at DESC LIMIT ?;" guard sqlite3_prepare_v2(db, sql, -1, &stmt, nil) == SQLITE_OK else { throw lastError() } defer { sqlite3_finalize(stmt) } - sqlite3_bind_int(stmt, 1, Int32(limit)) + var idx: Int32 = 1 + if let cursor { + sqlite3_bind_double(stmt, idx, cursor.capturedAt); idx += 1 + sqlite3_bind_double(stmt, idx, cursor.capturedAt); idx += 1 + sqlite3_bind_int64(stmt, idx, cursor.id); idx += 1 + } + sqlite3_bind_int(stmt, idx, Int32(limit)) var out: [ScreenshotRecord] = [] while sqlite3_step(stmt) == SQLITE_ROW { out.append(Self.row(from: stmt)) @@ -338,8 +364,8 @@ public final class ScreenshotStore: @unchecked Sendable { /// Runs a parsed `Query` and returns matching records. For Phase 1 this /// is a straightforward translation; in Phase 2 we'll add ranking via /// FTS5's bm25() for better relevance ordering. - public func search(_ query: Query, limit: Int = 200) throws -> [ScreenshotRecord] { - if query.isEmpty { return try recent(limit: limit) } + public func search(_ query: Query, limit: Int = 200, after cursor: Cursor? = nil) throws -> [ScreenshotRecord] { + if query.isEmpty { return try recent(limit: limit, after: cursor) } return try queue.sync { // Build WHERE incrementally. For FTS conditions we go through a @@ -374,6 +400,16 @@ public final class ScreenshotStore: @unchecked Sendable { binds.append(.double(range.upperBound.timeIntervalSince1970)) } + // Keyset pagination: continue strictly below the caller's cursor. + // Qualified with `s.` so it slots into the same WHERE/AND chain + // whether or not an FTS join is present. + if let cursor { + nonFTSClauses.append("(s.captured_at < ? OR (s.captured_at = ? AND s.id < ?))") + binds.append(.double(cursor.capturedAt)) + binds.append(.double(cursor.capturedAt)) + binds.append(.int64(cursor.id)) + } + var sql = "SELECT s.id, s.path, s.name, s.captured_at, s.mtime, s.size, s.width, s.height, s.ocr_text, s.pinned, s.pinned_at FROM screenshots s" if let ftsMatch { sql += " JOIN screenshots_fts f ON f.rowid = s.id" diff --git a/Tests/VistaCoreTests/ScreenshotStoreTests.swift b/Tests/VistaCoreTests/ScreenshotStoreTests.swift index d4f54d1..cd1d285 100644 --- a/Tests/VistaCoreTests/ScreenshotStoreTests.swift +++ b/Tests/VistaCoreTests/ScreenshotStoreTests.swift @@ -105,6 +105,65 @@ final class ScreenshotStoreTests: XCTestCase { XCTAssertEqual(results[0].name, "Mar.png") } + // MARK: - Keyset pagination + + // Walk every page of `recent()` via the (captured_at, id) cursor and + // assert we see each row exactly once, newest-first, with no dupes or + // gaps — including a tie where two rows share a captured_at (the `id` + // tiebreaker is what keeps that pair from being skipped or repeated). + func testRecentPaginatesWithoutDupesOrGaps() throws { + let base = Date(timeIntervalSince1970: 1_700_000_000) + // 25 rows on distinct timestamps + 2 sharing one timestamp = 27. + for i in 0..<25 { + try store.upsert(Self.sampleRecord(name: "S\(i).png", + capturedAt: base.addingTimeInterval(Double(i)), + text: "t")) + } + let tie = base.addingTimeInterval(100) + try store.upsert(Self.sampleRecord(name: "TieA.png", capturedAt: tie, text: "t")) + try store.upsert(Self.sampleRecord(name: "TieB.png", capturedAt: tie, text: "t")) + + var seen: [Int64] = [] + var cursor: ScreenshotStore.Cursor? = nil + let pageSize = 10 + while true { + let page = try store.recent(limit: pageSize, after: cursor) + seen.append(contentsOf: page.map(\.id)) + guard let last = page.last, page.count == pageSize else { break } + cursor = ScreenshotStore.Cursor(capturedAt: last.capturedAt.timeIntervalSince1970, id: last.id) + } + + XCTAssertEqual(seen.count, 27) + XCTAssertEqual(Set(seen).count, 27, "no row should appear twice across pages") + // Page-stitched order must match a single unbounded fetch. + let oneShot = try store.recent(limit: 1000).map(\.id) + XCTAssertEqual(seen, oneShot) + } + + // The cursor also has to thread through the FTS/date WHERE chain in + // `search()` — paginating a filtered result set, not just `recent()`. + func testSearchPaginatesWithCursor() throws { + let base = Date(timeIntervalSince1970: 1_700_000_000) + for i in 0..<15 { + try store.upsert(Self.sampleRecord(name: "Match\(i).png", + capturedAt: base.addingTimeInterval(Double(i)), + text: "needle")) + } + try store.upsert(Self.sampleRecord(name: "Other.png", capturedAt: base, text: "haystack")) + + let query = QueryParser.parse("needle") + let firstPage = try store.search(query, limit: 10) + XCTAssertEqual(firstPage.count, 10) + let cursor = ScreenshotStore.Cursor( + capturedAt: firstPage.last!.capturedAt.timeIntervalSince1970, + id: firstPage.last!.id) + let secondPage = try store.search(query, limit: 10, after: cursor) + + let combined = (firstPage + secondPage).map(\.id) + XCTAssertEqual(combined.count, 15, "only the 15 needle rows, no haystack leak") + XCTAssertEqual(Set(combined).count, 15) + } + // MARK: - Pinning func testPinSetsFlag() throws { From 9303b0bf41df9883d4bfc684c2080ce9410fc9fd Mon Sep 17 00:00:00 2001 From: Gordon Beeming Date: Wed, 10 Jun 2026 08:28:13 +1000 Subject: [PATCH 2/2] Fix keyset pagination tie order and unblock the scroll read Address review feedback on #9: - Add an id DESC tie-breaker to both pagination ORDER BY clauses so rows sharing a captured_at can't be skipped when a page boundary lands mid-tie. The cursor predicate already compares id; the sort now matches. - Run loadMore's query off the main actor. The store's serial queue is shared with the indexer's upserts, so a synchronous read could block the main thread behind a write batch and jank the scroll that triggered it. The detached read appends on the main actor with a staleness guard so a query/reload mid-flight drops the now-stale page. - Add a regression test for a tie group straddling the page boundary. Co-authored-by: Claude Co-authored-by: GitButler --- Sources/Vista/SearchViewModel.swift | 35 +++++++++++++------ Sources/VistaCore/ScreenshotStore.swift | 10 ++++-- .../VistaCoreTests/ScreenshotStoreTests.swift | 28 +++++++++++++++ 3 files changed, 61 insertions(+), 12 deletions(-) diff --git a/Sources/Vista/SearchViewModel.swift b/Sources/Vista/SearchViewModel.swift index e0d0c0d..406b783 100644 --- a/Sources/Vista/SearchViewModel.swift +++ b/Sources/Vista/SearchViewModel.swift @@ -112,19 +112,34 @@ public final class SearchViewModel { /// Driven by the grid's trailing-cell `onAppear` for infinite scroll. /// Selection is left untouched so paging in older rows never yanks the /// highlight away from where the user is. + /// + /// The query runs off the main actor: `store`'s serial queue is shared + /// with the background indexer's upserts, so a synchronous `search` here + /// could block the main thread behind a write batch — visible as scroll + /// jank exactly when paging fires. We hop to a detached task for the read + /// and only touch `results` back on the main actor. public func loadMore() { guard canLoadMore, !isLoadingMore, let last = results.last else { return } isLoadingMore = true - defer { isLoadingMore = false } - do { - let query = QueryParser.parse(queryText) - let cursor = ScreenshotStore.Cursor(capturedAt: last.capturedAt.timeIntervalSince1970, id: last.id) - let page = try store.search(query, limit: pageSize, after: cursor) - self.results.append(contentsOf: page) - self.canLoadMore = page.count == pageSize - } catch { - NSLog("vista: loadMore failed: \(error)") - self.canLoadMore = false + let query = QueryParser.parse(queryText) + let cursor = ScreenshotStore.Cursor(capturedAt: last.capturedAt.timeIntervalSince1970, id: last.id) + Task { [store, pageSize] in + defer { isLoadingMore = false } + do { + let page = try await Task.detached(priority: .userInitiated) { + try store.search(query, limit: pageSize, after: cursor) + }.value + // The await above is a suspension point: a fresh query or + // reload may have replaced `results` while we were reading. + // If the tail moved, this page is stale — drop it rather than + // appending rows that no longer follow what's on screen. + guard last.id == results.last?.id else { return } + results.append(contentsOf: page) + canLoadMore = page.count == pageSize + } catch { + NSLog("vista: loadMore failed: \(error)") + canLoadMore = false + } } } } diff --git a/Sources/VistaCore/ScreenshotStore.swift b/Sources/VistaCore/ScreenshotStore.swift index f8fec2a..926dc58 100644 --- a/Sources/VistaCore/ScreenshotStore.swift +++ b/Sources/VistaCore/ScreenshotStore.swift @@ -343,7 +343,11 @@ public final class ScreenshotStore: @unchecked Sendable { if cursor != nil { sql += " WHERE (captured_at < ? OR (captured_at = ? AND id < ?))" } - sql += " ORDER BY captured_at DESC LIMIT ?;" + // `id DESC` mirrors the cursor's tie-break. Without it, rows + // sharing a captured_at have undefined order, so a page boundary + // landing mid-tie could emit a low id and then skip the higher-id + // tied rows when the next page applies `id < cursor.id`. + sql += " ORDER BY captured_at DESC, id DESC LIMIT ?;" guard sqlite3_prepare_v2(db, sql, -1, &stmt, nil) == SQLITE_OK else { throw lastError() } defer { sqlite3_finalize(stmt) } var idx: Int32 = 1 @@ -421,7 +425,9 @@ public final class ScreenshotStore: @unchecked Sendable { } else if !nonFTSClauses.isEmpty { sql += " WHERE " + nonFTSClauses.joined(separator: " AND ") } - sql += " ORDER BY s.captured_at DESC LIMIT ?;" + // `s.id DESC` mirrors the keyset cursor's tie-break so pagination + // stays deterministic when rows share a captured_at — see recent(). + sql += " ORDER BY s.captured_at DESC, s.id DESC LIMIT ?;" binds.append(.int64(Int64(limit))) var stmt: OpaquePointer? diff --git a/Tests/VistaCoreTests/ScreenshotStoreTests.swift b/Tests/VistaCoreTests/ScreenshotStoreTests.swift index cd1d285..f5ed04c 100644 --- a/Tests/VistaCoreTests/ScreenshotStoreTests.swift +++ b/Tests/VistaCoreTests/ScreenshotStoreTests.swift @@ -140,6 +140,34 @@ final class ScreenshotStoreTests: XCTestCase { XCTAssertEqual(seen, oneShot) } + // Regression for the tie-at-page-boundary skip: when more rows share a + // captured_at than fit in one page, the page boundary lands *inside* the + // tie group. The cursor predicate (`id < ?`) only continues correctly if + // ORDER BY also breaks ties by `id DESC` — otherwise SQLite's arbitrary + // tie order can emit a low id first, then permanently skip the higher-id + // tied rows. Five rows on one timestamp + pageSize 2 forces the split. + func testRecentPaginationSurvivesTieStraddlingPageBoundary() throws { + let tie = Date(timeIntervalSince1970: 1_700_000_000) + for i in 0..<5 { + try store.upsert(Self.sampleRecord(name: "Tie\(i).png", capturedAt: tie, text: "t")) + } + + var seen: [Int64] = [] + var cursor: ScreenshotStore.Cursor? = nil + let pageSize = 2 + while true { + let page = try store.recent(limit: pageSize, after: cursor) + seen.append(contentsOf: page.map(\.id)) + guard let last = page.last, page.count == pageSize else { break } + cursor = ScreenshotStore.Cursor(capturedAt: last.capturedAt.timeIntervalSince1970, id: last.id) + } + + XCTAssertEqual(seen.count, 5, "no tied row skipped at the page boundary") + XCTAssertEqual(Set(seen).count, 5, "no tied row duplicated") + // Strictly descending id is the deterministic order the cursor relies on. + XCTAssertEqual(seen, seen.sorted(by: >)) + } + // The cursor also has to thread through the FTS/date WHERE chain in // `search()` — paginating a filtered result set, not just `recent()`. func testSearchPaginatesWithCursor() throws {