Skip to content

feat(telemetry): route 413/stale-call estimator through shared 3.5 divisor - #26

Merged
Kyzcreig merged 1 commit into
mainfrom
feat/unify-413-estimator-divisor
Jun 9, 2026
Merged

Kyzcreig merged 1 commit into
mainfrom
feat/unify-413-estimator-divisor

Conversation

@Kyzcreig

@Kyzcreig Kyzcreig commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator

What

Closes the last char/4 holdout. estimate_request_context_tokens (the stale-call / 413 payload-too-large oversize detector in chat_completion_helpers.py) hardcoded 5× // 4. Routes all five sites through the shared _ceil_chars_to_tokens() helper (added in #24), so the single COMPOSITION_CHARS_PER_TOKEN knob (default 3.5, env-tunable) now governs every estimator in the codebase:

Estimator Before After
display (compose_request_breakdown) 3.5 (#23) 3.5
compression trigger / preflight (estimate_*_rough) 3.5 (#24) 3.5
413 / stale-call (estimate_request_context_tokens) 4 3.5 ✅

Why

3.5 > 4 estimates higher, which is the safe direction for oversize detection — context-tier scaling and compression fire sooner, never later. Real Hermes payloads are JSON/code-dense (~3.5 chars/tok), so char/4 under-counted here too. Consistency: one knob, one rule, everywhere.

Safety

  • Empty payloads still return 0 (ceil(0) == 0).
  • All existing assertions in test_non_stream_stale_timeout.py are >=/> thresholds — a higher estimate only makes them safer. 12/12 pass.
  • Fixed one stale # char/4 comment.

Tests

12/12 test_non_stream_stale_timeout.py pass; ruff clean. Behavioral check: 4000 chars → 1143 tokens (was 1000).

Notes

…visor

The last char/4 holdout: estimate_request_context_tokens (the stale-call /
413 oversize detector in chat_completion_helpers.py) hardcoded 5x // 4.
Routes all five sites through the shared _ceil_chars_to_tokens() helper so
the single COMPOSITION_CHARS_PER_TOKEN knob (default 3.5) now governs ALL
estimators — display, compression trigger, preflight, AND 413 detection.

3.5 > 4 estimates higher, the safe direction for oversize detection (fires
context-tier scaling / compression sooner, never later). Empty payloads
still return 0 (ceil(0)=0). Existing threshold assertions (>=/>) only get
safer; fixed one stale char/4 comment. 12/12 stale-timeout tests pass,
ruff clean.
@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown

🔎 Lint report: feat/unify-413-estimator-divisor vs origin/main

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 9955 on HEAD, 9955 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 5159 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

@Kyzcreig
Kyzcreig merged commit 886877c into main Jun 9, 2026
23 checks passed
@Kyzcreig
Kyzcreig deleted the feat/unify-413-estimator-divisor branch June 9, 2026 14:06
@Kyzcreig
Kyzcreig restored the feat/unify-413-estimator-divisor branch September 21, 2026 10:32
ang-fleet-workers Bot pushed a commit that referenced this pull request Oct 4, 2026
Moves the floor `uses:` pin to debd27598fa6 (fleet-floor-workflows#26 merge
commit). #26 keys the floor concurrency group per-ref and cancels only on
pull_request; every other event (push:main backstop) gets a per-run group,
so a newer push no longer cancels or replaces it.

Diff old pin -> debd275 for the called workflow file: concurrency block +
a caller-paths comment only. Inputs and defaults unchanged; the scanner is
embedded in the workflow, so scanner bytes are identical. The other files
changed since the old pin (#22-#25: python-ci/js-ci inputs, steps-mode
actions, README, tests) are not called by this workflow.

Card t_ca5322d9.
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.

1 participant