diff --git a/tests/test_migration_060.py b/tests/test_migration_060.py index 97ec88ae..8b10fd09 100644 --- a/tests/test_migration_060.py +++ b/tests/test_migration_060.py @@ -11,7 +11,8 @@ isolated SQLite database per test, then asserts: bare row that merely starts with ```` — or even one with a matching ```` 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() diff --git a/tests/test_reconstruct_messages.py b/tests/test_reconstruct_messages.py index 9e2dcc2d..f464f281 100644 --- a/tests/test_reconstruct_messages.py +++ b/tests/test_reconstruct_messages.py @@ -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, ) diff --git a/tests/test_session.py b/tests/test_session.py index 9dbeae40..51a4970b 100644 --- a/tests/test_session.py +++ b/tests/test_session.py @@ -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 "" 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 diff --git a/tests/test_storage_source.py b/tests/test_storage_source.py new file mode 100644 index 00000000..f29cbc76 --- /dev/null +++ b/tests/test_storage_source.py @@ -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" diff --git a/tests/test_storage_source_reminders.py b/tests/test_storage_source_reminders.py deleted file mode 100644 index 6fc75f5f..00000000 --- a/tests/test_storage_source_reminders.py +++ /dev/null @@ -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" diff --git a/turnstone/core/memory.py b/turnstone/core/memory.py index 739efccc..04515e73 100644 --- a/turnstone/core/memory.py +++ b/turnstone/core/memory.py @@ -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: diff --git a/turnstone/core/session_routes.py b/turnstone/core/session_routes.py index d6a8398a..fbd207a6 100644 --- a/turnstone/core/session_routes.py +++ b/turnstone/core/session_routes.py @@ -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 diff --git a/turnstone/core/storage/_postgresql.py b/turnstone/core/storage/_postgresql.py index e09cf5a0..9316d9f0 100644 --- a/turnstone/core/storage/_postgresql.py +++ b/turnstone/core/storage/_postgresql.py @@ -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) diff --git a/turnstone/core/storage/_protocol.py b/turnstone/core/storage/_protocol.py index 745d576e..c6685f0a 100644 --- a/turnstone/core/storage/_protocol.py +++ b/turnstone/core/storage/_protocol.py @@ -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. """ ... diff --git a/turnstone/core/storage/_schema.py b/turnstone/core/storage/_schema.py index 26f7153b..ebec15d6 100644 --- a/turnstone/core/storage/_schema.py +++ b/turnstone/core/storage/_schema.py @@ -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 diff --git a/turnstone/core/storage/_sqlite.py b/turnstone/core/storage/_sqlite.py index d4d96284..f623f8dc 100644 --- a/turnstone/core/storage/_sqlite.py +++ b/turnstone/core/storage/_sqlite.py @@ -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) diff --git a/turnstone/core/storage/_utils.py b/turnstone/core/storage/_utils.py index 97710e6a..fbb1d890 100644 --- a/turnstone/core/storage/_utils.py +++ b/turnstone/core/storage/_utils.py @@ -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) diff --git a/turnstone/core/storage/migrations/versions/060_unwrap_tool_envelopes.py b/turnstone/core/storage/migrations/versions/060_unwrap_tool_envelopes.py index e8e0e33e..cc4cbed2 100644 --- a/turnstone/core/storage/migrations/versions/060_unwrap_tool_envelopes.py +++ b/turnstone/core/storage/migrations/versions/060_unwrap_tool_envelopes.py @@ -1,4 +1,4 @@ -"""Un-wrap legacy ```` advisory envelopes; null ``_reminders``. +"""Un-wrap legacy ```` 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 ```` @@ -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>`` → ````) 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 ```` 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))