Skip to content

test(chatcore): wait for queued call log writes - #10683

Merged
diegosouzapw merged 1 commit into
cheaper-inference:release/v3.8.50from
cryptiklemur:test/chatcore-isolate-stream-settings
Aug 20, 2026
Merged

diegosouzapw merged 1 commit into
cheaper-inference:release/v3.8.50from
cryptiklemur:test/chatcore-isolate-stream-settings

Conversation

@cryptiklemur

@cryptiklemur cryptiklemur commented Aug 18, 2026 •

Copy link
Copy Markdown
Contributor

CI note: Docs Gates currently inherits the release-base BOT_TOKEN / BOT_URL env-doc failure. the same base-red appears on unrelated #10780; this diff does not touch those variables.

The chatcore translation suite could read call_logs before queued writes finished. latest-record assertions then saw stale rows.

This PR only changes test timing. production behavior stays the same.

The shared helper now waits for waitForCallLogSaves(5000) before reading the latest call log. release/v3.8.50 owns the SSE comment behavior updated in #10726, so this branch leaves those assertions unchanged.

how to test

npx cross-env DISABLE_SQLITE_AUTO_BACKUP=true node --max-old-space-size=4096 --import tsx/esm --import ./open-sse/utils/setupPolyfill.ts --import ./tests/_setup/isolateDataDir.ts --test --test-force-exit --test-concurrency=1 tests/unit/chatcore-translation-paths.test.ts

The focused file passes after rebasing onto the current release tip. direct-file ESLint still reports 31 no-explicit-any findings present on the release base; this three-line diff adds none.

@diegosouzapw

Copy link
Copy Markdown
Collaborator

Thanks — this fixes a real latent bug: resolveTargetTokenLimit in open-sse/services/combo.ts was calling the async getCachedProviderConnectionById without await, so the per-connection rateLimitOverrides.tpm token-limit override was silently never applied in round-robin combo, and it imported from the localDb.js barrel instead of the canonical db/readCache.ts. The regression test plus the chatcore streaming-log assertion isolation (waiting for queued call-log saves via waitForCallLogSaves, scoping OMNIROUTE_SSE_COMMENTS=on to the one test and restoring it in afterEach/after) are exactly the right hardening, and they follow existing precedent (#9567 polling; sse-comments-optout-9305 env save/restore).

Note: the title and PR body call this a "test-only follow-up," but the PR also carries a production change in open-sse/services/combo.ts — worth reflecting in the title/conventional-commit (e.g. fix(combo)) so the changelog and CI gates (which treat production changes specially) match.

Before this can merge, the branch needs:

  1. Rebase/conflict resolution — GitHub reports mergeable=false / dirty against release/v3.8.50; the chatcore-translation-paths.test.ts and combo.ts hunks need re-applying onto the current tip.
  2. Get CI green — Unit Tests fast-path (4/4 shards) and Vitest are failing and this does not fall in the documented inherited-base-red list (🔴 Release branch not green: release/v3.8.50 #9985), so it looks branch-caused. If the async change to resolveTargetTokenLimit interacts with existing sync mocks of getCachedProviderConnectionById, or if the new combo-quota-token-limit.test.ts leaks SQLite handles (native runner hangs unless resetDbInstance() + test.after cleanup is present — see AGENTS.md PII/stream learnings), those need fixing. Please run the full unit + vitest suites locally and confirm.
  3. Merge integrity gate (changelog + generated skills, currently exit 2) — since production code changed, add a CHANGELOG entry for the combo fix.

Keep the existing tests green; do not weaken/remove the new assertions to pass. Happy to re-review once the conflict is resolved and CI is green.

@cryptiklemur
cryptiklemur force-pushed the test/chatcore-isolate-stream-settings branch from 6d862ad to 99d0414 Compare August 19, 2026 07:37
@cryptiklemur
cryptiklemur force-pushed the test/chatcore-isolate-stream-settings branch from 99d0414 to 38306b6 Compare August 20, 2026 04:28
@cryptiklemur cryptiklemur changed the title test(chatcore): isolate streaming log assertions test(chatcore): wait for queued call log writes Aug 20, 2026
@diegosouzapw
diegosouzapw merged commit 2a10d16 into cheaper-inference:release/v3.8.50 Aug 20, 2026
5 checks passed
diegosouzapw pushed a commit that referenced this pull request Aug 20, 2026
Merged — locally validated (110/110 focused tests, typecheck:core clean, gates green) after resolving base-drift against #10683 (both landed today, same test file — combined). Nice, well-tested cache-breakpoint fix. Thanks!
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
)

Merged — locally validated (70/70 combined chatcore-translation-paths tests, typecheck:core clean, gates green). Thanks!
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…inference#10684)

Merged — locally validated (110/110 focused tests, typecheck:core clean, gates green) after resolving base-drift against cheaper-inference#10683 (both landed today, same test file — combined). Nice, well-tested cache-breakpoint fix. Thanks!
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.

2 participants