fix: include hidden thinking in TPS timing - #413
Conversation
hetaoBackend
left a comment
There was a problem hiding this comment.
Reviewed the timing change and its consumers. LGTM overall: one nit inline and one non-blocking follow-up.
What I checked:
- Every pi provider that emits
thinking_startdoes so when a real reasoning block opens: Anthropicthinking/redacted_thinkingcontent_block_start, the Responsesreasoningitem, and Completions, Bedrock, Google, and Mistral on the first reasoning content. None synthesizes it early enough to pull prefill or queue wait into the decode window. observedAtMsis captured inpi-turn-runner/events.tsbefore the serialized queue, and later events wait for message id allocation. TheactiveAssistantMessageIdguard therefore does not drop the normalthinking_start.- The only consumers of
decode_duration_ms(TUITuiTurnOutputRateand local-runtime-v2 content application) divide provider output tokens by it, so moving the start boundary earlier is the right fix at this layer. - Locally, restoring
bridge.tsfrommainmakes 4 of the new tests fail (expected 20 to be 3000/60000). All 11 pass on this branch.
Non-blocking follow-up: OpenAI Chat Completions-style endpoints that hide reasoning entirely never emit thinking_start, while completion_tokens already includes reasoning_tokens (openai-completions.ts L1036-1037). TPS on that path stays inflated. The description covers this in general terms ("fully buffered streams without an early thinking event"); a tracking issue naming this path would make it easier to pick up later.
| observedAtMs?: number, | ||
| ): BridgedEvents { | ||
| // Output usage includes thinking tokens even when their text is hidden. | ||
| // Keep the timing boundary aligned with that usage, before queue delays. |
There was a problem hiding this comment.
Nit: the field doc for firstTokenMs (L167) still reads "First nonempty text/thinking/tool token". The boundary can now also be the first thinking_start, so please update it too, for example: First thinking_start or nonempty text/thinking/tool token; excludes first-token wait from throughput. Otherwise a later reader may fold this block back into the nonempty-delta check below and reintroduce the bug.
There was a problem hiding this comment.
Addressed in 4bff012: updated the firstTokenMs field doc to mention thinking_start.
|
Pushed d6ff1ed, unrelated to the TPS change: |
…ar-mode history) (#108) * fix: include hidden thinking in TPS timing (MiniMax-AI#413) * fix: include hidden thinking in TPS timing * docs: note thinking_start in first token timing comment * test(windows): allow the NTFS source check to exceed the default timeout --------- Co-authored-by: hetaoBackend <hetao7@pku.edu.cn> * fix: omit empty tools from OpenAI compaction requests (MiniMax-AI#199) * fix: omit empty tools on OpenAI compaction requests * fix: negotiate missing tools with bounded compatibility recovery * fix: simplify empty tools handling to omit absent definitions * fix(tui): keep regular-mode history stable during live runs (MiniMax-AI#416) Regular mode wrote rows into native scrollback as soon as they scrolled off screen. When such a row later changed (a parallel tool finishing, a streamed table widening, a list turning loose, a turn growing past the projection fold, the welcome status badge flipping, or a stopped prompt gaining its cancelled marker), the renderer cleared scrollback and replayed the whole session from the welcome logo. Only final rows now reach native history: - The transcript reports how many leading rows later updates cannot change, including the committed blocks of a streaming Markdown reply. - The chat layout keeps rows that may still change within one screen above the footer, showing their latest rows under a one-line notice when they do not fit. - The regular-mode projection only appends; established turns are not re-folded or dropped. - The welcome banner above a conversation is static; account notices that need action appear above the Composer. - Stopping a run no longer marks its delivered prompt as cancelled; the marker stays reserved for prompts returned to the Composer. To keep long sessions responsive, the transcript drops final rows far above the screen and the engine rebases its retained state (takeDiscardedRows) instead of rewriting history. Co-authored-by: tournierjc <tournierjc@users.noreply.github.com> --------- Co-authored-by: SaladDay <1203511142@qq.com> Co-authored-by: hetaoBackend <hetao7@pku.edu.cn> Co-authored-by: DanielWalnut <45447813+hetaoBackend@users.noreply.github.com> Co-authored-by: tournierjc <tournierjc@users.noreply.github.com>
…r, on 0.6.0/0.6.1 sync) (#110) * fix: remove forced branching from bash guidance (MiniMax-AI#358) Co-authored-by: minimax <adhere@minimaxi.com> * feat: sync reviewed 0.5.5 runtime improvements (MiniMax-AI#367) Preserve public privacy defaults and distribution boundaries while porting compaction, Bash timeout, memory output and update-proxy improvements. Record selective provenance without advancing the full source baseline. Assisted-by: codex-cli reason:public-source-sync-0.5.5 * docs: publish feedback and private security contact channels (MiniMax-AI#360) * docs: publish feedback and private security contact channels * docs: correct MiniMax Agent X contact link * fix(tui): distinguish Bash recaps and import models during onboarding (MiniMax-AI#369) * fix: measure token speed over model generation time (MiniMax-AI#373) * chore: bump version to 0.5.6 (MiniMax-AI#374) * feat(config): add M3.1 Flash Preview to the fallback catalog (MiniMax-AI#375) Include context window and effort controls in the first-run catalog while preserving existing managed snapshots. Docs-Impact: Existing model selection workflow is unchanged. Assisted-by: codex-cli reason:m31-flash-fallback-sync * chore: bump version to 0.5.7 (MiniMax-AI#377) * chore: bump version to 0.5.7 * test: drain stderr before nonzero Bash fixture exit * fix(tui): display hook system messages without adding model context (MiniMax-AI#376) * fix(tui): anchor restored sessions above inherited terminal history (MiniMax-AI#378) * fix(tui): keep slow terminal output off the input loop (MiniMax-AI#379) * chore: bump version to 0.5.8 (MiniMax-AI#380) * feat: cascade explicit session stops to owned background work (MiniMax-AI#381) Port the reviewed shared runtime behavior while retaining standalone composition. Keep conversation leave separate from explicit stop, suppress canceled task delivery, and track append activations through teardown. Assisted-by: codex-cli reason:selective-runtime-port Docs-Impact: document stop and session-leave behavior in docs/tui-capabilities.md Co-authored-by: chenhao <chenhao@minimaxi.com> * fix: query structured Windows source volume properties (MiniMax-AI#383) * feat(examples): add Pocket Pet demo and refresh README onboarding (MiniMax-AI#385) * feat(examples): add reproducible Pocket Pet demo and refresh onboarding * feat(examples): polish Pocket Pet interaction and demo * fix(tui): dismiss stale terminal errors when a new turn starts (MiniMax-AI#388) * fix(tui): preserve native scrollback during automatic updates (MiniMax-AI#389) * fix(tui): preserve native scrollback during projection updates * fix(tui): isolate transient overlays from native scrollback * fix(tui): stabilize native history and diff overlay frames (MiniMax-AI#390) * fix(tui): prevent idle transcript replay and diff overlay frames * fix(tui): retain prompt identity and deferred main-buffer geometry * test: pin the srt-macos probe PATH and scale the BYOK serial timeout budget (MiniMax-AI#397) * test(sandbox): pin the probe PATH instead of inheriting the ambient one The real sandbox-exec probes resolve their command through PATH, so inheriting the caller's let any wrapper ahead of /bin decide what `rm` means. On a machine that has run the agent, that is the recoverable-delete shim MCode installs into agent shells: `rm` resolves to the shim, the shim execs mavis-trash, the sandbox denies that trash move as a write outside the policy under test, and a correct allow-probe exits 1. The suite then reports a sandbox regression where the sandbox behaved correctly. Pin all three probe environments to the system binaries the probes actually use, matching the wrapper-argument test above them. This also drops the unguarded `process.env.PATH` in wrapProfile, which had no fallback and could hand a real sandbox spawn an undefined PATH. Fixes MiniMax-AI#394. * test(byok): scale the serial BYOK budget by platform The first case is 30+ serial CLI spawns, each booting the whole runtime, so its wall-clock cost tracks machine speed rather than the transport it asserts. The flat 90s budget only held with roughly 1.5x headroom on an idle machine, so a parallel build, a laptop under load, or a shared CI runner turned a passing transport into a `testTimeoutFailure` that named a product failure no assertion had actually observed. Budget it the way smoke.test.mjs already budgets runtime startup: a base allowance plus a Windows multiplier for slower process spawn. The test is not slowed down by a larger ceiling; it only stops failing for reasons unrelated to what it checks. Fixes MiniMax-AI#395. * feat: sync MiniMax Code 0.5.9 runtime and TUI changes (MiniMax-AI#398) TUI - Report active root-session background tasks as `background=N` in the `[V]` build-mode status line. Bash still owned by its foreground tool call is excluded, and the count holds at 1 after a turn settles until a fresh task list arrives. The documented minimum width is now 102 columns. Runtime - The MiniMax API-key route now shares the official model catalog with the managed route; the first-run catalog moves to `minimax-model-catalog.ts` with unchanged model definitions. - Apply byte limits to large media and accumulated history for BYOK requests. - Send the M3 thinking toggle as a thinking setting rather than a generic reasoning effort. - Allow edit and rewind after an interrupt that left only a background reminder in canonical history. - Read less data when listing session files and navigating long histories. - Measure token output rate from the time events are observed. - Add `worktreeRefreshBeforeCreate` (default on) to fetch the selected upstream before creating a worktree, and exclude nested roots from fork worktree fingerprints. - Add Ghostty to the external editor catalog. - Enable the Codex OAuth model settings entry by default. Prompts - Cron guidance now lives only in tool definitions; the legacy feature template is empty. Memory edits use the `memory` tool's `edit` operation. - Clarify multimodal tool discovery in the mcode-tools reminder. * chore: release MiniMax Code 0.5.9 (MiniMax-AI#399) * Revert MiniMax-AI#390 and MiniMax-AI#389 to restore TUI scrollback baseline (MiniMax-AI#396) * Revert "fix(tui): stabilize native history and diff overlay frames (MiniMax-AI#390)" This reverts commit 1e136bf. * Revert "fix(tui): preserve native scrollback during automatic updates (MiniMax-AI#389)" This reverts commit 9b9c07b. * fix(tui): sync 0.5.10 prompt and run recovery fixes (MiniMax-AI#410) * chore: release MiniMax Code 0.5.10 (MiniMax-AI#411) * fix: include hidden thinking in TPS timing (MiniMax-AI#413) * fix: include hidden thinking in TPS timing * docs: note thinking_start in first token timing comment * test(windows): allow the NTFS source check to exceed the default timeout --------- Co-authored-by: hetaoBackend <hetao7@pku.edu.cn> * fix: omit empty tools from OpenAI compaction requests (MiniMax-AI#199) * fix: omit empty tools on OpenAI compaction requests * fix: negotiate missing tools with bounded compatibility recovery * fix: simplify empty tools handling to omit absent definitions * fix(tui): keep regular-mode history stable during live runs (MiniMax-AI#416) Regular mode wrote rows into native scrollback as soon as they scrolled off screen. When such a row later changed (a parallel tool finishing, a streamed table widening, a list turning loose, a turn growing past the projection fold, the welcome status badge flipping, or a stopped prompt gaining its cancelled marker), the renderer cleared scrollback and replayed the whole session from the welcome logo. Only final rows now reach native history: - The transcript reports how many leading rows later updates cannot change, including the committed blocks of a streaming Markdown reply. - The chat layout keeps rows that may still change within one screen above the footer, showing their latest rows under a one-line notice when they do not fit. - The regular-mode projection only appends; established turns are not re-folded or dropped. - The welcome banner above a conversation is static; account notices that need action appear above the Composer. - Stopping a run no longer marks its delivered prompt as cancelled; the marker stays reserved for prompts returned to the Composer. To keep long sessions responsive, the transcript drops final rows far above the screen and the engine rebases its retained state (takeDiscardedRows) instead of rewriting history. * feat: sync and release MiniMax Code 0.6.0 (MiniMax-AI#421) Port the reviewed 0.6.0 runtime and TUI behavior into the standalone distribution and bump the release version to 0.6.0. - Allow /retry inside a /btw side conversation to resend its last message. - Default to MiniMax-M3.1-Flash-Preview when no model has been selected. - After an accepted Goal completion, keep the Turn open for one final reply that summarizes the result and deliverables; blocked proposals still end the Turn. - Do not retry provider safety refusals, including on BYOK, and classify them as content_filter; keep Anthropic refusal details in the error. - Retry TLS record verification failures before output like other transient network errors. - Keep a post-compaction reminder at the tail of a continuation instead of aborting the provider call and recompacting on every retry. * feat: sync and release MiniMax Code 0.6.1 (MiniMax-AI#422) Allow /doctor and /feedback in a /btw side conversation so a failed side response can be diagnosed or reported without leaving it. Neither command mutates the parent or side Session. /quit stays blocked in the side view, because leaving from there aborts only the side Turn and skips side Session cleanup. Bump the root and TUI source versions to 0.6.1. * test(tui): adopt upstream MiniMax-AI#410 literal-prompt assertion in compact-rail test Upstream 5683465 (MiniMax-AI#410) changed the 'renders intent and execution as one compact visual rail' expectation to keep the prompt's ** markers literal (fork prompt-literal behavior). The three-way merge kept the pre-MiniMax-AI#410 expectation; adopt the MiniMax-AI#410 form. * fix(config): adopt upstream MiniMax-AI#421 default model MiniMax-M3.1-Flash-Preview Upstream 9150441 (sync 0.6.0) flipped buildPresetEntry's defaultModel to minimax/MiniMax-M3.1-Flash-Preview; the three-way merge kept the main side for config.ts and dropped the flip, breaking the MiniMax-AI#421-pinned model-selection/management suites. Restore the upstream value. * fix(tui): keep side-session Esc and main run timer consistent (MiniMax-AI#423) In a /btw side conversation, an empty Escape no longer arms the double-Escape /edit shortcut or shows its "Press Esc again to edit" hint, since /edit is unavailable in side conversations. Escape still interrupts a live side response. Switching between the side and main views re-adopted the main Session's live Turn at the switch time, which reset the Running/Loading timer and shortened the settled Turn duration. A Turn start ledger now records the earliest observed start (submission, session.start events including hidden Sessions, and adoption), and both re-adoption and the activity line use it. Bump the root and TUI source versions to 0.6.2. * fix(tui): adopt upstream MiniMax-AI#423 side-session Esc and turn start ledger Record the earliest Turn start across side-session switches so the main run timer and settled duration stay stable, and ignore double-Escape /edit while a side conversation is open. Fork packages stay kinetick-code and @mavis/code at 0.6.9, and multi-tab live turn watchers stay in place. Co-authored-by: tournierjc <tournierjc@users.noreply.github.com> * test(tui): drop duplicate applyPendingModelSelection key in command-flow mock The duplicate key dates from the #55/#57 merge on origin/main (5bba9c7): both sides added the same mock key. vitest transforms tolerate it; ESLint does not. --------- Co-authored-by: AdhereZ <85055734+AdhereZ@users.noreply.github.com> Co-authored-by: minimax <adhere@minimaxi.com> Co-authored-by: DanielWalnut <45447813+hetaoBackend@users.noreply.github.com> Co-authored-by: AmsZuidas <254873068+amszuidas@users.noreply.github.com> Co-authored-by: chenhao <chenhao@minimaxi.com> Co-authored-by: SaladDay <1203511142@qq.com> Co-authored-by: hetaoBackend <hetao7@pku.edu.cn> Co-authored-by: hermes-agent <hermes-agent@users.noreply.github.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: tournierjc <tournierjc@users.noreply.github.com>
Change
The TUI can show thousands of TPS when a provider hides thinking text. The reported case showed 1,389 tok/s with a model configured as
claude-opus-5-5.outputTokensincludes thinking tokens, but the timer previously waited for a non-empty thinking or text delta. When thinking was hidden, it divided all output tokens by only the short final-answer interval.Start the timer at the first
thinking_start, using its originalobservedAtMs. Keep the existing fallback for other streams, message reset, and tool-time exclusion. The UI still shows TPS. No model-specific rules, caps, or estimated token counts are added.References:
thinking_start.These support the timing boundary; this PR does not copy either project's full algorithm.
Evidence
Two real mcode requests used a synthetic arithmetic prompt, one per MiniMax model. Each captured stream was replayed through the original and patched
EventBridge. The hidden replay removed only thinking deltas and keptthinking_start, text events, original timestamps, and the provider's output token count.Earlier real Claude-compatible requests also reproduced the failure. These rows compare both algorithms on the same captured request, not two separate speed benchmarks:
Timing data for the MiniMax replays
Milliseconds are relative to the client capture start. TPS = output tokens × 1,000 / duration. The original visible replay starts at
first_delta_ms; the original hidden replay starts atfirst_text_ms. Both patched replays start atthinking_start_ms. All end atmessage_stop_ms.The prompt asked for the smallest positive integer satisfying
n % 7 = 3,n % 11 = 5, andn % 13 = 7, with no tools and only the answer returned.Limits: these are client-side observations and event replays, not server generation timings or a test of Claude Code internals. The MiniMax hidden cases simulate suppression of thinking deltas; they do not establish how those servers implement native hidden thinking. Each model has only one sample. The final frame arrived about 120 ms after the last content delta in both MiniMax samples; that existing tail remains included. Fully buffered streams without an early thinking event remain outside this fix. The original screenshot has no matching trace, so its exact cause is unconfirmed.
Validation
pnpm verifyon911e34b: all 15 applicable gates passed on macOS arm64, Node 22.23.2 (full profile). Windows and release-package gates were not applicable.perf:full; the CI result is required before merge.Publication and contribution checks
Maintainer handoff
Publication scope and licenses: unchanged. Shared-source port: pending.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.