-
Notifications
You must be signed in to change notification settings - Fork 101
Show per-request tokens in the visualizer #804
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
Conversation
26d5519 to
4b02bb7
Compare
Co-authored-by: openhands <openhands@all-hands.dev>
4b02bb7 to
6350e2b
Compare
|
[Automatic Post]: I have assigned @neubig as a reviewer based on git blame information. Thanks in advance for the help! |
xingyaoww
left a comment
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.
Seems good to me! Although i'd hope we can maybe control this with a flag (eg display per-turn vs accumulated)
Another thing i've been thinking about would be to display something like
Input: 100K (+4k), where the +4K is the newly added per-request tokens
|
TBH I initially made a flag, but then I simplified, because I thought, is it really a need for the visualiser? I'm just thinking, this not a a full interface or end user app. Do you think we have a good use case for looking at accumulations? It's logged already (in stats). |
|
Ah, maybe we are not talking about the same numbers? There are actually 3 possibilities:
|
I think it is helpful to have some of these to get a sense of the current token usage 🤔 actually i kinda feel "per-request" token stats isn't all that informative to me though. Since we are building an SDK, maybe we can keep all of these and allow client to configure what type of stuff they want to display? |
|
@OpenHands Help me here. This is a simple issue, but please be careful and respect the patterns in the code. Read the full issue, including comments / reviews, and let's
Feel free to think it over, and understand this is an SDK, we want to offer good programmatic APIs, so maybe adjust to return each value in a consistent way or suggest and implement a better way. |
|
I'm on it! enyst can track my progress at all-hands.dev |
… add env/config flags - Add token_display utility with compute_token_display() returning a typed TokenDisplay - Support OPENHANDS_VIZ_TOKENS_MODE to switch between per_context (default) and accumulated - Add include_since_last flag (and OPENHANDS_VIZ_TOKENS_DELTA env) to optionally show (+delta) since last request - Refactor visualizer to use the utility and keep formatting consistent Co-authored-by: openhands <openhands@all-hands.dev>
…it__ - Export TokenDisplay, TokenDisplayMode, compute_token_display, get_default_mode_from_env Co-authored-by: openhands <openhands@all-hands.dev>
|
Summary of changes just pushed to feature/agent-sdk-11-visualizer-token-toggle I implemented a small, reusable token display API and refactored the visualizer to use it, plus added env/config controls as requested. What’s implemented
Checklist against your request
Files changed
Notes
|
- Merged main into branch with repo reorg (openhands-sdk/... layout) - Moved token_display under openhands-sdk/openhands/sdk/conversation/ - Rename env vars to OH_TOKENS_VIEW_MODE and OH_TOKENS_VIEW_DELTA Co-authored-by: openhands <openhands@all-hands.dev>
…gles - Tests for PER_CONTEXT with delta, ACCUMULATED mode, and visualizer env toggles Co-authored-by: openhands <openhands@all-hands.dev>
Coverage Report •
|
|||||||||||||||||||||||||||||||||||
Co-authored-by: openhands <openhands@all-hands.dev>
|
[Automatic Post]: I have assigned @xingyaoww as a reviewer based on git blame information. Thanks in advance for the help! |
|
@OpenHands please merge from main and resolve conflicts |
|
I'm on it! xingyaoww can track my progress at all-hands.dev |
Resolved conflicts in: - openhands-sdk/openhands/sdk/conversation/__init__.py: Added get_agent_final_response to __all__ exports along with existing token display utilities - openhands-sdk/openhands/sdk/conversation/visualizer.py: Kept the new token display implementation and removed duplicate code from main Co-authored-by: openhands <openhands@all-hands.dev>
SummarySuccessfully merged Changes MadeMerge Commit: Resolved conflicts in 2 files:
Verification✅ All pre-commit checks passed: ruff format, ruff lint, pycodestyle, pyright
The feature branch is now up-to-date with main and ready for review. |
|
Looks like there are a few issues preventing this PR from being merged!
If you'd like me to help, just leave a comment, like Feel free to include any additional details that might help me get this PR into a better state. You can manage your notification settings |
|
[Automatic Post]: It has been a while since there was any activity on this PR. @enyst, are you still working on it? If so, please go ahead, if not then please request review, close it, or request that someone else follow up. |
|
@OpenHands please understand this PR, then merge main branch and fix the conflicts |
|
I'm on it! enyst can track my progress at all-hands.dev |
…t new base visualizer architecture and token_display API. - Resolve merge conflict in visualizer/default.py by integrating token_display utilities and ConversationVisualizerBase - Provide ConversationVisualizer compatibility wrapper for tests - Fix MessageEvent panel duplication Co-authored-by: openhands <openhands@all-hands.dev>
|
Final summary of work What I did
Verification
Checklist
Next steps
|
|
[Automatic Post]: It has been a while since there was any activity on this PR. @enyst, are you still working on it? If so, please go ahead, if not then please request review, close it, or request that someone else follow up. |
1 similar comment
|
[Automatic Post]: It has been a while since there was any activity on this PR. @enyst, are you still working on it? If so, please go ahead, if not then please request review, close it, or request that someone else follow up. |
|
@OpenHands Read all comments/reviews and understand the goal of this PR, as it is after reviewer feedback. You do not need to keep this code. Just understand the comments. Then make a new branch from main and implement it. Open a new PR. |
|
I'm on it! enyst can track my progress at all-hands.dev |
|
Final summary of work What I did
Changes made (concise)
Verification
Checklist against your request
PR link Notes/next steps
|
This PR proposes to display in the default visualizer the tokens for the last request, not the accumulated values. I'd find it much more useful to keep an eye on the context window.
(The accumulated input tokens get into millions fast, and then they don't mean much really?)
IMHO this makes the default visualizer more useful, for tracking context window, reasoning tokens, what happened each request.
Tested and this is what it looks like:

Summary
Testing
Fixes #803
Agent Server images for this PR
• GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server
Variants & Base Images
eclipse-temurin:17-jdknikolaik/python-nodejs:python3.12-nodejs22golang:1.21-bookwormPull (multi-arch manifest)
# Each variant is a multi-arch manifest supporting both amd64 and arm64 docker pull ghcr.io/openhands/agent-server:9ac2d51-pythonRun
All tags pushed for this build
About Multi-Architecture Support
9ac2d51-python) is a multi-arch manifest supporting both amd64 and arm649ac2d51-python-amd64) are also available if needed