Skip to content

Token state not showing on load, or after message is finished.#5606

Merged
jamadeo merged 7 commits intomainfrom
dkatz/token-state-bugfix
Nov 6, 2025
Merged

Token state not showing on load, or after message is finished.#5606
jamadeo merged 7 commits intomainfrom
dkatz/token-state-bugfix

Conversation

@katzdave
Copy link
Collaborator

@katzdave katzdave commented Nov 6, 2025

Two fixes:

On client side use || an issue with truthyness of 0.

On server add token_state to Finish. This ensures the last turn is always included; update_session_metrics doesn't run until after the last Message.

fixes #5604

@katzdave katzdave marked this pull request as ready for review November 6, 2025 16:45
Copilot AI review requested due to automatic review settings November 6, 2025 16:45
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds token state tracking to the Finish event in the message stream, ensuring that final token counts are properly communicated to the frontend when a conversation completes.

  • Added token_state field to the Finish event in both backend and frontend type definitions
  • Extracted token state retrieval logic into a reusable get_token_state helper function
  • Updated frontend to use the token state from Finish events and changed nullish coalescing operators to logical OR operators
  • Removed a debug console.log statement

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
crates/goose-server/src/routes/reply.rs Added token_state to Finish event, extracted token state retrieval into get_token_state helper function
ui/desktop/src/hooks/useMessageStream.ts Updated Finish event type to include token_state and added call to setTokenState in Finish event handler
ui/desktop/src/hooks/useChatEngine.ts Removed debug console.log statement
ui/desktop/src/components/BaseChat.tsx Changed nullish coalescing operators (??) to logical OR operators (||) for token count fallbacks

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

initialValue={input || ''}
setView={setView}
totalTokens={tokenState?.totalTokens ?? sessionTokenCount}
totalTokens={tokenState?.totalTokens || sessionTokenCount}
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using logical OR (||) instead of nullish coalescing (??) means that a valid token count of 0 will be incorrectly treated as falsy and fall back to sessionTokenCount. Use ?? to only fallback on null/undefined values.

Suggested change
totalTokens={tokenState?.totalTokens || sessionTokenCount}
totalTokens={tokenState?.totalTokens ?? sessionTokenCount}

Copilot uses AI. Check for mistakes.
totalTokens={tokenState?.totalTokens || sessionTokenCount}
accumulatedInputTokens={
tokenState?.accumulatedInputTokens ?? sessionInputTokens ?? localInputTokens
tokenState?.accumulatedInputTokens || sessionInputTokens || localInputTokens
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using logical OR (||) instead of nullish coalescing (??) means that a valid token count of 0 will be incorrectly treated as falsy and fall through to the next fallback. Use ?? to only fallback on null/undefined values.

Copilot uses AI. Check for mistakes.
}
accumulatedOutputTokens={
tokenState?.accumulatedOutputTokens ?? sessionOutputTokens ?? localOutputTokens
tokenState?.accumulatedOutputTokens || sessionOutputTokens || localOutputTokens
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using logical OR (||) instead of nullish coalescing (??) means that a valid token count of 0 will be incorrectly treated as falsy and fall through to the next fallback. Use ?? to only fallback on null/undefined values.

Copilot uses AI. Check for mistakes.
Copy link
Collaborator

@jamadeo jamadeo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not quite following the different client-side sources of token counts, do we have a clear idea on why they all exist?

accumulated_input_tokens: 0,
accumulated_output_tokens: 0,
accumulated_total_tokens: 0,
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TokenState could derive Default (if it doesn't already) and this could be TokenState::default()

then this could alaso be get_session(session_id).await.map( .. ).unwrap_or_default()

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

}
accumulatedOutputTokens={
tokenState?.accumulatedOutputTokens ?? sessionOutputTokens ?? localOutputTokens
tokenState?.accumulatedOutputTokens || sessionOutputTokens || localOutputTokens
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are these three sources of token counts and do we need them all? tokenState, sessionOutputTokens, localOutputTokens

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fwiw this is all gone in basecamp2

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Im not convinced sessionOutputTokens is working right; I think that's the reason we were seeing this issue. The stream seems like a reasonable place to do this (although I suspect that might be used for session load because the stream code path is not hit there).

Also really don't know why we have token estimates in the client.

Will take a deeper pass and try to clean shop here.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fwiw this is all gone in basecamp2

👍

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's all going away soon, I wouldn't bother tracking it all down if this gets it working

Copilot AI review requested due to automatic review settings November 6, 2025 17:44
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +446 to 452
totalTokens={tokenState?.totalTokens || sessionTokenCount}
accumulatedInputTokens={
tokenState?.accumulatedInputTokens ?? sessionInputTokens ?? localInputTokens
tokenState?.accumulatedInputTokens || sessionInputTokens || localInputTokens
}
accumulatedOutputTokens={
tokenState?.accumulatedOutputTokens ?? sessionOutputTokens ?? localOutputTokens
tokenState?.accumulatedOutputTokens || sessionOutputTokens || localOutputTokens
}
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing from nullish coalescing (??) to logical OR (||) alters the behavior for falsy values. The nullish coalescing operator only falls back for null or undefined, while || also falls back for 0, empty string, false, etc. Since these are token counts that could legitimately be 0, using || means a value of 0 would incorrectly fall through to the next fallback. The original ?? operator should be retained to preserve correct behavior when token counts are zero.

Copilot uses AI. Check for mistakes.
@DOsinga
Copy link
Collaborator

DOsinga commented Nov 6, 2025

does this work in basecamp2?

…bugfix

* 'main' of github.com:block/goose:
  Change the other location too (#5608)
  feat(ui): bring back quick launcher (#5144)
  Support platform tools through CLI (#5570)
  Avoid web double write (#5601)
@jamadeo
Copy link
Collaborator

jamadeo commented Nov 6, 2025

does this work in basecamp2?

If I set ALPHA=true, I still see the issue with and without this PR

@jamadeo jamadeo merged commit c8d2d47 into main Nov 6, 2025
16 checks passed
wpfleger96 added a commit that referenced this pull request Nov 6, 2025
* main: (60 commits)
  fix: add standard context menu items to prevent empty right-click menu (#5616)
  Bump openapi in prepare-release (#5611)
  docs: add access control section to Developer tutorial (#5615)
  Token state not showing on load, or after message is finished. (#5606)
  Change the other location too (#5608)
  feat(ui): bring back quick launcher (#5144)
  Support platform tools through CLI (#5570)
  Avoid web double write (#5601)
  fix: gemini flash -> pro for mcp smoke tests (#5574)
  Manual compaction test and fix (#5568)
  fix: tidy up claude cli handling (#5594)
  Remove jetbrains (#5602)
  feat(githubcopilot): add support for newer Copilot AI Models (#5603)
  fix: customised recipe to yaml string to avoid minininjia parsing error (#5494)
  Add pending extension indicator to extension panel (#5493)
  Add environment subsition for auth blocks (#5439)
  acp: ToolCallLocations and working cancellation (#5588)
  feat(providers): add Mistral AI provider (#5009)
  Listen for ctrl-c during provider request (#5585)
  Also accept null as description, not just missing (#5589)
  ...
wpfleger96 added a commit that referenced this pull request Nov 6, 2025
* main: (31 commits)
  Standardize CLI argument flags and update documentation (#5516)
  Release 1.13.0
  fix: move goosehints/AGENTS.md handling to goose, and out of developer extension (#5575)
  fix: add standard context menu items to prevent empty right-click menu (#5616)
  Bump openapi in prepare-release (#5611)
  docs: add access control section to Developer tutorial (#5615)
  Token state not showing on load, or after message is finished. (#5606)
  Change the other location too (#5608)
  feat(ui): bring back quick launcher (#5144)
  Support platform tools through CLI (#5570)
  Avoid web double write (#5601)
  fix: gemini flash -> pro for mcp smoke tests (#5574)
  Manual compaction test and fix (#5568)
  fix: tidy up claude cli handling (#5594)
  Remove jetbrains (#5602)
  feat(githubcopilot): add support for newer Copilot AI Models (#5603)
  fix: customised recipe to yaml string to avoid minininjia parsing error (#5494)
  Add pending extension indicator to extension panel (#5493)
  Add environment subsition for auth blocks (#5439)
  acp: ToolCallLocations and working cancellation (#5588)
  ...
michaelneale added a commit that referenced this pull request Nov 7, 2025
* main: (21 commits)
  differentiate debug/release in cache key (#5613)
  Unify subrecipe and subagent execution through shared recipe pipeline (#5082)
  Standardize CLI argument flags and update documentation (#5516)
  Release 1.13.0
  fix: move goosehints/AGENTS.md handling to goose, and out of developer extension (#5575)
  fix: add standard context menu items to prevent empty right-click menu (#5616)
  Bump openapi in prepare-release (#5611)
  docs: add access control section to Developer tutorial (#5615)
  Token state not showing on load, or after message is finished. (#5606)
  Change the other location too (#5608)
  feat(ui): bring back quick launcher (#5144)
  Support platform tools through CLI (#5570)
  Avoid web double write (#5601)
  fix: gemini flash -> pro for mcp smoke tests (#5574)
  Manual compaction test and fix (#5568)
  fix: tidy up claude cli handling (#5594)
  Remove jetbrains (#5602)
  feat(githubcopilot): add support for newer Copilot AI Models (#5603)
  fix: customised recipe to yaml string to avoid minininjia parsing error (#5494)
  Add pending extension indicator to extension panel (#5493)
  ...
fbalicchia pushed a commit to fbalicchia/goose that referenced this pull request Nov 7, 2025
tlongwell-block added a commit that referenced this pull request Nov 7, 2025
* origin/main: (34 commits)
  Remove some logging (#5631)
  Use session IDs as task IDs for subagents instead of UUIDs (#5398)
  Fix the naming (#5628)
  fix: default tetrate model is broken, replace with haiku-4.5 (#5535) (#5587)
  Fetch less and use the right SHA (#5621)
  feat(ui): add custom macOS dock menu with New Window option (#5099)
  feat: remove hints from recipe prompts (#5622)
  docs: October 2025 Community All-Stars spotlight, Hacktoberfest edition (#5625)
  differentiate debug/release in cache key (#5613)
  Unify subrecipe and subagent execution through shared recipe pipeline (#5082)
  Standardize CLI argument flags and update documentation (#5516)
  Release 1.13.0
  fix: move goosehints/AGENTS.md handling to goose, and out of developer extension (#5575)
  fix: add standard context menu items to prevent empty right-click menu (#5616)
  Bump openapi in prepare-release (#5611)
  docs: add access control section to Developer tutorial (#5615)
  Token state not showing on load, or after message is finished. (#5606)
  Change the other location too (#5608)
  feat(ui): bring back quick launcher (#5144)
  Support platform tools through CLI (#5570)
  ...
Surendhar-N-D pushed a commit to Surendhar-N-D/goose that referenced this pull request Nov 17, 2025
arul-cc pushed a commit to arul-cc/goose that referenced this pull request Nov 17, 2025
BlairAllan pushed a commit to BlairAllan/goose that referenced this pull request Nov 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

(release 1.13.0) token counts are zero after first message

3 participants