Skip to content

fix(stats): keep reverted usage in session stats - #57928

Open
lakshya-dhariwal wants to merge 20 commits into
anomalyco:devfrom
lakshya-dhariwal:patch-2
Open

lakshya-dhariwal wants to merge 20 commits into
anomalyco:devfrom
lakshya-dhariwal:patch-2

Conversation

@lakshya-dhariwal

@lakshya-dhariwal lakshya-dhariwal commented Oct 21, 2026 •

Copy link
Copy Markdown

Issue for this PR

Closes #57878

Type of change

  • Bug fix

What does this PR do?

Committing a revert deletes the message rows from the boundary on, and SessionStats sums usage from the rows that remain, so reverted work drops out of opencode stats even though the session's stored counters still include it.

On commit, the usage of the assistant rows about to be deleted is now recorded as a durable UsageRecorded event with a new source: "revert", published before RevertEvent.Committed. Stats counts revert events the same way it already counts compaction ones, so the totals keep the reverted work. The projector does not apply these events to the session counters: the steps already added their usage there when they ended, and applying it again would double count.

Only token and cost totals are preserved; steps, prompts and per-model breakdowns still reflect the remaining messages, since UsageRecorded carries no step counts.

How did you verify your code works?

Added a test in session-revert.test.ts: run two steps with known usage, stage a revert to the second prompt, then prompt again to commit it. On current dev stats lose the reverted step (1000 of 2000 input tokens); with the change stats keep the full 2000 tokens and $1.00 cost, and the stored session counters are unchanged. The test fails without the fix and passes with it. The session-stats, session-projector and session-compaction suites all pass, and the client types were regenerated with bun run generate.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

Added by the opencode bot (AI-generated).

Related: #46371, #48306

Public API: adds a session_revert_usage table to the local database through a new migration (additive, backwards compatible)

@opencode-agent opencode-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't fix #57878 in a real run yet, and it adds new type errors.

How I checked: I ran the issue's reproduction (mock provider, opencode serve from source) on the base and on this branch. Both show 2 prompts · 2 steps · 2.2k tokens after the revert is committed, where 3.3k is expected. The local server's event bus doesn't save events (persist defaults to false; only workerd turns it on), so the event table stays empty and the new revert record is lost. The new test passes only because it builds the bus with persist: true. The issue's analysis warned about this (the reporter's event table was empty). The usage needs to be kept somewhere that always persists, for example a column or table written when the revert is committed, not a durable event.

I also forked a session and reverted it into the copied history: stats went from 2000 to 3000 input tokens and from $1.00 to $1.50, with no new model call (see inline). bun run typecheck in packages/core now fails; the base has no errors (see inline). The revert, stats and projector tests pass.

Public API: the durable session.usage.recorded event's source gains the value "revert"; the regenerated client types change to match. Producers aren't affected. Consumers that exhaustively match source ("title" | "compaction") will now see a new value.

Related: #46371, #48306

Comment thread packages/core/src/session/revert.ts Outdated

export const commit = Effect.fn("SessionRevert.commit")(function* (bus: Bus.Interface, session: SessionSchema.Info) {
if (!session.revert) return
const database = yield* Database.Service

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

commit now gets Database.Service from its environment, and that requirement leaks into its callers. bun run typecheck in packages/core fails with new errors in src/session.ts (475, 502), src/plugin/host.ts:570, src/config/plugin/command.ts:80, src/tool/plugin/subagent.ts:109 and test/session-shell.test.ts:198; the base has none. Passing the db in, or providing it where commit is called, would avoid widening the Session API types.

Comment thread packages/core/src/session/revert.ts Outdated
FROM ${SessionMessageTable}
WHERE ${SessionMessageTable.session_id} = ${session.id}
AND ${SessionMessageTable.type} = 'assistant'
AND ${SessionMessageTable.seq} >= ${boundary.seq}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This adds up every assistant row from the boundary onward, including rows a fork copied from its parent. Stats leaves those out with session.fork_session_id IS NULL OR message.time_created >= session.time_created (#31032). I forked a session after two steps, then staged and committed a revert in the fork to the copied second prompt: stats went from 2000 to 3000 input tokens and from $1.00 to $1.50, with no new model call. The sum needs the same fork filter, and a test with a fork.

This only reaches stats if the bus persists durable events. The local CLI and server run with persist false (only packages/server/src/workerd.ts sets events.persist: true), so the event table stays empty and stats never see this record. With the issue's reproduction against opencode serve from this branch, stats still show 2.2k tokens after the commit instead of 3.3k. The test passes only because it builds the bus with Bus.configured({ persist: true }). Even with persistence on, the event's created time is the commit time, so reverted usage would land in the commit's day and range, not when the steps ran.

@opencode-agent opencode-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both earlier points are fixed. With the issue's reproduction against opencode serve from this branch, stats now show 3 prompts · 3 steps · 3.3k tokens after the revert is committed. The reverted rows go into a table that the projector writes whether or not events are saved, they use the same fork filter as stats, and they keep their original times. bun run typecheck in packages/core is clean, and the revert, stats, projector and compaction tests pass, including the new fork test.

Public API: the event schema is no longer changed. The local database gains a session_revert_usage table through a new migration. The change is additive and backwards compatible; existing data is untouched.

One small thing: the branch is based on an older dev, and the 'restore upstream' commits copied newer files into it. GitHub's diff therefore shows unrelated-looking edits in projector.ts, revert.ts and session-event.ts that already match current dev. Rebasing on dev would remove them from the diff.

Merge upstream dev into patch-2

@opencode-agent opencode-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing new to raise. This push only merges dev, and the diff now shows only the stats change. The issue's reproduction against opencode serve from this branch still shows 3 prompts · 3 steps · 3.3k tokens after the revert is committed. The revert, stats, projector and compaction tests pass, and bun run typecheck in packages/core is clean.

Public API: unchanged. The local database gains a session_revert_usage table through a new migration; the change is additive and backwards compatible.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cli: stats totals differ from stored session usage

1 participant