Skip to content

perf(state): replace search_sessions full-table aggregate with correlated subquery - #21

Open
spfcraze wants to merge 1 commit into
mainfrom
fix/search-sessions-subquery
Open

perf(state): replace search_sessions full-table aggregate with correlated subquery#21
spfcraze wants to merge 1 commit into
mainfrom
fix/search-sessions-subquery

Conversation

@spfcraze

@spfcraze spfcraze commented Aug 1, 2026

Copy link
Copy Markdown
Owner

What does this PR do?

search_sessions() computed last_active via a LEFT JOIN over a derived table aggregating MAX(timestamp) GROUP BY session_id across EVERY message in the DB — O(all messages) per call, reached on every 'hermes -c'/'--resume' launch and ACP session-list. Replace with a correlated scalar subquery that resolves MAX(timestamp) per session row via the messages(session_id, ...) index — the exact pattern list_sessions_rich already uses for its last_active.

Related Issue

No direct issue — discovered via code review and reproduced live (see below).
Related PRs reviewed during the duplicate check (none covers this change):

Changes Made

  • fix/search-sessions-subquery — 2 file(s) changed vs base:
    • hermes_state.py
    • tests/test_hermes_state.py

hermes_state.py: one query rewrite in search_sessions() (derived-table LEFT JOIN -> correlated scalar subquery), same COALESCE fallback and ORDER BY. tests/test_hermes_state.py: +4 tests — MRU ordering with COALESCE-to-started_at fallback for message-less sessions, byte-exact parity vs the pre-fix join form across unfiltered/filtered/paginated calls, and a plan pin that drives the real method and asserts the issued query never materializes a GROUP BY over the messages table.

How to Test

Measured on a scratch DB with the real schema and indexes (2k sessions / 120k messages, median of 15 runs, repo venv): OLD 10.03 ms vs NEW 1.85 ms per LIMIT-20 call (5.4x). EXPLAIN QUERY PLAN: OLD materializes a SCAN of all 120k messages plus an AUTOMATIC COVERING INDEX for the join; NEW does per-row indexed lookups (idx_messages_session). Results byte-identical for both the page query and the full unbounded query. Gap grows with DB size — the aggregate cost is O(total messages), the correlated form is O(sessions).

Validation completed (recorded by prp):

  1. Sabotage check: pre-fix code fails the regression tests (1 failed), with the fix all pass (143 passed, 0 failed) — target tests/test_hermes_state.py.
  2. Suite tests/test_hermes_state.py: branch 143 passed / 0 failed vs baseline 140 passed / 0 failed — zero branch-only failures.
  3. Full state suites: tests/test_hermes_state.py 143/143, tests/hermes_state/ 33/33. Targeted fullcheck: 143 passed vs 140 baseline, zero branch-only failures. Sabotage revert-verified: plan-pin test fails on pre-fix main, 143/143 pass with the fix. Full repo-wide suite NOT run locally for this PR (skipped by decision; CI owns full-suite validation).
  4. Duplicate check: 73 potential matches reviewed — none covers this change.
  5. The full repo-wide suite was not run for this change; GitHub CI owns full-suite validation.

Logs

Sabotage verification output:

# base leg (pre-fix code + branch tests):
#   tests: 142 passed, 1 failed
# head leg (with fix):
#   tests: 143 passed, 0 failed

…ated subquery

search_sessions() computed last_active via a LEFT JOIN over a derived
table aggregating MAX(timestamp) GROUP BY session_id across EVERY
message in the DB — O(all messages) on every call, reached on each
'hermes -c'/'--resume' launch and ACP session-list. The correlated
scalar subquery resolves MAX(timestamp) per session row via the
messages(session_id, ...) index — the same pattern list_sessions_rich
already uses for its last_active.

Measured (real schema, 2k sessions / 120k messages, median of 15):
10.03 ms -> 1.85 ms per LIMIT-20 call (5.4x), with the OLD plan
materializing a SCAN of all messages and the NEW plan doing indexed
per-row lookups. Results byte-identical (page and full-table).

Tests: MRU ordering + COALESCE fallback behavior, byte-exact parity
against the pre-fix join form (unfiltered/filtered/paginated), and a
plan pin asserting the issued query never materializes a GROUP BY over
the messages table.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant