feat(assistant): add context seam breakdown#44
Merged
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR adds optional token usage breakdown tracking throughout the assistant and terminal modules. The ChangesToken Usage Breakdown Tracking
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 #44 +/- ##
==========================================
+ Coverage 60.49% 60.68% +0.19%
==========================================
Files 170 172 +2
Lines 16836 16901 +65
==========================================
+ Hits 10185 10257 +72
+ Misses 5634 5624 -10
- Partials 1017 1020 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![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/terminal/context_commands.go`:
- Around line 12-15: showContextInfo currently forwards any recognized slash
command to sendPrompt when tokenUsage is empty, causing "/context" to be treated
like a user prompt; change showContextInfo so that if original is the "/context"
command (e.g., original == "/context" or strings.HasPrefix(original,
"/context")), it always emits the context status message via the same logic used
when tokenUsage.HasAny() is true (instead of calling sendPrompt), and only call
sendPrompt for non-slash inputs; reference showContextInfo, sendPrompt, and
tokenUsage.HasAny when applying this conditional handling.
🪄 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: 234e852e-5155-49d8-8bc2-86592554292b
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (22)
internal/assistant/anthropic_internal_test.gointernal/assistant/client.gointernal/assistant/context_build.gointernal/assistant/context_build_test.gointernal/assistant/context_usage.gointernal/assistant/lifecycle.gointernal/assistant/runtime.gointernal/assistant/runtime_events_test.gointernal/assistant/runtime_test.gointernal/assistant/usage.gointernal/assistant/usage_events.gointernal/assistant/usage_test.gointernal/model/usage.gointernal/terminal/app.gointernal/terminal/autocomplete.gointernal/terminal/commands.gointernal/terminal/context_commands.gointernal/terminal/render_parity_test.gointernal/terminal/render_test.gointernal/terminal/token_usage.gointernal/terminal/token_usage_export_test.gointernal/terminal/token_usage_test.go
|
There was a problem hiding this comment.
🧹 Nitpick comments (2)
internal/model/usage_test.go (1)
11-49: ⚡ Quick winConvert these helper tests to a table-driven structure.
These tests validate core
TokenUsagebehavior and should follow the repository’s preferred table-driven style for consistency and easier case expansion.Suggested refactor sketch
func TestTokenUsageHelpers(t *testing.T) { - t.Parallel() - - usage := model.TokenUsage{ - Breakdown: nil, - ContextWindow: 100, - ContextTokens: 25, - InputTokens: 10, - OutputTokens: 15, - } - - assert.Equal(t, 25, usage.TotalTokens()) - assert.True(t, usage.HasAny()) - assert.Equal(t, 25, usage.ContextPercent()) + t.Parallel() + tests := []struct { + name string + usage model.TokenUsage + wantHasAny bool + wantTotal int + wantCtxPercent int + }{ + { + name: "normal usage", + usage: model.TokenUsage{ + Breakdown: nil, + ContextWindow: 100, + ContextTokens: 25, + InputTokens: 10, + OutputTokens: 15, + }, + wantHasAny: true, + wantTotal: 25, + wantCtxPercent: 25, + }, + { + name: "context percent capped", + usage: model.TokenUsage{ + Breakdown: nil, + ContextWindow: 100, + ContextTokens: 250, + }, + wantHasAny: true, + wantTotal: 0, + wantCtxPercent: 100, + }, + { + name: "empty usage", + usage: model.EmptyTokenUsage(), + wantHasAny: false, + wantTotal: 0, + wantCtxPercent: 0, + }, + } + + for _, tc := range tests { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tc.wantHasAny, tc.usage.HasAny()) + assert.Equal(t, tc.wantTotal, tc.usage.TotalTokens()) + assert.Equal(t, tc.wantCtxPercent, tc.usage.ContextPercent()) + }) + } }As per coding guidelines, "
**/*_test.go: Prefer table-driven tests for core behavior and regression tests for terminal rendering bugs".🤖 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/model/usage_test.go` around lines 11 - 49, Convert the three separate tests into a single table-driven test: create a slice of test cases (with name, TokenUsage fields: Breakdown, ContextWindow, ContextTokens, InputTokens, OutputTokens, and expected values for TotalTokens, HasAny, ContextPercent) and iterate with t.Run for each case, calling usage.TotalTokens(), usage.HasAny(), and usage.ContextPercent() and asserting expected values; include a case that uses model.EmptyTokenUsage() for the "empty" scenario, and ensure each subtest calls t.Parallel() to keep parallel behavior as in TestTokenUsageHelpers/TestEmptyTokenUsageHasNoUsage while keeping references to the TokenUsage methods (TotalTokens, HasAny, ContextPercent) and the EmptyTokenUsage helper.internal/terminal/token_usage_test.go (1)
152-188: ⚡ Quick winPrefer a table-driven test for the
/contextbehavior variants.These two tests share almost identical setup and can be consolidated into one table-driven test to make future regression cases cheaper to add.
♻️ Proposed refactor
-func TestShowContextInfoHandlesContextCommandWithoutUsage(t *testing.T) { - t.Parallel() - ... -} - -func TestShowContextInfoDisplaysSummaryAndBreakdown(t *testing.T) { - t.Parallel() - ... -} +func TestShowContextInfo(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + original string + usage model.TokenUsage + check func(t *testing.T, msg string) + }{ + { + name: "context command without usage", + original: "/context", + usage: model.EmptyTokenUsage(), + check: func(t *testing.T, msg string) { + assert.Equal(t, "context:", msg) + }, + }, + { + name: "context command with summary and breakdown", + original: "/context now", + usage: model.TokenUsage{ + Breakdown: map[string]int{ + testBreakdownExtensions: 0, + testBreakdownHistory: 1200, + testBreakdownSystem: 50, + }, + ContextWindow: 1000, + ContextTokens: 250, + }, + check: func(t *testing.T, msg string) { + assert.Contains(t, msg, "context:") + assert.Contains(t, msg, "- ctx 250/1.0k 25%") + assert.Contains(t, msg, "- breakdown:\n - history: 1.2k\n - system: 50") + }, + }, + } + + for _, tc := range tests { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + app := terminal.NewAppForTest() + app.SetTokenUsageForTest(tc.usage) + require.NoError(t, app.ShowContextInfoForTest(tc.original)) + messages := app.MessageContentsForTest() + require.NotEmpty(t, messages) + tc.check(t, messages[len(messages)-1]) + }) + } +}As per coding guidelines
**/*_test.go: Prefer table-driven tests for core behavior and regression tests for terminal rendering bugs.🤖 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/terminal/token_usage_test.go` around lines 152 - 188, Consolidate the two tests TestShowContextInfoHandlesContextCommandWithoutUsage and TestShowContextInfoDisplaysSummaryAndBreakdown into a single table-driven test: define cases for "no usage" and "with usage" that each create an app via terminal.NewAppForTest, optionally call app.SetTokenUsageForTest (only for the "with usage" case) and then call app.ShowContextInfoForTest with the appropriate command; for each case assert messages from app.MessageContentsForTest contain the expected snippets (e.g. "context:" for both, and the summary/breakdown lines only for the "with usage" case). Ensure each case runs t.Run and t.Parallel(), and reuse the same setup/teardown logic from the original tests (terminal.NewAppForTest, ShowContextInfoForTest, SetTokenUsageForTest) so future variants can be added as table entries.
🤖 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/model/usage_test.go`:
- Around line 11-49: Convert the three separate tests into a single table-driven
test: create a slice of test cases (with name, TokenUsage fields: Breakdown,
ContextWindow, ContextTokens, InputTokens, OutputTokens, and expected values for
TotalTokens, HasAny, ContextPercent) and iterate with t.Run for each case,
calling usage.TotalTokens(), usage.HasAny(), and usage.ContextPercent() and
asserting expected values; include a case that uses model.EmptyTokenUsage() for
the "empty" scenario, and ensure each subtest calls t.Parallel() to keep
parallel behavior as in TestTokenUsageHelpers/TestEmptyTokenUsageHasNoUsage
while keeping references to the TokenUsage methods (TotalTokens, HasAny,
ContextPercent) and the EmptyTokenUsage helper.
In `@internal/terminal/token_usage_test.go`:
- Around line 152-188: Consolidate the two tests
TestShowContextInfoHandlesContextCommandWithoutUsage and
TestShowContextInfoDisplaysSummaryAndBreakdown into a single table-driven test:
define cases for "no usage" and "with usage" that each create an app via
terminal.NewAppForTest, optionally call app.SetTokenUsageForTest (only for the
"with usage" case) and then call app.ShowContextInfoForTest with the appropriate
command; for each case assert messages from app.MessageContentsForTest contain
the expected snippets (e.g. "context:" for both, and the summary/breakdown lines
only for the "with usage" case). Ensure each case runs t.Run and t.Parallel(),
and reuse the same setup/teardown logic from the original tests
(terminal.NewAppForTest, ShowContextInfoForTest, SetTokenUsageForTest) so future
variants can be added as table entries.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 67fcde32-79a7-4529-bedf-8ba6a10e7659
📒 Files selected for processing (4)
internal/model/usage_test.gointernal/terminal/context_commands.gointernal/terminal/token_usage_export_test.gointernal/terminal/token_usage_test.go
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
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.
Summary
Validation