Skip to content

chore: remove prefix-cache-salt and reset-prefix-cache config flags - #2314

Merged
samsja merged 2 commits into
mainfrom
chore/remove-prefix-cache-salt-flags
Apr 18, 2026
Merged

chore: remove prefix-cache-salt and reset-prefix-cache config flags#2314
samsja merged 2 commits into
mainfrom
chore/remove-prefix-cache-salt-flags

Conversation

@samsja

@samsja samsja commented Apr 17, 2026

Copy link
Copy Markdown
Member

Summary

  • Drop the orchestrator.experimental.use_prefix_cache_salt flag — always salt inference requests with str(ckpt_step), for both training rollouts and evals.
  • Drop the inference.experimental.reset_prefix_cache_after_update flag — the /update_weights and /load_lora_adapter endpoints no longer reset the prefix cache. The salt alone is sufficient to invalidate stale KV states across policy updates, making the reset path redundant.
  • Remove the now-empty OrchestratorExperimentalConfig and InferenceExperimentalConfig classes and their parent experimental fields.
  • Tighten cache_salt typing in envs.py from str | None to str (always set).

🤖 Generated with Claude Code


Note

Medium Risk
Changes inference cache invalidation behavior by removing prefix-cache resets on weight/LoRA updates and making cache_salt mandatory and always derived from ckpt_step, which could affect serving performance/correctness if assumptions about vLLM cache salting are wrong.

Overview
Removes the configurable prefix-cache invalidation toggles (orchestrator.experimental.use_prefix_cache_salt and inference.experimental.reset_prefix_cache_after_update) and their associated wiring.

The orchestrator/scheduler now always injects cache_salt=str(ckpt_step) for training rollouts and evals (and cache_salt is now a required str throughout env APIs). Inference /update_weights and /load_lora_adapter no longer reset the vLLM prefix cache after applying updates.

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

Hardcode the defaults: always set cache_salt on inference requests
(keyed by ckpt_step) and never reset the prefix cache after weight or
LoRA updates. The salt alone is sufficient to invalidate stale KV
states across policy updates, so the reset path is redundant.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@samsja
samsja marked this pull request as ready for review April 17, 2026 23:50
Comment thread src/prime_rl/configs/orchestrator.py
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 26fdbc9. Configure here.

class OrchestratorExperimentalConfig(BaseConfig):
"""Experimental features for the orchestrator."""

use_prefix_cache_salt: Annotated[

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty experimental config classes left as dead code

Low Severity

After removing their only fields, OrchestratorExperimentalConfig and InferenceExperimentalConfig are now empty classes with no fields. They are still instantiated as experimental fields on OrchestratorConfig and InferenceConfig, but nothing in the codebase accesses .experimental anymore. The PR description explicitly states the intent to remove these classes and their parent fields, but this wasn't carried out.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 26fdbc9. Configure here.

@samsja
samsja merged commit 03689b8 into main Apr 18, 2026
8 of 9 checks passed
@mikasenghaas
mikasenghaas deleted the chore/remove-prefix-cache-salt-flags branch August 5, 2026 04:26
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