-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Make token counter safer #4924
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make token counter safer #4924
Conversation
crates/goose/src/token_counter.rs
Outdated
| if let Some(item_str) = item.as_str() { | ||
| func_token_count += enum_item; | ||
| func_token_count = | ||
| func_token_count.saturating_add_signed(ENUM_INIT); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this not supposed to be ENUM_ITEM?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops yes
crates/goose/src/token_counter.rs
Outdated
| if let Some(item_str) = item.as_str() { | ||
| func_token_count += enum_item; | ||
| func_token_count = | ||
| func_token_count.saturating_add_signed(ENUM_INIT); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also here.
How is this different from the one in AsyncTokenCounter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this code is clearly duplicated
crates/goose/src/token_counter.rs
Outdated
| Err(e) => panic!("Failed to initialize o200k_base tokenizer: {}", e), | ||
| } | ||
| tiktoken_rs::o200k_base().map(Arc::new).unwrap_or_else(|e| { | ||
| eprintln!("Failed to initialize o200k_base tokenizer: {}", e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks like a log message the user wouldn't know what to do with
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, I know, but I don't know what else we should do. maybe still crash?
|
Where are you seeing this actually get used, local models? The counting should be done generally in provider metadata with this as a fallback. |
we see crashes here and there. most recently: #4513 |
* 'main' of github.com:block/goose: fix: session timestamps (#4913) feat: lazy infinite scroller for session history view (#4922) chore: properly identify when to try oauth (#4918) Make token counter safer (#4924) Rename Hacktoberfest Blog to Hacktoberfest Content (#4926) Include Session ID appropriately in UI (#4901)
…-deeplink * 'main' of github.com:block/goose: (57 commits) Don't set agent props twice (#4872) fix: conversation fixer merges assistant text blocks and drops empty text messages (#4898) Batch fetch remaining issues for documentation updates fix: session timestamps (#4913) feat: lazy infinite scroller for session history view (#4922) chore: properly identify when to try oauth (#4918) Make token counter safer (#4924) Rename Hacktoberfest Blog to Hacktoberfest Content (#4926) Include Session ID appropriately in UI (#4901) fix mcp integration test flakiness (#4871) Add type field to empty schemas for anthropic (#4911) docs: remove subagents from experimental (#4907) CLI: dont show logs to user (#4902) patching the security scanner to redact environment variables (#4908) rmcp upgrade (#4792) feat: Use the screen, goose (#4905) added claude-sonnet-4-5 (#4906) Tiny: fix github casing (#4903) remove anyOf from create_task tool (#4897) chore(deps): bump tracing-subscriber from 0.3.19 to 0.3.20 (#4442) ...
* 'main' of github.com:block/goose: Don't set agent props twice (#4872) fix: conversation fixer merges assistant text blocks and drops empty text messages (#4898) Batch fetch remaining issues for documentation updates fix: session timestamps (#4913) feat: lazy infinite scroller for session history view (#4922) chore: properly identify when to try oauth (#4918) Make token counter safer (#4924) Rename Hacktoberfest Blog to Hacktoberfest Content (#4926) Include Session ID appropriately in UI (#4901) fix mcp integration test flakiness (#4871) Add type field to empty schemas for anthropic (#4911) docs: remove subagents from experimental (#4907) CLI: dont show logs to user (#4902) patching the security scanner to redact environment variables (#4908) rmcp upgrade (#4792) feat: Use the screen, goose (#4905) # Conflicts: # ui/desktop/src/components/ui/scroll-area.tsx
* main: Fix auto scroll to bottom during chat (#4923) Fix Typo, Add Description to Hacktoberfest Content Issue Template (#4931) Don't set agent props twice (#4872) fix: conversation fixer merges assistant text blocks and drops empty text messages (#4898) Batch fetch remaining issues for documentation updates fix: session timestamps (#4913) feat: lazy infinite scroller for session history view (#4922) chore: properly identify when to try oauth (#4918) Make token counter safer (#4924) Rename Hacktoberfest Blog to Hacktoberfest Content (#4926) Include Session ID appropriately in UI (#4901) fix mcp integration test flakiness (#4871)
* main: (44 commits) Add PR template (#4934) Using --resume with --name should still accept session IDs (#4937) Fix auto scroll to bottom during chat (#4923) Fix Typo, Add Description to Hacktoberfest Content Issue Template (#4931) Don't set agent props twice (#4872) fix: conversation fixer merges assistant text blocks and drops empty text messages (#4898) Batch fetch remaining issues for documentation updates fix: session timestamps (#4913) feat: lazy infinite scroller for session history view (#4922) chore: properly identify when to try oauth (#4918) Make token counter safer (#4924) Rename Hacktoberfest Blog to Hacktoberfest Content (#4926) Include Session ID appropriately in UI (#4901) fix mcp integration test flakiness (#4871) Add type field to empty schemas for anthropic (#4911) docs: remove subagents from experimental (#4907) CLI: dont show logs to user (#4902) patching the security scanner to redact environment variables (#4908) rmcp upgrade (#4792) feat: Use the screen, goose (#4905) ...
…-unification * 'main' of github.com:block/goose: (24 commits) Fix auto scroll to bottom during chat (#4923) Fix Typo, Add Description to Hacktoberfest Content Issue Template (#4931) Don't set agent props twice (#4872) fix: conversation fixer merges assistant text blocks and drops empty text messages (#4898) Batch fetch remaining issues for documentation updates fix: session timestamps (#4913) feat: lazy infinite scroller for session history view (#4922) chore: properly identify when to try oauth (#4918) Make token counter safer (#4924) Rename Hacktoberfest Blog to Hacktoberfest Content (#4926) Include Session ID appropriately in UI (#4901) fix mcp integration test flakiness (#4871) Add type field to empty schemas for anthropic (#4911) docs: remove subagents from experimental (#4907) CLI: dont show logs to user (#4902) patching the security scanner to redact environment variables (#4908) rmcp upgrade (#4792) feat: Use the screen, goose (#4905) added claude-sonnet-4-5 (#4906) Tiny: fix github casing (#4903) ... # Conflicts: # ui/desktop/src/components/BaseChat.tsx
* main: (30 commits) feat(nightly): build nightlies from main shas (block#4888) Add missing library for fedora/rhel/centos docs (block#4819) feat(process): Add GOVERNANCE and MAINTAINERS documents (block#4962) Pause test finder, have it run cargo fmt (block#4958) Disable the issue comment trigger on pr-comment-bundle (block#4961) fix(providers): update Claude Sonnet 4 model identifier (block#4884) fix redirect to extensions page after deeplink install and show toast with success message (block#4863) Remove wait-for-ready log (block#4956) docs: add a new goose tip (block#4940) Add PR template (block#4934) Using --resume with --name should still accept session IDs (block#4937) Fix auto scroll to bottom during chat (block#4923) Fix Typo, Add Description to Hacktoberfest Content Issue Template (block#4931) Don't set agent props twice (block#4872) fix: conversation fixer merges assistant text blocks and drops empty text messages (block#4898) Batch fetch remaining issues for documentation updates fix: session timestamps (block#4913) feat: lazy infinite scroller for session history view (block#4922) chore: properly identify when to try oauth (block#4918) Make token counter safer (block#4924) ...
Co-authored-by: Douwe Osinga <[email protected]> Signed-off-by: HikaruEgashira <[email protected]>
fixes things up a bit. most crucially dies less