Skip to content

fix(agent): namespace prompt_cache_key with logical session scope - #78956

Closed
StanleyStetson wants to merge 1 commit into
NousResearch:mainfrom
StanleyStetson:fix/78941-prompt-cache-logical-scope
Closed

fix(agent): namespace prompt_cache_key with logical session scope#78956
StanleyStetson wants to merge 1 commit into
NousResearch:mainfrom
StanleyStetson:fix/78941-prompt-cache-logical-scope

Conversation

@StanleyStetson

Copy link
Copy Markdown
Contributor

What does this PR do?

Hardens prompt_cache_key routing by adding logical session scope isolation:
prompt_cache_key = hash(logical_scope + "\0" + static_prefix)

Note

This is routing-scope hardening and collision protection between unrelated interactive sessions, NOT a proven cache eviction hit-rate fix.

Key Details & Scope Rules

  1. Cron jobs: Uses cron:<job_id> as logical scope (parsing job IDs and ignoring per-fire timestamps) to preserve warm cache hits across job executions (fix(cache): content-address prompt_cache_key so recurring cron jobs reuse the warm prefix #52295 / Recurring cron jobs are prompt-cache-cold on every fire (session_id is the cache key) #51395).
  2. Subagents / children: Uses session:<child_session_id> scope so subagents differ from parent sessions and sibling subagents.
  3. Interactive & Gateway sessions: Prefers stable gateway_session_key / conversation_id when available, or walks context-compression lineage root (parent_session_id) to maintain warm cache hits across compression turns.
  4. Explicit Overrides & Headers: Preserves explicit caller overrides (prompt_cache_key) and length bounding (_bounded_prompt_cache_key). Leaves real transcript session_id headers (session_id, x-client-request-id, x-grok-conv-id) untouched.

Fixes #78941

Verification

  • pytest tests/agent/transports/test_codex_transport.py tests/agent/transports/test_chat_completions.py (117 passed)
  • npm run fix

@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint provider/openai OpenAI / Codex Responses API P0 Critical — data loss, security, crash loop sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) labels Aug 4, 2026
@spfcraze

spfcraze commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Fresh interactive sessions now get a per-session prompt_cache_key (session:), dropping the previously-tested same-key-across-session-ids guarantee — while the issue's retest table shows the shared key delivering 77.23% cross-session reuse and could not reproduce the eviction this isolates against.

The diff removes test_cache_key_stable_across_session_ids — whose docstring calls same-key-across-session-ids "the whole point of the fix: repeated fires reuse the warm prefix" — and adds test_cache_key_independent_interactive_sessions_different_keys asserting the opposite for interactive sessions; only the cron case keeps a stable key. For a parentless CLI session get_conversation_root returns the session id unchanged, so the scope is the fresh session id itself.


Checked against 3c561e2cc — the PR head when this was written — and 36cb5ae55, main at the same moment. Verified mechanically: the diff grep and the issue text at the former; the code on main at the latter. The reading of intended scope is inference. If I have misread the intent here, please say so.

@StanleyStetson

StanleyStetson commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

Thanks — intent is intentional hardening per #78941, not a regression of #52295.

  • fix(cache): content-address prompt_cache_key so recurring cron jobs reuse the warm prefix #52295’s “whole point” we preserve: recurring cron fires of the same job stay on one stable scope (cron:<job_id>), covered by tests. We do not keep the broader side-effect that unrelated interactive sessions shared one content-only bucket.
  • Issue author already walked back sequential eviction as proven; this PR is routing-scope isolation / concentration hardening (stated in the PR body), accepting less cross-session prefix reuse between independent conversations.
  • Compression lineage + gateway_session_key keep warm within one logical conversation when physical session_id rotates; parentless CLI correctly scopes to that session id.

Happy to adjust wording or scope logic if maintainers prefer any further tuning to the isolation boundaries.

@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Thanks for the careful work here — the scenario-test coverage (9 precedence tests) and the explicit prompt_cache_key override handling were the strongest parts of any PR in the #78941 cluster, and your triage reply correctly framed this as routing-scope hardening rather than a proven hit-rate fix. Closing, though — the core is now implemented on main and the residual pieces need a design decision first.

1. Superseded by #79161 (merged this morning, closed #78941). Main now scopes prompt_cache_key by session with cron-timestamp normalization: _cache_scope_from_session_id() + an explicit scope_id param through both transports (agent/transports/codex.py), session_id threaded into _add_prompt_cache_key (chat_completions.py), and — the part this PR doesn't cover — auxiliary-client calls scoped via set_runtime_main(session_id=...) (turn_context.py). This PR conflicts in 3 files, and resolving in its favor would regress that aux coverage.

2. Two of the four scope rules don't behave as documented in production wiring (verified empirically):

  • Subagent isolation (rule 2) is dead code: the wiring passes is_subagent=getattr(agent, "is_subagent", False), but nothing in the codebase ever sets .is_subagent (delegates carry _delegate_depth/_parent_session_id). A real delegate child falls through to rule 4 and resolves to session:<parent> — parent, child, and all siblings collapse into one scope, the opposite of the PR body's claim. The precedence tests pass only because they set is_subagent=True explicitly.
  • Rule 4 conflates /branch children with compression continuations: get_conversation_root() walks parent_session_id without checking _branched_from, so a branch shares its root's scope.

3. The cron regex's speculative branches introduce the bug class this PR fixes. The \d{9,13} epoch alternation truncates job ids that end in 9–13 digits: cron_backup_1722800000cron:backup, cron_job_123456789cron:job — two distinct jobs collide into one scope. The only real producer format is cron_<job_id>_%Y%m%d_%H%M%S (cron/scheduler.py:3015); neither the epoch form nor the cron: prefix exists in-tree. Main's anchored ^(cron_.+)_\d{8}_\d{6}$ covers all actual inputs.

4. The genuinely novel pieces belong in #79017. Rotation-stable scoping (your gateway-key and lineage-root rules) is exactly the open design question there — with two constraints your current shape hits: gateway_session_key is per-chat and survives /new forever (two unrelated conversations in one chat would share a scope permanently), and get_conversation_root is up to 100 sequential SELECTs invoked per LLM API call from build_kwargs. Your precedence table and the explicit-override handling are referenced there as candidate design input — if the maintainer greenlights rotation-stable scoping, the right base is now main's _cache_scope_from_session_id without DB access on the hot path.

Appreciate the engagement on this one — the design conversation continues in #79017.

@StanleyStetson
StanleyStetson deleted the fix/78941-prompt-cache-logical-scope branch August 13, 2026 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P0 Critical — data loss, security, crash loop provider/openai OpenAI / Codex Responses API sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Cache]: content-only prompt_cache_key can concentrate unrelated sessions into one routing scope

4 participants