Fix token counting errors and improve context handling #2303
Conversation
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
Fixes Stack trace / error with Copilot + Sonnet related to counting tokens #2232
Unlimited number of subdirectory entries can easily blow up context length (eg. when one of the subdirectories is node_modules). Instead if the number of entries is high (currently hardcoded to a limit of 100) just pass the number of entries; the gatherer can request a directory listing explicitly when needed (the limit doesn't apply to toplevel entries).
It interferes with the prettier in the editor.
There was a problem hiding this comment.
Pull Request Overview
This PR fixes token counting errors and improves context handling by refactoring how token limits are computed and handled, while also removing deprecated dependencies.
- Removed outdated token count and reduction strategies.
- Refactored completion services to support proactive message truncation and improved error handling.
- Updated dependency versions, documentation links, and command strings for consistency.
Reviewed Changes
Copilot reviewed 46 out of 46 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/navie/src/services/message-token-reducer-service/ | Removed deprecated token counting and error parsing strategies. |
| packages/navie/src/services/google-vertexai-completion-service.ts | Renamed methods and refactored class to extend CompletionService. |
| packages/navie/src/services/completion-service.ts | Introduced PromptTooLongError and proactive message truncation logic. |
| packages/navie/src/services/anthropic-completion-service.ts | Updated error handling and adjusted service extensions. |
| packages/navie/src/commands/review-command.ts | Added context overflow callback and updated token limit messaging. |
| packages/cli/src/rpc/explain/collect-location-context.ts | Modified listDirectory to limit subentries and improve directory context extraction. |
| packages/navie/package.json & ESLint config | Updated dependency versions and documentation links for JavaScript. |
| packages/cli/src/cmds/record/action/guessTestCommands.ts | Adjusted command strings for improved consistency. |
Comments suppressed due to low confidence (2)
packages/navie/src/agents/gatherer.ts:34
- [nitpick] Confirm that passing 'onContextOverflow: "throw"' to the completion method aligns with the overall error handling strategy and provides consistent behavior during context overflows.
for await (const token of this.completion.complete(this.conversation, { onContextOverflow: 'throw', }))
packages/navie/src/cmds/record/action/guessTestCommands.ts:37
- Verify that nesting 'npx' commands as in 'npx appmap-node npx mocha' is intentional, as the redundancy may be confusing even if functionally correct.
command: 'npx appmap-node npx mocha',
… improve error messaging
dividedmind
force-pushed
the
fix/gatherer-context-limits
branch
from
b49f18d to
75c765d
Compare
kgilpin
approved these changes
May 23, 2025
|
🎉 This PR is included in version @appland/navie-v1.45.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version @appland/appmap-v3.189.4 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version @appland/search-v1.2.3 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This pull request introduces several updates, primarily focusing on replacing references to the
appmap-agent-jslibrary withappmap-node, improving directory traversal logic, enhancing token limit handling, and making various dependency and configuration adjustments. Below is a summary of the most important changes grouped by theme.Library Replacement
appmap-nodeinstead ofappmap-agent-jsacross multiple files, aligning with the new library name. (README.md[1]guessTestCommands.ts[2]agentProcessNotRunning.ts[3]documentation.js[4]Directory Traversal Enhancements
MAX_SUBENTRIESlimit to improve performance and clarity when listing directory contents. Added logic to summarize directories with excessive subentries and updated tests to validate this behavior. (collect-location-context.ts[1]collect-location-context.spec.ts[2] [3]Token Limit Handling Improvements
GathererandReviewCommandclasses by introducing more robust error handling and user feedback mechanisms. Added support for displaying token limit warnings and suggestions for resolving context size issues. (gatherer.ts[1]explain-command.ts[2]review-command.ts[3] [4]Dependency and Configuration Updates
prettier,ts-node, and addedjs-tiktoken. Removedprettierplugin configuration from.eslintrc.js. (package.json[1] [2].eslintrc.js[3] [4]Refactoring and Cleanup
AnthropicCompletionServiceclass to extend a baseCompletionServiceclass, removing redundant code and centralizing token reduction logic. (anthropic-completion-service.ts[1] [2] [3]Fixes #2232
Fixes #2290