fix: add persistent cost to prevent under-reporting spent value #7763
Conversation
…iving from the messages array max 100 messages are kept in the store/memory which leads to misreporting of the actual spend value
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
|
The following comment was made by an LLM, it may be inaccurate: No duplicate PRs found |
|
|
||
| const cost = createMemo(() => { | ||
| const total = messages().reduce((sum, x) => sum + (x.role === "assistant" ? x.cost : 0), 0) | ||
| const total = session().cost ?? 0 |
There was a problem hiding this comment.
you would want to fallback to the messages cost right cause otherwise opening old sessions would show 0
There was a problem hiding this comment.
It should be handled by the migration script for old sessions as well, but yeah it makes more sense to fallback to the old method instead of defaulting to zero. updated.
|
|
||
| export const getCost = fn(Identifier.schema("session"), async (id) => { | ||
| const read = await Storage.read<Info>(["session", Instance.project.id, id]) | ||
| return read.cost ?? 0 |
There was a problem hiding this comment.
prefer leaving as undefined so we can properly do checks in frontend right
There was a problem hiding this comment.
makes sense, updated
this informs the caller function that cost can be undefined so it can properly handle it, instead of just defaulting to 0.
| async (dir) => { | ||
| log.info("migrating session costs") | ||
| const startTime = Date.now() | ||
| let migratedCount = 0 | ||
|
|
||
| for await (const sessionPath of new Bun.Glob("session/*/*.json").scan({ | ||
| cwd: dir, | ||
| absolute: true, | ||
| })) { |
There was a problem hiding this comment.
do we have to do a migration?
There was a problem hiding this comment.
I think it's fine if only new things are updated, some people have a LOT of sessions
There was a problem hiding this comment.
would be awesome to have a way to run the migration manually if its not automatic.
There was a problem hiding this comment.
@rekram1-node with the fallback to deriving cost from messages present in the memory (from commit affc11a), migration is not strictly required in a sense that it will break the functionality but for older sessions the cost reporting will still be bugged.
from my local testing the migration was fine but again I don't know the extent of how many sessions people have, it could be a concern.
as @maharshi365 mentioned I can also add lazy migration, that only runs when a session is loaded, checks if the session has cost field, and run the migration if needed. I am currently not aware if users can manually run migrations in OC or not, also if it's fine from maintainers perspective to only updated new sessions I am fine with that as well, let me know @rekram1-node .
There was a problem hiding this comment.
I think one time migration is probably way more important than whatever slowness there is in startup. Its not just stale data, but incorrect cost data that is being shown.
3 participants
What does this PR do?
This PR adds a persistent cost field to sessions to fix incorrect cost reporting in the sidebar when sessions exceed 100 messages. Previously, cost was calculated from the last 100 messages only, causing under-reporting for long sessions.
Key changes:
How did you verify your code works?
Verified with before/after testing on the same long session:
OpenCode v1.1.12
OpenCode local
Fixes #7767 #6989