perf(host-core): reduce history costs in search and session reads

[moapop]

Problem

Four costs that grow with session history. Each was measured before the change
and again with the change ablated.

1. The workspace resolvers read a whole transcript to learn one project
   path — four call sites, one of them twice per call.
2. Every search statement sorted its complete match set. messages carried
   only the UNIQUE (session_id, seq) index the write path needs, while each
   statement ranks by created_at and cuts with a LIMIT, so
   EXPLAIN QUERY PLAN reports USE TEMP B-TREE FOR ORDER BY on the uncapped
   LIKE branch, both trigram branches and the per-session snippet lookup.
   snippet() also ran once per FTS match rather than once per returned row.
3. Two presence questions parsed whole transcripts
   (transcript_contains_id, read_tool_call).
4. The desktop walked a search query once per emitted page: page
   offset / 30 cost offset / 30 + 1 host calls, each paying the same
   whole-table aggregation, because OFFSET does not change it.

Result

measured	before	after
session summary vs full read, 21 MB	9.8 ms	0.049 ms (202x)
search.query uncapped LIKE, first 20 of 40k	30.46 ms	0.021 ms
per-session snippet lookup, 40k	0.183 ms	0.028 ms
search.query trigram, 40k	6.39 ms	3.52 ms
transcript_contains_id / read_tool_call, 21 MB	7.42 / 6.24 ms	4.39 / 4.51 ms
six search pages, 40k	11 calls / 27.4 ms	6 calls / 19.0 ms
hover existence check, 21 MB	22.3 ms / 21 MB	0.07 ms / 66 KB

Medians of five runs on one fixture per row. The ablation for the search indexes
is the same statements with the indexes dropped again, so the difference is
attributable to the index rather than to the statement text.

Implementation notes

• The indexes arrive the way idx_turns_ended_at arrives: an idempotent
  CREATE INDEX IF NOT EXISTS in boot_maintenance plus the schema
  declaration. No row is rewritten, no schema-version bump, no migration backup.
  The first launch after the upgrade builds both indexes once.
• The snippet() replacement repeats MATCH ?1 in its subquery. An FTS5
  auxiliary function reads the phrase list of the MATCH that positioned its
  cursor, so reaching a document by rowid alone returns the head of the document
  instead of the window around the hit. That faster form was rejected after a
  row-for-row comparison found 0 of 20 snippets identical; the shipped form is
  20 of 20.
• update_message stays O(history) deliberately: appending the replacement is
  396x cheaper and was rejected because read-path windows are physical line
  coordinates, so a search jump to a stamped message would open on the tail of
  the session.
• Transcript and revision lines still call sync_data() before the derived
  database transaction. The append-throughput optimization is withdrawn pending
  a transaction-safe durability design; no append-speed result is claimed here.

Verification

• cargo fmt --all -- --check — passed.
• cargo clippy -p host-core --all-targets --locked — passed with two
  non-blocking warnings in transcript cache/read code.
• cargo test -p host-core --locked — 779 passed, 0 failed.
• cargo build -p host-core --locked — passed.
• ELECTRON_MAJOR_VER=43 pnpm build:js — passed.
• pnpm --filter @pi-desktop/desktop typecheck — fails on existing
  LucideProps icon prop errors; the same class of errors reproduces on
  current main. The changed PR lines do not overlap those errors.
• pnpm lint — passed.
• pnpm -r --if-present test — passed across all workspaces, including
  desktop 3556/3556.
• pnpm test:e2e — 23/23 passed, 2 skipped because no API key was set.
• Changed desktop session tests — 27/27 passed.
• Architecture check and pnpm check:pr-base — passed; latest origin/main
  is an ancestor of this PR head.

Compatibility, migration, security

• No output, ordering or protocol shape changes. search.sessions gains an
  optional limit; omitted, it keeps the existing 30-row cursor contract.
• Transcript/revision sync remains ahead of the index transaction; a failed
  sync returns an error. No append durability contract is weakened. The
  append-speed optimization and its benchmark claim were removed.
• No dependency, lockfile, workflow or configuration change.
• No ADR: no architectural boundary, security boundary or frozen decision
  changes.
• No new E2E scenario: the changed behaviour is host-internal, and the host-core
  search tests plus the desktop search-merge expectations cover it; the latter
  are updated to pin the new request shape.

13 files changed, all modified — none added, none deleted.

[vastsa]

Review result: do not merge yet.

The measured search/session improvements are plausible, and I ran the relevant host-core coverage: cargo test -p host-core --locked passed 677/677, including the new session-search, transcript, index, and deferred-flush tests.

There is nevertheless a production data-safety regression in the append path. Before this PR, append_line() called file.sync_data()? before returning, so an append RPC did not succeed when the device flush failed. Now it returns after write_all + flush, queues sync_data() on a background thread, and only logs a flush error. main.rs drains only on clean shutdown; a crash/power loss or a failed sync can leave the transcript non-durable while the RPC and subsequent SQLite commit have already succeeded. That changes the persisted-data and error contract and can make the database claim a message that the transcript has lost.

This is not a cosmetic concern: the PR description itself calls it a reliability change, but no durable acknowledgement/error propagation or retry path replaces the old guarantee. Please preserve the existing durability semantics (or define and validate an explicit new contract) before landing. The PR head is also behind current origin/main (78d0e884, after #1200), so it needs a fresh integration candidate after correction.

[vastsa]

The durability regression is resolved in the current head: transcript writes are committed before the RPC response, and a failed sync_data() is returned as request failure. I verified cargo test -p host-core --locked: 705/705 passed after refreshing the branch onto current main.

I am not merging yet because GitHub reports no checks for this PR, while the current diff also changes desktop JS search/session paths. The local desktop typecheck could not be run from the isolated worktree because pnpm rejects the reused symlinked task-state directory. Please provide a CI candidate (or a clean JS build/typecheck/lint/test result) before landing this 12-file performance change.

[vastsa]

Found a remaining root-cause durability issue in the current deferred-flush design; please do not merge yet.

append_record() writes the JSONL line, then commits the SQLite messages row, while sync_data() is deferred until commit_writes_of() runs after the handler returns. If sync_data() fails, response_for() correctly returns an RPC error, but the SQLite row has already committed. A retry of the same append then hits message_indexed() and returns success/no-op without rewriting or re-registering the transcript flush. After a transient/device flush failure this can leave SQLite claiming the message exists while the transcript bytes are not durable; a crash at that point loses the message.

The new tests only prove that the deferred request reports the flush error; they do not assert the post-failure DB/transcript state or retry behavior. The fix needs to make the DB/index commit conditional on durable transcript success, or add an explicit durable-pending/retry/reconciliation state that prevents a retry from becoming a no-op until the transcript is synced.

[vastsa]

Thanks for the careful measurements and the substantial performance work. I re-checked the current head and cannot approve or merge it yet: the deferred flush still leaves a data-safety gap.

sessions::append_message calls append_record, which writes the transcript line and commits the SQLite message index inside the handler. append_line only registers the file for sync_data() in the request-local queue; commit_writes_of runs that queue after the handler has returned. If the flush fails, response_for correctly returns an RPC error, but the SQLite row remains committed. A retry then hits message_indexed() and returns success without rewriting or re-registering the transcript write. A failed device flush can therefore leave SQLite claiming a message whose transcript is not durable, and a later crash can lose that message.

The current a_request_only_answers_after_its_lines_reached_the_device test checks the RPC error, but not the database/transcript state or retry behavior. Please make the index commit conditional on a successful transcript flush (or add an explicit durable-pending/rollback path that prevents retry from becoming a no-op) and add a failing-flush-then-retry regression test that verifies the canonical transcript and index agree.

This head is also behind current main (PR base 4dee458, current origin/main c1afefe), and GitHub reports no checks for it. After correcting the persistence ordering, please refresh it from current main and provide the required green integration checks. The performance direction is valuable; these are landing blockers, not requests to redo the measurements.

[vastsa]

Thanks for the careful performance work. I verified the original durability issue and added a failing-flush-then-retry regression test. The transcript sync now completes before the derived SQLite transaction, so a failed sync cannot leave the index claiming a transcript line that was not persisted; the retry writes the line exactly once.

The append-throughput optimization and its benchmark claim have been removed from this PR because the deferred-flush design could not preserve that durability ordering. The remaining search and session-read improvements are retained. The PR integration merge ref (a701f65, tree df4c0ab) has the same executable tree as the validated task candidate.

Validation: host-core tests 779/779, workspace tests passed (desktop 3556/3556), E2E 23/23 with 2 API-key-dependent skips, changed desktop session tests 27/27, build, lint, formatting, and architecture checks passed. Desktop typecheck still reports existing LucideProps icon prop errors, reproduced on current main; the changed lines do not overlap them. Approving and merging.