fix(dashboard): sync live usage audit status#338
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughLive-logs now cache skipped usage by request_id, merge matching usage into audit entries at insert/merge points, refactor usage merge lifecycle and summary helpers, and update tests. Workflows add helpers to detect usage/audit pending state and use them in the chart model. ChangesLive Logs and Workflows Usage-to-Audit Integration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR updates live dashboard previews for usage and audit workflow status. The main changes are:
Confidence Score: 3/5I would fix the live usage merge bugs before merging.
internal/admin/dashboard/static/js/modules/live-logs.js Important Files Changed
Reviews (1): Last reviewed commit: "fix(dashboard): sync live usage audit st..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| }, ...currentEntries].slice(0, this.usageLog.limit || 50); | ||
| }; | ||
| this.applyLiveUsageToAudit(liveEntry); | ||
| if (this.usageLogHideCached && this.liveUsageEntryCached(liveEntry)) return; |
There was a problem hiding this comment.
When usageLogHideCached is enabled and a cached usage.completed event arrives before its audit row, applyLiveUsageToAudit(liveEntry) has no matching audit entry and this branch returns before storing the usage event anywhere. When the later audit.completed event arrives, mergeLiveAuditUsagePatch can only look in usageLog.entries, so the audit row never gets the cached usage summary or pending usage state.
| if (inputTokens > 0 && uncachedInputTokens + cachedInputTokens + cacheWriteInputTokens === 0) { | ||
| uncachedInputTokens = inputTokens; | ||
| } | ||
| const normalizedInputTokens = inputTokens || uncachedInputTokens + cachedInputTokens + cacheWriteInputTokens; |
There was a problem hiding this comment.
This uses input_tokens whenever it is nonzero, but split prompt-cache accounting reports cached reads and cache writes in separate fields. For a live usage event with input_tokens: 100 and cached_input_tokens: 50, the audit summary shows input_tokens: 100 instead of the normalized 150, which also makes the fallback total and cached ratio inconsistent with the server-side request summary.
| const normalizedInputTokens = inputTokens || uncachedInputTokens + cachedInputTokens + cacheWriteInputTokens; | |
| const segmentedInputTokens = uncachedInputTokens + cachedInputTokens + cacheWriteInputTokens; | |
| const normalizedInputTokens = segmentedInputTokens || inputTokens; |
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@internal/admin/dashboard/static/js/modules/live-logs.js`:
- Around line 233-239: The current liveUsageEntryForAudit(…) only searches
this.usageLog.entries and misses live usage events that were received but
filtered/skipped (see early return around lines 329-330); add a persistent map
(e.g., this.skippedLiveUsageByRequestId or this.cachedLiveUsage) on the same
object/class to store any live usage rows that were not inserted into the
visible table, populate that map where the code currently returns early for
filtered/cached entries, and then update liveUsageEntryForAudit to check this
map in addition to this.usageLog.entries (and remove entries from the map when
the row is later inserted/merged) so audits can be enriched by previously
skipped live usage events.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 806ee715-aa93-4e18-8ebf-54daddbe71ff
📒 Files selected for processing (4)
internal/admin/dashboard/static/js/modules/live-logs.jsinternal/admin/dashboard/static/js/modules/live-logs.test.cjsinternal/admin/dashboard/static/js/modules/workflows.jsinternal/admin/dashboard/static/js/modules/workflows.test.cjs
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@internal/admin/dashboard/static/js/modules/live-logs.js`:
- Around line 229-231: mergeLiveAuditUsagePatch currently reads a skipped usage
from skippedLiveUsageByRequestId (via liveUsageEntryForAudit) and merges it into
the audit entry with auditEntryWithLiveUsage but never removes that skipped
entry, causing retained stale entries; update mergeLiveAuditUsagePatch to, after
creating the merged entry, delete the corresponding key from
skippedLiveUsageByRequestId (or call the component method that evicts that
entry) so the skipped usage is evicted once attached, and apply the same
eviction change to the other similar function(s) handling live usage merging
around the other block referenced (the 349-362 region) to avoid cache growth and
stale summaries.
- Around line 306-323: When an existing usage row is updated (the branch that
uses mergeLiveUsagePatch(previous, incoming) and returns early after
applyLiveUsageToAudit), re-run the same hide/caching checks used for new rows:
after calling applyLiveUsageToAudit(merged) call the same conditional logic that
checks usageLogHideCached && liveUsageEntryCached(merged) and
usageLiveInsertAllowed(), and if either indicates the entry should be skipped
call storeSkippedLiveUsage(merged) and remove the merged entry from
this.usageLog.entries (and call removeSkippedLiveUsage(merged) as needed) so an
updated row that becomes cached is moved to skipped just like a newly inserted
row.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ccae2dc9-9b4d-460b-84f0-ee824c4adeea
📒 Files selected for processing (2)
internal/admin/dashboard/static/js/modules/live-logs.jsinternal/admin/dashboard/static/js/modules/live-logs.test.cjs
2 participants
Summary
Tests
Summary by CodeRabbit
New Features
Improvements
Tests