refactor: removed the token calculator logic#636
Open
blackmammoth wants to merge 2 commits intomainfrom
Open
Conversation
The information given was not accurate for the model providers
Contributor
📝 WalkthroughWalkthroughThis PR removes token-usage and token-budget tracking across backend, adapters, API surface, and frontend UI/state, deleting the token-usage endpoint, built-in /cost command, token-related normalized/status messages, and associated UI components/props. Changes
Sequence Diagram(s)sequenceDiagram
participant Provider as Provider Adapter
participant Server as Server (normalize/SDK)
participant WS as WebSocket Emission
participant Client as Frontend Client
Provider->>Server: emits result/turn.completed (with usage)
alt Old flow (before PR)
Server->>WS: emit normalized messages + status(token_budget)
WS->>Client: receive status(token_budget) -> update tokenBudget/UI
else New flow (after PR)
Server->>WS: emit normalized messages (no token_budget status)
WS->>Client: receive messages -> no tokenBudget updates
end
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
Contributor
|
@blackmammoth this removes more than it should actually. The tokens being returned from at least claude code are accurate. the visualization we have indeed is not relevant anymore. But the tokens are coming straight out of the sdk. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
server/projects.js (1)
1814-1814: Minor inconsistency in return structure.The non-paginated return at line 1814 returns
{ messages }while the paginated return (lines 1805-1811) returns{ messages, total, hasMore, offset, limit }. This asymmetry could cause issues for callers expecting consistent structure.Consider returning a consistent structure for both paths:
♻️ Suggested fix for consistent return structure
- return { messages }; + return { messages, total: messages.length, hasMore: false, offset: 0, limit: null };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/projects.js` at line 1814, The final non-paginated return currently returns only { messages } and should match the paginated shape used earlier ({ messages, total, hasMore, offset, limit }); update the return at the end of the function in server/projects.js so it returns the same keys (e.g., include total, hasMore, offset and limit with sensible defaults like total = messages.length, hasMore = false, offset = 0 and limit = messages.length or null) to ensure callers always receive a consistent response shape.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@server/projects.js`:
- Line 1814: The final non-paginated return currently returns only { messages }
and should match the paginated shape used earlier ({ messages, total, hasMore,
offset, limit }); update the return at the end of the function in
server/projects.js so it returns the same keys (e.g., include total, hasMore,
offset and limit with sensible defaults like total = messages.length, hasMore =
false, offset = 0 and limit = messages.length or null) to ensure callers always
receive a consistent response shape.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 400c9012-2e14-442a-98d9-d85e8f046b97
📒 Files selected for processing (19)
public/api-docs.htmlserver/claude-sdk.jsserver/index.jsserver/openai-codex.jsserver/projects.jsserver/providers/codex/adapter.jsserver/providers/gemini/adapter.jsserver/providers/types.jsserver/routes/agent.jsserver/routes/commands.jssrc/components/chat/hooks/useChatComposerState.tssrc/components/chat/hooks/useChatRealtimeHandlers.tssrc/components/chat/hooks/useChatSessionState.tssrc/components/chat/view/ChatInterface.tsxsrc/components/chat/view/subcomponents/ChatComposer.tsxsrc/components/chat/view/subcomponents/ChatInputControls.tsxsrc/components/chat/view/subcomponents/ClaudeStatus.tsxsrc/components/chat/view/subcomponents/TokenUsagePie.tsxsrc/stores/useSessionStore.ts
💤 Files with no reviewable changes (8)
- server/claude-sdk.js
- server/providers/codex/adapter.js
- server/routes/commands.js
- src/components/chat/view/ChatInterface.tsx
- src/stores/useSessionStore.ts
- server/index.js
- src/components/chat/view/subcomponents/TokenUsagePie.tsx
- src/components/chat/view/subcomponents/ChatInputControls.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/routes/agent.js`:
- Around line 549-554: The code currently deletes provider-native usage by
removing assistantMessage.message.usage before pushing to assistantMessages;
instead, stop deleting message.usage so the raw provider usage block from
parsed.data is preserved for non-streaming responses—remove the lines that clone
and delete assistantMessage.message.usage in the assistantMessage creation (and
the parallel block that does the same around the 1131–1139 pattern) so
parsed.data.message.usage is retained when pushing to assistantMessages.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 445ea302-be74-4b2a-8eb0-97580732b86f
📒 Files selected for processing (3)
public/api-docs.htmlserver/index.jsserver/routes/agent.js
💤 Files with no reviewable changes (1)
- server/index.js
✅ Files skipped from review due to trivial changes (1)
- public/api-docs.html
Comment on lines
+549
to
+554
| const assistantMessage = { ...parsed.data }; | ||
| if (assistantMessage.message?.usage) { | ||
| assistantMessage.message = { ...assistantMessage.message }; | ||
| delete assistantMessage.message.usage; | ||
| } | ||
| assistantMessages.push(assistantMessage); |
Contributor
There was a problem hiding this comment.
Preserve provider-native message.usage in non-streaming responses.
This strips the Claude SDK’s raw usage block from messages, and the non-streaming response later returns those filtered messages directly. That removes the only accurate token data for non-streaming clients while streaming clients still receive it, which is broader than removing the derived token calculator.
Suggested fix
- if (parsed.type === 'claude-response' && parsed.data && parsed.data.type === 'assistant') {
- const assistantMessage = { ...parsed.data };
- if (assistantMessage.message?.usage) {
- assistantMessage.message = { ...assistantMessage.message };
- delete assistantMessage.message.usage;
- }
- assistantMessages.push(assistantMessage);
+ if (parsed.type === 'claude-response' && parsed.data && parsed.data.type === 'assistant') {
+ assistantMessages.push({
+ ...parsed.data,
+ ...(parsed.data.message ? { message: { ...parsed.data.message } } : {})
+ });
}Also applies to: 1131-1139
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/routes/agent.js` around lines 549 - 554, The code currently deletes
provider-native usage by removing assistantMessage.message.usage before pushing
to assistantMessages; instead, stop deleting message.usage so the raw provider
usage block from parsed.data is preserved for non-streaming responses—remove the
lines that clone and delete assistantMessage.message.usage in the
assistantMessage creation (and the parallel block that does the same around the
1131–1139 pattern) so parsed.data.message.usage is retained when pushing to
assistantMessages.
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
3 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.
The information given was not accurate for the model providers. We can add it later on when we can get accurate responses from the sdks/spawned processes.
Summary by CodeRabbit