Fix/tui token count fix #552
Conversation
|
@krissetto kindly review this solution |
|
Hi @krissetto , i found out that there were some double counting issues - i am fxing them now |
|
Hey @riturajFi! 👋 is this ready for testing? If not we should put the PR back into draft for the meantime |
|
Hi @krissetto , the PR is ready for testing - i am just adding some comments, removing some unnecessary stuff and cleaning it up |
riturajFi
force-pushed
the
fix/tui_token_count_fix
branch
from
701b6c2 to
b2df510
Compare
|
Hi @krissetto , would love to see your reviews on this. Kindly suggest any changes and i shall do them asap. Would love seeing it merged. |
|
Hey @riturajFi! I'm not at my machine right now, but I'll give it a look and try it out as soon as I can over the weekend |
|
Thanks a lot @krissetto . Please assign me one more issue in tve meanwhile. I shall work on it! It's very insignful to work on first principle projects like these. |
|
sure! wanna take a look at #665? shouldn't require many changes |
krissetto
left a comment
krissetto
left a comment
There was a problem hiding this comment.
Hey @riturajFi, thanks for the contribution!
I pulled this code and ran a few tests. It looks like the counting is mostly working, but sometimes during task transfers and at the end of a request the count is not representative of the total so something unexpected is happening. I left a suggestion in the code because I think we can simplify this and make it more robust.
Also, looking at the UI aspects of this, i think it might be better to start a bit smaller in scope and only display the total count of tokens used by all sessions, in tokens and cost (ignoring context percentage for now). This way we can get your changes merged more quickly and iterate from there.
There are a few edge cases regarding what context percentage to show, how to organize the sidebar when there are more than just 1-2 sub-agent sessions, sub-agent sessions costs seem truncated, and I don't think we need the "transferred task" text either as it clutters the sidebar.
(ignore the costs all being 0 in the screenshot, I was using local models for this test)
There was a problem hiding this comment.
I think we can simplify things and don't really need this. Each session already contains its own usage data, so we could use less code to implement this total count by traversing the session tree (going from the root agent's session down to its sub-agent sessions) instead of having a separate tracking mechanism
There was a problem hiding this comment.
Actually, even simpler and less code to implement would probably be to append the values from the emitUsageEvent events in the TUI code, and not traverse sessions at all. What do you think?
| } | ||
|
|
||
| // formatCost renders small currency values with enough precision for tiny spends. | ||
| func formatCost(cost float64) string { |
There was a problem hiding this comment.
nit: I don't think we need to display sub 1cent spending, shorter numbers can help with formatting in the limited sidebar space too
|
Thanks @krissetto . I am on it |
2 participants
#504