diff --git a/cpo/specs/in-flight/generate-structured-message-history.md b/cpo/specs/in-flight/generate-structured-message-history.md new file mode 100644 index 000000000..80c810867 --- /dev/null +++ b/cpo/specs/in-flight/generate-structured-message-history.md @@ -0,0 +1,21 @@ +## Decision Log + +Request: `req_h5iyql3s` + +- D1 — Config key name: `messages` + The chat pre-hook returns a top-level `messages` key because `_run_talent()` copies non-`template_vars` keys directly onto config, `_apply_template_vars()` does not touch them, and no existing pre-hook key collides with `messages`. + +- D2 — Shape: plain dict + Structured history uses a plain `list[dict[str, str]]` with `role` and `content` because every provider adapter already accepts dict-based message lists or can map them locally without adding shared abstractions. + +- D3 — Owner-turn handling + `owner_message` triggers rely on the current owner turn already being present in the chat tail, while `talent_finished` and `talent_errored` triggers synthesize a final user turn like `[talent finished: ]` or `[talent errored: ]` to keep the model input user-final and explicit. + +- D4 — `chat.md` reconciliation + The flattened `$chat_stream_tail` prompt variable is removed from `talent/chat.md` so structured history lives only in `messages`, and the preview baseline is expected to change because it reflects the raw prompt template. + +- D5 — Talent-event markers in structured history + Mid-tail `talent_spawned`, `talent_finished`, and `talent_errored` events are dropped from the structured message list because they are side-channel metadata that disrupt conversational role alternation; only the current finished/errored trigger is preserved via the synthesized final user turn. + +- D6 — Google mapper + Google generate paths convert structured `{role, content}` dicts into Gemini-native `types.Content` objects, mapping `assistant` to `model`, while leaving `system_instruction` on `GenerateContentConfig` and keeping the legacy string/list-of-strings path unchanged. diff --git a/talent/chat.md b/talent/chat.md index 48e2f82a6..8e47ce2d8 100644 --- a/talent/chat.md +++ b/talent/chat.md @@ -24,8 +24,6 @@ $location $trigger_context -$chat_stream_tail - $active_talents $active_routines diff --git a/talent/chat_context.py b/talent/chat_context.py index 1f85e31f6..3bbfa5e19 100644 --- a/talent/chat_context.py +++ b/talent/chat_context.py @@ -11,7 +11,6 @@ from pathlib import Path from typing import Any from convey.chat_stream import read_chat_tail, reduce_chat_state -from think.chat_formatter import format_chat from think.utils import get_config, get_journal logger = logging.getLogger(__name__) @@ -249,13 +248,13 @@ def pre_process(context: dict) -> dict: day = _resolve_day(context, trigger_payload) template_vars = { "digest_contents": "", - "chat_stream_tail": "", "active_talents": "", "trigger_context": "", "location": "", "active_routines": "", "routine_suggestion": "", } + result = {"template_vars": template_vars} try: template_vars["digest_contents"] = _load_digest_contents() @@ -264,13 +263,36 @@ def pre_process(context: dict) -> dict: try: tail = read_chat_tail(day, limit=20) - if tail: - chunks, _meta = format_chat(tail) - body = "\n\n".join( - chunk["markdown"] for chunk in chunks if chunk.get("markdown") + messages: list[dict[str, str]] = [] + for event in tail: + if event["kind"] == "owner_message": + messages.append({"role": "user", "content": event["text"]}) + elif event["kind"] == "sol_message": + messages.append({"role": "assistant", "content": event["text"]}) + + if trigger_kind == "talent_finished": + messages.append( + { + "role": "user", + "content": ( + f"[talent {trigger_payload['name']} finished: " + f"{trigger_payload['summary']}]" + ), + } + ) + elif trigger_kind == "talent_errored": + messages.append( + { + "role": "user", + "content": ( + f"[talent {trigger_payload['name']} errored: " + f"{trigger_payload['reason']}]" + ), + } ) - if body: - template_vars["chat_stream_tail"] = f"## Recent Chat\n\n{body}" + + if messages: + result["messages"] = messages except Exception: logger.debug("Chat tail enrichment failed", exc_info=True) @@ -343,7 +365,7 @@ def pre_process(context: dict) -> dict: except Exception: logger.debug("Routine suggestion eligibility check failed", exc_info=True) - return {"template_vars": template_vars} + return result def _load_digest_contents() -> str: diff --git a/tests/baselines/api/sol/preview.json b/tests/baselines/api/sol/preview.json index 3c4a69621..6c8039513 100644 --- a/tests/baselines/api/sol/preview.json +++ b/tests/baselines/api/sol/preview.json @@ -1,5 +1,5 @@ { - "full_prompt": "## Instructions\n\n## Available Facets\n\n- **Capulet Industries** (`capulet`)\n Capulet Industries enterprise division\n - **Capulet Industries Entities**: Capulet Industries; Juliet Capulet; Nurse Angela; Paris Duke; Tybalt Capulet\n - **Capulet Industries Activities**: Meetings; Coding; Browsing; Email; Messaging; AI Conversation; Writing; Reading; Video; Gaming; Social Media; Planning; Productivity; Terminal; Design; Music\n\n- **Empty Entities Test** (`empty-entities`)\n - **Empty Entities Test Activities**: Meetings; Coding; Browsing; Email; Messaging; AI Conversation; Writing; Reading; Video; Gaming; Social Media; Planning; Productivity; Terminal; Design; Music\n\n- **Full Featured Facet** (`full-featured`)\n A facet for testing all features\n - **Full Featured Facet Entities**: First test entity; Second test entity; Third test entity with description\n - **Full Featured Facet Activities**: Meetings; Coding; Custom Activity; Email; Messaging\n\n- **Minimal Facet** (`minimal-facet`)\n - **Minimal Facet Activities**: Meetings; Coding; Browsing; Email; Messaging; AI Conversation; Writing; Reading; Video; Gaming; Social Media; Planning; Productivity; Terminal; Design; Music\n\n- **Montague Tech** (`montague`)\n Montague Tech startup operations\n - **Tester's Role**: CTO and co-founder of Montague Tech. Visionary full-stack engineer.\n - **Montague Tech Entities**: Balcony App; Balthasar Davi; Benvolio Montague; Friar Lawrence; Juliet Capulet; Mercutio Escalus; Mesh Routing; Montague Tech; Prince Escalus; Rosaline Prince; Schema Bridge; Verona Platform; Verona Ventures\n - **Montague Tech Activities**: Engineering; Meetings; Email; Messaging\n\n- **Priority Test** (`priority-test`)\n - **Priority Test Activities**: Meetings; Coding; Browsing; Email; Messaging; AI Conversation; Writing; Reading; Video; Gaming; Social Media; Planning; Productivity; Terminal; Design; Music\n\n- **Test Facet** (`test-facet`)\n A test facet for validating functionality\n - **Test Facet Entities**: Acme Corp; API Optimization; Bob Wilson; Dashboard Redesign; Docker; Jane Doe; John Smith; PostgreSQL; Tech Solutions Inc; Visual Studio Code\n - **Test Facet Activities**: Meetings; Coding; Browsing; Email; Messaging; AI Conversation; Writing; Reading; Video; Gaming; Social Media; Planning; Productivity; Terminal; Design; Music\n\n- **Verona** (`verona`)\n Cross-company Verona Platform collaboration\n - **Tester's Role**: Co-lead of the Verona Platform joint venture from Montague Tech.\n - **Verona Entities**: Balcony App; Friar Lawrence; Juliet Capulet; Verona Platform\n - **Verona Activities**: Engineering; Meetings; Design Review; Email; Messaging\n\n## Identity Frame\n\nYou are sol, responding to Tester inside the chat backend. You are not the research worker and you do not have tools in this step. Work only from the context already provided to you.\n\n## Current Digest\n\n$digest_contents\n\n$location\n\n$trigger_context\n\n$chat_stream_tail\n\n$active_talents\n\n$active_routines\n\n$routine_suggestion\n\n## Tonal Range\n\nMatch the owner's tone and stakes:\n- Be direct and brief for simple replies.\n- Be warm when the owner is sharing something difficult or personal.\n- Be analytical when the owner needs synthesis or a plan.\n- Be challenging only when there is a clear pattern worth naming.\n\n## Routine Etiquette\n\n- If a routine suggestion appears in context, mention it once and only at the end.\n- Do not raise routine suggestions on machine-driven follow-ups unless the context explicitly includes one.\n- Do not mention internal systems, hooks, or prompt assembly.\n\n## Import And Naming Awareness\n\n- If the owner is asking about imports, naming, or system readiness, answer plainly from the supplied context.\n- Request exec only when answering well requires deeper lookup, synthesis, or tool use.\n\n## When To Dispatch Exec\n\nSet `talent_request` only when the owner needs work that cannot be answered well from the supplied digest, chat history, active routines, and trigger context alone.\n\nDispatch exec for:\n- Journal exploration across days, entities, or transcripts\n- Multi-step synthesis or research\n- Meeting prep that needs fresh participant or activity lookup\n- Any request that clearly needs tool use or external state inspection\n\nDo not dispatch exec for:\n- Simple acknowledgements\n- Straightforward follow-up chat\n- Routine suggestions already supported by the supplied context\n- Brief guidance that can be answered from the current digest and chat tail\n\n## JSON Contract\n\nReturn exactly one JSON object matching `chat.schema.json`.\n\n- `message`: The owner-facing reply. Use `null` only when you genuinely have no safe or useful message to send.\n- `notes`: Brief internal summary of why you responded this way. Keep it factual and concise. Do not dump long reasoning.\n- `talent_request`: `null` unless exec should be dispatched. When dispatching, include:\n - `task`: the exact work exec should perform\n - `context`: optional structured hints that will help exec start fast\n\n## Output Rules\n\n- Return JSON only.\n- `message` should stand on its own without referring to hidden machinery.\n- If `talent_request` is present, the `message` should still be useful to the owner right now.\n- Prefer no dispatch over a weak or redundant dispatch.", + "full_prompt": "## Instructions\n\n## Available Facets\n\n- **Capulet Industries** (`capulet`)\n Capulet Industries enterprise division\n - **Capulet Industries Entities**: Capulet Industries; Juliet Capulet; Nurse Angela; Paris Duke; Tybalt Capulet\n - **Capulet Industries Activities**: Meetings; Coding; Browsing; Email; Messaging; AI Conversation; Writing; Reading; Video; Gaming; Social Media; Planning; Productivity; Terminal; Design; Music\n\n- **Empty Entities Test** (`empty-entities`)\n - **Empty Entities Test Activities**: Meetings; Coding; Browsing; Email; Messaging; AI Conversation; Writing; Reading; Video; Gaming; Social Media; Planning; Productivity; Terminal; Design; Music\n\n- **Full Featured Facet** (`full-featured`)\n A facet for testing all features\n - **Full Featured Facet Entities**: First test entity; Second test entity; Third test entity with description\n - **Full Featured Facet Activities**: Meetings; Coding; Custom Activity; Email; Messaging\n\n- **Minimal Facet** (`minimal-facet`)\n - **Minimal Facet Activities**: Meetings; Coding; Browsing; Email; Messaging; AI Conversation; Writing; Reading; Video; Gaming; Social Media; Planning; Productivity; Terminal; Design; Music\n\n- **Montague Tech** (`montague`)\n Montague Tech startup operations\n - **Tester's Role**: CTO and co-founder of Montague Tech. Visionary full-stack engineer.\n - **Montague Tech Entities**: Balcony App; Balthasar Davi; Benvolio Montague; Friar Lawrence; Juliet Capulet; Mercutio Escalus; Mesh Routing; Montague Tech; Prince Escalus; Rosaline Prince; Schema Bridge; Verona Platform; Verona Ventures\n - **Montague Tech Activities**: Engineering; Meetings; Email; Messaging\n\n- **Priority Test** (`priority-test`)\n - **Priority Test Activities**: Meetings; Coding; Browsing; Email; Messaging; AI Conversation; Writing; Reading; Video; Gaming; Social Media; Planning; Productivity; Terminal; Design; Music\n\n- **Test Facet** (`test-facet`)\n A test facet for validating functionality\n - **Test Facet Entities**: Acme Corp; API Optimization; Bob Wilson; Dashboard Redesign; Docker; Jane Doe; John Smith; PostgreSQL; Tech Solutions Inc; Visual Studio Code\n - **Test Facet Activities**: Meetings; Coding; Browsing; Email; Messaging; AI Conversation; Writing; Reading; Video; Gaming; Social Media; Planning; Productivity; Terminal; Design; Music\n\n- **Verona** (`verona`)\n Cross-company Verona Platform collaboration\n - **Tester's Role**: Co-lead of the Verona Platform joint venture from Montague Tech.\n - **Verona Entities**: Balcony App; Friar Lawrence; Juliet Capulet; Verona Platform\n - **Verona Activities**: Engineering; Meetings; Design Review; Email; Messaging\n\n## Identity Frame\n\nYou are sol, responding to Tester inside the chat backend. You are not the research worker and you do not have tools in this step. Work only from the context already provided to you.\n\n## Current Digest\n\n$digest_contents\n\n$location\n\n$trigger_context\n\n$active_talents\n\n$active_routines\n\n$routine_suggestion\n\n## Tonal Range\n\nMatch the owner's tone and stakes:\n- Be direct and brief for simple replies.\n- Be warm when the owner is sharing something difficult or personal.\n- Be analytical when the owner needs synthesis or a plan.\n- Be challenging only when there is a clear pattern worth naming.\n\n## Routine Etiquette\n\n- If a routine suggestion appears in context, mention it once and only at the end.\n- Do not raise routine suggestions on machine-driven follow-ups unless the context explicitly includes one.\n- Do not mention internal systems, hooks, or prompt assembly.\n\n## Import And Naming Awareness\n\n- If the owner is asking about imports, naming, or system readiness, answer plainly from the supplied context.\n- Request exec only when answering well requires deeper lookup, synthesis, or tool use.\n\n## When To Dispatch Exec\n\nSet `talent_request` only when the owner needs work that cannot be answered well from the supplied digest, chat history, active routines, and trigger context alone.\n\nDispatch exec for:\n- Journal exploration across days, entities, or transcripts\n- Multi-step synthesis or research\n- Meeting prep that needs fresh participant or activity lookup\n- Any request that clearly needs tool use or external state inspection\n\nDo not dispatch exec for:\n- Simple acknowledgements\n- Straightforward follow-up chat\n- Routine suggestions already supported by the supplied context\n- Brief guidance that can be answered from the current digest and chat tail\n\n## JSON Contract\n\nReturn exactly one JSON object matching `chat.schema.json`.\n\n- `message`: The owner-facing reply. Use `null` only when you genuinely have no safe or useful message to send.\n- `notes`: Brief internal summary of why you responded this way. Keep it factual and concise. Do not dump long reasoning.\n- `talent_request`: `null` unless exec should be dispatched. When dispatching, include:\n - `task`: the exact work exec should perform\n - `context`: optional structured hints that will help exec start fast\n\n## Output Rules\n\n- Return JSON only.\n- `message` should stand on its own without referring to hidden machinery.\n- If `talent_request` is present, the `message` should still be useful to the owner right now.\n- Prefer no dispatch over a weak or redundant dispatch.", "multi_facet": false, "name": "chat", "title": "Chat" diff --git a/tests/test_anthropic.py b/tests/test_anthropic.py index 694b0227e..039f388df 100644 --- a/tests/test_anthropic.py +++ b/tests/test_anthropic.py @@ -429,6 +429,29 @@ def test_claude_outfile_error(monkeypatch, tmp_path, capsys): class TestRunGenerateJsonSchema: + def test_structured_messages_passthrough(self, monkeypatch): + provider = importlib.reload( + importlib.import_module("think.providers.anthropic") + ) + mock_client = MagicMock() + mock_response = MagicMock() + mock_response.content = [SimpleNamespace(type="text", text="ok")] + mock_response.usage = None + mock_response.stop_reason = "end_turn" + mock_client.messages.create.return_value = mock_response + monkeypatch.setattr(provider, "_get_anthropic_client", lambda: mock_client) + messages = [ + {"role": "user", "content": "first"}, + {"role": "assistant", "content": "second"}, + {"role": "user", "content": "third"}, + ] + + provider.run_generate(messages, system_instruction="base") + + call_kwargs = mock_client.messages.create.call_args.kwargs + assert call_kwargs["messages"] == messages + assert call_kwargs["system"] == "base" + def test_no_schema_keeps_prompt_append(self, monkeypatch): provider = importlib.reload( importlib.import_module("think.providers.anthropic") @@ -477,6 +500,37 @@ class TestRunGenerateJsonSchema: } assert call_kwargs["system"] == "base" + def test_structured_messages_with_schema_uses_output_config(self, monkeypatch): + provider = importlib.reload( + importlib.import_module("think.providers.anthropic") + ) + mock_client = MagicMock() + mock_response = MagicMock() + mock_response.content = [SimpleNamespace(type="text", text="{}")] + mock_response.usage = None + mock_response.stop_reason = "end_turn" + mock_client.messages.create.return_value = mock_response + monkeypatch.setattr(provider, "_get_anthropic_client", lambda: mock_client) + schema = {"type": "object"} + messages = [ + {"role": "user", "content": "first"}, + {"role": "assistant", "content": "second"}, + {"role": "user", "content": "third"}, + ] + + provider.run_generate( + messages, + system_instruction="base", + json_schema=schema, + ) + + call_kwargs = mock_client.messages.create.call_args.kwargs + assert call_kwargs["messages"] == messages + assert call_kwargs["output_config"] == { + "format": {"type": "json_schema", "schema": schema} + } + assert call_kwargs["system"] == "base" + def test_fallback_on_bad_request(self, monkeypatch): provider = importlib.reload( importlib.import_module("think.providers.anthropic") diff --git a/tests/test_chat_context.py b/tests/test_chat_context.py index 327d4a72e..2e1216253 100644 --- a/tests/test_chat_context.py +++ b/tests/test_chat_context.py @@ -12,7 +12,6 @@ from convey.chat_stream import append_chat_event TEMPLATE_VAR_KEYS = { "digest_contents", - "chat_stream_tail", "active_talents", "trigger_context", "location", @@ -146,15 +145,11 @@ def test_chat_context_injects_digest_tail_trigger_location_and_routine_state( template_vars = _assert_template_vars_result(result) assert template_vars["digest_contents"] == "Digest notes for today." - assert "## Recent Chat" in template_vars["chat_stream_tail"] - assert ( - "**Alice** Please brief me for my meeting" in template_vars["chat_stream_tail"] - ) - assert "**Sol-agent** I can help with that." in template_vars["chat_stream_tail"] - assert ( - "*[exec spawned: Prepare the meeting brief]*" - in template_vars["chat_stream_tail"] - ) + assert result["messages"] == [ + {"role": "user", "content": "Please brief me for my meeting"}, + {"role": "assistant", "content": "I can help with that."}, + ] + assert all("exec spawned" not in msg["content"] for msg in result["messages"]) assert "## Active Execs" in template_vars["active_talents"] assert "Prepare the meeting brief" in template_vars["active_talents"] assert "## Trigger Context" in template_vars["trigger_context"] @@ -216,6 +211,61 @@ def test_chat_context_routine_suggestion_only_counts_owner_messages( assert len(save_calls) == 1 +def test_chat_context_talent_finished_appends_final_user_message(monkeypatch, tmp_path): + journal = tmp_path / "journal" + monkeypatch.setenv("_SOLSTONE_JOURNAL_OVERRIDE", str(journal)) + + append_chat_event( + "owner_message", + ts=_ts(10, 0), + text="What happened?", + app="home", + path="/app/home", + facet="work", + ) + append_chat_event( + "sol_message", + ts=_ts(10, 1), + use_id="use-chat-2", + text="Looking into it.", + notes="Acknowledged request.", + requested_exec=False, + requested_task=None, + ) + append_chat_event( + "talent_finished", + ts=_ts(10, 2), + use_id="use-exec-2", + name="exec", + summary="Found the latest notes.", + ) + + monkeypatch.setattr("think.routines.get_routine_state", lambda: []) + monkeypatch.setattr( + "think.routines.get_config", + lambda: {"_meta": {"suggestions_enabled": False, "suggestions": {}}}, + ) + monkeypatch.setattr("think.routines.save_config", lambda config: None) + + result = _load_chat_context_module().pre_process( + { + "day": "20260420", + "trigger_kind": "talent_finished", + "trigger_payload": { + "name": "exec", + "summary": "Found the latest notes.", + }, + } + ) + + _assert_template_vars_result(result) + assert result["messages"] == [ + {"role": "user", "content": "What happened?"}, + {"role": "assistant", "content": "Looking into it."}, + {"role": "user", "content": "[talent exec finished: Found the latest notes.]"}, + ] + + def test_chat_context_preserves_save_routines_config_side_effect(monkeypatch, tmp_path): journal = tmp_path / "journal" monkeypatch.setenv("_SOLSTONE_JOURNAL_OVERRIDE", str(journal)) @@ -260,8 +310,8 @@ def test_chat_context_routines_omitted_when_empty(monkeypatch, tmp_path): template_vars = _assert_template_vars_result(result) assert template_vars["active_routines"] == "" - assert template_vars["chat_stream_tail"] == "" assert template_vars["active_talents"] == "" + assert "messages" not in result def test_chat_context_enrichment_errors_are_graceful(monkeypatch, tmp_path): @@ -294,12 +344,12 @@ def test_chat_context_enrichment_errors_are_graceful(monkeypatch, tmp_path): template_vars = _assert_template_vars_result(result) assert template_vars["digest_contents"] == "" - assert template_vars["chat_stream_tail"] == "" assert template_vars["active_talents"] == "" assert template_vars["active_routines"] == "" assert template_vars["routine_suggestion"] == "" assert "Type: owner_message" in template_vars["trigger_context"] assert "/app/home" in template_vars["location"] + assert "messages" not in result def test_chat_context_drops_legacy_memory_imports(monkeypatch): diff --git a/tests/test_google.py b/tests/test_google.py index 25c1e21fe..a41778117 100644 --- a/tests/test_google.py +++ b/tests/test_google.py @@ -67,6 +67,15 @@ def make_mock_process(stdout_lines, return_code=0): return process +def _assert_structured_contents(contents): + assert [content.role for content in contents] == ["user", "model", "user"] + assert [[part.text for part in content.parts] for content in contents] == [ + ["first"], + ["second"], + ["third"], + ] + + def test_google_main(monkeypatch, tmp_path, capsys): setup_google_genai_stub(monkeypatch, with_thinking=False) sys.modules.pop("think.providers.google", None) @@ -262,6 +271,58 @@ def test_format_completion_message_none(): class TestRunGenerateJsonSchema: + def test_structured_messages_sync_mapped_to_google_contents(self, monkeypatch): + setup_google_genai_stub(monkeypatch, with_thinking=False) + sys.modules.pop("think.providers.google", None) + provider = importlib.reload(importlib.import_module("think.providers.google")) + + mock_client = MagicMock() + mock_client.models.generate_content.return_value = SimpleNamespace( + text="[]", + candidates=[], + usage_metadata=None, + ) + monkeypatch.setattr( + provider, "get_or_create_client", lambda _client=None: mock_client + ) + messages = [ + {"role": "user", "content": "first"}, + {"role": "assistant", "content": "second"}, + {"role": "user", "content": "third"}, + ] + + provider.run_generate(messages, model=GEMINI_FLASH) + + contents = mock_client.models.generate_content.call_args.kwargs["contents"] + _assert_structured_contents(contents) + + def test_structured_messages_async_mapped_to_google_contents(self, monkeypatch): + setup_google_genai_stub(monkeypatch, with_thinking=False) + sys.modules.pop("think.providers.google", None) + provider = importlib.reload(importlib.import_module("think.providers.google")) + + mock_client = MagicMock() + mock_client.aio.models.generate_content = AsyncMock( + return_value=SimpleNamespace( + text="[]", + candidates=[], + usage_metadata=None, + ) + ) + monkeypatch.setattr( + provider, "get_or_create_client", lambda _client=None: mock_client + ) + messages = [ + {"role": "user", "content": "first"}, + {"role": "assistant", "content": "second"}, + {"role": "user", "content": "third"}, + ] + + asyncio.run(provider.run_agenerate(messages, model=GEMINI_FLASH)) + + contents = mock_client.aio.models.generate_content.call_args.kwargs["contents"] + _assert_structured_contents(contents) + def test_no_schema_kwargs_unchanged(self, monkeypatch): setup_google_genai_stub(monkeypatch, with_thinking=False) sys.modules.pop("think.providers.google", None) diff --git a/tests/test_ollama.py b/tests/test_ollama.py index d5e13f4b9..279d759ad 100644 --- a/tests/test_ollama.py +++ b/tests/test_ollama.py @@ -476,6 +476,35 @@ class TestRunGenerate: assert messages[0] == {"role": "system", "content": "be concise"} assert messages[1] == {"role": "user", "content": "hello"} + def test_structured_messages_body(self): + provider = _ollama_provider() + mock_response = MagicMock() + mock_response.json.return_value = _make_ollama_response() + mock_response.raise_for_status = MagicMock() + input_messages = [ + {"role": "user", "content": "first"}, + {"role": "assistant", "content": "second"}, + {"role": "user", "content": "third"}, + ] + + with patch.object(provider, "_get_client") as mock_get: + mock_client = MagicMock() + mock_client.post.return_value = mock_response + mock_get.return_value = mock_client + + provider.run_generate( + input_messages, + model=OLLAMA_FLASH, + system_instruction="be concise", + ) + + call_kwargs = mock_client.post.call_args + body = call_kwargs.kwargs["json"] + assert body["messages"] == [ + {"role": "system", "content": "be concise"}, + *input_messages, + ] + def test_timeout(self): provider = _ollama_provider() mock_response = MagicMock() diff --git a/tests/test_openai.py b/tests/test_openai.py index 0fe998cb2..de05cfaa2 100644 --- a/tests/test_openai.py +++ b/tests/test_openai.py @@ -685,6 +685,32 @@ class TestRunGenerate: "total_tokens": 15, } + def test_structured_messages_passthrough(self): + provider = _openai_provider() + mock_client = MagicMock() + mock_client.responses.create = MagicMock() + mock_response = MagicMock() + mock_response.output_text = "Hello world" + mock_response.status = "completed" + mock_response.incomplete_details = None + mock_response.usage = None + mock_response.output = [] + mock_client.responses.create.return_value = mock_response + messages = [ + {"role": "user", "content": "first"}, + {"role": "assistant", "content": "second"}, + {"role": "user", "content": "third"}, + ] + + with patch( + "think.providers.openai._get_openai_client", return_value=mock_client + ): + provider.run_generate(messages, system_instruction="Be helpful") + + called_kwargs = mock_client.responses.create.call_args.kwargs + assert called_kwargs["input"] == messages + assert called_kwargs["instructions"] == "Be helpful" + def test_with_effort_suffix(self): provider = _openai_provider() mock_client = MagicMock() diff --git a/tests/test_talent_fallback.py b/tests/test_talent_fallback.py index 7c493b4f2..1ab68d1a2 100644 --- a/tests/test_talent_fallback.py +++ b/tests/test_talent_fallback.py @@ -266,6 +266,70 @@ def test_on_failure_retry_cogitate_uses_context_from_name(monkeypatch): assert seen["context"] == "talent.system.default" +def test_execute_generate_uses_messages_when_present(monkeypatch): + from think.talents import _execute_generate + + events = [] + seen = {} + messages = [ + {"role": "user", "content": "first"}, + {"role": "assistant", "content": "second"}, + {"role": "user", "content": "third"}, + ] + + def mock_generate_with_result(**kwargs): + seen["contents"] = kwargs["contents"] + return {"text": "ok", "usage": {"input_tokens": 1, "output_tokens": 1}} + + monkeypatch.setattr( + "think.talent.key_to_context", lambda _name: "talent.system.default" + ) + monkeypatch.setattr("think.models.generate_with_result", mock_generate_with_result) + + config = { + "name": "chat", + "messages": messages, + "transcript": "ignored transcript", + "user_instruction": "ignored instruction", + "prompt": "ignored prompt", + "health_stale": False, + } + + asyncio.run(_execute_generate(config, events.append)) + + assert seen["contents"] == messages + assert events[-1]["event"] == "finish" + + +def test_execute_generate_preserves_string_contents_order(monkeypatch): + from think.talents import _execute_generate + + events = [] + seen = {} + + def mock_generate_with_result(**kwargs): + seen["contents"] = kwargs["contents"] + return {"text": "ok", "usage": {"input_tokens": 1, "output_tokens": 1}} + + monkeypatch.setattr( + "think.talent.key_to_context", lambda _name: "talent.system.default" + ) + monkeypatch.setattr("think.models.generate_with_result", mock_generate_with_result) + + config = { + "name": "chat", + "transcript": "transcript", + "user_instruction": "instruction", + "prompt": "prompt", + "health_stale": False, + } + + asyncio.run(_execute_generate(config, events.append)) + + assert seen["contents"] == ["transcript", "instruction", "prompt"] + assert events[-1]["event"] == "finish" + + def test_on_failure_retry_generate(monkeypatch): from think.talents import _execute_generate diff --git a/think/providers/google.py b/think/providers/google.py index 8db17d52e..9496b516f 100644 --- a/think/providers/google.py +++ b/think/providers/google.py @@ -65,6 +65,28 @@ logger = logging.getLogger(__name__) _detected_backend: str | None = None +def _structured_to_google_contents( + messages: list[dict[str, str]], +) -> list[types.Content]: + """Map role/content dicts to Gemini-native Content objects.""" + mapped: list[types.Content] = [] + for msg in messages: + role = msg["role"] + if role == "user": + google_role = "user" + elif role == "assistant": + google_role = "model" + else: + raise ValueError(f"Unknown message role: {role!r}") + mapped.append( + types.Content( + role=google_role, + parts=[types.Part(text=msg["content"])], + ) + ) + return mapped + + # --------------------------------------------------------------------------- # Client and helper functions for generate/agenerate # --------------------------------------------------------------------------- @@ -473,6 +495,13 @@ def run_generate( client = get_or_create_client(client) if isinstance(contents, str): contents = [contents] + elif ( + isinstance(contents, list) + and contents + and isinstance(contents[0], dict) + and "role" in contents[0] + ): + contents = _structured_to_google_contents(contents) config = _build_generate_config( temperature=temperature, max_output_tokens=max_output_tokens, @@ -519,6 +548,13 @@ async def run_agenerate( client = get_or_create_client(client) if isinstance(contents, str): contents = [contents] + elif ( + isinstance(contents, list) + and contents + and isinstance(contents[0], dict) + and "role" in contents[0] + ): + contents = _structured_to_google_contents(contents) config = _build_generate_config( temperature=temperature, max_output_tokens=max_output_tokens, diff --git a/think/talents.py b/think/talents.py index feccdd0ca..49b66c43a 100644 --- a/think/talents.py +++ b/think/talents.py @@ -929,6 +929,7 @@ async def _execute_generate( from think.talent import key_to_context name = config["name"] + messages = config.get("messages") transcript = config.get("transcript", "") user_instruction = config.get("user_instruction", "") prompt = config.get("prompt", "") @@ -947,18 +948,21 @@ async def _execute_generate( 480, max(120, (max_output_tokens + thinking_budget) // 100) ) - # Build contents: transcript + instruction + prompt - contents = [] - if transcript: - contents.append(transcript) - if user_instruction: - contents.append(user_instruction) - if prompt: - contents.append(prompt) - - # Fallback if no contents - if not contents: - contents = ["No input provided."] + if messages and isinstance(messages, list): + contents = messages + else: + # Build contents: transcript + instruction + prompt + contents = [] + if transcript: + contents.append(transcript) + if user_instruction: + contents.append(user_instruction) + if prompt: + contents.append(prompt) + + # Fallback if no contents + if not contents: + contents = ["No input provided."] context = key_to_context(name) try: