mirror of
https://github.com/turnstonelabs/turnstone.git
synced 2026-08-12 23:12:23 -06:00
refactor(storage): drop the dead _reminders column instead of carrying it
Operator context moved to first-class system turns, leaving _reminders written by nothing and read by nothing. Nulling it (the prior 060 step) left a writable dead column — a foot-gun inviting accidental reuse. Drop it outright and remove every reference in one shot so there is no half-alive state: - migration 060: replace the wholesale null with batch_alter_table drop_column (per migration 027); downgrade re-adds the empty column to match the 059 schema (the envelope un-wrap stays irreversible). - _schema.py: remove the column. - _sqlite / _postgresql: drop the reminders save param, the INSERT/bulk values, and both SELECT columns. - reconstruct_messages: the row tuple is now 8/9-tuple (event_id shifts from index 9 to 8); _utils + the _row test helper updated. - _protocol / memory save_message: drop the reminders param + docstrings. - tests: replace the reminders-roundtrip tests with a _source-only file and a 060 drop-column assertion; remove the obsolete legacy-reminders wire test. No production caller passed reminders=, and the SELECT no longer reads the column, so an un-migrated DB simply ignores any residual values.
This commit is contained in:
@@ -11,7 +11,8 @@ isolated SQLite database per test, then asserts:
|
||||
bare row that merely starts with ``<tool_output>`` — or even one with a
|
||||
matching ``</tool_output>`` close but no advisory — is left untouched (the
|
||||
known-issue #1 false positive);
|
||||
* the ``_reminders`` side-channel column is nulled;
|
||||
* the dead ``_reminders`` side-channel column is dropped outright (not nulled
|
||||
and carried forward as a writable foot-gun);
|
||||
* the migration is idempotent (a second run is a no-op);
|
||||
* a plain non-envelope row is untouched.
|
||||
"""
|
||||
@@ -200,7 +201,7 @@ class TestMigration060:
|
||||
finally:
|
||||
engine.dispose()
|
||||
|
||||
def test_nulls_reminders_column(self, tmp_path: Path) -> None:
|
||||
def test_drops_reminders_column(self, tmp_path: Path) -> None:
|
||||
db_path = tmp_path / "060-reminders.db"
|
||||
cfg = _alembic_cfg(db_path)
|
||||
command.upgrade(cfg, "059")
|
||||
@@ -208,6 +209,7 @@ class TestMigration060:
|
||||
engine = sa.create_engine(f"sqlite:///{db_path}")
|
||||
try:
|
||||
with engine.begin() as conn:
|
||||
# The column still exists at 059, so a legacy value can be seeded.
|
||||
_seed_row(
|
||||
conn,
|
||||
role="user",
|
||||
@@ -215,14 +217,23 @@ class TestMigration060:
|
||||
tool_call_id="call_d",
|
||||
_reminders='[{"type":"correction","text":"watch it"}]',
|
||||
)
|
||||
# At 059 the column is present.
|
||||
assert "_reminders" in {
|
||||
c["name"] for c in sa.inspect(engine).get_columns("conversations")
|
||||
}
|
||||
|
||||
command.upgrade(cfg, "060")
|
||||
|
||||
# 060 drops it outright (no dead column carried forward); the row
|
||||
# itself survives.
|
||||
cols = {c["name"] for c in sa.inspect(engine).get_columns("conversations")}
|
||||
assert "_reminders" not in cols
|
||||
assert "_source" in cols # the live sibling stays
|
||||
with engine.connect() as conn:
|
||||
reminders = conn.execute(
|
||||
sa.text("SELECT _reminders FROM conversations WHERE tool_call_id = 'call_d'")
|
||||
content = conn.execute(
|
||||
sa.text("SELECT content FROM conversations WHERE tool_call_id = 'call_d'")
|
||||
).scalar_one()
|
||||
assert reminders is None
|
||||
assert content == "hello"
|
||||
finally:
|
||||
engine.dispose()
|
||||
|
||||
|
||||
@@ -16,13 +16,12 @@ def _row(
|
||||
pdata=None,
|
||||
tool_calls=None,
|
||||
source=None,
|
||||
reminders=None,
|
||||
):
|
||||
"""Build a 9-element conversation row tuple (id, role, ...).
|
||||
"""Build an 8-element conversation row tuple (id, role, ...).
|
||||
|
||||
Trailing ``source`` / ``reminders`` mirror the persisted twins of
|
||||
the in-memory ``_source`` / ``_reminders`` side-channels added in
|
||||
migration 050.
|
||||
Trailing ``source`` is the persisted twin of the in-memory ``_source``
|
||||
side-channel. (The ``_reminders`` column that used to ride here was
|
||||
dropped in migration 060 — operator context lives in ``system`` turns.)
|
||||
"""
|
||||
return (
|
||||
next(_row_ids),
|
||||
@@ -33,7 +32,6 @@ def _row(
|
||||
pdata,
|
||||
tool_calls,
|
||||
source,
|
||||
reminders,
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -4254,35 +4254,6 @@ class TestReminderSidechannelIsolation:
|
||||
assert extracted_user == "first message body"
|
||||
assert "SECRET_NUDGE_TEXT" not in extracted_user
|
||||
|
||||
def test_wire_pass_does_not_render_legacy_reminders(self, tmp_db):
|
||||
"""A pre-migration row whose ``_reminders`` column survived
|
||||
``load_messages`` must NOT leak onto the wire: the wire transform
|
||||
only folds first-class ``system`` turns and ignores the dead
|
||||
``_reminders`` side-channel entirely (no envelope splice anymore).
|
||||
"""
|
||||
from turnstone.core.memory import register_workstream, save_message
|
||||
|
||||
register_workstream("resume_no_resplice")
|
||||
save_message(
|
||||
"resume_no_resplice",
|
||||
"user",
|
||||
"second turn",
|
||||
reminders=json.dumps(
|
||||
[{"type": "denial", "text": "HISTORICAL_REMINDER_BODY"}],
|
||||
separators=(",", ":"),
|
||||
),
|
||||
)
|
||||
save_message("resume_no_resplice", "assistant", "ok")
|
||||
|
||||
session = _make_session()
|
||||
assert session.resume("resume_no_resplice") is True
|
||||
|
||||
session.messages.append({"role": "user", "content": "live new turn"})
|
||||
wire = session._prepare_wire_messages(session.messages)
|
||||
rendered = "\n".join(m["content"] for m in wire if isinstance(m.get("content"), str))
|
||||
assert "HISTORICAL_REMINDER_BODY" not in rendered
|
||||
assert "<system-reminder>" not in rendered
|
||||
|
||||
def test_fork_preserves_source(self, tmp_db):
|
||||
"""A forked workstream's resumed transcript carries the wake
|
||||
marker (``_source = "system_nudge"``). The bulk-row builder
|
||||
|
||||
@@ -0,0 +1,37 @@
|
||||
"""Tests for the ``_source`` storage column round-tripping through both backends.
|
||||
|
||||
``_source`` mirrors the in-memory side channel — which producer synthesised the
|
||||
row (a wake ``system_nudge`` or an operator-context kind on a ``system`` turn).
|
||||
(The sibling ``_reminders`` column was dropped in migration 060; operator
|
||||
context lives in first-class ``system`` turns now.)
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
|
||||
class TestSourceRoundtrip:
|
||||
def test_source_roundtrip(self, backend):
|
||||
backend.register_workstream("s1")
|
||||
backend.save_message("s1", "user", "", source="system_nudge")
|
||||
msgs = backend.load_messages("s1")
|
||||
assert len(msgs) == 1
|
||||
assert msgs[0]["role"] == "user"
|
||||
assert msgs[0]["content"] == ""
|
||||
assert msgs[0].get("_source") == "system_nudge"
|
||||
|
||||
def test_source_absent_when_not_set(self, backend):
|
||||
backend.register_workstream("s1")
|
||||
backend.save_message("s1", "user", "hello")
|
||||
msgs = backend.load_messages("s1")
|
||||
assert "_source" not in msgs[0]
|
||||
|
||||
def test_nul_bytes_stripped_from_source(self, backend):
|
||||
"""NUL bytes must be stripped from ``_source`` at the storage layer.
|
||||
|
||||
Producers strip NUL today, but the layer is the tripwire if a future
|
||||
producer forgets — and PostgreSQL TEXT columns reject NUL outright.
|
||||
"""
|
||||
backend.register_workstream("s1")
|
||||
backend.save_message("s1", "user", "", source="system_nudge\x00")
|
||||
msgs = backend.load_messages("s1")
|
||||
assert msgs[0].get("_source") == "system_nudge"
|
||||
@@ -1,158 +0,0 @@
|
||||
"""Tests for ``_source`` / ``_reminders`` round-tripping through both
|
||||
storage backends.
|
||||
|
||||
Persisting the in-memory side-channels lets multi-tab / multi-device
|
||||
replay show the same metacognitive bubble shape the originating tab
|
||||
saw live — see ``docs/design/watch-card-ux.md`` §1.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
|
||||
import sqlalchemy as sa
|
||||
|
||||
from turnstone.core.storage._schema import conversations
|
||||
|
||||
|
||||
class TestSourceRoundtrip:
|
||||
def test_source_roundtrip(self, backend):
|
||||
backend.register_workstream("s1")
|
||||
backend.save_message("s1", "user", "", source="system_nudge")
|
||||
msgs = backend.load_messages("s1")
|
||||
assert len(msgs) == 1
|
||||
assert msgs[0]["role"] == "user"
|
||||
assert msgs[0]["content"] == ""
|
||||
assert msgs[0].get("_source") == "system_nudge"
|
||||
|
||||
def test_source_absent_when_not_set(self, backend):
|
||||
backend.register_workstream("s1")
|
||||
backend.save_message("s1", "user", "hello")
|
||||
msgs = backend.load_messages("s1")
|
||||
assert "_source" not in msgs[0]
|
||||
|
||||
|
||||
class TestRemindersRoundtrip:
|
||||
def test_reminders_roundtrip(self, backend):
|
||||
backend.register_workstream("s1")
|
||||
payload = [
|
||||
{
|
||||
"type": "watch_triggered",
|
||||
"text": "$ ls\nfile.txt\n",
|
||||
"watch_name": "w1",
|
||||
"command": "ls",
|
||||
"poll_count": 2,
|
||||
"max_polls": 100,
|
||||
"is_final": False,
|
||||
}
|
||||
]
|
||||
backend.save_message(
|
||||
"s1",
|
||||
"user",
|
||||
"",
|
||||
source="system_nudge",
|
||||
reminders=json.dumps(payload, separators=(",", ":")),
|
||||
)
|
||||
msgs = backend.load_messages("s1")
|
||||
assert msgs[0].get("_reminders") == payload
|
||||
# Optional fields preserved verbatim.
|
||||
rem = msgs[0]["_reminders"][0]
|
||||
assert rem["watch_name"] == "w1"
|
||||
assert rem["command"] == "ls"
|
||||
assert rem["poll_count"] == 2
|
||||
assert rem["max_polls"] == 100
|
||||
assert rem["is_final"] is False
|
||||
|
||||
def test_reminders_null_renders_as_no_key(self, backend):
|
||||
"""Absent vs. empty-list should map to the same shape on the
|
||||
load side: ``_reminders`` simply not present in the dict.
|
||||
Mirrors the ``_attachments_meta`` precedent in
|
||||
``reconstruct_messages``.
|
||||
"""
|
||||
backend.register_workstream("s1")
|
||||
backend.save_message("s1", "user", "hello")
|
||||
msgs = backend.load_messages("s1")
|
||||
assert "_reminders" not in msgs[0]
|
||||
|
||||
def test_tool_reminders_roundtrip(self, backend):
|
||||
backend.register_workstream("s1")
|
||||
# Build a minimal valid history: assistant turn with one
|
||||
# tool_call followed by the tool result that carries the
|
||||
# tool-channel reminder. Without the assistant turn the
|
||||
# tool row would be orphaned and stripped by the repair pass.
|
||||
tc_json = json.dumps(
|
||||
[
|
||||
{
|
||||
"id": "c1",
|
||||
"type": "function",
|
||||
"function": {"name": "bash", "arguments": "{}"},
|
||||
}
|
||||
]
|
||||
)
|
||||
backend.save_message("s1", "user", "go")
|
||||
backend.save_message("s1", "assistant", None, tool_calls=tc_json)
|
||||
payload = [{"type": "tool_error", "text": "command failed"}]
|
||||
backend.save_message(
|
||||
"s1",
|
||||
"tool",
|
||||
"boom",
|
||||
tool_call_id="c1",
|
||||
reminders=json.dumps(payload, separators=(",", ":")),
|
||||
)
|
||||
msgs = backend.load_messages("s1")
|
||||
# Find the tool message and assert reminders survived load.
|
||||
tool_msgs = [m for m in msgs if m.get("role") == "tool"]
|
||||
assert len(tool_msgs) == 1
|
||||
assert tool_msgs[0].get("_reminders") == payload
|
||||
|
||||
def test_nul_bytes_stripped_from_source_and_reminders(self, backend):
|
||||
"""NUL bytes must be stripped at the storage layer.
|
||||
|
||||
Producers (``sanitize_payload`` on the watch dispatch path,
|
||||
constants for non-watch nudges) already strip NUL today so
|
||||
nothing in production reaches this clamp — but the layer is
|
||||
the tripwire if a future producer forgets, mirroring how
|
||||
``content`` and ``provider_data`` are sanitized. PostgreSQL
|
||||
TEXT columns reject NUL outright, so the sanitization is also
|
||||
a hard correctness invariant on that backend.
|
||||
|
||||
``json.dumps`` already escapes NUL inside string values to
|
||||
``\\u0000`` so a real NUL byte can't enter ``_reminders`` via
|
||||
the normal encode path — the test feeds a raw NUL directly to
|
||||
cover the bypass case (a future producer that hand-builds the
|
||||
column string).
|
||||
"""
|
||||
backend.register_workstream("s1")
|
||||
backend.save_message(
|
||||
"s1",
|
||||
"user",
|
||||
"",
|
||||
source="system_nudge\x00",
|
||||
reminders='[{"type":"watch_triggered","text":"ok\x00bad"}]',
|
||||
)
|
||||
msgs = backend.load_messages("s1")
|
||||
assert msgs[0].get("_source") == "system_nudge"
|
||||
assert msgs[0].get("_reminders") == [{"type": "watch_triggered", "text": "okbad"}]
|
||||
|
||||
def test_malformed_reminders_json_does_not_crash_load(self, backend):
|
||||
"""A garbage string in the column must not abort the whole
|
||||
load — mirrors the ``provider_data`` JSON-decode-suppress
|
||||
pattern. Concretely: write a row with valid columns BUT a
|
||||
corrupted ``_reminders`` value via raw SQL, then verify the
|
||||
load returns the message with no ``_reminders`` key (rather
|
||||
than raising or surfacing the garbage).
|
||||
"""
|
||||
backend.register_workstream("s1")
|
||||
msg_id = backend.save_message("s1", "user", "hello")
|
||||
with backend._engine.connect() as conn:
|
||||
conn.execute(
|
||||
sa.update(conversations)
|
||||
.where(conversations.c.id == msg_id)
|
||||
.values(_reminders="this is not json {{")
|
||||
)
|
||||
conn.commit()
|
||||
msgs = backend.load_messages("s1")
|
||||
assert len(msgs) == 1
|
||||
# Garbage suppressed silently — key absent, content intact.
|
||||
assert "_reminders" not in msgs[0]
|
||||
assert msgs[0]["content"] == "hello"
|
||||
@@ -42,7 +42,6 @@ def save_message(
|
||||
provider_data: str | None = None,
|
||||
tool_calls: str | None = None,
|
||||
source: str | None = None,
|
||||
reminders: str | None = None,
|
||||
event_id: int | None = None,
|
||||
) -> int:
|
||||
"""Log a message to the conversations table.
|
||||
@@ -50,10 +49,9 @@ def save_message(
|
||||
Returns the inserted row id, or ``0`` on failure (preserving the
|
||||
module's no-raise contract).
|
||||
|
||||
``source`` / ``reminders`` mirror the in-memory ``_source`` /
|
||||
``_reminders`` side-channels (``reminders`` JSON-encoded). Both
|
||||
default to ``None`` for the common case where no metacog payload
|
||||
rides the row.
|
||||
``source`` is the persisted twin of the in-memory ``_source``
|
||||
side-channel (which producer synthesised the row); ``None`` for the
|
||||
common case of an ordinary user/assistant/tool row.
|
||||
|
||||
``event_id`` is the per-ws SSE ring-buffer high-water mark at save
|
||||
time (``SessionUIBase._event_id``); the caller in ``session.py``
|
||||
@@ -70,7 +68,6 @@ def save_message(
|
||||
provider_data,
|
||||
tool_calls=tool_calls,
|
||||
source=source,
|
||||
reminders=reminders,
|
||||
event_id=event_id,
|
||||
)
|
||||
except Exception:
|
||||
|
||||
@@ -2965,8 +2965,8 @@ def make_history_handler(cfg: SessionEndpointConfig) -> Handler:
|
||||
)
|
||||
# Final structural projection: flatten nested tool_calls,
|
||||
# collapse multipart content, surface the
|
||||
# ``_source`` / ``_reminders`` / ``_attachments_meta``
|
||||
# side-channels top-level, and derive
|
||||
# ``_source`` / ``_attachments_meta`` side-channels
|
||||
# top-level, and derive
|
||||
# ``denied`` / ``is_error`` / ``pending``. Runs last (reads
|
||||
# decorate's in-place verdict/advisory mutations + the
|
||||
# stamped ``reasoning``) and returns a fresh list, so the
|
||||
|
||||
@@ -290,14 +290,12 @@ class PostgreSQLBackend:
|
||||
provider_data: str | None = None,
|
||||
tool_calls: str | None = None,
|
||||
source: str | None = None,
|
||||
reminders: str | None = None,
|
||||
event_id: int | None = None,
|
||||
) -> int:
|
||||
now = datetime.now(UTC).strftime("%Y-%m-%dT%H:%M:%S")
|
||||
content = sanitize_text(content)
|
||||
provider_data = sanitize_text(provider_data)
|
||||
source = sanitize_text(source)
|
||||
reminders = sanitize_text(reminders)
|
||||
with self._conn() as conn:
|
||||
result = conn.execute(
|
||||
sa.insert(conversations)
|
||||
@@ -311,7 +309,6 @@ class PostgreSQLBackend:
|
||||
provider_data=provider_data,
|
||||
tool_calls=tool_calls,
|
||||
_source=source,
|
||||
_reminders=reminders,
|
||||
event_id=event_id,
|
||||
)
|
||||
.returning(conversations.c.id)
|
||||
@@ -343,7 +340,6 @@ class PostgreSQLBackend:
|
||||
"provider_data": sanitize_text(row.get("provider_data")),
|
||||
"tool_calls": row.get("tool_calls"),
|
||||
"_source": sanitize_text(row.get("source")),
|
||||
"_reminders": sanitize_text(row.get("reminders")),
|
||||
}
|
||||
)
|
||||
with self._conn() as conn:
|
||||
@@ -369,7 +365,6 @@ class PostgreSQLBackend:
|
||||
conversations.c.provider_data,
|
||||
conversations.c.tool_calls,
|
||||
conversations.c._source,
|
||||
conversations.c._reminders,
|
||||
conversations.c.event_id,
|
||||
)
|
||||
.where(conversations.c.ws_id == ws_id)
|
||||
@@ -388,7 +383,6 @@ class PostgreSQLBackend:
|
||||
conversations.c.provider_data,
|
||||
conversations.c.tool_calls,
|
||||
conversations.c._source,
|
||||
conversations.c._reminders,
|
||||
conversations.c.event_id,
|
||||
)
|
||||
.where(conversations.c.ws_id == ws_id)
|
||||
|
||||
@@ -158,7 +158,6 @@ class StorageBackend(Protocol):
|
||||
provider_data: str | None = None,
|
||||
tool_calls: str | None = None,
|
||||
source: str | None = None,
|
||||
reminders: str | None = None,
|
||||
event_id: int | None = None,
|
||||
) -> int:
|
||||
"""Log a message to the conversations table.
|
||||
@@ -168,10 +167,9 @@ class StorageBackend(Protocol):
|
||||
use this to associate the row after save.
|
||||
|
||||
``source`` is the persisted twin of the in-memory ``_source``
|
||||
side-channel (today only ``"system_nudge"`` for wake-driven
|
||||
empty user turns). ``reminders`` is a JSON-encoded list mirroring
|
||||
``_reminders``; both are NULL for the common case where a
|
||||
message carries no metacog payload.
|
||||
side-channel — which producer synthesised the row (a wake
|
||||
``"system_nudge"`` or an operator-context kind on a ``system`` turn);
|
||||
NULL for ordinary user/assistant/tool rows.
|
||||
|
||||
``event_id`` is the per-ws SSE ring-buffer high-water mark at save
|
||||
time (``SessionUIBase._event_id``) — the ``Last-Event-ID`` resume
|
||||
@@ -186,8 +184,8 @@ class StorageBackend(Protocol):
|
||||
Each dict must include ``ws_id``, ``role``, and ``content``
|
||||
(which may be ``None`` for assistant messages with only tool_calls).
|
||||
Optional keys: ``tool_name``, ``tool_call_id``, ``provider_data``,
|
||||
``tool_calls``, ``source``, ``reminders``. Timestamp and
|
||||
workstream updated-at are handled internally.
|
||||
``tool_calls``, ``source``. Timestamp and workstream updated-at
|
||||
are handled internally.
|
||||
"""
|
||||
...
|
||||
|
||||
|
||||
@@ -38,14 +38,14 @@ conversations = sa.Table(
|
||||
sa.Column("tool_call_id", sa.Text),
|
||||
sa.Column("provider_data", sa.Text),
|
||||
sa.Column("tool_calls", sa.Text),
|
||||
# Sibling-key columns mirroring the in-memory ``_source`` /
|
||||
# ``_reminders`` side-channels. ``_source`` audits which producer
|
||||
# synthesised the row (today only ``"system_nudge"`` for wake-driven
|
||||
# empty user turns); ``_reminders`` stores the JSON-encoded reminder
|
||||
# list ``[{type, text, ...optional}]`` so multi-tab / multi-device
|
||||
# replay sees the same bubble shape the originating tab saw live.
|
||||
# ``_source`` mirrors the in-memory side channel: which producer
|
||||
# synthesised the row — a ``system_nudge`` wake turn, or one of the
|
||||
# operator-context kinds on a first-class ``system`` turn (output_guard /
|
||||
# user_interjection / tool_error / watch_triggered / … — see
|
||||
# ``tool_advisory.SYSTEM_TURN_SOURCES``). (The sibling ``_reminders`` column
|
||||
# that rode here was dropped in migration 060 — operator context lives in
|
||||
# ``system`` turns now, so it was dead weight.)
|
||||
sa.Column("_source", sa.Text),
|
||||
sa.Column("_reminders", sa.Text),
|
||||
# SSE ``Last-Event-ID`` resume cursor: the per-ws ``_event_id``
|
||||
# ring-buffer high-water mark at the moment this row was saved (see
|
||||
# ``SessionUIBase._enqueue``). Distinct id-space from the ``id`` PK
|
||||
|
||||
@@ -324,14 +324,12 @@ class SQLiteBackend:
|
||||
provider_data: str | None = None,
|
||||
tool_calls: str | None = None,
|
||||
source: str | None = None,
|
||||
reminders: str | None = None,
|
||||
event_id: int | None = None,
|
||||
) -> int:
|
||||
now = datetime.now(UTC).strftime("%Y-%m-%dT%H:%M:%S")
|
||||
content = sanitize_text(content)
|
||||
provider_data = sanitize_text(provider_data)
|
||||
source = sanitize_text(source)
|
||||
reminders = sanitize_text(reminders)
|
||||
with self._conn() as conn:
|
||||
result = conn.execute(
|
||||
sa.insert(conversations),
|
||||
@@ -345,7 +343,6 @@ class SQLiteBackend:
|
||||
"provider_data": provider_data,
|
||||
"tool_calls": tool_calls,
|
||||
"_source": source,
|
||||
"_reminders": reminders,
|
||||
"event_id": event_id,
|
||||
},
|
||||
)
|
||||
@@ -391,7 +388,6 @@ class SQLiteBackend:
|
||||
"provider_data": sanitize_text(row.get("provider_data")),
|
||||
"tool_calls": row.get("tool_calls"),
|
||||
"_source": sanitize_text(row.get("source")),
|
||||
"_reminders": sanitize_text(row.get("reminders")),
|
||||
}
|
||||
)
|
||||
with self._conn() as conn:
|
||||
@@ -430,7 +426,6 @@ class SQLiteBackend:
|
||||
conversations.c.provider_data,
|
||||
conversations.c.tool_calls,
|
||||
conversations.c._source,
|
||||
conversations.c._reminders,
|
||||
conversations.c.event_id,
|
||||
)
|
||||
.where(conversations.c.ws_id == ws_id)
|
||||
@@ -449,7 +444,6 @@ class SQLiteBackend:
|
||||
conversations.c.provider_data,
|
||||
conversations.c.tool_calls,
|
||||
conversations.c._source,
|
||||
conversations.c._reminders,
|
||||
conversations.c.event_id,
|
||||
)
|
||||
.where(conversations.c.ws_id == ws_id)
|
||||
|
||||
@@ -336,12 +336,12 @@ def reconstruct_messages(
|
||||
) -> list[dict[str, Any]]:
|
||||
"""Reconstruct OpenAI message format from stored conversation rows.
|
||||
|
||||
Each *row* is a 9- or 10-tuple ``(id, role, content, tool_name,
|
||||
tool_call_id, provider_data, tool_calls_json, source, reminders_json
|
||||
[, event_id])``, ordered chronologically by row id. ``source`` /
|
||||
``reminders_json`` mirror the ``_source`` / ``_reminders`` in-memory
|
||||
side channels so multi-tab / multi-device replay sees the same bubble
|
||||
shape the originating tab saw live. The optional 10th element
|
||||
Each *row* is an 8- or 9-tuple ``(id, role, content, tool_name,
|
||||
tool_call_id, provider_data, tool_calls_json, source [, event_id])``,
|
||||
ordered chronologically by row id. ``source`` is rehydrated as the
|
||||
``_source`` side channel. (The legacy ``_reminders`` column that used to
|
||||
ride here was dropped in migration 060 — operator context lives in
|
||||
first-class ``system`` turns now.) The optional 9th element
|
||||
``event_id`` (migration 059, the per-ws SSE ``Last-Event-ID`` resume
|
||||
cursor) is surfaced as the ``_event_id`` side-channel; legacy 9-tuple
|
||||
fixtures omit it (handled by the defensive unpack below).
|
||||
@@ -374,15 +374,14 @@ def reconstruct_messages(
|
||||
provider_data,
|
||||
tool_calls_json,
|
||||
source,
|
||||
reminders_json,
|
||||
) = row[:9]
|
||||
# ``event_id`` (10th column, migration 059) is the per-ws SSE
|
||||
) = row[:8]
|
||||
# ``event_id`` (9th column, migration 059) is the per-ws SSE
|
||||
# ring-buffer high-water mark stamped at save time — the
|
||||
# ``Last-Event-ID`` resume cursor space. Surfaced as the
|
||||
# ``_event_id`` side-channel so ``make_history_handler`` can
|
||||
# compute the resume cursor + locate the in-flight-turn boundary.
|
||||
# Defensive length check keeps pre-event_id 9-tuple fixtures valid.
|
||||
event_id = row[9] if len(row) > 9 else None
|
||||
# Defensive length check keeps pre-event_id 8-tuple fixtures valid.
|
||||
event_id = row[8] if len(row) > 8 else None
|
||||
|
||||
if role == "user":
|
||||
parts: list[dict[str, Any]] = []
|
||||
@@ -412,12 +411,6 @@ def reconstruct_messages(
|
||||
umsg = {"role": "user", "content": content or ""}
|
||||
if source:
|
||||
umsg["_source"] = str(source)
|
||||
if reminders_json:
|
||||
# Mirrors the ``provider_data`` / ``tool_calls`` JSON
|
||||
# decode pattern below: malformed JSON in the column is
|
||||
# swallowed silently rather than aborting load.
|
||||
with contextlib.suppress(json.JSONDecodeError, TypeError):
|
||||
umsg["_reminders"] = json.loads(reminders_json)
|
||||
if event_id is not None:
|
||||
umsg["_event_id"] = int(event_id)
|
||||
messages.append(umsg)
|
||||
@@ -440,12 +433,6 @@ def reconstruct_messages(
|
||||
"tool_call_id": tc_id or "",
|
||||
"content": content or "",
|
||||
}
|
||||
if reminders_json:
|
||||
# Tool-channel reminders (``tool_error`` / ``repeat``)
|
||||
# ride the same column so replay shows the same below-
|
||||
# the-tool bubble the originating tab rendered live.
|
||||
with contextlib.suppress(json.JSONDecodeError, TypeError):
|
||||
tmsg["_reminders"] = json.loads(reminders_json)
|
||||
if event_id is not None:
|
||||
tmsg["_event_id"] = int(event_id)
|
||||
messages.append(tmsg)
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
"""Un-wrap legacy ``<tool_output>`` advisory envelopes; null ``_reminders``.
|
||||
"""Un-wrap legacy ``<tool_output>`` advisory envelopes; drop ``_reminders``.
|
||||
|
||||
Operator-context (output-guard findings, user interjections, metacognitive
|
||||
nudges) used to ride two transient carriers baked into stored rows: a
|
||||
@@ -7,7 +7,7 @@ nudges) used to ride two transient carriers baked into stored rows: a
|
||||
side-channel column. Both are replaced by first-class ``{"role":"system"}``
|
||||
turns, so the legacy carriers are now dead on the read path.
|
||||
|
||||
This migration drains the carriers in place — UPDATE only, no row insertion:
|
||||
This migration retires both carriers:
|
||||
|
||||
* **Envelopes** — every tool row whose ``content`` is a wrapped envelope is
|
||||
rewritten to the bare tool output. The embedded ``<system-reminder>``
|
||||
@@ -21,11 +21,14 @@ This migration drains the carriers in place — UPDATE only, no row insertion:
|
||||
original escape is reversed: re-activating the wrapper-tag escapes
|
||||
(``<system-reminder>`` → ``<system-reminder>``) would un-defang
|
||||
injection the old escape had neutralised, so those entities are left as-is.
|
||||
* **Reminders** — ``conversations._reminders`` is nulled wholesale; nothing
|
||||
writes the column anymore and the read path no longer projects it.
|
||||
* **Reminders** — the ``conversations._reminders`` column is dropped outright
|
||||
(``batch_alter_table``, per migration 027). Nothing writes it and the read
|
||||
path no longer reads it, so carrying it forward would only leave a writable
|
||||
dead column as a foot-gun.
|
||||
|
||||
``downgrade()`` is a documented no-op: the un-wrap is lossy (the advisory
|
||||
blocks and the reminder JSON are discarded), so the original rows cannot be
|
||||
``downgrade()`` re-adds the (empty) ``_reminders`` column so the schema matches
|
||||
the 059 state, but does NOT reverse the envelope un-wrap — that is lossy (the
|
||||
advisory blocks are discarded), so the original wrapped rows cannot be
|
||||
reconstructed.
|
||||
|
||||
Revision ID: 060
|
||||
@@ -114,7 +117,6 @@ def upgrade() -> None:
|
||||
"conversations",
|
||||
sa.column("id", sa.Integer),
|
||||
sa.column("content", sa.Text),
|
||||
sa.column("_reminders", sa.Text),
|
||||
)
|
||||
|
||||
# (1) Un-wrap legacy ``<tool_output>`` envelopes in place. Only rows whose
|
||||
@@ -149,15 +151,20 @@ def upgrade() -> None:
|
||||
.values(content=unwrapped)
|
||||
)
|
||||
|
||||
# (2) Null the dead ``_reminders`` side-channel column wholesale.
|
||||
bind.execute(
|
||||
sa.update(conversations)
|
||||
.where(conversations.c._reminders.isnot(None))
|
||||
.values(_reminders=None)
|
||||
)
|
||||
# (2) Drop the dead ``_reminders`` column outright. Operator context now
|
||||
# lives in first-class ``system`` turns; nothing writes the column and
|
||||
# ``reconstruct_messages`` no longer reads it. Dropping it (rather than
|
||||
# nulling and carrying it forward) removes the foot-gun of a writable
|
||||
# dead column. ``batch_alter_table`` so SQLite (table rebuild) and
|
||||
# PostgreSQL (native ALTER) both work — see migration 027.
|
||||
with op.batch_alter_table("conversations") as batch_op:
|
||||
batch_op.drop_column("_reminders")
|
||||
|
||||
|
||||
def downgrade() -> None:
|
||||
# No-op: the un-wrap discards the advisory blocks and the reminder JSON,
|
||||
# so the original wrapped rows cannot be reconstructed.
|
||||
pass
|
||||
# Re-add the (empty) column so the schema matches the 059 state. The
|
||||
# envelope un-wrap (step 1) is NOT reversed — it discards the advisory
|
||||
# blocks, so the original wrapped rows cannot be reconstructed; the
|
||||
# re-added column is therefore always NULL.
|
||||
with op.batch_alter_table("conversations") as batch_op:
|
||||
batch_op.add_column(sa.Column("_reminders", sa.Text, nullable=True))
|
||||
|
||||
Reference in New Issue
Block a user