- Issue created by @murz
- @murz opened merge request.
- 🇬🇧United Kingdom yautja_cetanu
I don't know if this is relevant or not but one thing to remember is we don't always have token usage information. Depends on the provider and we can't always accurately calculate tokens, we can use tiktoken to approximate it to what it would be with chatgpt.
- 🇦🇲Armenia murz Yerevan, Armenia
@yautja_cetanu, good poin! so in that case we can extend the proposed TokenUsageDto with a method to calculate tokens using tiktoken. But this will already not be DTO, but a smarter object, and it seems we should rename it to the TokenUsageData, or something.
- 🇦🇲Armenia murz Yerevan, Armenia
Still decided to keep the TokenUsageDto as a simple DTO, not something complex. But on every stage we can put new values to it, so use the same tiktoken to calculate the average usage. MR now is ready for review.
- 🇧🇪Belgium svendecabooter Gent
Isn't this (partially) implemented in 📌 Add token usage to streamed chat Active , or am I reading this incorrectly?
- 🇦🇲Armenia murz Yerevan, Armenia
Yes, this is implemented in the 📌 Add token usage to streamed chat Active , but managing five separate properties + getters-setters is not convenient, because after the char response we should pass them to events, from events to logs, and so on.
So, this improvement wraps all the token usage into one DTO object, but keeps the backward functionality (as deprecated functions). - 🇧🇪Belgium svendecabooter Gent
Thanks for the clarification.
Sounds like a good improvement to me. - 🇩🇪Germany marcus_johansson
Thank you - I tried to think of any use cases where we need this object to go beyond being a DTO, but all of the ones I could think of happens outside this object.
So it looks good and is getting merged to 1.2.x-dev.
- 🇩🇪Germany marcus_johansson
Saw that the new deprecations will have to be handled.