Skip to content

fix: codex_responses prompt caching — session routing headers + cache_write_tokens field - #10006

Open
zicochaos wants to merge 1 commit into
NousResearch:mainfrom
zicochaos:fix/codex-prompt-caching
Open

zicochaos wants to merge 1 commit into
NousResearch:mainfrom
zicochaos:fix/codex-prompt-caching

Conversation

@zicochaos

Copy link
Copy Markdown
Contributor

Problem

Prompt caching does not work when using codex_responses API mode with OpenAI-compatible providers (e.g. theclawbay). Every request is a cache miss despite prompt_cache_key being set in the request body.

Root cause

Two issues:

1. Missing session routing headers (run_agent.py)

The OpenAI client is initialized without session_id or x-client-request-id headers for codex providers. These headers are required for server-side cache routing — they tell the backend to route requests to the same server that holds the cached prompt prefix.

The official Codex CLI sends these unconditionally. Hermes sets default_headers for OpenRouter, GitHub Copilot, Kimi, and Qwen — but never for Codex/theclawbay.

2. Wrong field name for cache_write_tokens (agent/usage_pricing.py)

The codex_responses branch reads cache_creation_tokens (Anthropic naming convention) instead of cache_write_tokens (OpenAI Responses API naming). This means cache write tokens are always reported as 0.

Fix

Patch 1: Session routing headers

After session_id is assigned during __init__, inject session_id and x-client-request-id into default_headers for codex_responses mode. Also applied in _apply_client_headers_for_base_url() so headers survive /model switches.

Patch 2: cache_write_tokens field

Read cache_write_tokens first (OpenAI naming), fall back to cache_creation_tokens for backward compatibility.

Tests

  • test_codex_responses_reads_cache_write_tokens_field — verifies correct field is read
  • test_codex_responses_falls_back_to_cache_creation_tokens — backward compat
  • test_codex_responses_injects_session_routing_headers — verifies headers are set

Affected files

File Change
run_agent.py Inject session routing headers for codex_responses mode (+22 lines)
agent/usage_pricing.py Read cache_write_tokens before cache_creation_tokens (+3/-1 lines)
tests/agent/test_usage_pricing.py 2 new tests
tests/run_agent/test_run_agent.py 1 new test

@Marcuss2

Copy link
Copy Markdown

This will likely fix the issue I am encountering, I seem to go trough Codex limits much faster with Hermes than with Kilo.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint provider/openai OpenAI / Codex Responses API labels Apr 26, 2026
@markojak

Copy link
Copy Markdown

Linking this into the #17459 cache/time-awareness cluster.

This remains relevant for Codex/Responses caching, but should align with the umbrella direction:

Related: #16235, #15866, #17335, #17459, #17476.

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for isolating both caching concerns. The cache-write accounting portion remains relevant on current main: agent/usage_pricing.py:857-860 still reads only cache_creation_tokens for codex_responses.

Problems

  • The session-header hunk is stale. AIAgent.__init__ now delegates to agent/agent_init.py, where the session ID is assigned at agent/agent_init.py:1217-1226; this PR is currently reported as conflicting.
  • Current main already emits session_id and x-client-request-id from the active Responses transport for the Codex backend (agent/transports/codex.py:324-346), with coverage in tests/agent/transports/test_codex_transport.py:184-228. Injecting persistent client defaults is no longer the active request path.

Suggested changes

  • Salvage the cache_write_tokens-first fallback and its normalization tests.
  • Rework any broader compatible-provider header support in ResponsesApiTransport.build_kwargs() and add a transport-level regression test for the intended endpoint classification.

Automated hermes-sweeper review.

@alt-glitch alt-glitch added the sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) label Jul 12, 2026
@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 12, 2026
@sheldon-im

Copy link
Copy Markdown

I looked more closely at the current OpenAI prompt-caching documentation.

For GPT-5.6 and later model families, OpenAI reports cache writes separately as input_tokens_details.cache_write_tokens. Cache writes are also now billed separately, at 1.25× the normal input-token rate.

Hermes already includes GPT-5.6 cache-write pricing, so accurately reading this field seems important for keeping cache usage and cost accounting consistent.

I reproduced the current behavior with a small usage fixture containing cached_tokens and cache_write_tokens. The write count was recorded as zero because the codex_responses normalization path currently checks cache_creation_tokens.

So, independently of the now-stale session-header changes, the cache_write_tokens-first fallback and its tests still appear useful.

OpenAI documentation:
https://developers.openai.com/api/docs/guides/prompt-caching

@alt-glitch alt-glitch removed the sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades label Jul 13, 2026
@teknium1 teknium1 added the area/sessions Session lifecycle, resume, persistence, history label Jul 19, 2026
@alt-glitch alt-glitch added the needs-decision Awaiting maintainer decision before any implementation label Jul 19, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Related to #47126 and open #57012. The cache-write accounting change remains relevant, but this branch's session-header routing differs from the current transport-level affinity approach; please rebase and consolidate the residual work.

@israellot

Copy link
Copy Markdown
Contributor

Adding independent wire evidence for the cache_write_tokens accounting hunk, which the sweeper review flagged as the salvageable portion — it has become more important since this PR was filed.

The field is live on the wire. Two-call probe today (2026-07-23) against openai.gpt-5.6-sol through an OpenAI-compatible Responses relay (Amazon Bedrock Mantle, store: false), >1024-token identical prefix, stable prompt_cache_key:

  • call 1: input_tokens_details: {"cache_write_tokens": 1513, "cached_tokens": 0}
  • call 2: input_tokens_details: {"cache_write_tokens": 0, "cached_tokens": 1513}

Current main's codex_responses branch reads only cache_creation_tokens, so that 1513-token write normalizes to 0 and gets folded into regular input.

Why it now matters beyond telemetry: as @sheldon-im noted, GPT-5.6+ bills cache writes at 1.25× the uncached input rate (prompt caching guide), and Hermes already carries GPT-5.6 cache-write pricing — the broken field read is the only thing between current main and correct cost reporting for 5.6 traffic. On 5.5 the drop was invisible (writes free/unreported); on 5.6 it systematically under-prices every cache-writing request.

We've adopted exactly the salvage shape the sweeper suggested into our fork (cache_write_tokens first, cache_creation_tokens fallback, normalization tests for both paths): YallaPlay/hermes-agent@07d1fa1 (Co-authored-by preserved). Happy to see this PR rebased to just that hunk — the header portion is superseded by the transport-level session_id/x-client-request-id emission on current main, which we can confirm works against Bedrock Mantle (~97% hit rate on GPT-5.5 with content-addressed cache keys).

@alt-glitch alt-glitch added area/usage-cost Token accounting, usage reporting, billing, cost tracking P3 Low — cosmetic, nice to have duplicate This issue or pull request already exists and removed P2 Medium — degraded but workaround exists needs-decision Awaiting maintainer decision before any implementation sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) area/sessions Session lifecycle, resume, persistence, history labels Jul 23, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Duplicate of #70057 on the remaining current-main-relevant cache_write_tokens accounting fix. #70057 also handles explicit zero values and adds broader usage/insights coverage; this branch's session-header hunk is stale against the active transport path.

@alt-glitch alt-glitch added P2 Medium — degraded but workaround exists and removed P3 Low — cosmetic, nice to have labels Jul 23, 2026
@teknium1

Copy link
Copy Markdown
Collaborator

Status update: the agent/usage_pricing.py half of this PR — reading details.cache_write_tokens with cache_creation_tokens fallback in the codex_responses branch — is now on main via PR #85769. You were the first submitter of that fix (April 15), and that's noted in the merged PR's thread.

The session-routing-headers half (run_agent.py: session_id / x-client-request-id default headers so prompt_cache_key routes to the server holding the cached prefix) is NOT covered by that merge and this PR stays open for that part. When it gets reviewed it will need a rebase onto current main since the usage_pricing hunk is now redundant.

@alt-glitch alt-glitch added needs-decision Awaiting maintainer decision before any implementation P3 Low — cosmetic, nice to have and removed duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists labels Aug 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/usage-cost Token accounting, usage reporting, billing, cost tracking comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint needs-decision Awaiting maintainer decision before any implementation P3 Low — cosmetic, nice to have provider/openai OpenAI / Codex Responses API sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants