fix(agent): same-model review fork keeps the parent's affinity header and Portal conversation root (#109964)
Trimmed salvage of #110045 (deltas 1 + 2 only), stacked on the #110009 scope inheritance: - `declared_conversation_scope` treats an inherited value as a DECLARED scope only when it carries the `gwk_` prefix. A rotated CLI parent publishes no affinity scope (None → sticky key falls back to the conversation root); the fork now publishes exactly the same instead of an explicit physical lineage root. `resolve_prompt_cache_scope` honors any inherited value directly, so the body `prompt_cache_key` still matches. - `build_cache_parity_fork` snapshots `parent._conversation_root_id()` as `_cached_conversation_root`; with `_session_db=None` the fork's own walk fell back to the parent's PHYSICAL id, so after a compression rotation the review's Portal `conversation=` tag fragmented usage attribution across one logical conversation. Dropped from the original: copying `_gateway_session_key` onto the persistence-detached fork (no cache-identity consumer reads it there; the compression-boundary hooks were deliberately severed by `_detach_fork_compression`), and the defensive hasattr/callable/try wrapper around `_conversation_root_id()`.
This commit is contained in:
@@ -970,6 +970,10 @@ def build_cache_parity_fork(
|
||||
inherited_scope = resolve_prompt_cache_scope_safe(agent)
|
||||
if inherited_scope:
|
||||
review_agent._inherited_cache_scope = inherited_scope
|
||||
# Same reason for the Portal ``conversation=`` tag: with no DB the fork's own
|
||||
# _conversation_root_id() falls back to the parent's PHYSICAL id, so after a compression
|
||||
# rotation the review's usage was attributed to a different conversation than its parent.
|
||||
review_agent._cached_conversation_root = agent._conversation_root_id()
|
||||
_inherit_parent_tool_surface(review_agent, agent)
|
||||
_detach_fork_compression(review_agent)
|
||||
# Compaction bounds a single request; this bounds the WHOLE review (checked in
|
||||
|
||||
@@ -87,23 +87,21 @@ def declared_conversation_scope(agent: Any) -> Optional[str]:
|
||||
"""Host-declared logical conversation scope (``gwk_<sha256[:24]>``), or None.
|
||||
|
||||
Hashes ``(source, gateway_session_key, generation)``. None (fall back to the physical id)
|
||||
when no key is declared, for an explicit fork child, and on any DB error (fail closed
|
||||
rather than merge a fork onto its parent's key).
|
||||
when no key is declared, for a background-review fork (``_persist_disabled``), for an
|
||||
explicit fork child, and on any DB error (fail closed rather than merge a fork onto its
|
||||
parent's key).
|
||||
|
||||
A same-model background-review fork (``_persist_disabled=True``, ``_session_db=None``)
|
||||
resolves the parent's *already-resolved* scope via ``_inherited_cache_scope`` BEFORE the
|
||||
``_persist_disabled`` fail-closed branch (#109964): the fork's entire purpose is prefix
|
||||
parity with the parent, but the exclusion below plus the missing DB made both resolvers
|
||||
diverge — the header path (affinity/sticky session) and the body path
|
||||
(``prompt_cache_key``) keyed the fork into a different bucket, costing one cold
|
||||
~full-context request per review. The inherited value is stamped once at fork time by
|
||||
``build_cache_parity_fork`` (no DB access from the fork), so persistence stays fully
|
||||
detached. Nothing sets the attribute for routed (different-model) forks, ``/branch``
|
||||
children, delegate/tool children, or fresh sessions — the fail-closed default stands.
|
||||
The one sanctioned exception is a same-model cache-parity fork (#109964): its whole purpose
|
||||
is prefix parity with the parent, yet ``_persist_disabled`` + ``_session_db=None`` made both
|
||||
resolvers key it into a different bucket (one cold ~full-context request per review).
|
||||
``build_cache_parity_fork`` stamps the parent's ALREADY-RESOLVED scope as
|
||||
``_inherited_cache_scope`` (no DB access from the fork). Only a ``gwk_`` value is a declared
|
||||
scope; a physical lineage root stays out of the affinity header so the fork publishes
|
||||
exactly what its parent publishes (None → consumers fall back to the conversation root).
|
||||
"""
|
||||
inherited = getattr(agent, "_inherited_cache_scope", None)
|
||||
if inherited:
|
||||
return str(inherited)
|
||||
if isinstance(inherited, str) and inherited.startswith(_DECLARED_SCOPE_PREFIX):
|
||||
return inherited
|
||||
key = str(getattr(agent, "_gateway_session_key", "") or "").strip()
|
||||
if not key or getattr(agent, "_persist_disabled", False):
|
||||
return None
|
||||
@@ -143,12 +141,12 @@ def declared_conversation_scope(agent: Any) -> Optional[str]:
|
||||
|
||||
|
||||
def resolve_prompt_cache_scope(agent: Any) -> str:
|
||||
"""Rotation-stable cache-scope id: declared scope, else the compression-lineage root of
|
||||
``agent.session_id`` (the physical id without ancestry/DB). Memoized on the agent.
|
||||
|
||||
A same-model review fork's inherited scope (``_inherited_cache_scope``) resolves through
|
||||
:func:`declared_conversation_scope` above, which honors it first — fixing header and body
|
||||
paths together (#109964)."""
|
||||
"""Rotation-stable cache-scope id: the inherited parent scope of a same-model cache-parity
|
||||
fork, else the declared scope, else the compression-lineage root of ``agent.session_id``
|
||||
(the physical id without ancestry/DB). Memoized on the agent."""
|
||||
inherited = getattr(agent, "_inherited_cache_scope", None)
|
||||
if isinstance(inherited, str) and inherited:
|
||||
return inherited
|
||||
sid = str(getattr(agent, "session_id", None) or "")
|
||||
if not sid:
|
||||
return ""
|
||||
|
||||
@@ -1333,6 +1333,9 @@ class AIAgent(
|
||||
def _conversation_root_id(self) -> Optional[str]:
|
||||
"""Session-lineage ROOT id for Portal usage attribution, so one conversation keeps a single
|
||||
``conversation=`` tag across compression rotation; subagents resolve via ``_parent_session_id``."""
|
||||
cached = getattr(self, "_cached_conversation_root", None)
|
||||
if cached:
|
||||
return str(cached)
|
||||
sid = getattr(self, "session_id", None)
|
||||
if not sid:
|
||||
return None
|
||||
|
||||
@@ -520,10 +520,14 @@ def test_same_model_fork_inherits_parent_cache_scope_gateway_key(tmp_path):
|
||||
def test_same_model_fork_inherits_parent_cache_scope_rotated_lineage(tmp_path):
|
||||
"""#109964 invariant 1 (rotated-lineage case): a CLI parent whose lineage
|
||||
root != current physical id must also pass its scope to the fork. Pre-fix
|
||||
the parent resolved 'root-sid' while the fork fell to the physical id."""
|
||||
the parent resolved 'root-sid' while the fork fell to the physical id.
|
||||
|
||||
Every identity the fork publishes must equal the parent's: body cache key, the
|
||||
affinity header (None for both — a physical root is not a declared ``gwk_`` scope)
|
||||
and the Portal ``conversation=`` root (fork has no DB to walk the lineage)."""
|
||||
import run_agent
|
||||
from agent.background_review import build_cache_parity_fork
|
||||
from agent.prompt_cache_scope import resolve_prompt_cache_scope
|
||||
from agent.prompt_cache_scope import declared_conversation_scope, resolve_prompt_cache_scope
|
||||
from hermes_state import SessionDB
|
||||
|
||||
db = SessionDB(db_path=tmp_path / "state.db")
|
||||
@@ -543,6 +547,9 @@ def test_same_model_fork_inherits_parent_cache_scope_rotated_lineage(tmp_path):
|
||||
assert resolve_prompt_cache_scope(agent) == "root-sid"
|
||||
assert getattr(fork, "_inherited_cache_scope", None) == "root-sid"
|
||||
assert resolve_prompt_cache_scope(fork) == "root-sid"
|
||||
assert declared_conversation_scope(fork) is declared_conversation_scope(agent) is None
|
||||
fork_root = run_agent.AIAgent._conversation_root_id(fork)
|
||||
assert fork_root == agent._conversation_root_id() == "root-sid"
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user