Skip to content

perf(lcm): bound retention diagnostics - #507

Draft
HashemKhalifa wants to merge 4 commits into
ScriptedAlchemy:masterfrom
HashemKhalifa:perf/retention-scan-bounds-20260812
Draft

perf(lcm): bound retention diagnostics#507
HashemKhalifa wants to merge 4 commits into
ScriptedAlchemy:masterfrom
HashemKhalifa:perf/retention-scan-bounds-20260812

Conversation

@HashemKhalifa

@HashemKhalifa HashemKhalifa commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Summary

  • make lcm_doctor retention mode run only the retention diagnostic instead of unrelated payload, FTS, lifecycle, cleanup, and GC checks
  • bound unscoped discovery to 20 registered sessions and raw-message analysis to 10,000 messages
  • report truncated and incomplete coverage explicitly, and exclude incomplete sessions from candidate classification
  • preserve the existing complete retention diagnostic in non-retention doctor modes

Root cause

Retention mode reused the complete doctor diagnostic pipeline and aggregated the full raw-message and summary stores before returning candidates. LIMIT was applied only after those aggregations, so the work performed still grew with the complete session history.

Before and after

Area Before After Why the new behavior is better
Diagnostic scope Ran the complete doctor pipeline, including payload, FTS, lifecycle, cleanup, and GC diagnostics Runs only the retention diagnostic Avoids unrelated CPU, filesystem, and database work
Unscoped session discovery Derived sessions by grouping the complete matching raw-message store Reads at most 21 rows from the indexed session registry and samples at most 20 sessions Discovery cost is bounded independently of raw-history size
Raw-message analysis Aggregated every matching raw message before applying a result limit Analyzes at most 10,000 messages in total, plus at most one sentinel row for each of 20 sampled sessions Query work has a hard upper bound instead of growing with the database
Scoped session analysis Aggregated the complete selected session Analyzes at most 10,000 messages plus one sentinel row A single very large session cannot force an unbounded scan
Summary lookup Grouped all matching summary nodes Performs at most 20 LIMIT 1 presence probes, only for completely sampled sessions Avoids full summary aggregation and unnecessary payload reads
Partial coverage A result limit did not reveal that the underlying aggregation was unbounded Reports session and message truncation, lists incomplete sessions, and excludes them from candidates Prevents partial aggregates from being presented as definitive retention candidates
Candidate count Up to 20 candidates after full-store aggregation Up to 20 conservative candidates from completely sampled sessions Preserves the public result limit without unsafe partial classifications
Other doctor modes Complete diagnostics Unchanged complete diagnostics The performance boundary is isolated to explicit retention mode
Database mutation None None Retention diagnosis remains read-only, including when apply=true
Database size Unchanged Unchanged This PR improves query cost; it does not delete, compact, or rewrite history

Why this should perform better

The previous cost was proportional to the amount of stored history because the database had to complete full grouped aggregations before it could apply the result limit. The new retention-only path is proportional to explicit fixed budgets: 21 registry rows, 10,000 analyzed raw messages plus at most 20 sentinels, and at most 20 summary-presence probes. That bounds CPU and disk I/O while returning conservative, auditable coverage metadata when the store is larger than the budget.

Unscoped retention diagnosis can intentionally omit sessions beyond the first 20 or sessions whose raw messages exceed their share of the scan budget. Callers can detect this through scan_truncated, session_scan_truncated, incomplete_session_count, and incomplete_sessions.

Follow-up TODOs before ready for review

The current implementation is a safe bounded first-result path, not the intended final exhaustive-retention design. Keep this PR in draft until these items are resolved or explicitly moved to a separately linked follow-up:

  • Add an opaque continuation cursor covering both the session position and raw-message position.
  • Ensure each repeated request advances coverage instead of rescanning the same first 20 sessions.
  • Allow bounded pagination to eventually cover every eligible session and message without a full-store query.
  • Accumulate exact per-session statistics across pages without presenting partial aggregates as final candidates.
  • Return complete=true only after the requested scope has been exhaustively covered; otherwise return explicit progress and continuation metadata.
  • Define cursor invalidation and consistency behavior when new messages arrive between pages.
  • Add end-to-end tests proving forward progress, no duplicate/omitted rows, deterministic resume, and eventual completeness across oversized stores.
  • Evaluate incremental per-session retention counters so routine diagnostics can avoid repeatedly reading raw history.
  • Add a matched, isolated before/after benchmark before claiming wall-clock or percentage speedups.

Validation metrics

Evidence Result
Focused retention tests 5 passed, 0 failed
Public MCP retention regression 1 passed, 0 failed
Large-store adversarial fixture 30,003 raw messages across oversized sessions
Covered failure modes unrelated diagnostics, boundary partiality, multi-session starvation, unbounded session discovery, and non-retention compatibility
Formatting cargo fmt --check passed
Patch hygiene git diff --check passed
Independent final review of the current bounded implementation ship, no findings

No wall-clock speedup is claimed here because a trustworthy isolated, matched before/after benchmark was not completed. The performance claim is the exact algorithmic/query bound above, backed by adversarial tests rather than cache-sensitive timing.

@changeset-bot

changeset-bot Bot commented Aug 12, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: d8e9997

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

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