Conversation
|
The following comment was made by an LLM, it may be inaccurate: Related PRs Found#12721 - feat(tui): add tokens per second to response footer
#42372 - feat(app): show tokens-per-second in context usage indicator
The PR description explicitly acknowledges these relationships and indicates that #12721 should take precedence if rebased. The current PR (#46108) is a cleaner implementation of the same design on a current branch. |
|
Tested this PR on Windows 11 (native, Windows Terminal, bun 1.3.14) from
Example from a quick session — same model, back-to-back responses:
That per-response variance visibility is exactly the use case described in #43857 — the slower response was same-model, so it was invisible until now. Works as expected on Windows; hope this can get merged. |
|
ae0e69a to
8a662fd
Compare
Upstream PR anomalyco#12721 by @JohnC0de, reimplemented against the current TUI. The tokens-per-second formula and its validity guards are theirs; that PR targeted `packages/opencode/src/cli/cmd/tui/routes/session/`, which no longer exists, so this reimplements the feature against `packages/tui/` and adds the `time.firstToken` stamp the calculation needs. Co-authored-by: João Carlos Magalhães de Castro <88864312+JohnC0de@users.noreply.github.com>
isValidForTPS had no caller outside its own tests. Its checks duplicate getMessageTPS, which returns undefined for exactly the same rejection cases, so the export widened the public surface without adding reach. The two properties its block covered that getMessageTPS did not — a negative token total and an explicitly null finish — move into the getMessageTPS block, so coverage is unchanged.
8a662fd to
7d2cd35
Compare
2 participants
Issue for this PR
Closes #6096
Type of change
Credit
This is @JohnC0de's #12721 rebased onto current
dev— same design (TPS calc incore/so non-TUI consumers can use it,firstTokenrecorded inprocessor.ts), which is itself a clean take on @edlsh's #5497. #12721 only went stale — it'sdirtyagainstdevnow. If @JohnC0de rebases it, theirs should land and I'll close this. Take anything from here that helps.What does this PR do?
Adds a
tok/sreadout to the assistant footer, after the duration:18.3s · 131 tok/s.processor.tsstampstime.firstTokenon the first streamed event (tool-only turns included). TPS isgeneratedTokens / ((completed − firstToken) / 1000), output plus reasoning. The calc lives inpackages/core/src/session/tokens.tsso the SDK can reuse it;firstTokenis optional on the message schema and persisted, so a session-level average can come later without a migration.One departure from #12721: tool-call turns are not filtered out. A turn that ends in a tool call is one provider step with its own usage and timestamps, and in practice it's most turns — on my box over three days, 83% of assistant turns in top-level sessions and 98% in subagent sessions end in
tool-calls, so filtering them made the readout look intermittent. Sampled tool-call turns give ordinary rates (47–183 tok/s). Errored turns, summaries, and unfinished turns are still skipped, and the 250 ms floor stays — it's what suppresses non-streamed responses that arrive in one chunk (a 228-token turn landing in 43 ms would otherwise read as 5,000 tok/s).How did you verify your code works?
The timestamp guard is compiler-enforced, not test-enforced: delete the narrowing and it's a type error (
'completed' is possibly 'undefined'). The zero-token, sub-threshold, rounding, and finish-reason guards each go red when reverted.packages/coreTPS tests: 17 pass / 0 failpackages/opencodefull suite: 3396 pass / 0 failbun typecheckclean inopencode,core,tuiScreenshots / recordings
Footer gains a
· NN tok/ssegment; no new surface.Checklist