Skip to content

[codex] Fix train renderer pool cache#1889

Draft
xeophon wants to merge 1 commit into
mainfrom
codex/daily-bug-scan-20260627-train-renderer-pools
Draft

[codex] Fix train renderer pool cache#1889
xeophon wants to merge 1 commit into
mainfrom
codex/daily-bug-scan-20260627-train-renderer-pools

Conversation

@xeophon

@xeophon xeophon commented Jun 27, 2026

Copy link
Copy Markdown
Member

Summary

  • Cache v1 TrainClient renderer pools by effective renderer model and serialized chat_template_kwargs.
  • Add a regression test proving identical kwargs reuse a pool while changed or absent kwargs get distinct pools.

Root Cause

Commit a8036b441 / PR #1876 threaded per-request chat_template_kwargs into create_renderer_pool, but TrainClient still cached a single self._pool. The first request could therefore lock in one renderer pool and leak its chat-template settings into later requests with different kwargs or no kwargs.

Impact

Training/eval paths using the v1 train renderer client can now safely mix requests with different chat-template options, such as Qwen thinking settings, without cross-request pool contamination.

Validation

  • uv run pytest tests/v1/test_train_client.py
  • uv run ruff check --fix verifiers/v1/clients/train.py tests/v1/test_train_client.py
  • git diff --check

Note: repo-wide uv run ruff check --fix is currently blocked by an unrelated existing Python 3.11 f-string parse issue in verifiers/v1/cli/dashboard/eval.py:194. The pre-commit/pre-push hooks also reintroduced unrelated uv.lock exclude-newer churn, so the commit and push used the already-passing targeted checks while keeping uv.lock out of the PR.

Note

Fix TrainClient._renderer_pool to cache pools per model and chat template kwargs

  • Replaces the single shared _pool attribute in train.py with a _pools dict keyed by (renderer_model, serialized_chat_template_kwargs).
  • Creates a new renderer pool when the key is absent, and reuses the existing pool when the key matches — enabling distinct pools per unique combination of model and chat template kwargs.
  • Adds a unit test in test_train_client.py verifying that identical kwargs reuse the same pool, differing kwargs (e.g. enable_thinking) produce a new pool, and omitting kwargs produces a third pool.
  • Behavioral Change: callers that previously shared one pool across all invocations will now get distinct pools when chat_template_kwargs differ.

Macroscope summarized e8dce84.

@xeophon xeophon added the codex label Jun 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant