Skip to content

perf(#4842): pre-aggregate latest-message ordering for missing-index cron scans - #4962

Closed
rodboev wants to merge 5 commits into
nesquena:masterfrom
rodboev:pr/4842-latest-message-ordering
Closed

rodboev wants to merge 5 commits into
nesquena:masterfrom
rodboev:pr/4842-latest-message-ordering

Conversation

@rodboev

@rodboev rodboev commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Thinking Path

  • The earlier #4842 slices already removed the per-row cron sidecar I/O path and the streaming cache churn, but the cron-only sidebar rescue can still hit multi-second cold rebuilds when state.db has no usable idx_messages_session.
  • The common indexed path needs to stay on the existing correlated ordering, because that is still the faster plan on a healthy migrated store.
  • This follow-up stays independent of #4952, keeps indexed scans unchanged, and only switches the cron-only capped pass to a pre-aggregated latest-message relation when the best-effort index prime cannot restore the normal path.

What Changed

  • api/agent_sessions.py: keep the existing indexed ordering path, keep the defensive idx_messages_session prime, and add a cron-only fallback that joins a pre-aggregated latest_messages CTE only when the messages index is still unavailable after that prime.
  • tests/test_issue2628_cli_sessions_perf.py: update the source-shape guard to the missing-index gate, keep the resumed-old-session coverage, and add a deterministic progress-budget proof that compares the old correlated ordering against the new fallback on a read-only missing-index database.

Why It Matters

This fixes the cold cron fallback that still goes quadratic on degraded or read-only stores, without regressing the normal indexed sidebar path. Cron-heavy installs keep the same rows and ordering, but the missing-index branch no longer pays one latest-message lookup per session row.

Verification

  • pytest tests/test_issue2628_cli_sessions_perf.py tests/test_issue3887_index_prime.py tests/test_issue3172_cron_session_limit.py tests/test_issue3930_source_filter_pushdown.py tests/test_issue3585_cron_session_overflow.py -v --timeout=60

Full-suite CI context, not a required local check unless requested: pytest tests/ -v --timeout=60.

Upstream

Follow-up for #4842.

Attribution: Nathan's request for help and Rod Boev's profiling plus proposed SQL shape defined this slice.

Model Used

GPT 5.5 via Codex CLI

@greptile-apps

greptile-apps Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds a faster cron-only fallback for degraded session scans. The main changes are:

  • Detects whether idx_messages_session is present before choosing the query shape.
  • Keeps the indexed correlated ordering path for normal migrated stores.
  • Uses a pre-aggregated latest-message CTE for cron-only missing-index scans.
  • Expands performance tests for missing-index cron ordering.

Confidence Score: 5/5

The changed flow looks mergeable after a small deterministic-ordering cleanup.

  • The normal indexed path remains unchanged.
  • The missing-index cron fallback can pick arbitrary tied sessions at the candidate limit boundary.
  • No blocking security or data-loss issue was found in the changed code.

api/agent_sessions.py

Important Files Changed

Filename Overview
api/agent_sessions.py Adds the index-presence gate and cron-only latest-message CTE fallback; the fallback candidate window still needs deterministic tie handling.
tests/test_issue2628_cli_sessions_perf.py Adds missing-index database helpers and progress-budget coverage for the cron fallback path.

Reviews (1): Last reviewed commit: "fix(sidebar): restrict cron pre-aggregat..." | Re-trigger Greptile

Comment thread api/agent_sessions.py
@nesquena-hermes nesquena-hermes added the size:L Large PR (>10 files or >250 LOC) label Jun 26, 2026
nesquena-hermes added a commit that referenced this pull request Jun 26, 2026
…ng-index scans (#4962)

Release YB (v0.51.672): pre-aggregate cron sidebar ordering for missing-index scans (#4962)
@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Shipped in v0.51.672 (Release YB, just deployed) — thanks @rodboev! The cron sidebar rescue scan on a state.db missing idx_messages_session now uses a single pre-aggregated latest-message ordering instead of the correlated per-row subquery; the indexed path is unchanged. Gate: Codex SAFE — verified result-equivalence with a direct SQL probe (no-message / null-timestamp / tied-latest-message all match old output), pre-agg fires only on missing-index, no interaction with #4952. Suite 10654. Verified on prod.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L Large PR (>10 files or >250 LOC)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants