fix(tests): dispose test translator budgets in every file that creates them

[FredAmartey]

Summary

• tests/helpers/translator-budget.ts disposes the budgets it hands out, and resets the translator aggregate, from a module-level afterEach. Bun evaluates a module once per process, and a hook registered while it loads belongs to the test file that imported it first. In a shared process only that file gets the cleanup. The other files that reach the helper (128 test files import it directly or through passthrough-key-url.ts) keep their budgets and the aggregate for the rest of the run.
• A two-file check shows the mechanism: a helper that calls afterEach when it loads runs it after the first file's tests and never after the second's. Under --isolate each file loads its own copy and gets its own hook, which is why CI does not see this.
• tests/images/loop.test.ts asserts the number of live budgets. When gemini-inline.test.ts imported the helper first, loop sees 8 where it expects 1, and 4 of its tests fail. loop is one of the files Batched test runner cannot run on macOS, and one-process runs give batch-size-dependent failure counts #5439 lists as failing only in batches.
• createTestTranslatorBudget now also registers the cleanup with onTestFinished, which belongs to the running test whatever file it is in. It throws outside a test, so a budget created at module scope or in beforeAll keeps the afterEach as its only cleanup, as before. Under --isolate the only change is a second cleanup that finds nothing left. No runtime code changes.

Verification

Head c5c586ad6 on dev 6d5d501a6, Bun 1.4.0 (the bun dependency in package.json). The measurements below were taken at dev 965a6e5b1. None of the 9 dev commits since then touches the helper, src/lib/translator-budget.ts or tests/images, and the gemini-inline and loop pair and the static checks were rerun on this head:

• Driven red first: bun test tests/images/gemini-inline.test.ts tests/images/loop.test.ts in one process gives 78 pass, 4 fail on untouched dev and 82 pass, 0 fail with this change.
• In one process, the 128 test files that reach the helper give 14 failures on dev and 9 with this change. The 4 loop budget failures are gone and nothing new fails. The other 9 fail the same way on dev, 7 of them import errors from the image-test mocks fixed in fix(tests): put the real modules back after the image tests mock them #5605.
• The same 128 files under --isolate, the way CI runs them: 3431 pass, 0 fail.
• Full suite with CI's flags: every batch of shards 1/4 to 4/4 as scripts/ci/run-bun-test-batches.sh slices them (12 files per batch, bun test --isolate --timeout 60000, CI=true), run past failing batches because the script stops a shard at its first failure, with the four shards in parallel on one macOS machine: 28969 pass, 4 fail across 1506 files. None of the four files reaches the helper. codex-runtime.test.ts and openai-provider-option-e2e.test.ts also fail in their batches on untouched dev at the same base; the second is one of the tests [Bug]: four tests exceed their timeouts in the unsharded macOS control lane, first observed once the lane was able to finish #4997 tracks. codex-shim.test.ts and macos-serial-lanes.test.ts failed only under the four-shard load: their batches pass on this head and on dev when rerun on their own.
• bun run typecheck, bun run structure:check, bun run privacy:scan and git diff --check: passed.

Checklist

• Scope stays focused and avoids unrelated cleanup.
• Docs or release notes were updated when needed.
• Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

• Required local validation passed; commands, results, and any full-suite exception are documented.

• I pushed my PR to the latest dev commit.

• I resolved all correct Codex and CodeRabbit findings.

• My PR is ready for review.

Summary by CodeRabbit

• Tests
  • Improved test cleanup for translator budgets.
  • Ensured tracked budgets are disposed and related state is reset after each test, including tests without an active test context.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: d451303f-c977-4fe5-81d0-05de6b66464b

📥 Commits

Reviewing files that changed from the base of the PR and between 6d5d501 and c5c586a.

📒 Files selected for processing (1)

• tests/helpers/translator-budget.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.

---

📝 Walkthrough

Walkthrough

The translator budget test helper now uses shared cleanup for tracked budgets. It registers cleanup with onTestFinished during active tests and retains afterEach cleanup when no test is active.

Changes

Translator budget cleanup

Layer / File(s)	Summary
Per-test cleanup registration and fallback tests/helpers/translator-budget.ts	The helper imports onTestFinished at line 1. Lines 11–33 add shared cleanup that disposes tracked budgets, clears the set, and resets aggregate state. Active tests use onTestFinished; calls outside a test use afterEach.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to c5c58

The cleanup change is mergeable with no remaining actionable risk identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: ensuring test translator budgets are disposed in every file that creates them.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

✅ Deterministic PR hygiene checks passed.

[lidge-jun]

리뷰 · 우선순위 64 / 80

이 PR은 테스트용 번역 예산(translator budget) 청소가, Bun을 --isolate 없이 한 프로세스에서 여러 파일을 돌릴 때 첫 번째로 헬퍼를 불러온 파일에만 돌아가던 구멍을 막습니다. 예전에는 tests/helpers/translator-budget.ts가 모듈이 로드될 때 afterEach만 걸어 두었는데, Bun은 모듈을 프로세스당 한 번만 읽으므로 그 훅은 “처음 import한 테스트 파일” 소속이 됩니다. 그래서 gemini-inline.test.ts가 먼저 헬퍼를 가져오면, 나중에 도는 loop.test.ts는 예산을 안 치우고 살아 있는 예산 개수가 1이 아니라 8처럼 보여 네 테스트가 깨졌습니다. CI는 --isolate라 파일마다 모듈을 다시 읽어 같은 증상이 안 났고, #5439에 적힌 “묶음으로만 실패” 쪽과 맞습니다. 지금은 createTestTranslatorBudget이 예산을 만들 때 onTestFinished로도 같은 청소 함수를 등록해서, 어느 파일의 테스트 안이든 그 테스트가 끝나면 예산을 버리고 aggregate도 리셋합니다. 테스트 밖(모듈 최상단·beforeAll)에서 만들면 onTestFinished가 던져서 예전처럼 afterEach만 남습니다. 런타임 코드는 안 건드리고 헬퍼 한 파일(+18/−6)만 바꿉니다. base는 dev이고 types/config 분할·프리뷰 배포와는 무관합니다.

라인 - tests/helpers/translator-budget.ts · 빈 catch — onTestFinished가 던지는 이유를 “테스트 밖”으로만 보고 모두 삼킵니다. API가 바뀌거나 다른 이유로 실패해도 조용히 넘어가 afterEach(첫 import 파일만)에만 기대게 됩니다.

라인 - afterEach(disposeTestTranslatorBudgets) — 모듈 로드 때 한 번만 등록되는 구조는 그대로입니다. 테스트 밖·beforeAll에서 예산을 만드는 뒤쪽 import 파일은 예전과 같이 공유 프로세스에서 새지 않을 수 있습니다. 작성자도 적어 두었고, 흔한 패턴은 팩토리/test 안에서 withTestTranslatorBudget·createTestTranslatorBudget을 호출하는 쪽이라 이번 고침으로 loop 실패는 잡힙니다.

라인 - onTestFinished(disposeTestTranslatorBudgets) — 예산 하나마다 훅을 걸지만, 콜백은 liveTestBudgets 전체를 dispose하고 aggregate를 리셋합니다. 한 테스트에서 예산을 여러 개 만들면 같은 청소가 여러 번 등록됩니다. 결과는 맞고, --isolate에서는 두 번째 청소가 비어 있어 해롭지 않습니다.

라인 - Draft / 호스트 CI — Draft라 CodeRabbit은 스킵했고, 지금 보이는 잡은 hygiene·label 통과와 resolve-pr pending 정도입니다. 작성자 로컬로는 gemini-inline+loop 한 프로세스 82 pass, 헬퍼 닿는 128파일 한 프로세스에서 loop 예산 실패 4건 제거, --isolate·샤드 검증·typecheck/structure/privacy는 통과했다고 적혀 있습니다. 남은 한 프로세스 실패 9건은 이번 범위 밖(이미지 mock 등)으로 보입니다.

메인테이너의 판단이 필요한 지점

빈 catch를 그대로 둘지, “테스트 밖”일 때만 삼키도록 메시지를 좁힐지. 모듈 스코프/beforeAll 예산까지 공유 프로세스에서 완전히 막으려면 afterEach 잔여 한계를 후속으로 볼지, 이번엔 테스트 안 경로만으로 충분한지. Draft를 Ready로 올린 뒤 호스트 테스트 샤드 초록을 머지 조건으로 둘지.

너의 추천

원인·고침·검증이 한줄로 맞고, #5439류 묶음 실패를 줄이는 데 바로 도움이 됩니다. Ready로 바꾼 뒤 호스트 CI(테스트 포함)만 확인하고 머지하는 쪽을 권합니다. 빈 catch 좁히기와 테스트 밖 경로 잔여 누수는 필수는 아니고, 원하면 후속 이슈로 남겨도 됩니다. 닫을 types/config 중복 PR은 없습니다.

이 댓글은 grok-bot이 작성했습니다

[github-actions]

✅ READY

• all PR quality gates passed; the review readiness checklist is complete.

Review readiness checklist

• ✅ Required local validation passed; commands, results, and any full-suite exception are documented.
• ✅ I pushed my PR to the latest dev commit.
• ✅ I resolved all correct Codex and CodeRabbit findings.
• ✅ My PR is ready for review.

✅ 4/4 boxes ticked.

This pull request is already Ready for Review.
The review-ready label marks this PR as ready; review automation runs independently.
Maintainers: @lidge-jun @Ingwannu

[lidge-jun]

Carried onto dev in bundle PR #5672 (squash-merged as 7dd1db2), rebuilt on current dev as commit c151c59 on the lane branch with a Co-authored-by trailer for you, so the credit stays on the merged commit. Closing this one as superseded. Thank you for the fix.