fix(state): durable _row_id adoption after rewind is opt-in (TUI) instead of every warm caller
On origin/main only the TUI copied durable `_row_id`s onto the installed warm prefix (its clients address follow-ups by row); folding the loop into `rewind_user_turn` made CLI /undo grow `_row_id` on a resumed history that never had it. Live CLI rows already carry ids from the flush, so for them it was a no-op, but a resumed transcript changed shape. The loop now runs only with `adopt_row_ids=True`, which the TUI passes; the CLI history shape is unchanged. Review follow-up on #109610.
This commit is contained in:
@@ -47,7 +47,7 @@ class SessionRewindMixin:
|
||||
|
||||
def rewind_user_turn(
|
||||
self, session_id: str, user_ordinal: int, *, warm_history: Optional[List[Dict[str, Any]]] = None,
|
||||
require_retryable: bool = False, require_composite: bool = False,
|
||||
require_retryable: bool = False, require_composite: bool = False, adopt_row_ids: bool = False,
|
||||
) -> RewindOutcome:
|
||||
"""Rewind the active transcript to just before user turn ``user_ordinal`` (0 = oldest; negative counts
|
||||
back from the newest and clamps to the oldest, so ``-n`` is ``/undo n``). ``warm_history`` (CLI/TUI):
|
||||
@@ -55,7 +55,9 @@ class SessionRewindMixin:
|
||||
transcript, else ``RuntimeError`` and nothing changes; its (richer) prefix is what gets installed.
|
||||
``require_retryable``: the live payload must be losslessly replayable as text (``ValueError`` from
|
||||
:func:`retryable_user_text` before any write). ``require_composite``: the target must be a compaction
|
||||
carrier. Out-of-range / wrong-shape targets raise :class:`RewindTargetUnavailableError`."""
|
||||
carrier. ``adopt_row_ids`` (TUI): copy durable ``_row_id`` identities onto the installed warm prefix so
|
||||
clients can address follow-ups by row; the CLI leaves its history shape alone. Out-of-range /
|
||||
wrong-shape targets raise :class:`RewindTargetUnavailableError`."""
|
||||
from agent.context_compressor import (
|
||||
_DB_PERSISTED_MARKER, history_before_user_originated_turn, retryable_user_text,
|
||||
split_user_originated_turn)
|
||||
@@ -103,7 +105,7 @@ class SessionRewindMixin:
|
||||
raise RuntimeError("rewind did not retain its compaction handoff")
|
||||
durable_prefix[-1].update({"_row_id": replacement_id, _DB_PERSISTED_MARKER: True})
|
||||
prefix[-1] = durable_prefix[-1]
|
||||
if prefix is not durable_prefix and len(prefix) == len(durable_prefix) and all(
|
||||
if adopt_row_ids and prefix is not durable_prefix and len(prefix) == len(durable_prefix) and all(
|
||||
warm.get("role") == durable_message.get("role")
|
||||
and bool(warm.get("display_kind")) == bool(durable_message.get("display_kind"))
|
||||
and _comparison_content(warm) == _comparison_content(durable_message)
|
||||
|
||||
@@ -97,6 +97,24 @@ def test_every_surface_persists_the_same_active_set(db, n, rich):
|
||||
assert all("QUJD" in c for r, c in active if r == "user")
|
||||
|
||||
|
||||
def test_cli_undo_leaves_the_warm_history_shape_alone_while_the_tui_adopts_row_ids(db):
|
||||
"""``_row_id`` adoption is the TUI's contract (clients address follow-ups by durable row); a CLI history
|
||||
that had no ids before /undo must not grow them."""
|
||||
from hermes_cli.cli_session_mixin import CLISessionMixin
|
||||
for sid in ("shape-cli", "shape-tui"):
|
||||
_seed(db, sid)
|
||||
warm = db.get_messages_as_conversation("shape-cli")
|
||||
assert not any("_row_id" in m for m in warm)
|
||||
cli = CLISessionMixin.__new__(CLISessionMixin)
|
||||
cli._session_db, cli.session_id, cli.conversation_history, cli.agent = db, "shape-cli", warm, None
|
||||
cli._prefill_input_buffer = MagicMock()
|
||||
cli.undo_last(1)
|
||||
assert len(cli.conversation_history) == 4 and not any("_row_id" in m for m in cli.conversation_history)
|
||||
|
||||
installed, _view, _count = _rewind_via("tui", db, "shape-tui", 1)
|
||||
assert [m["_row_id"] for m in installed] == [row[0] for row in _active_rows(db, "shape-tui") if row[3]]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("surface", SURFACES)
|
||||
def test_out_of_range_target_changes_nothing_on_every_surface(db, surface):
|
||||
sid = f"oob-{surface}"
|
||||
|
||||
@@ -384,7 +384,8 @@ def _rewind_active_session_history(
|
||||
if db is None:
|
||||
raise RuntimeError("session database is unavailable")
|
||||
outcome = db.rewind_user_turn(
|
||||
session_key, user_ordinal, warm_history=history, require_retryable=require_retryable)
|
||||
session_key, user_ordinal, warm_history=history, require_retryable=require_retryable,
|
||||
adopt_row_ids=True)
|
||||
installed, live_view, rewound_count = outcome.prefix, outcome.live_view, outcome.rewound_count
|
||||
else:
|
||||
target_index = user_indices[user_ordinal]
|
||||
|
||||
Reference in New Issue
Block a user