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..406b783 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,53 @@ 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. + /// + /// 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 + 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 c99e031..926dc58 100644 --- a/Sources/VistaCore/ScreenshotStore.swift +++ b/Sources/VistaCore/ScreenshotStore.swift @@ -318,15 +318,45 @@ 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 < ?))" + } + // `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) } - 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 +368,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 +404,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" @@ -385,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 d4f54d1..f5ed04c 100644 --- a/Tests/VistaCoreTests/ScreenshotStoreTests.swift +++ b/Tests/VistaCoreTests/ScreenshotStoreTests.swift @@ -105,6 +105,93 @@ 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) + } + + // 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 { + 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 {