Skip to content

test(kimi): stop drawing the refresh window inside the assertion - #11380

Merged
diegosouzapw merged 1 commit into
cheaper-inference:release/v3.8.50from
ntdat812:test/kimi-health-jitter-flake-11361
Aug 24, 2026
Merged

diegosouzapw merged 1 commit into
cheaper-inference:release/v3.8.50from
ntdat812:test/kimi-health-jitter-flake-11361

Conversation

@ntdat812

Copy link
Copy Markdown
Contributor

Refs #11361 — the Node 26 nightly failure on release/v3.8.51 is not a Node-compat
break. It is a test that draws a random number and then asserts the outcome.

⚠️ base-red inherited: #9985 — release/v3.8.50 is not green; the failures this PR
inherits are the ones tracked there, not introduced here.

What the nightly actually reported

From the run linked in #11361 (Node 26 Compat Tests (4/4)):

ℹ tests 8280
ℹ pass  8275
ℹ fail  1

✖ failing tests:
test at tests/unit/token-health-check-kimi.test.ts:1:681
✖ triggers refresh when Kimi token is within jittered expiration window
  AssertionError [ERR_ASSERTION]: false !== true

One failure out of 8280, in a test whose own name contains the word "jittered".

Why it fails, on any Node

checkKimiWebConnectionIfNeeded decides the refresh window by drawing it:

const jitterSec = 60 + Math.floor(Math.random() * 180);   // [60, 239]
const expiringSoon = isKimiTokenExpiringSoon(token, jitterSec);   // remainingSec <= jitterSec

The test builds a token expiring in 90 s and asserts a refresh happened. That is
true only when the draw lands at 90 or above — 150 of the 180 possible values.

computed:  16.67% failure rate  ((90 - 60) / 180)
measured:  4 of 20 local runs failed (20%)

Nothing about Node 24/26 is involved; the nightly just rolled low. The same coin has
presumably been flipped on every run of this suite since the test was written.

Change

The spread becomes defaultKimiRefreshJitterSec() and the window becomes injectable
as jitterSecFn, so a caller can decide it rather than roll for it.

Production behaviour is unchanged. The only caller
(src/lib/tokenHealthCheck.ts:611) passes nothing and keeps the random spread —
which exists for a real reason (a fleet of connections must not stampede the token
endpoint at the same instant), so it is kept, not removed.

The test now passes jitterSecFn: () => 120 for the in-window case, and two cases
are added: a token outside the window is left alone, and the default spread stays
inside [60, 240) over 2000 draws.

Verification

5 consecutive runs of the updated file:   4 pass / 0 fail, every time
20 runs of the previous file:             4 failures
npx tsc --noEmit                          clean
eslint (repo suppressions)                clean

Changelog fragment: changelog.d/maintenance/kimi-health-check-jitter-determinism.md.

I would not close #11361 on this alone — it asks to triage "which Node version/shard
broke", and the answer is "none of them, on this run". If a later nightly shows a
genuine Node 24/26 break, it will now be visible instead of buried under a 1-in-6 coin.

@ntdat812
ntdat812 requested a review from diegosouzapw as a code owner August 24, 2026 11:04
`checkKimiWebConnectionIfNeeded` spreads its refresh over [60, 240)
seconds before expiry so a fleet of connections cannot stampede the token
endpoint. The test built a token expiring in 90 s and asserted a refresh
happened — true only when the draw lands at 90 or above, i.e. 150 of the
180 possible values.

So the test fails 1 run in 6 by construction: 16.67% computed, 4 of 20
measured locally. That is what the Node 26 nightly hit, and it was filed
as a Node-compat break (cheaper-inference#11361).

The spread is now `defaultKimiRefreshJitterSec()` and the window is
injectable as `jitterSecFn`, so a caller can decide it instead of rolling
for it. Production is unchanged: the only caller
(`tokenHealthCheck.ts:611`) passes nothing and keeps the random spread.

Cases added for a token outside the window, and for the default spread
staying inside [60, 240).

Refs cheaper-inference#11361
@ntdat812

Copy link
Copy Markdown
Contributor Author

CI note, so the red is not left ambiguous.

Unit Tests fast-path — openapi-coverage.test.ts — is base-side, not this PR.

AssertionError: OpenAPI operation coverage regressed: 34.5% < floor 34.6%

The same assertion fails identically on #11374, #11376, #11377 and #11380, whose
diffs are disjoint (bin/cli/*, src/sse/services/auth.ts, src/shared/utils/wsPath.ts,
src/lib/tokenHealthCheckKimi.ts). None of them adds or removes an API route, so
none can move that ratio. The floor sits 0.1 point above what release/v3.8.50
currently documents — routes landed without their openapi entries.

Build (advisory) and dast-smoke are the inherited base-red already tracked in #9985.

I have not touched the coverage floor: rebaselining a gate I did not break is not
mine to do, and it would hide the drift. Say the word if you would rather it be
bumped, or if you want the undocumented operations written up.

@ntdat812

Copy link
Copy Markdown
Contributor Author

CI update — all three reds are base-side; none are reachable from this diff.

Unit Tests fast-path — same openapi-coverage.test.ts failure triaged above
(34.5% < floor 34.6%), just surfacing under the shard name now. That test reads exactly
two inputs, src/app/api/**/route.ts and docs/openapi.yaml. Both are byte-identical to
the base:

$ git rev-parse origin/release/v3.8.50:src/app/api  HEAD:src/app/api
969e4cfef5784db139716e16c68ba0458dd7ed72
969e4cfef5784db139716e16c68ba0458dd7ed72
$ git rev-parse origin/release/v3.8.50:docs/openapi.yaml  HEAD:docs/openapi.yaml
540f2abd52dba4f7a143613146eed70ca7f0c8e3
540f2abd52dba4f7a143613146eed70ca7f0c8e3

Identical inputs cannot produce a different coverage number, so this PR cannot have moved it.
Still not lowering a gate I did not break.

Build (advisory) and dast-smoke — both continue-on-error: true, and both died on
The operation was canceled with no compile error and no assertion, mid Creating an optimized production build / mid CLI-bundle build. Not diff-specific: #11378 (different author, disjoint
diff) is red on the same three checks right now, and #11381/#11382 are red on
Fast Production Build/dast-smoke. dast-smoke's job failed on 4 of the 5 other branches
sampled from the last 20 runs of that workflow.

Nothing to change here. Happy to rebase if you want a fresh run.

@diegosouzapw
diegosouzapw merged commit d5d730c into cheaper-inference:release/v3.8.50 Aug 24, 2026
13 of 16 checks passed
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…aper-inference#11380)

Validado em lote combinado (batch-0824g, junto de cheaper-inference#11388/cheaper-inference#11397/cheaper-inference#11415/cheaper-inference#11418) contra o tip de release/v3.8.50: typecheck:core limpo, file-size/changelog/complexity/cognitive-complexity OK, 62/62 testes focados passando.

Diagnóstico correto e bem documentado: a falha do nightly Node 26 era um teste que sorteia um número e depende do resultado, não uma quebra de compatibilidade. Comportamento de produção inalterado (a janela de jitter continua aleatória; só o teste ganhou controle sobre ela). Obrigado pela investigação detalhada!
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.

🌙 nightly-compat: Node 24/26 failures on release/v3.8.51

2 participants