Skip to content

Scope proposal for #442 — output-rate still fails open when request duration is absent #486

Open

Description

@shersdk

Scope proposal for #442. I've got a 14-line fix ready in turn-output-rate.ts with a regression test. Happy to open a PR from a fork, or hand the patch over for internal porting and sync, whichever is easier.

What's still broken after #454

#454 added a burst guard, but it can only fire when request_duration_ms is present:

// packages/tui/src/tui/controller/projection/turn-output-rate.ts:80
if (!nonnegativeFinite(requestMs) || decodeMs >= requestMs * BURST_MAX_DECODE_SHARE)
  return false;

Only a configured metrics client emits that field. turn.ts:202 gates the callback on deps.metrics, and metricsClient is optional. decode_duration_ms has no such dependency; it just needs includeDetailedUsage. So without a metrics client the guard never fires, and the reported rate counts a buffered burst as decode time.

Reproduction

Two samples that differ only in whether request_duration_ms is present:

rate.beginTurn('turn-1');

// present -> correctly suppressed by #454
rate.apply('turn-1', {
  type: 'message',
  message: { id: 'a', role: 'assistant',
    usage: { outputTokens: 395, decodeDurationMs: 64, requestDurationMs: 4_000 } },
}); // undefined

// absent -> aggregated, and reported as 6172 tok/s
rate.apply('turn-1', {
  type: 'message',
  message: { id: 'b', role: 'assistant',
    usage: { outputTokens: 395, decodeDurationMs: 64 } },
}); // 6171.875

One caveat on those numbers. 6172 tok/s and the 395 output tokens are what the TUI actually reported. The 64 ms window is derived from them (395 / 6172), not captured from a log. The sample shape is right; treat the millisecond figure as illustrative.

The change

Check an absolute plausibility bound before the request-duration test, so the guard stops depending on an optional field:

const MAX_PLAUSIBLE_OUTPUT_TOKENS_PER_SECOND = 4_000;

function bufferedBurst(tokens, decodeMs, requestMs) {
  if (decodeMs > 0 && tokens / (decodeMs / 1_000) > MAX_PLAUSIBLE_OUTPUT_TOKENS_PER_SECOND)
    return true;
  if (!nonnegativeFinite(requestMs) || decodeMs >= requestMs * BURST_MAX_DECODE_SHARE)
    return false;
  return tokens > (decodeMs / 1_000) * BURST_MIN_TOKENS_PER_SECOND;
}

Two files, +36 lines including the test. 4000 sits well above the 1900 tok/s sample #454 deliberately preserves. Zero-duration samples keep folding the way they do today; an existing test pins that, so I left it alone.

The 4000 value is my judgement call, not something I pulled from telemetry. If you have real provider numbers for the ceiling, that is the one line worth swapping.

Evidence

12/12 in turn-output-rate.test.ts. Reverting only the source change makes the new case fail with expected 6171.875 to be undefined, so it pins the defect rather than passing vacuously. pnpm typecheck is clean.

On pnpm verify --profile platform, test:capabilities reports 3 failures: tui-keybindings, plus two update-runtime Homebrew-receipt cases. Running those files against unmodified main in the same environment gives the same 3, so they're pre-existing and unrelated. Full gate inventory available on request.

Activity

  1. changed the title [-][Bug]: Scope proposal — output-rate fails open when request duration is absent (#442)[/-] [+]Scope proposal for #442 — output-rate still fails open when request duration is absent[/+] on Oct 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions