Repository navigation
perf(conversations): track message stats in threads log so list_threads is O(threads.jsonl) - #1517
Merged
senamakel merged 3 commits intoMay 12, 2026
Conversation
…ds is O(threads.jsonl)
`ConversationStore::list_threads` was reading every per-thread messages
JSONL file from disk on every call — purely to compute `message_count`
and `last_message_at` for sorting. For a user with many threads and a
year of chat history this could be hundreds of MB of disk reads per call,
all under the global `CONVERSATION_STORE_LOCK`, blocking every other
conversation RPC. This is the root cause behind the 30s
`openhuman.threads_list` timeouts surfaced in Sentry (OPENHUMAN-REACT-X)
and only gets worse as users age in.
Push the stats into `threads.jsonl` itself:
- `ThreadLogEntry::MessageAppended { thread_id, last_message_at }` is
emitted from `append_message` under the existing lock; the index fold
increments `message_count` and overwrites `last_message_at`.
- `ThreadLogEntry::Stats { thread_id, message_count, last_message_at }`
is an absolute snapshot, used to backfill legacy threads (data written
before this commit). The first `list_threads` after upgrade does one
slow per-file read for any thread with no stat history and persists a
`Stats` entry — every subsequent call hits the fast path.
- `list_threads_unlocked` no longer opens per-thread files. The hot path
is one sequential read of `threads.jsonl` + sort.
- `thread_summary_unlocked` (single-thread queries) also reads index
stats first, falling back to a per-thread file read only when stats
are unknown.
Tests cover:
- after warm-up, deleting every per-thread file leaves count + last_at
intact (proves the fast path)
- legacy log without stat history backfills correctly + persists a
`Stats` snapshot
- legacy `threads.jsonl` written before this commit still parses
- deleting a thread clears stats from the folded index
- the existing 15 tests all still pass
Open follow-up: the threads log now grows by one entry per message
append. Compaction (rewriting `threads.jsonl` as a folded snapshot
periodically) is a sane future improvement but not needed here.
Refs OPENHUMAN-REACT-X
Contributor
📝 WalkthroughWalkthroughThis PR adds a TypeScript error formatter utility for thread-load failures and introduces Rust-side message-count and last-message timestamp caching to the JSONL thread index, with automatic legacy backfill for existing data. ChangesError Formatter Utility
Thread Stats Indexing and Backfill
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
…anches The mount-time .catch() block added in tinyhumansai#1513 normalises Redux's SerializedError plain object into a string for console.warn. Pull that logic into an exported pure helper so it can be unit-tested without mounting the full page, then exercise every branch (Error, SerializedError-shaped object, plain object, string, null/undefined, non-string message field). Coverage Gate on changed lines goes from 0% (the original 4-line inline catch was untested) to ≥80%. No behavior change beyond the refactor.
Member
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/pages/Conversations.tsx (1)
275-313: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winConvert
.then().catch()chain to async/await in the mount effectThe guideline requires using async/await for promise handling instead of
.then()chains in TypeScript code. Refactor theloadThreads()chain to use async/await syntax.Proposed refactor
useEffect(() => { let cancelled = false; - - void dispatch(loadThreads()) - .unwrap() - .then(data => { + void (async () => { + try { + const data = await dispatch(loadThreads()).unwrap(); if (cancelled) return; const threadStateForSelect = store.getState().thread; if (data.threads.length > 0) { const persistedId = threadStateForSelect.selectedThreadId; const resumeId = persistedId && data.threads.some(t => t.id === persistedId) ? persistedId : data.threads[0].id; dispatch(setSelectedThread(resumeId)); void dispatch(loadThreadMessages(resumeId)); } else { void handleCreateNewThread(); } - }) - .catch(err => { + } catch (err) { if (cancelled) return; console.warn('[conversations] loadThreads failed on mount:', formatThreadLoadError(err)); - }); + } + })();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/pages/Conversations.tsx` around lines 275 - 313, Replace the .then().catch() chain on dispatch(loadThreads()).unwrap() with an async IIFE or an async function inside the mount effect: await the result via const data = await dispatch(loadThreads()).unwrap(); then run the same selection logic (read store.getState().thread into threadStateForSelect, compute persistedId/resumeId, dispatch(setSelectedThread(resumeId)) and void dispatch(loadThreadMessages(resumeId)) or call void handleCreateNewThread()) while guarding with if (cancelled) returns after the await; wrap the await+logic in try/catch and in the catch call the same warning using formatThreadLoadError(err) (keeping the cancelled check there too). Keep references to dispatch, loadThreads, setSelectedThread, loadThreadMessages, handleCreateNewThread, formatThreadLoadError and cancelled so the behavior is unchanged.
🧹 Nitpick comments (1)
src/openhuman/memory/conversations/store.rs (1)
451-461: 💤 Low valueConsider adding a debug log for the fallback path.
When
thread_summary_unlockedfalls back to the slowmeasure_messages_unlockedpath for legacy threads, a debug log would help diagnose unexpected per-thread file reads during performance investigations. This aligns with the coding guideline to add "copious debug/trace logs...on hard-to-infer branches."🔧 Proposed addition
_ => { + debug!( + "{LOG_PREFIX} thread_summary fallback to file read thread_id={}", + thread_id + ); let (count, last_at) = self.measure_messages_unlocked(thread_id)?; (count, last_at.unwrap_or_else(|| entry.created_at.clone())) }As per coding guidelines: "Add copious debug/trace logs while implementing features...on hard-to-infer branches."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/memory/conversations/store.rs` around lines 451 - 461, thread_summary_unlocked currently falls back to the slow measure_messages_unlocked path when entry.message_count or entry.last_message_at is missing; add a debug log in that fallback branch to record that the slow path was taken and include identifying info (thread_id, entry.created_at, and any available entry.id or thread key) and the resulting (count, last_at) to aid performance debugging; place the log call just before or immediately after calling measure_messages_unlocked so it only executes on the legacy path and use the same logger used elsewhere in this module for consistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@app/src/pages/Conversations.tsx`:
- Around line 275-313: Replace the .then().catch() chain on
dispatch(loadThreads()).unwrap() with an async IIFE or an async function inside
the mount effect: await the result via const data = await
dispatch(loadThreads()).unwrap(); then run the same selection logic (read
store.getState().thread into threadStateForSelect, compute persistedId/resumeId,
dispatch(setSelectedThread(resumeId)) and void
dispatch(loadThreadMessages(resumeId)) or call void handleCreateNewThread())
while guarding with if (cancelled) returns after the await; wrap the await+logic
in try/catch and in the catch call the same warning using
formatThreadLoadError(err) (keeping the cancelled check there too). Keep
references to dispatch, loadThreads, setSelectedThread, loadThreadMessages,
handleCreateNewThread, formatThreadLoadError and cancelled so the behavior is
unchanged.
---
Nitpick comments:
In `@src/openhuman/memory/conversations/store.rs`:
- Around line 451-461: thread_summary_unlocked currently falls back to the slow
measure_messages_unlocked path when entry.message_count or entry.last_message_at
is missing; add a debug log in that fallback branch to record that the slow path
was taken and include identifying info (thread_id, entry.created_at, and any
available entry.id or thread key) and the resulting (count, last_at) to aid
performance debugging; place the log call just before or immediately after
calling measure_messages_unlocked so it only executes on the legacy path and use
the same logger used elsewhere in this module for consistency.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2c997279-d41b-49d8-b875-1937f33c13c2
📒 Files selected for processing (4)
app/src/pages/Conversations.tsxapp/src/pages/__tests__/Conversations.test.tsxsrc/openhuman/memory/conversations/store.rssrc/openhuman/memory/conversations/store_tests.rs
AusAgentSmith
pushed a commit
to AusAgentSmith/openhuman
that referenced
this pull request
May 23, 2026
…ds is O(threads.jsonl) (tinyhumansai#1517)
senamakel
added a commit
to nocstah/openhuman
that referenced
this pull request
Sep 11, 2026
…ds is O(threads.jsonl) (tinyhumansai#1517)\n
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ConversationStore::list_threadsno longer reads every per-thread messages JSONL on every call. It foldsmessage_countandlast_message_atstraight out ofthreads.jsonl.ThreadLogEntryvariants:MessageAppended(incremental, emitted fromappend_message) andStats(absolute snapshot, used to backfill legacy data).list_threadscall after upgrade — one slow per-file read each, then a persistedStatssnapshot so every subsequent call is fast.threads.jsonlstill parses (#[serde(default)]on new fields,Upsert/Deleteshape unchanged).Problem
Sentry OPENHUMAN-REACT-X surfaced as a 30 s timeout on
openhuman.threads_list(unhandled-rejection band-aid landed in #1513). Tracing the call into the core revealed the actual root cause:That's O(threads² + total messages bytes) on every
list_threads, all inside the globalCONVERSATION_STORE_LOCK— so any other conversation RPC queues behind it. A user with 100 threads + a year of chat history can easily push a single call past 30 s on slow disk.Solution
Push stats into
threads.jsonlso folding the log gives us everything:Upsert(unchanged)Delete(unchanged)MessageAppended { thread_id, last_message_at }append_messageunder the existing lockStats { thread_id, message_count, last_message_at }The fold in
thread_index_unlockedcarries(message_count: Option<usize>, last_message_at: Option<String>)per thread.Nonemeans "no stat history yet" →list_threads_unlockeddoes the one-time slow read and writes aStatsentry.thread_summary_unlocked(single-thread queries) reads the index first and only falls back to a per-thread file read when stats are unknown.The hot path is now one sequential read of
threads.jsonl+ a sort. No per-thread file opens.Trade-off / follow-up
The log now grows by one entry per message append. A compaction pass (rewriting
threads.jsonlas a folded snapshot when it crosses a size threshold) is a sane future improvement but not needed for correctness. Not in scope for this PR.Submission Checklist
Statssnapshot, old-format log still parses, delete clears index stats. Existing 15 tests instore_tests.rsall still pass.store.rsis exercised by the new tests (MessageAppendedfold,Statsfold, backfill loop,measure_messages_unlocked,thread_summary_unlockedindex-first path).Impact
list_threadsdrops from O(threads² + total message bytes) to O(threads.jsonl). For a user with a year of chat history, expect a multi-second → sub-millisecond improvement on warm cache, plus the globalCONVERSATION_STORE_LOCKis released sooner so concurrent conversation RPCs stop queuing.legacy_log_without_stats_still_parses). Backward-compatible writes are not relevant — onceMessageAppended/Statsentries are in the log, an older binary opening the same workspace would skip them (serdetagged-enum default behaviour is to fail; this would surface as warnings via the existingread_jsonlline-skip path). In practice this is fine because we don't expect downgrades.~80 bytes). For a user with 10 k messages that's ~800 KB. Acceptable until we add compaction.Related
threads.jsonl(rewrite as folded snapshot when log size > threshold)AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
Validation Run
cargo test --lib conversations::store— 19 / 19 pass (15 pre-existing + 4 new)cargo fmt -- --check src/openhuman/memory/conversations/store{.rs,_tests.rs}clean;cargo check --manifest-path Cargo.tomlcleanValidation Blocked
Behavior Changes
list_threadsreturns the same data shape and ordering, computed from the threads-log index instead of from per-thread file reads.append_messageemits aMessageAppendedentry tothreads.jsonl.CONVERSATION_STORE_LOCKimproves concurrent-RPC behaviour. Same UX otherwise.Parity Contract
thread_summary_unlockedfalls back to the old per-file read path when index stats are missing;list_threadstriggers a one-shot backfill that persists aStatssnapshot, restoring the fast path on the next call.Duplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
Bug Fixes
Performance
Tests