feat(cache): per-session cache affinity — cache_hot + route_cache + session - #15
Merged
Conversation
…he + session
Agents re-send a large, stable prefix every turn; the provider's prompt-cache
discount only lands if the SAME peer keeps serving the conversation. This adds
cache-aware routing so a policy can keep an agent's session pinned to the peer
that already holds its prefix hot.
Form delta (the four natures)
Definition: a new observation field cache_hot (Bool) — true for the candidate
whose route is the one this session most recently used successfully. Plus
route_cache, the host-side per-session memory of that route, and a session id on
the request that ties them together.
Zero engine change (verified): cache_hot is a HOST-declared extension field via
the fields.lua schema{extensions} seam, injected once in LLMRouterHost; the
core/ submodule is untouched — signature, ops and goldens unchanged. Route
identity stays 100% host-internal; the algebra only ever observes the Bool (same
contract as latency_ms/success_rate, engine #14).
Locus note: the design first proposed stamping cache_hot in offers_sync, but
offers_sync is request-blind (built once per refresh, no session). The correct
seam is ctx.request: build_ctx already exposes the whole contract as ctx.request,
so the host resolves the session's hot route into contract.cache_hot_route per
request and the field getter reconstructs each candidate's route key (exactly as
_fold_route_outcome does) and compares — no engine, no offers_sync, no per-source
edits.
Invariants: /v1 evolves additively (optional session in; nothing removed). The
central fold (_fold_route_outcome) folds route_cache alongside reliability/latency
on each outcome; no-op without a session. A new/unknown session gets no affinity
(default false — no phantom stickiness).
Essence (tests):
- tests/test_route_cache.py (10): route_cache fold (success-only, per-session),
the central-hook integration, and the cache_hot field marking exactly the hot
route so a policy scoring it picks it.
- features/10_agent_cache.feature (+ steps): end-to-end over /v1 — an agent's
session lifts its working route's score, and a brand-new session gets no
affinity. Runs on the fixed gpt-5.3-codex-spark family.
Goods: +bonum (cache efficiency), no degradation of unum/verum; net-small; no
new deps.
|
Warning Review limit reached
More reviews will be available in 58 minutes and 5 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses rolling per-developer review limits. Reviews become available again as older review attempts age out of the rolling limit window. Please see our Fair Usage Limits Policy for further information. 📝 WalkthroughWalkthroughA new ChangesAgent prompt-cache hot-route affinity
Sequence Diagram(s)sequenceDiagram
participant Client
participant shim.py
participant LLMRouterHost
participant route_cache
participant LLMProvider
Client->>shim.py: POST /v1/chat/completions {session: "sid"}
shim.py->>route_cache: hot_route("sid")
route_cache-->>shim.py: "providerA|familyX|peerA" (or None)
shim.py->>LLMRouterHost: contract {session, cache_hot_route}
LLMRouterHost->>LLMRouterHost: cache_hot field scores candidate matching cache_hot_route
LLMRouterHost->>LLMProvider: routed call
LLMProvider-->>LLMRouterHost: response (ok=True)
LLMRouterHost->>route_cache: observe("sid", rkey, ok=True)
route_cache-->>LLMRouterHost: affinity updated
LLMRouterHost-->>shim.py: response + x_router decision_trace
shim.py-->>Client: OpenAI-compat response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@features/steps/steps.py`:
- Around line 334-339: The key() function in step_rankings_identical extracts
tuples of (served_model_id, score) from rows, but when served_model_id is
missing (returns None from r.get()), the comparison silently passes by comparing
None values instead of catching the missing identity field. Modify the key()
function to explicitly check that served_model_id is not None for each row, and
raise an assertion error with a clear message if the served_model_id field is
missing or None, ensuring the test fails when ranked rows lack route identity
rather than producing false positives.
- Around line 307-309: The step_rerank_unknown function uses a fixed unknown
session ID that becomes cached after the first call to _rerank(), causing
subsequent scenario runs to not actually test an unknown session. Generate a
unique unknown session ID each time the step_rerank_unknown function is called
instead of using a constant or pre-defined ID value, so that the
route_cache.observe() behavior does not interfere with testing the unknown
session path across multiple test runs.
In `@route_cache.py`:
- Around line 37-49: The _hot dictionary grows unbounded with each new session
added in the observe function, creating a memory exhaustion risk in production
when handling high-cardinality session traffic. Implement bounded retention for
_hot by adding a size cap check before inserting a new session entry (when not
session or not ok is false). When the dictionary reaches the size limit,
implement an eviction strategy such as removing the oldest entry or implementing
an LRU-style eviction. Optionally, consider adding TTL-based eviction by
tracking timestamps for each session entry and periodically cleaning expired
entries. Apply this logic within the _lock context to maintain thread safety.
In `@tests/test_route_cache.py`:
- Around line 164-185: The test_cache_hot_false_when_no_hot_route_in_contract
function currently only verifies that both candidates survive filtering by
checking that both served_model_id values are present in the ranked results. To
strengthen the assertion and truly validate no affinity lift occurs, add an
additional check after the current assertion to verify that both candidates have
equal scores in the ranked results. This equal-score validation will confirm
that no phantom cache_hot boost or affinity stickiness is being applied to
either candidate, which is the actual behavior being tested.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9d632351-c5cf-489a-bbb6-a367cf5528aa
📒 Files selected for processing (6)
features/10_agent_cache.featurefeatures/steps/steps.pyllm_router_host.pyroute_cache.pyshim.pytests/test_route_cache.py
Comment on lines
+307
to
+309
| @when('the agent re-ranks the same turn with unknown session "{sid}" as "{label}"') | ||
| def step_rerank_unknown(context, sid, label): | ||
| context.ranks[label] = _rerank(context, sid) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the “unknown session” one-shot per scenario run.
_rerank() sends this session through /v1; after a successful call, route_cache.observe() can make the fixed "never-seen-zzz" ID hot, so reruns against the same stack no longer test an unknown session.
Use a unique unknown session ID
+import uuid `@when`('the agent re-ranks the same turn with unknown session "{sid}" as "{label}"')
def step_rerank_unknown(context, sid, label):
- context.ranks[label] = _rerank(context, sid)
+ unknown_sid = f"{sid}-{uuid.uuid4().hex}"
+ context.ranks[label] = _rerank(context, unknown_sid)🤖 Prompt for 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.
In `@features/steps/steps.py` around lines 307 - 309, The step_rerank_unknown
function uses a fixed unknown session ID that becomes cached after the first
call to _rerank(), causing subsequent scenario runs to not actually test an
unknown session. Generate a unique unknown session ID each time the
step_rerank_unknown function is called instead of using a constant or
pre-defined ID value, so that the route_cache.observe() behavior does not
interfere with testing the unknown session path across multiple test runs.
Comment on lines
+334
to
+339
| @then('the rankings "{a}" and "{b}" are identical') | ||
| def step_rankings_identical(context, a, b): | ||
| def key(rows): | ||
| return [(r.get("served_model_id"), r.get("score")) for r in rows] | ||
| ka, kb = key(context.ranks[a]), key(context.ranks[b]) | ||
| assert ka == kb, f"rankings differ:\n {a}={ka}\n {b}={kb}" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Don’t collapse ranked-row identity to None.
The nearby comment says /v1 trace rows do not echo served_model_id, but key() uses only that field. This can pass while comparing score vectors rather than the actual rankings.
Fail closed when ranked rows lack route identity
`@then`('the rankings "{a}" and "{b}" are identical')
def step_rankings_identical(context, a, b):
+ def route_id(r):
+ provider = r.get("provider_id") or jpath(r, "candidate.provider_id")
+ family = r.get("model_family") or jpath(r, "candidate.model_family")
+ served = r.get("served_model_id") or jpath(r, "candidate.served_model_id")
+ peer = (r.get("peer_id") or jpath(r, "offer.peer_id") or
+ jpath(r, "candidate.peer_id") or jpath(r, "candidate.offer.peer_id"))
+ assert served or peer, f"ranked row lacks route identity: {r}"
+ return (provider, family, peer or served)
+
def key(rows):
- return [(r.get("served_model_id"), r.get("score")) for r in rows]
+ return [(route_id(r), r.get("score")) for r in rows]
ka, kb = key(context.ranks[a]), key(context.ranks[b])
assert ka == kb, f"rankings differ:\n {a}={ka}\n {b}={kb}"🤖 Prompt for 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.
In `@features/steps/steps.py` around lines 334 - 339, The key() function in
step_rankings_identical extracts tuples of (served_model_id, score) from rows,
but when served_model_id is missing (returns None from r.get()), the comparison
silently passes by comparing None values instead of catching the missing
identity field. Modify the key() function to explicitly check that
served_model_id is not None for each row, and raise an assertion error with a
clear message if the served_model_id field is missing or None, ensuring the test
fails when ranked rows lack route identity rather than producing false
positives.
Comment on lines
+37
to
+49
| # session -> route_key of the last SUCCESSFUL call on that session. | ||
| _hot: dict[str, str] = {} | ||
|
|
||
|
|
||
| def observe(session: "str | None", key: str, ok: bool) -> None: | ||
| """Fold one call outcome. A successful call makes its route the session's hot | ||
| route (it now holds the prefix). Failures are ignored (no honest signal), and | ||
| a missing session is a no-op — affinity only exists for sessions the caller | ||
| names.""" | ||
| if not session or not ok: | ||
| return | ||
| with _lock: | ||
| _hot[session] = key |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Bound session-state growth to prevent memory exhaustion.
_hot only grows for new successful sessions and has no production eviction path. Since shim.py forwards client-provided session, high-cardinality traffic can grow this map unbounded and eventually pressure/OOM the host process. Add bounded retention (size cap and/or TTL) in the write/read path.
Suggested bounded-retention approach
+from collections import OrderedDict
+import time
+
_lock = threading.Lock()
-# session -> route_key of the last SUCCESSFUL call on that session.
-_hot: dict[str, str] = {}
+# session -> (route_key, last_seen_epoch_s)
+_hot: "OrderedDict[str, tuple[str, float]]" = OrderedDict()
+_MAX_HOT_SESSIONS = 100_000
+_HOT_TTL_S = 3600
def observe(session: "str | None", key: str, ok: bool) -> None:
@@
if not session or not ok:
return
with _lock:
- _hot[session] = key
+ _hot[session] = (key, time.time())
+ _hot.move_to_end(session)
+ while len(_hot) > _MAX_HOT_SESSIONS:
+ _hot.popitem(last=False)
def hot_route(session: "str | None") -> "str | None":
@@
if not session:
return None
- return _hot.get(session)
+ now = time.time()
+ with _lock:
+ v = _hot.get(session)
+ if v is None:
+ return None
+ key, ts = v
+ if now - ts > _HOT_TTL_S:
+ _hot.pop(session, None)
+ return None
+ return key🤖 Prompt for 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.
In `@route_cache.py` around lines 37 - 49, The _hot dictionary grows unbounded
with each new session added in the observe function, creating a memory
exhaustion risk in production when handling high-cardinality session traffic.
Implement bounded retention for _hot by adding a size cap check before inserting
a new session entry (when not session or not ok is false). When the dictionary
reaches the size limit, implement an eviction strategy such as removing the
oldest entry or implementing an LRU-style eviction. Optionally, consider adding
TTL-based eviction by tracking timestamps for each session entry and
periodically cleaning expired entries. Apply this logic within the _lock context
to maintain thread safety.
Comment on lines
+164
to
+185
| def test_cache_hot_false_when_no_hot_route_in_contract(host): | ||
| # No cache_hot_route in the contract -> the field is false for everyone, so | ||
| # the affinity scorer adds nothing (no phantom stickiness for a new session). | ||
| base = {"model_family": "glm-5.2", "capabilities": {}} | ||
| a = {**base, "provider_id": "antseed", "served_model_id": "A", | ||
| "offer": {"model_family": "glm-5.2", "peer_id": "peerA"}} | ||
| b = {**base, "provider_id": "antseed", "served_model_id": "B", | ||
| "offer": {"model_family": "glm-5.2", "peer_id": "peerB"}} | ||
| policy = ["policy", | ||
| ["and", ["meets_req"], ["not", ["is", "disabled"]]], | ||
| ["gate", ["is", "cache_hot"], ["lit", 1]], | ||
| ["argmax"], ["id"], | ||
| ["always", {"action": "next_candidate"}]] | ||
| ranked, _ = host.rank({ | ||
| "prompt": "x", "profile": "default", "policy_ir": policy, | ||
| "requirements": {"model_family": "glm-5.2"}, # isolate to our two peers | ||
| "extra_candidates": [a, b], | ||
| }) | ||
| assert ranked | ||
| # cache_hot is false for everyone (no hot route in the contract): both peers | ||
| # survive, the affinity scorer adds nothing, no phantom stickiness. | ||
| assert {r["candidate"]["served_model_id"] for r in ranked} == {"A", "B"} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Strengthen the “no hot route” assertion to verify no affinity lift.
The current assertion only proves both candidates survive filtering. A phantom cache_hot boost on one candidate could still pass this test. Add an equal-score check (under this cache_hot-only policy) to validate true no-affinity behavior.
Suggested test hardening
def test_cache_hot_false_when_no_hot_route_in_contract(host):
@@
assert ranked
# cache_hot is false for everyone (no hot route in the contract): both peers
# survive, the affinity scorer adds nothing, no phantom stickiness.
assert {r["candidate"]["served_model_id"] for r in ranked} == {"A", "B"}
+ scores = {r["candidate"]["served_model_id"]: r["score"] for r in ranked}
+ assert scores["A"] == scores["B"]🤖 Prompt for 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.
In `@tests/test_route_cache.py` around lines 164 - 185, The
test_cache_hot_false_when_no_hot_route_in_contract function currently only
verifies that both candidates survive filtering by checking that both
served_model_id values are present in the ranked results. To strengthen the
assertion and truly validate no affinity lift occurs, add an additional check
after the current assertion to verify that both candidates have equal scores in
the ranked results. This equal-score validation will confirm that no phantom
cache_hot boost or affinity stickiness is being applied to either candidate,
which is the actual behavior being tested.
The cache_hot getter re-serialized the route key (provider|family|peer) in Lua — a second source for an identity already defined once by route_reliability.route_key in Python. Format or peer-derivation drift across the Python/Lua boundary would silently lose affinity (cache_hot all false, no error). Bridge the single route_key into Lua as the host_route_key global and call it from the getter, so the serialization has exactly one source. Behaviour unchanged: route_cache tests 10/0; full suite identical A/B (the 3 provider_filter_flow failures are pre-existing — the branch's core/ submodule is at engine #17 and needs #18 for provider_eq; rebase on main).
jmlago
added a commit
that referenced
this pull request
Jun 23, 2026
* feat(compact): /v1/compact — append-only context sealing (stateless) An agent loop grows its context until it must be compacted. Done naively (re-summarize everything) the seal rewrites the prefix and destroys the prompt cache — caching and compaction fight each other. /v1/compact seals only the AGED middle into one cheaply-routed summary and splices it back, so the frozen system prefix and the recent tail stay byte-identical and everything upstream stays cache-hot (it composes with the cache_hot affinity from #15). Form delta Definition: POST /v1/compact {messages, keep_recent, policy_ir?, max_tokens?} -> {messages, compacted}. A STATELESS transform — array in, array out. The host stores no conversation; the agent stays sovereign over its context. Consumer surface (the ingress catch-all forwards it with caller auth), so agents (SubZeroClaw, opencode) can call it. Splice (append-only): frozen = a leading system message; aged = the middle; recent = last keep_recent. The aged span is summarized by a cheaply-routed call (default: cheapest healthy route; the caller can pin a model — the BDD routes it to the local Ollama). Result: frozen + [sealed summary] + recent. The prefix and tail are never rewritten. Invariants: anti-orphan guard — a leading `tool` message in `recent` whose assistant lives in the dropped `aged` span would 400 the next provider call; it is dropped at the seam. No-op (compacted=false) when there is nothing worth sealing or the seal returns empty (never lose content). Additive: a new consumer route; nothing else changes. Essence: - tests/test_compact.py (3, hermetic via TestClient + mocked summarizer): the append-only splice, the short-input no-op, the anti-orphan seam. - features/11_agent_compact.feature: end-to-end over /v1/compact, summary routed to the local Ollama, asserting compaction + prefix + verbatim tail. Goods: +bonum (token/cache efficiency on long agent loops); net-small; no deps; no engine change. * feat(compact): suggest compaction via x_router.compact at a token threshold The companion to /v1/compact: the host decides WHEN to seal, the agent doesn't own the threshold. When a call's input crosses settings `compaction.at_tokens` (default 24000, operator-tunable, dashboard Config tab), the response carries x_router.compact=true so an agent (SubZeroClaw, opencode) knows to POST /v1/compact next. Measured on the real prompt_tokens the call reported, so it costs nothing; additive (OpenAI clients ignore x_router); set on both the streaming and non-streaming response builders. Tests: tests/test_compact.py — flag true over the threshold, false under.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Agents re-send a large, stable prefix every turn; the provider's prompt-cache
discount only lands if the SAME peer keeps serving the conversation. This adds
cache-aware routing so a policy can keep an agent's session pinned to the peer
that already holds its prefix hot.
Form delta (the four natures)
Definition: a new observation field cache_hot (Bool) — true for the candidate
whose route is the one this session most recently used successfully. Plus
route_cache, the host-side per-session memory of that route, and a session id on
the request that ties them together.
Zero engine change (verified): cache_hot is a HOST-declared extension field via
the fields.lua schema{extensions} seam, injected once in LLMRouterHost; the
core/ submodule is untouched — signature, ops and goldens unchanged. Route
identity stays 100% host-internal; the algebra only ever observes the Bool (same
contract as latency_ms/success_rate, engine #14).
Locus note: the design first proposed stamping cache_hot in offers_sync, but
offers_sync is request-blind (built once per refresh, no session). The correct
seam is ctx.request: build_ctx already exposes the whole contract as ctx.request,
so the host resolves the session's hot route into contract.cache_hot_route per
request and the field getter reconstructs each candidate's route key (exactly as
_fold_route_outcome does) and compares — no engine, no offers_sync, no per-source
edits.
Invariants: /v1 evolves additively (optional session in; nothing removed). The
central fold (_fold_route_outcome) folds route_cache alongside reliability/latency
on each outcome; no-op without a session. A new/unknown session gets no affinity
(default false — no phantom stickiness).
Essence (tests):
the central-hook integration, and the cache_hot field marking exactly the hot
route so a policy scoring it picks it.
session lifts its working route's score, and a brand-new session gets no
affinity. Runs on the fixed gpt-5.3-codex-spark family.
Goods: +bonum (cache efficiency), no degradation of unum/verum; net-small; no
new deps.
Summary by CodeRabbit
New Features
sessionfield to chat API requests for session trackingTests