Conversation
|
not a maintainer but: @dannote seeing how things tend to evolve, I'd recommend switching bool for enum of smth like prompCacheKeyStyle: none | openai, so that it's evolvable for the inevitable future where someone will want to send it via smth. a bit different. (or if you're reasonably sure this won't end up like session-id mess as it's not headers it might be fine) |
|
Traced the old vs new conditions — the refactor is behavior-preserving: direct One gap worth flagging: this only covers the Chat Completions shape. The Responses adapter sends On the bool vs enum thread: agree with @petrroll — with |
|
Thanks for the idea, @petrroll! Switched the boolean to |
gaoanze888
left a comment
There was a problem hiding this comment.
Reviewed exact head 490bebc99, including the Chat Completions and Responses policies plus models.json provider/model override propagation.
The tri-state behavior is coherent and backward-compatible:
- Chat Completions
autoretains the existing direct-OpenAI / supported-long-retention gate. - Responses
autoretains its existing key-on behavior. enabledallows compatible proxies to receive the session key without claiming long-retention support.disabledandcacheRetention: "none"suppress the key as documented.- session-affinity headers remain independent, and caller headers still override generated affinity headers.
I also checked config schema acceptance, provider inheritance, per-model override precedence, missing session IDs, Unicode clamping, and the relation to #6654 (key policy vs caller-supplied key value). Azure/Codex adapters remain outside the documented scope of this flag.
Validation:
- prompt-cache adapter tests: 30/30
- model registry/config tests: 85/85
npm run check- full exact-head merge into current
origin/main(71dca871b) applies cleanly - clean
git diff --check
No blocker found.
This comment is AI-generated by /wr
Chat Completions only sends
prompt_cache_keyto direct OpenAI URLs or when long retention is requested and supported. A compatible proxy therefore cannot receive pi's session key with default short retention, even if it accepts that field.Add
compat.supportsPromptCacheKeyto opt in independently of long retention:falseexplicitly suppresses the key; unset preserves existing behavior.cacheRetention: "none"still omits it. This does not change retention duration or session-affinity headers.In a live test through llm_proxy with a Codex backend, a follow-up without the key returned 0/1,931 cached input tokens; a separate pair with a stable key returned 1,792/1,931.
Related to #6654, but addresses the URL/retention gate that PR explicitly leaves unchanged, rather than adding a key-value override.
Validation:
npm run check,npm run build:offline, and./test.shpass. Added coverage for proxy opt-in, unchanged defaults, disabled caching, independent retention, missing session IDs, Unicode key clamping, and models.json provider/model overrides. A filesystem-watcher test timed out on the first full run, then passed individually and in the full rerun.AI-assisted implementation and PR description.