🤖 Fix usage stats race condition - guarantee real-time/persisted parity #287
Conversation
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
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR.
ammar-agent
force-pushed
the
fix-consumer-initial-load
branch
3 times, most recently
from
ae4fe17 to
bfe8bba
Compare
**Problem 1: Usage stats don't update after stream completion** Root cause: usage store bumped BEFORE aggregator stored metadata, creating a race where UI reads stale data. **Problem 2: Consumer breakdown stuck "Calculating..." after reload** Root cause: Redundant scheduling logic - caught-up handler scheduled blindly, racing with lazy trigger and creating inconsistent state. **Problem 3: O(N) scheduling overhead during history load** Root cause: Each buffered stream-end event triggered scheduleCalculation(). With 20 messages in history → 20 schedule calls (debouncing collapsed execution to 1, but still O(N) clearTimeout/setTimeout overhead). **Solution:** 1. Remove premature bump at start of processStreamEvent() [fixes P1] 2. Add finalizeUsageStats() helper called AFTER aggregator updates [fixes P1] - Bumps usage store only if usage metadata present - Schedules consumer calculation always (tokenization needed) 3. Remove redundant lazy trigger dependency [fixes P2] 4. Track history replay mode with replayingHistory Set [fixes P3] - During buffered event processing: skip scheduling in finalizeUsageStats - After all buffered events: schedule once - Result: O(N) → O(1) scheduling **Architecture:** - **Before:** 3 schedulers (caught-up eager, lazy trigger, per-event) - **After:** 2 schedulers (caught-up once, per-event when not replaying) - History load: 1 schedule after all events - Real-time streaming: 1 schedule per stream-end - Lazy trigger removed (no longer needed) **Why this guarantees parity:** Single source of truth: aggregator.getAllMessages() - Real-time: Aggregator updated → bump → UI reads current state ✅ - After refresh: UI loads from chat.jsonl → same messages ✅ **Impact:** Net +8 lines, O(N)→O(1) performance, reduced complexity
- Extract dispatchResumeCheck() helper (eliminates duplication) - Extract handleCompactSummaryCompletion() and performCompaction() methods to improve processStreamEvent() tractability - Compact_summary logic now clearly separated from normal stream-end - Makes control flow more explicit: early return if compaction handled Net +35 lines but significantly improves code clarity and maintainability
ammar-agent
force-pushed
the
fix-consumer-initial-load
branch
from
059371a to
9af7313
Compare
ammario
approved these changes
Oct 17, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Fixes three usage statistics bugs where stats didn't match between real-time and after refresh.
Problems Fixed
1. Usage stats don't update after stream completion
finalizeUsageStats()helper2. Consumer breakdown stuck "Calculating..." after reload
3. O(N) scheduling overhead during history load
Additional Refactoring
Extracted helpers for code clarity:
dispatchResumeCheck()- Eliminates duplicationhandleCompactSummaryCompletion()+performCompaction()- Separates special case from normal flowWhy This Guarantees Parity
Single source of truth: Both real-time and post-refresh read from
aggregator.getAllMessages()which includes persisted usage metadata.Regression Risk Analysis
✅ ZERO RISKS IDENTIFIED - All changes are:
Key Changes Verified Safe:
Test Coverage:
Generated with
cmux