From 11ed0871c2a5b0886fde249e6206be0e702b4e98 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Wed, 22 Jul 2026 11:08:07 -0700 Subject: [PATCH] =?UTF-8?q?test:=20fix=20CI=20red=20on=20#3130=20=E2=80=94?= =?UTF-8?q?=20withEnv=20for=20batch-concurrency=20env=20+=20close=20resetG?= =?UTF-8?q?ateway=20shard-order=20poison=20window?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two real failures surfaced by this PR's re-sharding: 1. verify/check-test-isolation: embed-batch-concurrency.test.ts mutated process.env directly (R1). Now uses withEnv(). 2. test (9) source-health "expected 1280 dimensions, not 1536": a file whose last afterEach calls resetGateway() leaves the gateway slot empty during the NEXT file's beforeAll (which runs before any beforeEach can restore the legacy 1536 pin), so initSchema() sizes the embedding column from the zembed-1/1280 defaults and every 1536-d fixture in that file fails. Which pair collides depends on shard composition, so adding test files (as this PR does) can surface it anywhere. The legacy-embedding preload now also repairs the empty slot in a global afterEach (preload after-hooks run after file-local ones), closing the window at the root instead of patching one victim file. Reproduced locally with a poison/afterEach-reset file followed by a schema-creating file: embedding column typmod 1280 before the fix, 1536 after. check-test-isolation, typecheck, and the affected suites all pass. Co-Authored-By: Claude Fable 5 --- test/embed-batch-concurrency.test.ts | 7 +++--- test/helpers/legacy-embedding-preload.ts | 30 +++++++++++++++++++++--- 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/test/embed-batch-concurrency.test.ts b/test/embed-batch-concurrency.test.ts index 6dd198d60..7fcd3ca25 100644 --- a/test/embed-batch-concurrency.test.ts +++ b/test/embed-batch-concurrency.test.ts @@ -24,6 +24,7 @@ import { __setEmbedTransportForTests, } from '../src/core/ai/gateway.ts'; import { embedBatch } from '../src/core/embedding.ts'; +import { withEnv } from './helpers/with-env.ts'; const DIMS = 1536; @@ -69,7 +70,6 @@ describe('embedBatch bounded parallelism (#1818)', () => { }); afterEach(() => { __setEmbedTransportForTests(null); - delete process.env.GBRAIN_EMBED_BATCH_CONCURRENCY; }); test('default pool dispatches sub-batches in parallel, order preserved', async () => { @@ -92,9 +92,10 @@ describe('embedBatch bounded parallelism (#1818)', () => { }); test('GBRAIN_EMBED_BATCH_CONCURRENCY env bounds the pool when option unset', async () => { - process.env.GBRAIN_EMBED_BATCH_CONCURRENCY = '2'; const tracker = installTrackingTransport(); - await embedBatch(texts, { onBatchComplete: () => {} }); + await withEnv({ GBRAIN_EMBED_BATCH_CONCURRENCY: '2' }, async () => { + await embedBatch(texts, { onBatchComplete: () => {} }); + }); expect(tracker.maxInFlight()).toBeGreaterThan(1); expect(tracker.maxInFlight()).toBeLessThanOrEqual(2); }); diff --git a/test/helpers/legacy-embedding-preload.ts b/test/helpers/legacy-embedding-preload.ts index 257c21d35..60200b443 100644 --- a/test/helpers/legacy-embedding-preload.ts +++ b/test/helpers/legacy-embedding-preload.ts @@ -19,7 +19,7 @@ * overwrites this preload. */ import { configureGateway, getEmbeddingDimensions } from '../../src/core/ai/gateway.ts'; -import { beforeEach } from 'bun:test'; +import { afterEach, beforeEach } from 'bun:test'; const LEGACY_CONFIG = { embedding_model: 'openai:text-embedding-3-large', @@ -52,7 +52,7 @@ applyLegacy(); // 2. file-local beforeAll → may overwrite to ZE/1280 // Since beforeAll runs once per file BEFORE the first beforeEach, // file-local beforeAll wins for that file's tests. ✓ -beforeEach(() => { +function applyLegacyIfEmpty() { try { // Only re-apply if the gateway was reset (or never configured). // Tests that explicitly configured a different model in their @@ -62,4 +62,28 @@ beforeEach(() => { } catch { applyLegacy(); } -}); +} + +beforeEach(applyLegacyIfEmpty); + +// PR #3130 shard-order fix: beforeEach alone leaves ONE window open — a file +// whose LAST afterEach calls resetGateway() poisons the NEXT file's +// beforeAll, which runs BEFORE any beforeEach fires. A beforeAll there that +// does engine.initSchema() then sizes the embedding column from the gateway +// DEFAULTS (zembed-1/1280d) instead of the pinned legacy 1536, and every +// 1536-d Float32Array fixture in that file dies with +// "expected 1280 dimensions, not 1536". Which file pair collides is a +// function of shard composition, so adding/removing ANY test file can +// surface it (that is exactly how it bit shard 9). +// +// Preload hooks are registered before any file-local hooks, and bun runs +// after-hooks inside-out (file-local afterEach first, then this one), so +// this repairs the empty slot immediately after the poisoning reset — +// before the next file's beforeAll can observe it. +// +// Known remaining window: a file whose afterAll() resets the gateway (no +// hook runs between its afterAll and the next file's beforeAll). Files +// that reset in afterAll and can precede a schema-creating file should +// re-apply their own config, or the victim file should configureGateway() +// explicitly in its beforeAll. +afterEach(applyLegacyIfEmpty);