feat(openai): forward prompt_cache_key for server-side prefix cache routing#515
Conversation
…outing Send the session ID as OpenAI's prompt_cache_key on every completion request so consecutive turns route to a backend replica that already holds the conversation prefix in its prompt cache, instead of re-billing the full ~11k-token system+tools prefix each turn on OpenAI-compatible endpoints (which get no cache_control breakpoints, unlike Anthropic). Keyless requests serialize byte-identically to before, and ZERO_DISABLE_PROMPT_CACHE_KEY=1 suppresses the field for strict endpoints that reject unknown parameters. The Codex (Responses API) flavor is deliberately untouched: the ChatGPT backend is a separate, stricter surface.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughAdds a ChangesPrompt cache key plumbing
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/providers/openai/provider.go`:
- Around line 425-431: The kill switch check in the provider mapping currently
disables prompt-cache-key forwarding for any non-empty
ZERO_DISABLE_PROMPT_CACHE_KEY value, which is broader than intended. Update the
logic in the prompt cache key handling inside the OpenAI provider mapping so
only truthy enablement values (for example “1” or equivalent) disable
forwarding, and treat “0”, “false”, or other non-truthy values as no-op. Keep
the behavior localized to the request.PromptCacheKey mapping path and preserve
the existing documented env var behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 09a46d6f-c204-4c70-963d-aed99b2ea8d2
📒 Files selected for processing (5)
internal/agent/loop.gointernal/providers/openai/provider.gointernal/providers/openai/provider_test.gointernal/providers/openai/types.gointernal/zeroruntime/types.go
Zero automated PR reviewVerdict: No blockers found Blockers
Validation
ScopeHead: This deterministic review checks validation status and basic diff hygiene. A human reviewer still owns product judgment and design quality. |
Vasanthdev2004
left a comment
There was a problem hiding this comment.
Read the whole thing — clean, well-scoped win. Forwarding the session ID as prompt_cache_key so OpenAI-compatible endpoints (which get no cache_control breakpoints) can route consecutive turns to a warm replica is the right lever for that ~11k-prefix re-bill, and it's done thoroughly: all five request-construction paths carry the key, omitempty keeps keyless requests byte-identical to before, the Codex/Responses surface is left alone, and the test covers serialization, keyless omission, and the kill switch under -race.
Privacy is fine — I checked, the session ID is an opaque zero_<timestamp>_<nano>_<counter> with no path or username in it, so it's a routing key, not user data. And the 6 local internal/cli failures are the opengateway-profile artifact you called out, not this change — CI is green across all nine checks, which confirms it.
One real thing worth fixing before it lands (CodeRabbit flagged this too): the kill switch reads os.Getenv(...) == "", so any non-empty value disables forwarding — including ZERO_DISABLE_PROMPT_CACHE_KEY=0 and =false, which read as "keep it on" but would turn it off. Low-probability footgun on its own, but it's inconsistent with how the other boolean flag parses (ZERO_FORMAT_ON_WRITE treats 0/false as unset). Match that truthy parse so the two behave the same.
One design call I'd flag rather than block: it's default-on for every openai-compatible endpoint, so a strict one that rejects unknown params would 400 until the user finds the kill switch. Rare, recoverable, and prompt_cache_key is a standard param now, so opt-out is a defensible default — just noting it's the spot a user could get surprised.
Approving. Fold in the env-var parse and it's a clean merge.
ZERO_DISABLE_PROMPT_CACHE_KEY=0 or =false previously disabled forwarding because any non-empty value tripped the check. Parse it the same way ZERO_FORMAT_ON_WRITE parses boolean flags, so explicitly-falsy values are a no-op and only truthy values flip the toggle.
Summary
Send the session ID as OpenAI's prompt_cache_key on every completion request so consecutive turns route to a backend replica that already holds the conversation prefix in its prompt cache, instead of re-billing the full ~11k-token system+tools prefix each turn on OpenAI-compatible endpoints (which get no cache_control breakpoints, unlike Anthropic).
Keyless requests serialize byte-identically to before, and ZERO_DISABLE_PROMPT_CACHE_KEY=1 suppresses the field for strict endpoints that reject unknown parameters. The Codex (Responses API) flavor is deliberately untouched: the ChatGPT backend is a separate, stricter surface.
Checklist
Checklist
issue-approvedlabel.No linked issue — core-team change, same waiver as feat: agent quality, caching, retry, and tooling upgrades #506.
go build ./...,go vet ./..., andgo test ./...pass locally.Build and vet clean. Full test suite passes except the 6 pre-existing
internal/clifailures caused by a localopengatewayprovider profile("openai provider opengateway requires official baseURL") — byte-identical
failure set on
main, unrelated to this change.gofmtclean.-racewhere relevant).TestOpenAIRequestPromptCacheKeycovers key serialization, omission forkeyless requests, and the
ZERO_DISABLE_PROMPT_CACHE_KEYkill switch.internal/providers/openai,internal/zeroruntime, andinternal/agentall pass under
-race.N/A — no UI changes (request serialization only).
Summary by CodeRabbit
New Features
Bug Fixes