mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-09-25 03:42:06 +00:00
xarmian
f8ed3e10a7
* fix(search): require explicit selection on Enter; add bare-number go-to (BUG-864, BUG-910) The command palette had two related issues: - BUG-864: Pressing Enter armed the first search result automatically — the user could close the modal and navigate without ever pressing an arrow key. selectedIdx now starts at -1 and only advances on ArrowDown/ArrowUp. - BUG-910: Typing a bare number (e.g. "843") returned no results because parseItemRef requires PREFIX-NUMBER and FTS doesn't index item_number. Backend (internal/store): - Add parseItemNumber() helper alongside parseItemRef. - In Search(), add a bare-numeric direct-lookup path that mirrors the existing ref-lookup block but without a collection prefix filter. item_number is unique per workspace (idx_items_workspace_number) so this resolves to at most one direct hit, prepended with rank=-1000. Frontend (CommandPalette.svelte): - selectedIdx defaults to -1; reset to -1 (not 0) on modal open and after every search. - Enter on a non-numeric query is a no-op unless the user has arrow-selected. - Numeric queries are a deliberate exception: Enter on a bare-number query flushes the debounce, navigates directly to the matching item, and lets the search palette double as a quick "go to item N" jump. Tests: - TestSearch_BareNumericQueryFindsItemByNumber covers the new path. - TestParseItemNumber covers helper edge cases. * fix(search): exclude direct hits from FTS WHERE to keep pagination correct Codex review (round 1) on PR #320: > Numeric direct hits are appended before the FTS query, but the later > pagination only removes duplicates after SQL LIMIT/OFFSET. If item #2 > also matches FTS for query "2" through its title/content, that > duplicate consumes an FTS slot, so page 1 can return fewer than `limit` > results and later pages can repeat/skip rows. Hoist the direct-hit (ref + numeric) snapshot to before the FTS query is built, then append `AND i.id NOT IN (...)` to both the SELECT and COUNT FTS queries. After a successful count, add refCount back so SearchResponse.Total still reflects the full result set (since FTS itself no longer counts those rows). The flaw also applied to the pre-existing parseItemRef path; this fix covers both. The post-LIMIT dedup loop is now defense-in-depth. New test TestSearch_BareNumericQueryDedupsAgainstFTS guards the case: an item whose title/content literally contains its own item_number (so it matches both the direct lookup and FTS) appears exactly once in Results and Total counts it exactly once. * fix(search): paginate direct hits properly across workspaces Codex review (round 2) on PR #320: > P1: Bare numeric direct hits break pagination in global search. > item_number is only unique per workspace, so q=1 with WorkspaceIDs > spanning N workspaces returns N direct hits — all appended without > being sliced to Limit. limit=1 with three workspaces each having #1 > returns three results on page 0, and offset=1 drops all direct hits > then returns FTS rows instead of the second direct hit. The same flaw applied to the pre-existing parseItemRef path: the global search "TASK-5" can match TASK-5 in multiple workspaces. Fix: - Add deterministic ORDER BY i.workspace_id, i.id to both ref and bare- numeric direct-hit lookups so pagination is stable across pages. - Replace the offset==0/offset>0 branching pagination with a uniform slice: directStart = min(Offset, refCount); directEnd = min(Offset+Limit, refCount); results = results[directStart:directEnd]; ftsLimit = Limit - directConsumed; ftsOffset = max(Offset - refCount, 0). This honours (offset, limit) whether direct hits, FTS, or both fill the page. Total stays correct because the FTS count was already excluding direct hits (round-1 fix) and we add refCount back unconditionally. New test TestSearch_BareNumericQueryPaginatesAcrossWorkspaces creates three workspaces each with item #1 and verifies that limit=1 with offsets 0/1/2 returns three different direct hits in stable order, and limit=10 returns all three. * chore: gofmt — column alignment in struct field declarations CI Go (SQLite) lint failed on two files: - internal/store/store_test.go (TestParseItemNumber, this PR's new test) — unaligned column widths and inconsistent comment spacing. - internal/config/config.go (drive-by) — pre-existing alignment regression in the Config struct that snuck in via an earlier landed PR; included here because it blocks merge. No semantic changes — `gofmt -w` only.