Skip to content

Fix cache salt forwarding in train client - #2463

Merged
mikasenghaas merged 1 commit into
mainfrom
fix/forward-cache-salt
Aug 29, 2026
Merged

Fix cache salt forwarding in train client#2463
mikasenghaas merged 1 commit into
mainfrom
fix/forward-cache-salt

Conversation

@eligotts

@eligotts eligotts commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Summary

Forward cache_salt through Renderers’ dedicated generate argument instead of leaving it inside sampling_params, where it does not reach vLLM.

This preserves policy-version prefix-cache isolation for pipeline RL: existing rollouts retain their starting cache namespace, while fresh rollouts miss KV cached under older weights.

Validation

  • uv run ruff check verifiers/v1/clients/train.py
  • Commit hooks: Ruff check, Ruff format, and ty

Note

Pass cache_salt as separate argument in TrainClient.get_response

Extracts cache_salt from sampling_params via pop and forwards it as a distinct argument to the generate call in train.py. sampling_params no longer carries cache_salt when forwarded to generate.

Macroscope summarized 7367a4a.


Note

Low Risk
Single-path wiring change in the train client; no auth or data-model changes, aligned with the legacy renderer client pattern.

Overview
Fixes cache_salt not reaching vLLM when the train client calls renderers.client.generate.

TrainClient.get_response now pops cache_salt from sampling_params (same pattern as chat_template_kwargs) and passes it as a dedicated cache_salt argument to generate, instead of leaving it inside sampling_params where it was ignored.

This restores prefix-cache namespace isolation for pipeline RL rollouts that key KV cache on policy version.

Reviewed by Cursor Bugbot for commit 7367a4a. Bugbot is set up for automated code reviews on this repo. Configure here.

@eligotts
eligotts marked this pull request as ready for review August 29, 2026 05:05
@macroscopeapp

macroscopeapp Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 7367a4a

Macroscope's review found this PR approvable — This is a focused, single-file bug fix that forwards an existing cache-salt value through the renderer's intended dedicated argument. Requests without a cache salt retain their prior behavior, while configured train rollouts regain the intended prefix-cache isolation.

You can add or adjust custom eligibility rules. Learn more.

@mikasenghaas
mikasenghaas merged commit 8a993ec into main Aug 29, 2026
13 checks passed
@mikasenghaas
mikasenghaas deleted the fix/forward-cache-salt branch August 29, 2026 16:31
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