feat(context): use provider usage for compaction - #90
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe pull request threads provider-reported token usage anchors through context building and usage estimation, adds split-turn summaries for compaction when a boundary splits an in-progress turn, and refactors session-store append logic to persist per-entry token-usage metadata and track/clear anchored usage during context replay. ChangesToken usage anchoring and split-turn context compaction
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ 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)
Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #90 +/- ##
==========================================
+ Coverage 69.24% 69.40% +0.15%
==========================================
Files 215 217 +2
Lines 19104 19236 +132
==========================================
+ Hits 13229 13350 +121
- Misses 4722 4729 +7
- Partials 1153 1157 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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/database/session_store.go`:
- Around line 582-597: The function updateContextUsageAnchor currently swallows
the error returned by dataFromEntry; change its signature to return error (func
updateContextUsageAnchor(...) error), check and return the err when
dataFromEntry fails, and update all callers to handle/propagate that error; keep
the existing early-returns for non-message/assistant and for nil/empty usage,
but ensure any dataFromEntry error is returned instead of ignored so callers of
updateContextUsageAnchor (and related callers in session store flow) can handle
or log it appropriately.
🪄 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: CHILL
Plan: Pro
Run ID: 4398eb74-c627-4b71-b5d6-40c718e93c4b
📒 Files selected for processing (10)
internal/assistant/context_build.gointernal/assistant/context_compaction.gointernal/assistant/context_compaction_internal_test.gointernal/assistant/context_usage_led.gointernal/assistant/context_usage_led_internal_test.gointernal/assistant/runtime_context.gointernal/assistant/runtime_entries.gointernal/database/entity.gointernal/database/session_store.gointernal/database/session_usage_test.go
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/database/session_store.go (1)
661-680: 💤 Low valueUnreachable
return nilat line 679.The switch statement exhaustively returns in all branches (including the fallthrough for unhandled types at lines 670-676), making the final
return nildead code.♻️ Suggested cleanup
case EntryTypeCompaction, EntryTypeModelChange, EntryTypeThinkingLevelChange, EntryTypeCustom, EntryTypeLabel, EntryTypeSessionInfo: return nil + default: + return nil } - - return nil }🤖 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 `@internal/database/session_store.go` around lines 661 - 680, The trailing "return nil" in appendRetainedCompactionTailEntry is redundant/unreachable; add an explicit default case in the switch that returns nil (or move the existing final return into a default) and then remove the final standalone "return nil" so all code paths return from inside the switch; update the switch in appendRetainedCompactionTailEntry accordingly.
🤖 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.
Nitpick comments:
In `@internal/database/session_store.go`:
- Around line 661-680: The trailing "return nil" in
appendRetainedCompactionTailEntry is redundant/unreachable; add an explicit
default case in the switch that returns nil (or move the existing final return
into a default) and then remove the final standalone "return nil" so all code
paths return from inside the switch; update the switch in
appendRetainedCompactionTailEntry accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0352b69e-014a-4daf-9777-ad6826ef2d68
📒 Files selected for processing (2)
internal/database/session_store.gointernal/database/session_usage_internal_test.go
|



Summary
Validation