diff --git a/tests/test_providers.py b/tests/test_providers.py index 2ffdb9d7..fee76fbf 100644 --- a/tests/test_providers.py +++ b/tests/test_providers.py @@ -224,6 +224,25 @@ class TestOpenAIProvider: sanitize_messages([original]) assert original["content"] is None + def test_sanitize_messages_strips_underscore_sibling_keys(self) -> None: + """Internal sibling metadata (``_reminders``, ``_reminders_delivered``, + ``_attachments_meta``, ``_provider_content``) must be stripped + before the wire — the OpenAI-compat APIs reject unknown fields.""" + msgs = [ + { + "role": "user", + "content": "hi", + "_reminders": [{"type": "correction", "text": "watch"}], + "_reminders_delivered": True, + "_attachments_meta": [{"kind": "image"}], + } + ] + result = sanitize_messages(msgs) + assert result == [{"role": "user", "content": "hi"}] + assert "_reminders" not in result[0] + assert "_reminders_delivered" not in result[0] + assert "_attachments_meta" not in result[0] + # -- sanitize_messages: orphan detection ----------------------------------- def test_sanitize_orphaned_tool_call_synthesized(self) -> None: diff --git a/tests/test_session.py b/tests/test_session.py index bed7cb28..839607dc 100644 --- a/tests/test_session.py +++ b/tests/test_session.py @@ -1,6 +1,7 @@ """Tests for turnstone.core.session — ChatSession construction.""" import base64 +import contextlib import json from unittest.mock import MagicMock, patch @@ -46,6 +47,9 @@ class NullUI: def on_error(self, message): pass + def on_user_reminder(self, reminders): + pass + def on_state_change(self, state): pass @@ -1928,18 +1932,22 @@ class TestMetacognitiveBuffers: # while readers of the buffer don't have to unbox. assert session._pending_tool_advisories == [("tool_error", "check memories")] - def test_splice_appends_system_reminder_to_string_content(self, tmp_db): + def test_attach_writes_reminders_sidechannel_for_string_content(self, tmp_db): session = _make_session() session._queue_user_advisory("correction", "ALERT_TEXT") msg = {"role": "user", "content": "hello there"} - session._splice_pending_user_advisories(msg) - assert msg["content"].startswith("hello there") - assert "" in msg["content"] - assert "ALERT_TEXT" in msg["content"] - assert "" in msg["content"] + session._attach_pending_user_reminders(msg) + # Content is untouched — the splice now writes a side-channel + # only. ```` rendering happens later inside + # ``_apply_reminders_for_provider`` against a transient copy + # so ``self.messages`` and every downstream consumer (UI replay, + # compaction, title gen, channel adapters) see clean text. + assert msg["content"] == "hello there" + assert "" not in msg["content"] + assert msg["_reminders"] == [{"type": "correction", "text": "ALERT_TEXT"}] assert session._pending_user_advisories == [] - def test_splice_appends_to_trailing_text_part_of_list_content(self, tmp_db): + def test_attach_writes_reminders_sidechannel_for_list_content(self, tmp_db): session = _make_session() session._queue_user_advisory("denial", "WATCH_OUT") msg = { @@ -1949,49 +1957,35 @@ class TestMetacognitiveBuffers: {"type": "image_url", "image_url": {"url": "data:image/png;base64,..."}}, ], } - session._splice_pending_user_advisories(msg) - # Splice lands on the trailing text part — image part is untouched. - text_part = msg["content"][0] - image_part = msg["content"][1] - assert "look at this image" in text_part["text"] - assert "WATCH_OUT" in text_part["text"] - assert "" in text_part["text"] - assert image_part == { - "type": "image_url", - "image_url": {"url": "data:image/png;base64,..."}, - } + session._attach_pending_user_reminders(msg) + # Content (including parts) is untouched — neither text part + # nor image part is mutated. The reminder lives on the + # sibling key. + assert msg["content"][0] == {"type": "text", "text": "look at this image"} + assert msg["content"][1]["type"] == "image_url" + assert msg["_reminders"] == [{"type": "denial", "text": "WATCH_OUT"}] - def test_splice_inserts_text_part_when_list_has_no_text(self, tmp_db): - session = _make_session() - session._queue_user_advisory("resume", "REMINDER") - msg = { - "role": "user", - "content": [ - {"type": "image_url", "image_url": {"url": "data:image/png;base64,..."}}, - ], - } - session._splice_pending_user_advisories(msg) - # New text part appended at the end. - assert len(msg["content"]) == 2 - assert msg["content"][0]["type"] == "image_url" - assert msg["content"][1]["type"] == "text" - assert "REMINDER" in msg["content"][1]["text"] - - def test_splice_noop_when_buffer_empty(self, tmp_db): + def test_attach_noop_when_buffer_empty(self, tmp_db): session = _make_session() msg = {"role": "user", "content": "untouched"} - session._splice_pending_user_advisories(msg) + session._attach_pending_user_reminders(msg) assert msg["content"] == "untouched" + # No reminders → no side-channel key set (so a downstream + # ``msg.get("_reminders")`` is falsey without needing to test + # for an empty list). + assert "_reminders" not in msg - def test_splice_combines_multiple_queued_nudges(self, tmp_db): + def test_attach_combines_multiple_queued_nudges(self, tmp_db): session = _make_session() session._queue_user_advisory("denial", "FIRST") session._queue_user_advisory("correction", "SECOND") msg = {"role": "user", "content": "user text"} - session._splice_pending_user_advisories(msg) - assert msg["content"].count("") == 2 - assert "FIRST" in msg["content"] - assert "SECOND" in msg["content"] + session._attach_pending_user_reminders(msg) + # Both queued nudges land in order on the side-channel. + assert msg["_reminders"] == [ + {"type": "denial", "text": "FIRST"}, + {"type": "correction", "text": "SECOND"}, + ] # Both nudges drained. assert session._pending_user_advisories == [] @@ -2068,8 +2062,11 @@ class TestMetacognitiveBuffers: Drives `send()` end-to-end with a mocked stream that raises GenerationCancelled to exit the loop after the user message has - been appended and spliced. Asserts the nudge actually rode along - on the user message body and the buffer drained.""" + been appended and spliced. Asserts the nudge landed on the user + message's ``_reminders`` side-channel and the buffer drained. + The cancel-handler path also clears the user-advisory buffer, + so checking len after a cancel is a covering assertion for + both behaviours.""" from turnstone.core.session import GenerationCancelled session = _make_session() @@ -2085,27 +2082,33 @@ class TestMetacognitiveBuffers: ): session.send("first user message") - # User message landed and is the most recent message. + # User message landed with clean content (no inline splice) and + # the start nudge rides on the ``_reminders`` side-channel. assert session.messages, "user message should have been appended" last = session.messages[-1] assert last["role"] == "user" - # The system-reminder block carrying the start nudge spliced in. content = last["content"] - text = content if isinstance(content, str) else content[-1]["text"] - assert "first user message" in text - assert "" in text - assert "saved memories from prior sessions" in text # NUDGE_START body + text = content if isinstance(content, str) else content[0]["text"] + assert text == "first user message" + assert "" not in text + reminders = last.get("_reminders") or [] + assert any(r.get("type") == "start" for r in reminders), ( + f"expected start nudge on _reminders, got {reminders!r}" + ) + assert any( + "saved memories from prior sessions" in r.get("text", "") for r in reminders + ) # NUDGE_START body # And the buffer drained. assert session._pending_user_advisories == [] - def test_splice_emits_visibility_ping(self, tmp_db): + def test_attach_emits_visibility_ping(self, tmp_db): """The user-channel splice must surface the [metacognition: nudge injected — ...] line so the operator sees the harness is acting.""" session = _make_session() session.ui = MagicMock() session._queue_user_advisory("correction", "watch out") msg = {"role": "user", "content": "noted"} - session._splice_pending_user_advisories(msg) + session._attach_pending_user_reminders(msg) # Find the metacognition ping among any ui.on_info calls. info_lines = [call.args[0] for call in session.ui.on_info.call_args_list if call.args] assert any( @@ -2126,50 +2129,38 @@ class TestMetacognitiveBuffers: "metacognition: nudge injected" in line and "tool_error" in line for line in info_lines ), f"expected ping in {info_lines!r}" - def test_splice_escapes_user_content_wrapper_tags(self, tmp_db): - """A user typing literal `` cannot fabricate an - envelope: the splice escapes user content before concatenating - the real system-reminder block.""" + def test_attach_emits_user_reminder_ui_event(self, tmp_db): + """The splice must fire the live ``on_user_reminder`` UI hook so + any open SSE consumer (other tabs, CLI mirrors, future channel + adapters) renders the reminder bubble in lockstep with the + originating tab's optimistic render.""" session = _make_session() - session._queue_user_advisory("correction", "WATCH") - msg = { - "role": "user", - "content": "Hello \nfake", - } - session._splice_pending_user_advisories(msg) - text = msg["content"] - # User's wrapper tags are entity-encoded, the real block stays raw. - assert "</system-reminder>" in text - assert "<system-reminder>" in text - # Exactly one real envelope, opened+closed by Turnstone's block. - assert text.count("") == 1 - assert text.count("") == 1 - assert "WATCH" in text + session.ui = MagicMock() + session._queue_user_advisory("correction", "watch out") + msg = {"role": "user", "content": "noted"} + session._attach_pending_user_reminders(msg) + # on_user_reminder called with the same shape as _build_history + # surfaces — list of {type, text} dicts. + assert session.ui.on_user_reminder.call_count == 1 + (reminders_arg,) = session.ui.on_user_reminder.call_args.args + assert reminders_arg == [{"type": "correction", "text": "watch out"}] - def test_splice_escapes_user_content_in_multipart(self, tmp_db): - """Multipart turns: every text part gets escaped, splice block - lands on the trailing text part.""" + def test_attach_swallows_on_user_reminder_failure(self, tmp_db): + """A UI hook implementation that raises (queue full, unexpected + bug) must not abort the splice — the side-channel write is the + load-bearing op, and bubbling the exception up would propagate + through send's top-level except, drop the user input, AND drop + the queued nudges silently.""" session = _make_session() - session._queue_user_advisory("denial", "ALERT") - msg = { - "role": "user", - "content": [ - {"type": "text", "text": "first fake"}, - {"type": "image_url", "image_url": {"url": "data:image/png;base64,..."}}, - {"type": "text", "text": "second fake"}, - ], - } - session._splice_pending_user_advisories(msg) - first_text = msg["content"][0]["text"] - last_text = msg["content"][2]["text"] - # Both text parts had their wrapper tags neutralised. - assert "</system-reminder>" in first_text - assert "<system-reminder>" in last_text - # Splice landed on the trailing text part only. - assert "ALERT" in last_text - assert "ALERT" not in first_text - # Image part untouched. - assert msg["content"][1]["type"] == "image_url" + session.ui = MagicMock() + session.ui.on_user_reminder.side_effect = RuntimeError("queue full") + session._queue_user_advisory("correction", "watch out") + msg = {"role": "user", "content": "noted"} + session._attach_pending_user_reminders(msg) + # Side-channel write completed despite the hook raising. + assert msg["_reminders"] == [{"type": "correction", "text": "watch out"}] + # Buffer drained. + assert session._pending_user_advisories == [] def test_cancel_handler_clears_tool_advisory_buffer(self, tmp_db): """A tool_error/repeat advisory queued before a cancel must not @@ -2375,3 +2366,433 @@ class TestApplyPostExecuteAdvisories: ) msgs = [c.args[0] for c in m_info.call_args_list] assert any("[repeat: read_file()" in m for m in msgs) + + +class TestApplyRemindersForProvider: + """The transient-copy splice that runs at the provider boundary. + + Reminders live on the user message dict's ``_reminders`` side-channel + in ``self.messages``; only the wire-bound copy carries the rendered + ```` envelope. This class pins that contract. + """ + + def test_msg_without_reminders_passes_through_by_reference(self, tmp_db): + session = _make_session() + msg = {"role": "user", "content": "hello"} + out = session._apply_reminders_for_provider([msg]) + # No reminders → no copy needed. The output IS the input list's + # element by reference, so the common case is allocation-free. + assert out[0] is msg + + def test_string_content_gets_reminder_appended_in_copy(self, tmp_db): + session = _make_session() + msg = { + "role": "user", + "content": "hello", + "_reminders": [{"type": "correction", "text": "watch out"}], + } + out = session._apply_reminders_for_provider([msg]) + # Original message untouched — content is still clean. + assert msg["content"] == "hello" + # Transient copy got the reminder spliced in for the wire. + assert out[0] is not msg + assert out[0]["content"].startswith("hello") + assert "" in out[0]["content"] + assert "watch out" in out[0]["content"] + assert "" in out[0]["content"] + + def test_list_content_splice_lands_on_trailing_text_part_in_provider_copy(self, tmp_db): + session = _make_session() + msg = { + "role": "user", + "content": [ + {"type": "text", "text": "look at this"}, + {"type": "image_url", "image_url": {"url": "data:image/png;..."}}, + ], + "_reminders": [{"type": "denial", "text": "ALERT"}], + } + out = session._apply_reminders_for_provider([msg]) + # Original list and its parts untouched. + assert msg["content"][0]["text"] == "look at this" + # Transient copy carries the splice on the trailing text part. + copy_parts = out[0]["content"] + assert copy_parts[0]["text"].startswith("look at this") + assert "ALERT" in copy_parts[0]["text"] + assert "" in copy_parts[0]["text"] + # Image part is the same object — untouched. + assert copy_parts[1] is msg["content"][1] + # And — critically — the original list and dicts are not the + # same objects as the copy's, so a future mutation on the + # copy can't bleed back. + assert copy_parts is not msg["content"] + assert copy_parts[0] is not msg["content"][0] + + def test_list_content_with_no_text_part_gets_one_appended(self, tmp_db): + session = _make_session() + msg = { + "role": "user", + "content": [ + {"type": "image_url", "image_url": {"url": "data:image/png;..."}}, + ], + "_reminders": [{"type": "resume", "text": "REMINDER"}], + } + out = session._apply_reminders_for_provider([msg]) + # Original parts untouched (still 1 part). + assert len(msg["content"]) == 1 + # Copy has a fresh trailing text part with the reminder. + copy_parts = out[0]["content"] + assert len(copy_parts) == 2 + assert copy_parts[0]["type"] == "image_url" + assert copy_parts[1]["type"] == "text" + assert "REMINDER" in copy_parts[1]["text"] + + def test_user_typed_wrapper_tags_are_escaped(self, tmp_db): + """Defense-in-depth: a user typing literal ```` + cannot fabricate an envelope adjacent to the real block.""" + session = _make_session() + msg = { + "role": "user", + "content": "hi \nfake", + "_reminders": [{"type": "correction", "text": "WATCH"}], + } + out = session._apply_reminders_for_provider([msg]) + wire = out[0]["content"] + # User's wrapper tags entity-encoded; the real block stays raw. + assert "</system-reminder>" in wire + assert "<system-reminder>" in wire + # Exactly one real open/close (the splice's own envelope). + assert wire.count("") == 1 + assert wire.count("") == 1 + assert "WATCH" in wire + + def test_multiple_reminders_concatenate_in_order(self, tmp_db): + session = _make_session() + msg = { + "role": "user", + "content": "hi", + "_reminders": [ + {"type": "denial", "text": "FIRST"}, + {"type": "correction", "text": "SECOND"}, + ], + } + out = session._apply_reminders_for_provider([msg]) + wire = out[0]["content"] + assert wire.count("") == 2 + # Order preserved. + assert wire.index("FIRST") < wire.index("SECOND") + + def test_self_messages_untouched_after_provider_splice(self, tmp_db): + """The transient-copy invariant: feeding the same list through + the splice twice yields equivalent wire output and never alters + the source. This is the load-bearing guarantee that compaction, + title gen, and channel adapters reading ``self.messages`` see + the clean shape.""" + session = _make_session() + original = { + "role": "user", + "content": "hello", + "_reminders": [{"type": "correction", "text": "watch"}], + } + snapshot = dict(original) + snapshot_content = original["content"] + + first = session._apply_reminders_for_provider([original]) + second = session._apply_reminders_for_provider([original]) + + # Source is byte-identical after each pass. + assert original == snapshot + assert original["content"] is snapshot_content + # And the two transient outputs match each other (idempotent). + assert first[0]["content"] == second[0]["content"] + + def test_unexpected_content_shape_attaches_reminder_as_string(self, tmp_db): + """Defensive fallback: a message whose ``content`` is neither a + string nor a list (None, dict, etc. — shouldn't reach the splice + in practice, but providers do disagree on edge cases) gets the + reminder block attached as a fresh string content rather than + silently dropped.""" + session = _make_session() + msg = { + "role": "user", + "content": None, + "_reminders": [{"type": "correction", "text": "WATCH"}], + } + out = session._apply_reminders_for_provider([msg]) + wire = out[0]["content"] + assert isinstance(wire, str) + assert wire # non-empty + assert "" in wire + assert "WATCH" in wire + # Source untouched. + assert msg["content"] is None + + def test_delivered_flag_skips_splice_for_already_delivered(self, tmp_db): + """Once ``_mark_reminders_delivered`` flips the flag the next + provider call must not re-render the same reminder — model sees + it once, not on every subsequent send.""" + session = _make_session() + msg = { + "role": "user", + "content": "hi", + "_reminders": [{"type": "correction", "text": "WATCH"}], + } + # First pass — flag is False, splice happens. + first = session._apply_reminders_for_provider([msg]) + assert "WATCH" in first[0]["content"] + # Mark delivered: simulate the post-stream-success hook. + msg["_reminders_delivered"] = True + # Second pass — flag is True, msg passes through by reference. + second = session._apply_reminders_for_provider([msg]) + assert second[0] is msg + assert second[0]["content"] == "hi" + assert "WATCH" not in second[0]["content"] + + def test_delivered_flag_does_not_strip_reminders_key(self, tmp_db): + """``_reminders`` must persist after delivery so ``/history`` + replay (reconnecting tabs) still surfaces the bubble. Only + wire-side replay is suppressed.""" + session = _make_session() + msg = { + "role": "user", + "content": "hi", + "_reminders": [{"type": "correction", "text": "WATCH"}], + "_reminders_delivered": True, + } + session._apply_reminders_for_provider([msg]) + assert msg["_reminders"] == [{"type": "correction", "text": "WATCH"}] + + +class TestMarkRemindersDelivered: + """``_mark_reminders_delivered`` flips the wire-suppression flag on + every user message in ``self.messages`` that carries reminders, + enabling the once-per-session-not-per-turn semantic that pairs with + ``_apply_reminders_for_provider``'s skip path.""" + + def test_marks_all_undelivered_messages(self, tmp_db): + session = _make_session() + session.messages.extend( + [ + { + "role": "user", + "content": "first", + "_reminders": [{"type": "start", "text": "A"}], + }, + {"role": "assistant", "content": "ok"}, + { + "role": "user", + "content": "second", + "_reminders": [{"type": "correction", "text": "B"}], + }, + ] + ) + session._mark_reminders_delivered() + assert session.messages[0]["_reminders_delivered"] is True + assert session.messages[2]["_reminders_delivered"] is True + # Assistant message — no reminders → no flag added. + assert "_reminders_delivered" not in session.messages[1] + + def test_idempotent_on_already_delivered(self, tmp_db): + """Re-running the mark must not flip an already-delivered + flag back or add spurious keys to messages without reminders.""" + session = _make_session() + session.messages.append( + { + "role": "user", + "content": "x", + "_reminders": [{"type": "start", "text": "A"}], + "_reminders_delivered": True, + } + ) + before_keys = set(session.messages[0].keys()) + session._mark_reminders_delivered() + assert set(session.messages[0].keys()) == before_keys + assert session.messages[0]["_reminders_delivered"] is True + + def test_no_reminders_no_flag(self, tmp_db): + """Messages without ``_reminders`` are untouched — no spurious + ``_reminders_delivered`` key gets added.""" + session = _make_session() + session.messages.append({"role": "user", "content": "plain"}) + session._mark_reminders_delivered() + assert "_reminders_delivered" not in session.messages[0] + + +class TestUpdateTokenTableMsgsParam: + """``_update_token_table(msgs=...)`` reuses the wire-bound message + list already built for the stream call instead of re-applying the + reminder splice (perf-2). Critical given the delivered-flag flow: + after ``_mark_reminders_delivered`` runs, a fresh + ``_apply_reminders_for_provider`` would skip every just-delivered + reminder and undercount calibration chars.""" + + def test_uses_provided_msgs_skips_re_application(self, tmp_db): + session = _make_session() + session._last_usage = {"prompt_tokens": 100, "completion_tokens": 50} + session.messages.append( + { + "role": "user", + "content": "hi", + "_reminders": [{"type": "correction", "text": "x"}], + } + ) + # Patch _apply_reminders_for_provider to detect re-application. + with patch.object( + session, + "_apply_reminders_for_provider", + wraps=session._apply_reminders_for_provider, + ) as m_apply: + pre_built = session._apply_reminders_for_provider(session._full_messages()) + calls_after_prebuild = m_apply.call_count + session._update_token_table({"role": "assistant", "content": "ok"}, msgs=pre_built) + # Calibration must not have called _apply_reminders_for_provider + # again. + assert m_apply.call_count == calls_after_prebuild + + def test_falls_back_to_apply_when_msgs_missing(self, tmp_db): + """The optional kwarg has a fallback so callers that don't (or + can't) pre-build the wire copy still get a sane calibration — + just one that may undercount if reminders have already been + flagged delivered.""" + session = _make_session() + session._last_usage = {"prompt_tokens": 100, "completion_tokens": 50} + session.messages.append({"role": "user", "content": "hi"}) + with patch.object( + session, + "_apply_reminders_for_provider", + wraps=session._apply_reminders_for_provider, + ) as m_apply: + session._update_token_table({"role": "assistant", "content": "ok"}) + # Fallback path applies the splice. + assert m_apply.call_count == 1 + + +class TestUserAdvisoryCancelClear: + """Pre-existing bug surfaced by the side-channel audit — cancel + handlers cleared ``_pending_tool_advisories`` but not the user-channel + buffer, so a queued user-channel nudge from a cancelled batch leaked + into the next user turn. Stage 1 fix lives at the three cancel + branches inside ``send``. + """ + + def test_generation_cancelled_clears_user_advisory_buffer(self, tmp_db): + from turnstone.core.session import GenerationCancelled + + session = _make_session() + session._queue_user_advisory("denial", "leftover") + with ( + patch.object(session, "_visible_memory_count", return_value=0), + patch.object( + session, + "_create_stream_with_retry", + side_effect=GenerationCancelled(), + ), + ): + session.send("user input") + assert session._pending_user_advisories == [] + + def test_keyboard_interrupt_clears_user_advisory_buffer(self, tmp_db): + session = _make_session() + session._queue_user_advisory("correction", "leftover") + with ( + patch.object(session, "_visible_memory_count", return_value=0), + patch.object( + session, + "_create_stream_with_retry", + side_effect=KeyboardInterrupt(), + ), + contextlib.suppress(KeyboardInterrupt), + ): + session.send("user input") + assert session._pending_user_advisories == [] + + def test_unexpected_exception_clears_user_advisory_buffer(self, tmp_db): + session = _make_session() + session._queue_user_advisory("resume", "leftover") + with ( + patch.object(session, "_visible_memory_count", return_value=0), + patch.object( + session, + "_create_stream_with_retry", + side_effect=RuntimeError("boom"), + ), + contextlib.suppress(RuntimeError), + ): + session.send("user input") + assert session._pending_user_advisories == [] + + +class TestReminderSidechannelIsolation: + """The side-channel design's load-bearing guarantee: any reader of + ``self.messages`` that goes through ``content`` cannot see reminders. + Compaction, title generation, agent message lists, channel adapters + — all read ``content``, so the side-channel is invisible by + construction. These tests pin that contract for the two in-process + consumers most likely to leak (compaction and the title-extraction + loop). + """ + + def test_format_messages_for_summary_does_not_see_reminders(self, tmp_db): + """Compaction feeds ``self.messages`` straight into a summarising + prompt — if a reminder leaked into ``content`` it would land in + the summary text and outlive the turn it advised.""" + session = _make_session() + session.messages.append( + { + "role": "user", + "content": "user said this", + "_reminders": [{"type": "correction", "text": "SECRET_NUDGE_TEXT"}], + } + ) + session.messages.append({"role": "assistant", "content": "ok"}) + summary = session._format_messages_for_summary(session.messages) + assert "SECRET_NUDGE_TEXT" not in summary + assert "" not in summary + assert "user said this" in summary + + def test_first_user_message_extraction_does_not_see_reminders(self, tmp_db): + """Title generation pulls the first user message's ``content`` for + the title prompt. Replicates the inner extraction loop and pins + that the side-channel is invisible — the content slot stays + clean even when ``_reminders`` is populated.""" + session = _make_session() + session.messages.append( + { + "role": "user", + "content": "first message body", + "_reminders": [{"type": "start", "text": "SECRET_NUDGE_TEXT"}], + } + ) + # Mirror the loop at session.py:_generate_title that pulls the + # first user message into the title prompt. + extracted_user = "" + for m in session.messages: + content = m.get("content") or "" + if isinstance(content, list): + content = " ".join(p.get("text", "") for p in content if isinstance(p, dict)) + if m["role"] == "user" and not extracted_user: + extracted_user = content[:300] + break + assert extracted_user == "first message body" + assert "SECRET_NUDGE_TEXT" not in extracted_user + + +class TestSessionUIBaseUserReminderHook: + """``on_user_reminder`` enqueues a ``user_reminder`` SSE event with + the same shape ``_build_history`` surfaces, so live tabs and + reconnecting tabs render the same reminder payload.""" + + def test_on_user_reminder_enqueues_sse_event(self): + from turnstone.core.session_ui_base import SessionUIBase + + class _RecordingUI(SessionUIBase): + def __init__(self) -> None: + super().__init__() + self.events: list[dict] = [] + + def _enqueue(self, data: dict) -> None: # type: ignore[override] + self.events.append(data) + + ui = _RecordingUI() + reminders = [{"type": "correction", "text": "watch out"}] + ui.on_user_reminder(reminders) + assert ui.events == [{"type": "user_reminder", "reminders": reminders}] diff --git a/tests/test_workstream_endpoints.py b/tests/test_workstream_endpoints.py index f81c4e57..3804f53f 100644 --- a/tests/test_workstream_endpoints.py +++ b/tests/test_workstream_endpoints.py @@ -899,6 +899,138 @@ class TestHistoryInteractive: assert client.get(base, params={"limit": 999}).status_code == 200 +class TestBuildHistoryReminderPropagation: + """``_build_history`` must surface the ``_reminders`` side-channel on + each entry so a tab reconnecting via ``/history`` renders the same + metacognitive nudge bubble the originating tab saw via the live + ``user_reminder`` SSE event. + """ + + def _session_with_messages(self, messages: list[dict]) -> MagicMock: + session = MagicMock() + session.messages = messages + return session + + def test_reminders_sidechannel_surfaces_on_entry(self): + from turnstone.server import _build_history + + session = self._session_with_messages( + [ + { + "role": "user", + "content": "ah no", + "_reminders": [{"type": "correction", "text": "watch out"}], + } + ] + ) + history = _build_history(session) + assert history[0]["content"] == "ah no" + assert history[0]["reminders"] == [{"type": "correction", "text": "watch out"}] + + def test_no_reminders_key_when_sidechannel_absent(self): + from turnstone.server import _build_history + + session = self._session_with_messages([{"role": "user", "content": "just a message"}]) + history = _build_history(session) + assert "reminders" not in history[0] + + def test_no_reminders_key_when_sidechannel_empty(self): + from turnstone.server import _build_history + + session = self._session_with_messages([{"role": "user", "content": "hi", "_reminders": []}]) + history = _build_history(session) + assert "reminders" not in history[0] + + def test_multiple_reminders_preserved_in_order(self): + from turnstone.server import _build_history + + session = self._session_with_messages( + [ + { + "role": "user", + "content": "x", + "_reminders": [ + {"type": "denial", "text": "FIRST"}, + {"type": "correction", "text": "SECOND"}, + ], + } + ] + ) + history = _build_history(session) + assert history[0]["reminders"] == [ + {"type": "denial", "text": "FIRST"}, + {"type": "correction", "text": "SECOND"}, + ] + + def test_reminders_coexist_with_attachments(self): + from turnstone.server import _build_history + + session = self._session_with_messages( + [ + { + "role": "user", + "content": [ + {"type": "text", "text": "look"}, + {"type": "image_url", "image_url": {"url": "data:..."}}, + ], + "_reminders": [{"type": "correction", "text": "watch"}], + } + ] + ) + history = _build_history(session) + assert history[0]["content"] == "look" + assert history[0]["attachments"] == [{"kind": "image", "filename": "", "mime_type": ""}] + assert history[0]["reminders"] == [{"type": "correction", "text": "watch"}] + + def test_malformed_reminders_filtered_out(self): + """Defensive: a non-dict element in the list (corruption / bug) + is dropped rather than crashing the history serialisation.""" + from turnstone.server import _build_history + + session = self._session_with_messages( + [ + { + "role": "user", + "content": "x", + "_reminders": [ + {"type": "correction", "text": "ok"}, + "not-a-dict", + {"type": "denial"}, # missing text + ], + } + ] + ) + history = _build_history(session) + # Non-dicts dropped; missing-text fills with empty string. + assert history[0]["reminders"] == [ + {"type": "correction", "text": "ok"}, + {"type": "denial", "text": ""}, + ] + + def test_clean_message_passes_through_unchanged(self): + """No reminders, plain content — _build_history is a no-op for the + reminder field and ``content`` rides through verbatim.""" + from turnstone.server import _build_history + + session = self._session_with_messages( + [{"role": "user", "content": "just a normal message"}] + ) + history = _build_history(session) + assert history[0]["content"] == "just a normal message" + assert "reminders" not in history[0] + + def test_assistant_content_with_literal_reminder_tag_unchanged(self): + """Assistant output may legitimately reference the tag (e.g. when + the model is explaining the reminder system itself). No + transformation should ever apply to assistant content.""" + from turnstone.server import _build_history + + content = "Here is a tag in assistant output." + session = self._session_with_messages([{"role": "assistant", "content": content}]) + history = _build_history(session) + assert history[0]["content"] == content + + class TestDetailInteractive: """Interactive parity for the lifted ``GET /v1/api/workstreams/{ws_id}``. diff --git a/turnstone/core/session.py b/turnstone/core/session.py index 4252521f..4f1edce5 100644 --- a/turnstone/core/session.py +++ b/turnstone/core/session.py @@ -91,6 +91,7 @@ from turnstone.core.providers import create_provider from turnstone.core.safety import is_command_blocked, sanitize_command from turnstone.core.sandbox import execute_math_sandboxed from turnstone.core.storage._registry import get_storage +from turnstone.core.tool_advisory import escape_wrapper_tags, render_system_reminder from turnstone.core.tool_search import ToolSearchManager from turnstone.core.tools import ( AGENT_AUTO_TOOLS, @@ -276,6 +277,7 @@ class SessionUI(Protocol): def on_plan_review(self, content: str) -> str: ... def on_info(self, message: str) -> None: ... def on_error(self, message: str) -> None: ... + def on_user_reminder(self, reminders: list[dict[str, str]]) -> None: ... def on_state_change(self, state: str) -> None: ... def on_rename(self, name: str) -> None: ... def on_intent_verdict(self, verdict: dict[str, Any]) -> None: @@ -1643,6 +1645,106 @@ class ChatSession: """System messages + conversation history.""" return self.system_messages + self.messages + def _apply_reminders_for_provider( + self, + messages: list[dict[str, Any]], + ) -> list[dict[str, Any]]: + """Return a transient copy of *messages* with ``_reminders`` rendered + inline for the model. + + Metacognitive user-channel nudges live on the message dict's + ``_reminders`` side-channel, not inside ``content`` — so + ``self.messages`` and every downstream consumer (UI replay, + compaction, title gen, channel adapters, DB) see clean user + text. Only the wire-bound copy carries the rendered reminder. + + For each message that has ``_reminders`` AND has not yet been + flagged delivered, build a shallow copy and splice the reminders + as ```` blocks onto the trailing edge of the + copy's ``content`` — string content gets a tail block, list + content gets the block on the trailing text part (or a new text + part if there isn't one). Messages without ``_reminders`` (or + already delivered) pass through unchanged (same object + reference) so the common case is allocation-free. + + **Reminder lifecycle.** After a successful provider stream + ``_mark_reminders_delivered`` flips ``_reminders_delivered`` to + ``True`` on every user message that carried reminders into that + call, so subsequent provider calls skip them — the model sees + each reminder once, the turn it advised. The + ``_reminders`` key itself stays on the message dict for the + lifetime of the in-memory session so ``/history`` (reconnecting + tabs, multi-tab live mirrors) still renders the same nudge + bubbles the originating tab saw; only the wire-side replay is + suppressed. Compaction is the natural full drain (it replaces + ``self.messages`` wholesale). + + ``sanitize_messages`` later drops both leading-underscore sibling + keys (``_reminders`` and ``_reminders_delivered``) on the way to + the wire, so the provider sees only ``content`` with the + reminder spliced in. + + **Read-only contract on the returned list.** The pass-through + path returns the original ``msg`` by reference; callers must + not mutate the returned dicts in place (today's only callers — + ``sanitize_messages`` + provider conversion — construct new + dicts, so the contract holds). Mutations on the spliced copy + are safe; mutations on a pass-through reference would bleed + back into ``self.messages``. + """ + out: list[dict[str, Any]] = [] + for msg in messages: + reminders = msg.get("_reminders") + if not reminders or msg.get("_reminders_delivered"): + out.append(msg) + continue + block = "\n\n" + "\n\n".join( + render_system_reminder(r.get("text", "")) for r in reminders + ) + copy = dict(msg) + content = copy.get("content") + if isinstance(content, str): + copy["content"] = escape_wrapper_tags(content) + block + elif isinstance(content, list): + # Shallow-copy the parts list and any text parts we'll + # mutate so the original list/dicts in self.messages stay + # untouched. + new_parts = [ + dict(p) if isinstance(p, dict) and p.get("type") == "text" else p + for p in content + ] + text_parts = [ + p for p in new_parts if isinstance(p, dict) and p.get("type") == "text" + ] + for part in text_parts: + part["text"] = escape_wrapper_tags(part.get("text", "")) + if text_parts: + text_parts[-1]["text"] = text_parts[-1]["text"] + block + else: + new_parts.append({"type": "text", "text": block}) + copy["content"] = new_parts + else: + # Unexpected shape (None, etc.) — attach as a text-only + # content rather than dropping the reminder silently. + copy["content"] = block.lstrip() + out.append(copy) + return out + + def _mark_reminders_delivered(self) -> None: + """Flag every user-message ``_reminders`` as delivered. + + Called after a successful provider stream. Subsequent calls to + ``_apply_reminders_for_provider`` skip messages with this flag, + so the model sees each reminder exactly once (the turn it + advised). The flag is a sibling key like ``_reminders`` itself; + ``sanitize_messages`` strips both before the wire and + ``_build_history`` ignores the delivered flag entirely so UI + replay parity is preserved across reconnects. + """ + for msg in self.messages: + if msg.get("_reminders") and not msg.get("_reminders_delivered"): + msg["_reminders_delivered"] = True + def _emit_state(self, state: str) -> None: """Notify UI of a workstream state transition. @@ -2123,7 +2225,7 @@ class ChatSession: # ``user_input`` only (line below) so these blocks stay # ephemeral — they advise the next assistant turn and do not # persist across reloads. - self._splice_pending_user_advisories(user_msg) + self._attach_pending_user_reminders(user_msg) self.messages.append(user_msg) self._msg_tokens.append(max(1, int(self._msg_char_count(user_msg) / self._chars_per_token))) # DB row stores the raw text only; attachments are joined back in @@ -2203,7 +2305,7 @@ class ChatSession: try: while True: self._check_cancelled(my_generation) - msgs = self._full_messages() + msgs = self._apply_reminders_for_provider(self._full_messages()) if self.debug: self._debug_print_request(msgs) @@ -2240,7 +2342,7 @@ class ChatSession: self.ui.on_thinking_stop() try: self._compact_messages(auto=True) - msgs = self._full_messages() + msgs = self._apply_reminders_for_provider(self._full_messages()) self.ui.on_thinking_start() stream = self._create_stream_with_retry(msgs) except Exception: @@ -2262,7 +2364,19 @@ class ChatSession: if self._generation != my_generation: return - self._update_token_table(assistant_msg) + # Reuse the wire-bound ``msgs`` we already built for the + # stream call instead of re-applying the reminder splice + # (perf-2). After mark-delivered runs below, a fresh + # _apply_reminders_for_provider would skip the + # just-rendered reminders and undercount; passing the + # already-rendered list keeps calibration char count + # aligned with what the provider actually counted. + self._update_token_table(assistant_msg, msgs=msgs) + # Reminders that rode this stream have now reached the + # model; flag delivered so the next provider call skips + # them (one-shot semantics for the wire; UI replay still + # surfaces ``_reminders`` for reconnect parity). + self._mark_reminders_delivered() self._print_status_line() # Report usage for EVERY API call self.messages.append(assistant_msg) self._msg_tokens.append( @@ -2506,10 +2620,7 @@ class ChatSession: # Drain any queued user messages so they appear in the # conversation and are visible on the next send(). self._flush_queued_messages() - # Tool-channel nudges queued earlier in this generation - # (tool_error, repeat) belong to the abandoned batch — drop - # them so they don't bleed into the next send()'s tool loop. - self._pending_tool_advisories.clear() + self._drain_pending_advisories() # No need to clear _cancel_event — it's replaced per-generation # in send(), so this generation's event is simply discarded. self.ui.on_info("[Generation cancelled]") @@ -2519,15 +2630,29 @@ class ChatSession: except KeyboardInterrupt as exc: self._synthesize_cancelled_results("Interrupted by user.") self._flush_queued_messages() - self._pending_tool_advisories.clear() + self._drain_pending_advisories() self._record_fatal_error(exc) raise except Exception as exc: self._flush_queued_messages() - self._pending_tool_advisories.clear() + self._drain_pending_advisories() self._record_fatal_error(exc) raise + def _drain_pending_advisories(self) -> None: + """Drop both advisory channels' pending buffers. + + Both channels are scoped to the current generation: tool-channel + nudges (``tool_error``, ``repeat``) queued earlier in this batch + and user-channel nudges (``correction``, ``denial``, …) queued + during ``_check_metacognitive_nudge`` but not yet drained. When + a generation is abandoned (cancel, KeyboardInterrupt, unexpected + exception) both must drop so they don't bleed into the next + send's tool loop or next user turn. + """ + self._pending_tool_advisories.clear() + self._pending_user_advisories.clear() + def _synthesize_cancelled_results(self, reason: str) -> None: """Synthesize tool_result messages for orphaned tool_calls after cancel. @@ -3060,8 +3185,24 @@ class ChatSession: text_chars, images, doc_chars = self._msg_text_chars(msg) return text_chars + doc_chars + int(images * self._IMAGE_TOKENS * self._chars_per_token) - def _update_token_table(self, assistant_msg: dict[str, Any]) -> None: - """Update per-message token estimates using API usage data.""" + def _update_token_table( + self, + assistant_msg: dict[str, Any], + *, + msgs: list[dict[str, Any]] | None = None, + ) -> None: + """Update per-message token estimates using API usage data. + + *msgs* (optional) is the wire-bound message list already built + for the stream call — passing it avoids a redundant + ``_apply_reminders_for_provider`` walk and, more importantly, + ensures the char count matches the bytes the provider counted + even after ``_mark_reminders_delivered`` has flipped the flag + on the reminders that rode the stream. When *msgs* is None the + caller didn't pre-build (rare path) — fall back to applying + the splice on the fly, but be aware the result will be + reminder-free if delivered flags are already set. + """ if not self._last_usage: return @@ -3071,8 +3212,16 @@ class ChatSession: # Calibrate chars_per_token ratio from actual usage. # Images get a fixed token budget (subtracted). Documents # tokenize non-linearly depending on provider — excluded from - # calibration so they don't skew the text ratio. - all_msgs = self._full_messages() # system + self.messages (before append) + # calibration so they don't skew the text ratio. ``all_msgs`` + # must reflect what the provider actually counted in + # ``prompt_tokens``: when called from the loop with the + # pre-built ``msgs``, that's exact; without it, fall back to + # applying the splice fresh (post-mark-delivered the result + # may undercount, but no caller currently takes this path + # after a successful stream). + all_msgs = ( + msgs if msgs is not None else self._apply_reminders_for_provider(self._full_messages()) + ) # system + self.messages (before append) active_tools = self._get_active_tools() or [] tool_def_chars = sum(len(json.dumps(t)) for t in active_tools) text_chars = 0 @@ -5307,55 +5456,60 @@ class ChatSession: def _queue_user_advisory(self, nudge_type: str, text: str) -> None: """Queue a metacognitive nudge for the next user turn. - Drains in ``_append_user_turn`` as a ```` block - appended to the user message body. Used for nudges that respond - to user behaviour: ``correction``, ``denial``, ``resume``, + Drains in ``_append_user_turn`` onto the user message dict's + ``_reminders`` side-channel. Used for nudges that respond to + user behaviour: ``correction``, ``denial``, ``resume``, ``start``, ``completion``. """ self._pending_user_advisories.append((nudge_type, text)) - def _splice_pending_user_advisories(self, user_msg: dict[str, Any]) -> None: - """Drain ``_pending_user_advisories`` into *user_msg*'s content. + def _attach_pending_user_reminders(self, user_msg: dict[str, Any]) -> None: + """Drain ``_pending_user_advisories`` onto *user_msg*'s ``_reminders`` + sibling key (a side-channel, never inside ``content``). - Mutates *user_msg* in place — the caller appends it after. - Renders each queued nudge as a ```` block - (same envelope as ``wrap_tool_result``) and attaches them to - the trailing edge of the user content. Every text segment in - the user content is passed through ``escape_wrapper_tags`` - first so a user typing literal ```` cannot - fabricate an envelope the model would treat as a - Turnstone-issued reminder. For attachment-bearing turns the - blocks land on the trailing text part so they stay glued to - the same multipart turn. + Mutates *user_msg* in place — the caller appends it after. The + rendered ```` envelope is built later in + ``_apply_reminders_for_provider`` against a transient copy, so + the model still sees the reminder spliced into ``content`` at + the wire boundary while ``self.messages`` and every downstream + consumer (UI replay, compaction, title gen, channel adapters, + DB) see clean user text. + + ``_reminders`` rides the leading-underscore convention used by + other internal sibling metadata (``_attachments_meta``, + ``_provider_content``); ``sanitize_messages`` strips it before + the wire on its own pass. + + Also fires the live ``on_user_reminder`` UI hook so any open + SSE consumers (other browser tabs, CLI mirrors, eventual + channel adapters) can render the reminder bubble in lockstep + with the originating tab's optimistic render. Hook failures + are logged and swallowed: the side-channel write is the + load-bearing op, and a UI implementation throwing here must + not abort the user's send (which would otherwise drop both the + user message and the queued nudges). """ if not self._pending_user_advisories: return - from turnstone.core.tool_advisory import escape_wrapper_tags, render_system_reminder items = list(self._pending_user_advisories) self._pending_user_advisories.clear() - block = "\n\n" + "\n\n".join(render_system_reminder(text) for _, text in items) - content = user_msg["content"] - if isinstance(content, str): - user_msg["content"] = escape_wrapper_tags(content) + block - else: - text_parts = [p for p in content if isinstance(p, dict) and p.get("type") == "text"] - for part in text_parts: - part["text"] = escape_wrapper_tags(part.get("text", "")) - if text_parts: - text_parts[-1]["text"] = text_parts[-1]["text"] + block - else: - content.append({"type": "text", "text": block}) + reminders = [{"type": nudge_type, "text": text} for nudge_type, text in items] + user_msg["_reminders"] = reminders self._emit_nudge_ping(nudge_type for nudge_type, _ in items) + try: + self.ui.on_user_reminder(reminders) + except Exception: + log.warning("ui.on_user_reminder failed; reminder still attached", exc_info=True) def _emit_nudge_ping(self, types: Iterable[str]) -> None: """Surface the ``[metacognition: nudge injected — …]`` UI line. Centralised so both drain sites (tool channel via ``_collect_advisories``, user channel via - ``_splice_pending_user_advisories``) emit the same wording. + ``_attach_pending_user_reminders``) emit the same wording. """ joined = ", ".join(types) if joined: diff --git a/turnstone/core/session_ui_base.py b/turnstone/core/session_ui_base.py index f0e5f0af..fadc6df0 100644 --- a/turnstone/core/session_ui_base.py +++ b/turnstone/core/session_ui_base.py @@ -1293,6 +1293,20 @@ class SessionUIBase: def on_error(self, message: str) -> None: self._enqueue({"type": "error", "message": message}) + def on_user_reminder(self, reminders: list[dict[str, str]]) -> None: + """Surface a metacognitive nudge as its own UI element. + + Reminders live on the user message dict's ``_reminders`` + side-channel and are spliced into ``content`` only at the + provider boundary; this event is what lets every connected + SSE consumer (other browser tabs, CLI mirrors, future channel + adapters) render the reminder bubble in lockstep with the + originating tab. The history-replay path surfaces the same + shape via ``_build_history`` so a tab reconnecting later + renders the same bubble. + """ + self._enqueue({"type": "user_reminder", "reminders": reminders}) + # ------------------------------------------------------------------ # Broadcast hooks — kind-specific transport. # diff --git a/turnstone/eval.py b/turnstone/eval.py index 5a038989..34f979e2 100644 --- a/turnstone/eval.py +++ b/turnstone/eval.py @@ -139,6 +139,9 @@ class NullUI: def on_error(self, message: str) -> None: pass + def on_user_reminder(self, reminders: list[dict[str, str]]) -> None: + pass + def on_state_change(self, state: str) -> None: pass diff --git a/turnstone/server.py b/turnstone/server.py index 39d41121..706532f4 100644 --- a/turnstone/server.py +++ b/turnstone/server.py @@ -359,6 +359,12 @@ def _build_history( issued the tool calls is also marked ``"denied": True`` so the client can render the correct badge. """ + # Metacognitive nudges live on the user message dict's ``_reminders`` + # side-channel and are surfaced separately on each entry so the UI + # can render them as their own bubble (live + replay). ``content`` + # never carries the ```` envelope — that splice is + # transient, applied to a wire-bound copy in + # ``ChatSession._apply_reminders_for_provider``. history = [] for msg in session.messages: content = msg.get("content") @@ -404,6 +410,23 @@ def _build_history( entry = {"role": msg["role"], "content": content} if attachments_meta: entry["attachments"] = attachments_meta + # Surface the ``_reminders`` side-channel so a tab reconnecting + # via /history renders the same metacognitive nudge bubble as + # the originating tab saw via the live ``user_reminder`` SSE + # event. Reminders are in-memory only (not persisted to DB), + # so this only fires for the originating session. + reminders = msg.get("_reminders") + if isinstance(reminders, list): + # Filter first so an all-malformed _reminders doesn't set the + # field to []; absent vs. empty-list should mean the same + # thing on the wire. + clean_reminders = [ + {"type": str(r.get("type") or ""), "text": str(r.get("text") or "")} + for r in reminders + if isinstance(r, dict) + ] + if clean_reminders: + entry["reminders"] = clean_reminders if msg.get("tool_calls"): entry["tool_calls"] = [ { diff --git a/turnstone/ui/static/app.js b/turnstone/ui/static/app.js index 32008447..996f8094 100644 --- a/turnstone/ui/static/app.js +++ b/turnstone/ui/static/app.js @@ -557,6 +557,28 @@ Pane.prototype.handleEvent = function (evt) { this.addErrorMessage(evt.message); break; + case "user_reminder": + // Metacognitive nudges — render as their own bubble above the + // user message they advise. The originating tab's optimistic + // addUserMessage already ran when the user clicked send, so by + // the time this SSE event arrives the just-sent user bubble is + // at the bottom of messagesEl and addUserReminder's "anchor to + // most recent .msg.user" lookup finds it correctly. + // + // Multi-tab caveat: the server emits no user_message SSE event + // today, so a non-originating tab open on the same workstream + // sees the reminder without a paired user-message render — the + // anchor falls on a stale prior user bubble, mis-positioning + // the reminder. The next /history reload corrects it (the + // entry["reminders"] propagation in _build_history is + // anchor-stable because replayHistory runs addUserMessage first + // for every turn). Acceptable cost for stage 1; closing the + // gap is a follow-up that adds a user_message SSE event. + if (Array.isArray(evt.reminders) && evt.reminders.length) { + this.addUserReminder(evt.reminders); + } + break; + case "message_queued": // Confirmation from server that a queued message was accepted. // The UI already showed the message optimistically in addQueuedMessage. @@ -669,6 +691,41 @@ Pane.prototype.removeThinkingIndicator = function () { if (el) el.remove(); }; +Pane.prototype.addUserReminder = function (reminders) { + // Render each metacognitive reminder as its own bubble visually + // anchored above the user message it advises. Always called AFTER + // the corresponding addUserMessage (live: optimistic local render + // ran before the SSE event arrived; replay: replayHistory now + // renders the user message first), so "most recent .msg.user" is + // always THIS turn's bubble — insertBefore drops the reminder + // directly above it. When no .msg.user exists at all (e.g. a + // non-originating tab receiving a stage-1 reminder before any user + // turn has rendered) we append; the next /history reload corrects + // any anchor anomaly. + this.removeEmptyState(); + var userBubbles = this.messagesEl.querySelectorAll(".msg.user"); + var anchor = userBubbles.length ? userBubbles[userBubbles.length - 1] : null; + for (var i = 0; i < reminders.length; i++) { + var r = reminders[i] || {}; + var el = document.createElement("div"); + el.className = "msg user-reminder"; + var labelEl = document.createElement("span"); + labelEl.className = "msg-user-reminder-label"; + labelEl.textContent = "metacog" + (r.type ? " · " + String(r.type) : ""); + var textEl = document.createElement("span"); + textEl.className = "msg-user-reminder-text"; + textEl.textContent = r.text || ""; + el.appendChild(labelEl); + el.appendChild(textEl); + if (anchor) { + this.messagesEl.insertBefore(el, anchor); + } else { + this.messagesEl.appendChild(el); + } + } + this.scrollToBottom(true); +}; + Pane.prototype.addUserMessage = function (text, attachments) { this.removeEmptyState(); var el = document.createElement("div"); @@ -911,7 +968,15 @@ Pane.prototype.replayHistory = function (messages) { for (var i = 0; i < messages.length; i++) { var msg = messages[i]; if (msg.role === "user") { + // addUserMessage first so addUserReminder's "anchor to most + // recent .msg.user" lookup finds THIS message's bubble (not the + // previous user message's, which would mis-place the reminder + // above the wrong turn). insertBefore then drops the reminder + // directly above the just-rendered user bubble. this.addUserMessage(msg.content || "", msg.attachments || null); + if (Array.isArray(msg.reminders) && msg.reminders.length) { + this.addUserReminder(msg.reminders); + } lastToolBlock = null; } else if (msg.role === "assistant") { if (msg.tool_calls && msg.tool_calls.length) { diff --git a/turnstone/ui/static/style.css b/turnstone/ui/static/style.css index a6f4a63e..63cbd4e5 100644 --- a/turnstone/ui/static/style.css +++ b/turnstone/ui/static/style.css @@ -642,6 +642,28 @@ .msg.user { color: var(--fg-bright); } +/* Metacognitive reminder — pinned above the user message it advises. + Yellow accent reads as "advisory metadata" against the amber-ish + user colour and the cyan tool cards; deliberately quieter than the + user bubble so it doesn't compete for attention. */ +.msg.user-reminder { + align-self: flex-end; + max-width: min(78ch, 100%); + background: var(--bg-surface); + border: 1px solid var(--border); + border-left: 3px solid var(--yellow); + color: var(--fg-dim); + font-size: 12px; + padding: 6px 10px; + margin-bottom: 2px; + white-space: pre-wrap; +} +.msg.user-reminder .msg-user-reminder-label { + color: var(--yellow); + font-weight: 600; + margin-right: 6px; + text-transform: lowercase; +} /* .msg.assistant / .msg.info / .msg.error alignment + baseline visuals come from shared_static/chat.css. Interactive UI adds a pre-wrap override for info messages and a tightened tool-message shape with