perf(daemon): make sdk_messages subtype filters sargable (#2330)

[lsm]

Opening the 167k-message coder session stalled ~2.35s because the messages.bySession background-task sidecar filtered COALESCE(message_subtype,'') — non-sargable, since ~78% of sdk_messages rows have message_subtype IS NULL. The (session_id, parent_tool_use_id) index sought every top-level row and filtered row-by-row.

This adds a VIRTUAL generated column message_subtype_norm = COALESCE(message_subtype,'') plus a (session_id, message_subtype_norm, parent_tool_use_id) index (migration 168; also in createTables() for fresh DBs), and swaps every COALESCE(message_subtype,'') filter in live-query-handlers.ts to use it. Semantically identical by construction; EXPLAIN now shows a covering seek on the new index for the sidecar. VIRTUAL keeps the ALTER TABLE schema-only — no table rewrite, safe on the 15GB production DB.

Closes #2330.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[greptile-apps]

Greptile Summary

This PR fixes a 2.35s stall when opening large sessions by making sdk_messages subtype filters sargable. It adds a VIRTUAL generated column message_subtype_norm = COALESCE(message_subtype, '') and a (session_id, message_subtype_norm, parent_tool_use_id) covering index, then replaces every COALESCE(message_subtype, '') expression in live-query-handlers.ts with the column reference.

• Migration 169 applies the schema change to existing databases using ALTER TABLE ... ADD COLUMN (schema-only, no rewrite) with an idempotency guard via pragma_table_xinfo — necessary because generated columns are invisible to pragma_table_info.
• createTables() and createIndexes() are updated for fresh databases, and all test helpers are kept in sync.
• sdk-message-repository.ts contains structurally identical COALESCE(message_subtype, '') queries (pagination, getBackgroundTaskMessages) that were not updated and remain non-sargable against the new index.

Confidence Score: 5/5

Safe to merge — the schema change is additive and write-free (VIRTUAL column), the migration is idempotent, and the query substitution is semantically identical by construction.

The change is narrowly scoped: a VIRTUAL generated column, one new index, and mechanical COALESCE-to-column-ref substitutions. The migration correctly guards against re-application using pragma_table_xinfo, and tests verify both the invariant and the index-seek behavior. No correctness risk is introduced.

Files Needing Attention: sdk-message-repository.ts contains identical COALESCE(message_subtype, '') query patterns that were not updated and will not benefit from the new index.

Important Files Changed

Filename	Overview
packages/daemon/src/lib/rpc-handlers/live-query-handlers.ts	All COALESCE(message_subtype, '') filter expressions replaced with message_subtype_norm; BACKGROUND_TASK_METADATA_SQL exported for test access. Semantically identical substitution.
packages/daemon/src/storage/schema/index.ts	Adds the VIRTUAL generated column and composite index to createTables()/createIndexes() for fresh databases; well-commented with semantic justification.
packages/daemon/src/storage/schema/migrations.ts	Migration 169 correctly adds the column (using pragma_table_xinfo to detect generated columns invisible to pragma_table_info) and index; idempotent and no-op when sdk_messages is absent.
packages/daemon/tests/unit/4-space-storage/storage/migrations/migration-169-subtype-norm-generated-column.test.ts	Good coverage of migration semantics, idempotency, and sargability. The sargability test uses a negative assertion (not.toContain('SCAN sdk_messages')) rather than the stronger positive toContain('SEARCH') used in the companion live-query test.
packages/daemon/tests/unit/2-handlers/rpc-handlers/live-query-messages.test.ts	Two new tests added: one validates index seek (SEARCH) for the sidecar's subtype IN filter, one verifies the invariant that message_subtype_norm always matches COALESCE(message_subtype, '').
packages/daemon/tests/unit/2-handlers/rpc-handlers/live-query-handlers.test.ts	Stub table DDL updated to include the generated column so existing handler tests continue to pass against the new schema.
packages/daemon/tests/unit/helpers/space-test-db.ts	Test helper updated with the generated column and new index, keeping the test schema in sync with production createTables().

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["Query: sdk_messages\nWHERE session_id = ?\nAND message_subtype_norm IN (...)"] --> B{Index available?}
    B -- "Before (COALESCE)" --> C["COALESCE(message_subtype,'') = ...\nNon-sargable — wraps column in fn"]
    C --> D["Old index: (session_id, parent_tool_use_id)\nSeeks ALL top-level rows, filters row-by-row\n~78% NULL subtypes → ~2.35s stall"]
    B -- "After (generated column)" --> E["message_subtype_norm = ...\nSargable — bare column reference"]
    E --> F["New index: (session_id, message_subtype_norm, parent_tool_use_id)\nDirect seek to matching subtypes only\nCovering index — no table rows read"]
    F --> G["VIRTUAL column: schema-only ALTER TABLE\nNo table rewrite, safe on 15GB DB"]
    G --> H["Migration 169\npragma_table_xinfo guard\n(generated cols invisible to pragma_table_info)"]

Loading

_{Reviews (3): Last reviewed commit: "Merge remote-tracking branch 'origin/dev..." | Re-trigger Greptile}

[lsm]

🤖 Review by glm-5.1[1m] (NeoKai)

Model: glm-5.1[1m] | Client: NeoKai | Provider: Z.ai

Recommendation: APPROVE (posted as COMMENT because reviewer shares the PR author's account; GitHub rejects self-APPROVE).

Independent review of the sargability fix (#2330). The change adds a VIRTUAL generated column message_subtype_norm = COALESCE(message_subtype,'') + a (session_id, message_subtype_norm, parent_tool_use_id) index (migration 168, also in createTables()), and swaps all 12 COALESCE(message_subtype,'') filters in live-query-handlers.ts to use it. Verdict: clean, no blocking findings.

Correctness — verified

• All 12 swaps are semantically identical by construction: the generated column's expression is exactly COALESCE(message_subtype,''), so message_subtype_norm <op> X ≡ COALESCE(message_subtype,'') <op> X. No three-valued-logic risk on NOT IN either, since the norm column is never NULL.
• New invariant tests assert message_subtype_norm = COALESCE(message_subtype,'') for every row (drift count == 0).

Migration 168 — safe & idempotent

• VIRTUAL (not STORED) ⇒ schema-only ALTER TABLE, no table rewrite — correct call for the 15GB DB. Index build is the only I/O cost, inherent to any index migration.
• The column guard correctly uses pragma_table_xinfo rather than pragma_table_info (the shared tableHasColumn helper) — generated columns are hidden from the latter, so this is the right check and avoids a "duplicate column name" error on re-run. CREATE INDEX IF NOT EXISTS + tableExists guard make it idempotent and a no-op without sdk_messages. Idempotency/no-op both covered by the new migration test.
• Full runMigrations path (incl. 168) runs clean — confirmed via space-runtime.test.ts (197 pass).

Integration safety — proven

• message_subtype_norm is referenced only in live-query-handlers.ts. That SQL is consumed only by the 4 rpc-handlers live-query test files (all pass: 145 + 43 + 197 across the affected suites, 0 fail). The runtime/storage tests with inline sdk_messages fixtures run queries from sdk-message-repository.ts/space-agent-tools.ts, which still use the unswapped COALESCE(message_subtype,'') (out of scope for this PR) — so they can't break, and don't.
• No regression path from the swap itself: adding an index never forces its use, and the reference change is semantically neutral, so the planner either picks the new index (faster) or keeps the existing (session_id, parent_tool_use_id) index (same as before).
• Typecheck clean; oxlint clean on the changed files.

One non-blocking nit (optional): the sidecar EXPLAIN test asserts not.toContain('SCAN sdk_messages USING'), which greptile flagged. It's adequate as-is because the load-bearing assertion toContain('idx_sdk_messages_session_subtype_parent') is the real guard and would itself fail under any table-scan regression; but tightening to toContain('SEARCH') would be unambiguously future-proof. Not required to merge.

Scope note (no action needed here): the same COALESCE(message_subtype,'') anti-pattern remains in sdk-message-repository.ts and a couple of evolution/space-agent-tools files — deliberately out of scope for this PR (issue scopes the swap to live-query-handlers.ts; the rest is tracked by #2335/#2336/#2338).

Counts: P0 0 · P1 0 · P2 0 · P3 1 (optional, the assertion nit above).

[lsm]

🤖 Review by glm-5.1[1m] (NeoKai)

Model: glm-5.1[1m] | Client: NeoKai | Provider: Z.ai

Recommendation: APPROVE — re-review of the corrected head f4916d6 (post conflict-resolution push), confirming the post-approval changes are sound before merge.

Reviewed the full delta from the approved head 1bac57642 → f4916d6. The branch's own changes (isolated from the merged origin/dev content) are exactly two:

1. Assertion tightening (d3f7856ef, the requested Option A): not.toContain('SCAN sdk_messages USING') → toContain('SEARCH'); load-bearing toContain('idx_sdk_messages_session_subtype_parent') retained. Correct.

2. Merge of origin/dev + migration renumber (f4916d621): the conflict was a real migration-number collision — dev shipped M168 = idx_node_executions(agent_session_id) in #2343, same number this branch used. Resolution is correct and surgical:

   • dev's runMigration168 (node_executions) preserved intact under marker migration_168.
   • the generated-column migration renumbered to runMigration169 under distinct marker migration_169 (monotonic 167→168→169). This avoids the silent-skip trap: a DB that ran dev's M168 will still run the generated-column migration as M169.
   • createTables() still defines message_subtype_norm + the composite index for fresh DBs.
   • Test files renamed/re-imported consistently; migration-168_test.ts correctly tests node_executions, migration-169-...test.ts tests the generated column. No stale generated-column-as-168 references remain.

No unrelated coder changes (the 47-file delta is dev's merged content, #2316/#2318/#2319/#2340/#2341/#2342/#2343 — visible in the merge commit, not author changes). CI green (run 30783302632); 0 unresolved threads; mergeStateStatus: CLEAN. Safe to squash-merge.