From 6b77700752693ffbbc2348edfac3b0e16875acd3 Mon Sep 17 00:00:00 2001 From: lei_lei <96427312+leilei3167@users.noreply.github.com> Date: Thu, 3 Sep 2026 03:36:47 +0000 Subject: [PATCH 1/4] fix(reflect): keep tools schema stable across forced turns and synthesis Ollama/LM Studio used to narrow tools[] per named tool_choice and the final synthesis swapped the system prompt / dropped tools, which forces hybrid models to full-reprefill. Encode forced tools as a message suffix and make in-budget synthesis a prefix extension instead. Fixes #3865 --- .../engine/providers/openai_compatible_llm.py | 31 +++++-- .../hindsight_api/engine/reflect/agent.py | 79 ++++++++++++----- .../hindsight_api/engine/reflect/prompts.py | 23 +++++ .../tests/test_lmstudio_tool_choice.py | 51 +++++------ .../tests/test_reflect_agent.py | 84 +++++++++++++------ .../tests/test_reflect_split_synthesis.py | 16 +++- .../test_tool_choice_required_downgrade.py | 26 +++--- 7 files changed, 216 insertions(+), 94 deletions(-) diff --git a/hindsight-api-slim/hindsight_api/engine/providers/openai_compatible_llm.py b/hindsight-api-slim/hindsight_api/engine/providers/openai_compatible_llm.py index 8d8642959a..14543678e0 100644 --- a/hindsight-api-slim/hindsight_api/engine/providers/openai_compatible_llm.py +++ b/hindsight-api-slim/hindsight_api/engine/providers/openai_compatible_llm.py @@ -1467,14 +1467,29 @@ async def call_with_tools( request_tool_choice: str | None if tool_choice.mode is LLMToolChoiceMode.NAMED: forced_name = tool_choice.selected_function_name - filtered = [tool for tool in tools if tool.get("function", {}).get("name") == forced_name] - if len(filtered) != 1: + matching = [tool for tool in tools if tool.get("function", {}).get("name") == forced_name] + if len(matching) != 1: raise ValueError( f"Named tool_choice must reference exactly one declared tool; " - f"found {len(filtered)} definitions for {forced_name!r}" + f"found {len(matching)} definitions for {forced_name!r}" ) - tools = filtered - request_tool_choice = LLMToolChoiceMode.REQUIRED.value + # Providers that silently drop tool_choice="required" (ollama/lmstudio) + # used to narrow tools[] to the forced function so the call stayed + # practically forced under auto. That mutates the tools schema every + # forced turn and forces hybrid/Ollama models to full-reprefill + # (#3865). Keep tools byte-identical and encode the force as a + # conversation suffix instead. + if self._drops_tool_choice_required(): + messages = list(messages) + [ + { + "role": "user", + "content": f"You must call `{forced_name}` now.", + } + ] + request_tool_choice = None + else: + tools = matching + request_tool_choice = LLMToolChoiceMode.REQUIRED.value elif tool_choice.mode is LLMToolChoiceMode.AUTO: request_tool_choice = None else: @@ -1489,9 +1504,9 @@ async def call_with_tools( # LM Studio and Ollama silently drop tool_choice="required", returning an # empty tool_calls array instead of forcing a call (#1563/#1179). # Downgrade to auto (None) so the model still gets to call a tool. Named - # tool_choice dicts were already normalized to "required" + a single - # filtered tool above, so the call stays practically forced even under - # auto. Generic OpenAI-compatible endpoints retain the canonical + # tool_choice on these providers already kept the full tools list and + # appended a force suffix above (#3865); required-only calls just omit + # the field. Generic OpenAI-compatible endpoints retain the canonical # ``required`` contract regardless of whether they use a custom base URL. if request_tool_choice == LLMToolChoiceMode.REQUIRED.value and self._drops_tool_choice_required(): request_tool_choice = None diff --git a/hindsight-api-slim/hindsight_api/engine/reflect/agent.py b/hindsight-api-slim/hindsight_api/engine/reflect/agent.py index 7cecef5a8c..c96d0beb86 100644 --- a/hindsight-api-slim/hindsight_api/engine/reflect/agent.py +++ b/hindsight-api-slim/hindsight_api/engine/reflect/agent.py @@ -15,7 +15,7 @@ from ...cancellation import OperationCancelledError from ...config import get_config -from ..llm_interface import LLM_TOOL_CHOICE_AUTO, LLMToolChoice +from ..llm_interface import LLM_TOOL_CHOICE_AUTO, LLM_TOOL_CHOICE_NONE, LLMToolChoice from ..llm_trace import LLMQueueWait, reset_queue_wait_sink, set_queue_wait_sink from ..llm_transport import describe_llm_error from .models import DirectiveInfo, LLMCall, ReflectAgentResult, StructuredOutputResult, TokenUsageSummary, ToolCall @@ -28,6 +28,7 @@ build_final_prompt, build_final_system_prompt, build_reduce_prompt, + build_stable_synthesis_nudge, build_system_prompt_for_tools, split_context_history, ) @@ -740,18 +741,25 @@ async def _tracked_llm_call(prompt: str, trace_scope: str, system_prompt: str, c return response.strip() async def _forced_final_synthesis(iterations_completed: int) -> ReflectAgentResult: - """Answer without tools from the accumulated tool results. - - When the accumulated results fit the prompt budget this is one LLM call, - exactly as before. When they exceed it, they are SPLIT — not truncated: - each budget-sized chunk is compressed in parallel into dated, cited - claims, and one reduce call synthesizes the answer from every chunk's - claims. The old behavior dropped any over-budget block whole (plus all - older ones), which produced confident "no information" answers carrying - hundreds of citations the synthesis model never saw (#3122). + """Answer from the accumulated tool results. + + When the accumulated results fit the prompt budget this is one LLM call + that *extends* the existing agent conversation: same system prompt, same + ``tools`` array, plus a user nudge to produce the final answer with no + further tool calls (#3865). Swapping the system prompt / dropping tools + used to diverge tools-first chat templates at token 3 and force a full + prefill on hybrid/Ollama models. + + When results exceed the budget they are SPLIT — not truncated: each + budget-sized chunk is compressed in parallel into dated, cited claims, + and one reduce call synthesizes the answer from every chunk's claims. + The old behavior dropped any over-budget block whole (plus all older + ones), which produced confident "no information" answers carrying + hundreds of citations the synthesis model never saw (#3122). Split + synthesis still uses dedicated prompts because the conversation no + longer fits as a prefix extension. """ nonlocal total_input_tokens, total_output_tokens, total_cached_tokens, total_thoughts_tokens - final_system = build_final_system_prompt(bank_profile.get("mission"), llm_output_language, directives) chunks = split_context_history(context_history, max_context_tokens) # Every call below uses the transport-level cap, never the caller's # max_tokens: that is a visible-length target carried as a prompt @@ -759,17 +767,42 @@ async def _forced_final_synthesis(iterations_completed: int) -> ReflectAgentResu # thinking models mid-word — or, on the map calls, starve the evidence # extraction. if len(chunks) <= 1: - prompt = build_final_prompt( - query, - context_history, - bank_profile, - context, - max_context_tokens=max_context_tokens, - max_tokens=max_tokens, - llm_output_language=llm_output_language, + # Prefix-stable synthesis: keep system + tools, append a nudge. + synth_messages = list(messages) + [ + { + "role": "user", + "content": build_stable_synthesis_nudge( + max_tokens=max_tokens, + llm_output_language=llm_output_language, + ), + } + ] + llm_start = time.time() + result = await llm_config.call_with_tools( + messages=synth_messages, + tools=tools, + scope="reflect", + tool_choice=LLM_TOOL_CHOICE_NONE, + temperature=get_config().llm_temperature_reflect, + max_completion_tokens=synthesis_max_completion_tokens, ) - answer = await _tracked_llm_call(prompt, "final", final_system, synthesis_max_completion_tokens) + llm_duration = int((time.time() - llm_start) * 1000) + total_input_tokens += result.input_tokens + total_output_tokens += result.output_tokens + total_cached_tokens += getattr(result, "cached_tokens", 0) or 0 + total_thoughts_tokens += getattr(result, "thoughts_tokens", 0) or 0 + llm_trace.append( + { + "scope": "final", + "duration_ms": llm_duration, + "input_tokens": result.input_tokens, + "output_tokens": result.output_tokens, + } + ) + answer = (result.content or "").strip() else: + final_system = build_final_system_prompt(bank_profile.get("mission"), llm_output_language, directives) + log = logger.warning if len(chunks) > _SPLIT_SYNTHESIS_WARN_CHUNKS else logger.info log( f"[REFLECT {reflect_id}] Retrieved data exceeds the context budget; " @@ -799,7 +832,7 @@ async def _forced_final_synthesis(iterations_completed: int) -> ReflectAgentResu answer = await _tracked_llm_call(prompt, "final", final_system, synthesis_max_completion_tokens) if not (answer or "").strip(): - # Tools were disabled for this call and the model still returned nothing. + # The synthesis call returned no text. # There is no answer to hand back, so the run failed -- see #2959 for why # a placeholder here is worse than an exception. raise ReflectNoAnswerError( @@ -846,7 +879,7 @@ async def _forced_final_synthesis(iterations_completed: int) -> ReflectAgentResu is_last = iteration == max_iterations - 1 if is_last: - # Force text response on last iteration - no tools + # Force final synthesis on last iteration (prefix-stable when in budget) return await _forced_final_synthesis(iteration + 1) # Proactive context-window guard: if accumulated messages would exceed the @@ -1000,7 +1033,7 @@ async def _forced_final_synthesis(iterations_completed: int) -> ReflectAgentResu f"produced no usable tool call (the transport may not support function calling)." + detail ) # Model tool-called earlier and is now stopping: fall through to a clean - # forced final synthesis (tools disabled, prose expected). + # forced final synthesis (prefix-stable when in budget; prose expected). return await _forced_final_synthesis(iteration + 1) # The model produced at least one tool call reflect could parse: it can diff --git a/hindsight-api-slim/hindsight_api/engine/reflect/prompts.py b/hindsight-api-slim/hindsight_api/engine/reflect/prompts.py index cb002d26b4..78aa2b3fda 100644 --- a/hindsight-api-slim/hindsight_api/engine/reflect/prompts.py +++ b/hindsight-api-slim/hindsight_api/engine/reflect/prompts.py @@ -705,6 +705,29 @@ def _length_directive(max_tokens: int | None) -> str | None: ) +def build_stable_synthesis_nudge( + max_tokens: int | None = None, + llm_output_language: str | None = None, +) -> str: + """User-message suffix for prefix-stable final synthesis (#3865). + + Keeps the agent system prompt and ``tools`` array unchanged so synthesis is + a pure extension of the conversation. Hybrid / Ollama templates that render + tools before system text can then resume from a prior checkpoint instead of + full-reprefilling when the old path swapped the system prompt and dropped tools. + """ + parts = [ + "Produce the final answer now. Do not make any further tool calls.", + "", + "## Instructions", + _FINAL_INSTRUCTIONS, + ] + length_directive = _length_directive(max_tokens) + if length_directive is not None: + parts.append(length_directive) + return "\n".join(parts) + output_language_directive(llm_output_language) + + def build_final_prompt( query: str, context_history: list[dict], diff --git a/hindsight-api-slim/tests/test_lmstudio_tool_choice.py b/hindsight-api-slim/tests/test_lmstudio_tool_choice.py index 9d87f38d2e..62f512a4d5 100644 --- a/hindsight-api-slim/tests/test_lmstudio_tool_choice.py +++ b/hindsight-api-slim/tests/test_lmstudio_tool_choice.py @@ -7,8 +7,10 @@ LM Studio (and Ollama) reject this format with HTTP 400: "Tool choice of type 'function' is not supported. Use 'auto', 'none', or 'required'." -The fix should convert named tool_choice to "required" and filter the tools list -to only the requested tool for providers that don't support named tool_choice. +For providers that drop tool_choice="required" (lmstudio/ollama), keep the full +tools list byte-identical and append a user-message suffix forcing the named +tool (#3865). Other providers still convert named tool_choice to "required" + a +filtered tools list. """ import json @@ -157,13 +159,17 @@ async def test_lmstudio_named_tool_choice_no_longer_causes_400(self): assert result.tool_calls[0].name == "search_mental_models" sent_kwargs = mock_create.call_args.kwargs - # The named dict is normalized to "required" + a single filtered tool, - # then "required" is downgraded to auto (omitted) because LM Studio - # silently drops it (#1563/#1179/#1877). The single filtered tool keeps - # the call forced in practice. See test_tool_choice_required_downgrade.py. + # "required" is omitted because LM Studio silently drops it + # (#1563/#1179/#1877). Tools stay full-schema; a user suffix forces the + # named tool (#3865). See test_tool_choice_required_downgrade.py. assert "tool_choice" not in sent_kwargs - assert len(sent_kwargs["tools"]) == 1 - assert sent_kwargs["tools"][0]["function"]["name"] == "search_mental_models" + assert [t["function"]["name"] for t in sent_kwargs["tools"]] == [ + "search_mental_models", + "search_observations", + "recall", + "done", + ] + assert sent_kwargs["messages"][-1]["content"] == "You must call `search_mental_models` now." @pytest.mark.asyncio @pytest.mark.parametrize( @@ -228,11 +234,9 @@ class TestExpectedFixBehavior: {"type": "function", "function": {"name": "search_mental_models"}} The fix should: - 1. Convert tool_choice to "required" - 2. Filter tools to only the requested tool - - These tests currently FAIL (because the fix is not yet implemented). - After the fix is applied, they should PASS. + 1. Omit tool_choice (these servers drop "required") + 2. Keep the full tools list + 3. Append a user suffix forcing the named tool """ @pytest.mark.asyncio @@ -260,21 +264,21 @@ async def test_fix_converts_named_tool_choice_and_downgrades(self): assert result.tool_calls[0].name == "search_mental_models" sent_kwargs = mock_create.call_args.kwargs - # Fix: dict was normalized then "required" downgraded to auto (omitted) + # Fix: "required" omitted; tools stay full; suffix forces the named tool assert "tool_choice" not in sent_kwargs - # Fix: tools filtered to just the requested one (keeps the call forced) - assert len(sent_kwargs["tools"]) == 1 - assert sent_kwargs["tools"][0]["function"]["name"] == "search_mental_models" + assert len(sent_kwargs["tools"]) == len(REFLECT_TOOLS) + assert sent_kwargs["messages"][-1]["content"] == "You must call `search_mental_models` now." @pytest.mark.asyncio @pytest.mark.parametrize( "forced_tool_name", ["search_mental_models", "search_observations", "recall"], ) - async def test_fix_filters_tools_to_requested_tool(self, forced_tool_name: str): + async def test_fix_keeps_full_tools_and_appends_force_suffix(self, forced_tool_name: str): """ - After fix: tools list is filtered to only the forced tool so the model - can only call that one tool (equivalent to the named tool_choice behavior). + After fix (#3865): tools list stays the full reflect schema and a user + suffix asks for the forced tool (equivalent to named tool_choice without + mutating tools[]). """ llm = _make_lmstudio_llm() named_tool_choice = LLMToolChoice.named(forced_tool_name) @@ -291,11 +295,10 @@ async def test_fix_filters_tools_to_requested_tool(self, forced_tool_name: str): ) sent_kwargs = mock_create.call_args.kwargs - # "required" is downgraded to auto (omitted) for lmstudio; the single - # filtered tool keeps the call forced. + # "required" is omitted for lmstudio; tools stay full; suffix forces. assert "tool_choice" not in sent_kwargs - assert len(sent_kwargs["tools"]) == 1 - assert sent_kwargs["tools"][0]["function"]["name"] == forced_tool_name + assert len(sent_kwargs["tools"]) == len(REFLECT_TOOLS) + assert sent_kwargs["messages"][-1]["content"] == f"You must call `{forced_tool_name}` now." @pytest.mark.asyncio async def test_fix_also_applies_to_openai_provider(self): diff --git a/hindsight-api-slim/tests/test_reflect_agent.py b/hindsight-api-slim/tests/test_reflect_agent.py index 3c14186a8e..acda45b1cd 100644 --- a/hindsight-api-slim/tests/test_reflect_agent.py +++ b/hindsight-api-slim/tests/test_reflect_agent.py @@ -970,9 +970,12 @@ async def test_normalizes_tool_names_in_other_tools(self, mock_llm, mock_functio @pytest.mark.asyncio async def test_stop_after_evidence_uses_forced_final_synthesis(self, mock_llm, mock_functions): """A model that tool-called at least once and then stops (no tool call) is a - legitimate completion: reflect does a clean forced final-synthesis call (tools - disabled) rather than salvaging free text or raising ReflectToolCallError. + legitimate completion: reflect does a prefix-stable forced final-synthesis + call (same tools, tool_choice=none) rather than salvaging free text or + raising ReflectToolCallError (#3865). """ + from hindsight_api.engine.llm_interface import LLM_TOOL_CHOICE_NONE + mock_functions["search_mental_models_fn"].return_value = { "mental_models": [{"id": "mm-1", "name": "Prefs", "content": "Fresh content.", "is_stale": False}] } @@ -981,13 +984,15 @@ async def test_stop_after_evidence_uses_forced_final_synthesis(self, mock_llm, m self._mm_call(), # Turn 1: model stops with plain text and no tool call. LLMToolCallResult(tool_calls=[], content="I have enough to answer.", finish_reason="stop"), + # Forced final synthesis (prefix-stable). + LLMToolCallResult( + tool_calls=[], + content="Synthesized final answer.", + finish_reason="stop", + input_tokens=40, + output_tokens=12, + ), ] - mock_llm.call = AsyncMock( - return_value=( - "Synthesized final answer.", - TokenUsage(input_tokens=40, output_tokens=12, total_tokens=52), - ) - ) cap = 64 result = await run_reflect_agent( @@ -1003,23 +1008,37 @@ async def test_stop_after_evidence_uses_forced_final_synthesis(self, mock_llm, m # Answer comes from the clean forced-final call, not the turn-1 free text. assert result.text == "Synthesized final answer." - assert mock_llm.call.await_count == 1 - # The forced-final synthesis no longer hard-caps the transport at the page - # budget (that truncates thinking models mid-word, #3365): the call is - # uncapped by default and the page length reaches the model as a prompt - # directive instead. - assert mock_llm.call.await_args.kwargs["max_completion_tokens"] is None - final_prompt = mock_llm.call.await_args.kwargs["messages"][1]["content"] + assert mock_llm.call.await_count == 0 + assert mock_llm.call_with_tools.await_count == 3 + final_kwargs = mock_llm.call_with_tools.await_args_list[2].kwargs + assert final_kwargs["tool_choice"] is LLM_TOOL_CHOICE_NONE + assert final_kwargs["tools"] # tools kept on the wire (#3865) + assert final_kwargs["max_completion_tokens"] is None + final_prompt = final_kwargs["messages"][-1]["content"] + assert "Produce the final answer now" in final_prompt assert f"approximately {cap} tokens" in final_prompt @pytest.mark.asyncio async def test_max_iterations_reached(self, mock_llm, mock_functions): """Test that agent stops after max iterations even with errors.""" - # LLM keeps calling unknown tools - mock_llm.call_with_tools.return_value = LLMToolCallResult( - tool_calls=[LLMToolCall(id="1", name="unknown_tool", arguments={})], - finish_reason="tool_calls", - ) + from hindsight_api.engine.llm_interface import LLM_TOOL_CHOICE_NONE + + # LLM keeps calling unknown tools, then final synthesis returns prose. + def _side_effect(**kwargs): + if kwargs.get("tool_choice") is LLM_TOOL_CHOICE_NONE: + return LLMToolCallResult( + tool_calls=[], + content="Fallback answer from final iteration", + finish_reason="stop", + input_tokens=100, + output_tokens=50, + ) + return LLMToolCallResult( + tool_calls=[LLMToolCall(id="1", name="unknown_tool", arguments={})], + finish_reason="tool_calls", + ) + + mock_llm.call_with_tools.side_effect = _side_effect result = await run_reflect_agent( llm_config=mock_llm, @@ -1033,6 +1052,7 @@ async def test_max_iterations_reached(self, mock_llm, mock_functions): # Should have a result even if no memories found assert result is not None assert result.iterations == 3 + assert result.text == "Fallback answer from final iteration" @pytest.mark.asyncio async def test_wall_clock_timeout(self, mock_llm: MagicMock, mock_functions: dict[str, AsyncMock]) -> None: @@ -1271,7 +1291,17 @@ async def test_proactive_guard_fires_when_budget_exceeded(self, mock_llm, mock_f async def test_context_overflow_error_skips_retry(self, mock_llm, mock_functions_with_large_output): """A context_length_exceeded error from the LLM should NOT be retried — it should immediately fall back to final synthesis.""" - mock_llm.call_with_tools.side_effect = Exception("context_length_exceeded: messages resulted in 150000 tokens.") + mock_llm.call_with_tools.side_effect = [ + Exception("context_length_exceeded: messages resulted in 150000 tokens."), + # Empty evidence still uses prefix-stable synthesis (#3865). + LLMToolCallResult( + tool_calls=[], + content="Synthesized answer from gathered evidence.", + finish_reason="stop", + input_tokens=50, + output_tokens=20, + ), + ] result = await run_reflect_agent( llm_config=mock_llm, @@ -1283,10 +1313,10 @@ async def test_context_overflow_error_skips_retry(self, mock_llm, mock_functions ) assert result is not None - # Should have attempted only 1 iteration (no retry on overflow error) - assert mock_llm.call_with_tools.call_count == 1 - # Final synthesis was called - mock_llm.call.assert_called_once() + assert result.text == "Synthesized answer from gathered evidence." + # One failed tool-loop attempt + one prefix-stable synthesis call. + assert mock_llm.call_with_tools.call_count == 2 + mock_llm.call.assert_not_called() class TestNoAnswerFailsHard: @@ -1379,8 +1409,7 @@ async def test_empty_document_mode_answer_raises(self, mock_llm, mock_functions) @pytest.mark.asyncio async def test_empty_final_synthesis_raises(self, mock_llm, mock_functions): - """The forced final synthesis (tools disabled) returning nothing also fails.""" - mock_llm.call.return_value = (" ", TokenUsage(input_tokens=10, output_tokens=0, total_tokens=10)) + """The forced final synthesis returning nothing also fails.""" # Gather evidence, then stop tool-calling: the agent falls through to the # forced synthesis, which is where the empty text comes from. mock_llm.call_with_tools.side_effect = [ @@ -1389,6 +1418,7 @@ async def test_empty_final_synthesis_raises(self, mock_llm, mock_functions): finish_reason="tool_calls", ), LLMToolCallResult(content="", tool_calls=[], finish_reason="stop"), + LLMToolCallResult(content=" ", tool_calls=[], finish_reason="stop", input_tokens=10, output_tokens=0), ] with pytest.raises(ReflectNoAnswerError) as exc_info: diff --git a/hindsight-api-slim/tests/test_reflect_split_synthesis.py b/hindsight-api-slim/tests/test_reflect_split_synthesis.py index 8f4114a1fd..8d70879547 100644 --- a/hindsight-api-slim/tests/test_reflect_split_synthesis.py +++ b/hindsight-api-slim/tests/test_reflect_split_synthesis.py @@ -228,7 +228,9 @@ async def test_caller_max_tokens_is_a_directive_not_a_transport_cap(self): @pytest.mark.asyncio async def test_fitting_history_stays_single_call(self): - """No overflow → exactly the pre-existing single forced-synthesis call.""" + """No overflow → exactly one prefix-stable forced-synthesis call (#3865).""" + from hindsight_api.engine.llm_interface import LLM_TOOL_CHOICE_NONE + small = {"memories": [{"id": "mem-1", "text": "one small fact"}]} llm = self._mock_llm("Direct answer.") # First turn recalls; second turn stops with no tool calls → forced synthesis. @@ -238,6 +240,13 @@ async def test_fitting_history_stays_single_call(self): finish_reason="tool_calls", ), LLMToolCallResult(tool_calls=[], finish_reason="stop", content="done"), + LLMToolCallResult( + tool_calls=[], + finish_reason="stop", + content="Direct answer.", + input_tokens=10, + output_tokens=5, + ), ] result = await run_reflect_agent( @@ -251,6 +260,11 @@ async def test_fitting_history_stays_single_call(self): assert result.text == "Direct answer." scopes = [c.scope for c in result.llm_trace] assert scopes == ["agent_1", "agent_2", "final"], f"unexpected scopes {scopes}" + final_kwargs = llm.call_with_tools.await_args_list[2].kwargs + assert final_kwargs["tool_choice"] is LLM_TOOL_CHOICE_NONE + assert final_kwargs["tools"] + assert "Produce the final answer now" in final_kwargs["messages"][-1]["content"] + llm.call.assert_not_called() @pytest.mark.asyncio async def test_map_prompts_partition_the_evidence(self): diff --git a/hindsight-api-slim/tests/test_tool_choice_required_downgrade.py b/hindsight-api-slim/tests/test_tool_choice_required_downgrade.py index 6f1b902e19..02ee76f2b0 100644 --- a/hindsight-api-slim/tests/test_tool_choice_required_downgrade.py +++ b/hindsight-api-slim/tests/test_tool_choice_required_downgrade.py @@ -9,10 +9,11 @@ bank holds the answer. The fix downgrades ``"required"`` to auto (omitted) for these providers so the -model still gets to call a tool. Named ``tool_choice`` dicts are normalized to -``"required"`` + a single filtered tool first, so forced calls stay practically -forced even under auto. Generic OpenAI-compatible endpoints, including custom -base URLs, retain ``"required"`` because URL shape does not declare endpoint +model still gets to call a tool. Named ``tool_choice`` keeps the full ``tools`` +array byte-identical and appends a user-message suffix ("You must call `name` +now") so forced calls stay practically forced without mutating the tools schema +(#3865). Generic OpenAI-compatible endpoints, including custom base URLs, still +narrow tools + send ``"required"`` because URL shape does not declare endpoint capabilities. These are fast, deterministic unit tests: the OpenAI client's ``create`` is @@ -159,10 +160,10 @@ async def test_required_preserved_for_cloud_provider(): async def test_named_tool_choice_forced_call_survives_downgrade(): """Reflect forces a tool via a named dict; it must still effectively force. - The dict is normalized to ``required`` + a single filtered tool, then the - downgrade drops ``required`` to auto. With only one tool available the call - stays practically forced, so the model still emits the tool call instead of - the empty-tool_calls failure mode. + For ollama/lmstudio the full tools schema stays on the wire and a user-message + suffix asks for the named tool (#3865). ``required`` is omitted because these + servers drop it. The model still emits the tool call instead of the + empty-tool_calls failure mode. """ llm = _make_llm("lmstudio", "http://localhost:1234/v1") named = LLMToolChoice.named("recall") @@ -178,9 +179,12 @@ async def test_named_tool_choice_forced_call_survives_downgrade(): sent = mock_create.call_args.kwargs # required was dropped (the silent-drop trigger is gone) ... assert "tool_choice" not in sent - # ... but tools were narrowed to just the forced one, keeping the call forced. - assert len(sent["tools"]) == 1 - assert sent["tools"][0]["function"]["name"] == "recall" + # ... but tools stay byte-identical; the force is a conversation suffix. + assert [tool["function"]["name"] for tool in sent["tools"]] == ["recall", "done"] + assert sent["messages"][-1] == { + "role": "user", + "content": "You must call `recall` now.", + } # and the model returns the tool call rather than an empty array. assert [tc.name for tc in result.tool_calls] == ["recall"] From 450e9f35340e6f076ecc1a5835f5cea0e64a930d Mon Sep 17 00:00:00 2001 From: lei_lei <96427312+leilei3167@users.noreply.github.com> Date: Thu, 3 Sep 2026 04:41:57 +0000 Subject: [PATCH 2/4] fix(reflect): align page-length tests with prefix-stable synthesis Drop unused build_final_prompt import (ruff) and give forced-synthesis mocks a third call_with_tools response for tool_choice=none synthesis. --- .../hindsight_api/engine/reflect/agent.py | 1 - .../test_reflect_page_length_decoupling.py | 33 +++++++++++++++---- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/hindsight-api-slim/hindsight_api/engine/reflect/agent.py b/hindsight-api-slim/hindsight_api/engine/reflect/agent.py index c96d0beb86..68f217a8d1 100644 --- a/hindsight-api-slim/hindsight_api/engine/reflect/agent.py +++ b/hindsight-api-slim/hindsight_api/engine/reflect/agent.py @@ -25,7 +25,6 @@ _extract_directive_rules, build_agent_user_prompt, build_chunk_claims_prompt, - build_final_prompt, build_final_system_prompt, build_reduce_prompt, build_stable_synthesis_nudge, diff --git a/hindsight-api-slim/tests/test_reflect_page_length_decoupling.py b/hindsight-api-slim/tests/test_reflect_page_length_decoupling.py index 9b6c8e775a..dff1f9dc5e 100644 --- a/hindsight-api-slim/tests/test_reflect_page_length_decoupling.py +++ b/hindsight-api-slim/tests/test_reflect_page_length_decoupling.py @@ -80,8 +80,12 @@ def _mock_functions(): } -def _stop_after_evidence(llm): - """Turn 0 tool-calls, turn 1 stops with plain text -> forced final synthesis.""" +def _stop_after_evidence(llm, final_answer: str = "Synthesized final answer."): + """Turn 0 tool-calls, turn 1 stops with plain text -> forced final synthesis. + + In-budget forced synthesis is a third call_with_tools (same tools, + tool_choice=none + nudge), not llm.call (#3865). + """ from hindsight_api.engine.response_models import LLMToolCall, LLMToolCallResult llm.call_with_tools = AsyncMock( @@ -91,12 +95,21 @@ def _stop_after_evidence(llm): finish_reason="tool_calls", ), LLMToolCallResult(tool_calls=[], content="I have enough.", finish_reason="stop"), + LLMToolCallResult( + tool_calls=[], + content=final_answer, + finish_reason="stop", + input_tokens=40, + output_tokens=12, + ), ] ) @pytest.mark.asyncio async def test_forced_synthesis_uncapped_by_default(monkeypatch): + from hindsight_api.engine.llm_interface import LLM_TOOL_CHOICE_NONE + monkeypatch.delenv("HINDSIGHT_API_REFLECT_MAX_COMPLETION_TOKENS", raising=False) clear_config_cache() try: @@ -113,9 +126,13 @@ async def test_forced_synthesis_uncapped_by_default(monkeypatch): **_mock_functions(), ) assert result.text == "Synthesized final answer." - # Uncapped transport; page length reached the model via the prompt. - assert llm.call.await_args.kwargs["max_completion_tokens"] is None - assert "approximately 64 tokens" in llm.call.await_args.kwargs["messages"][1]["content"] + assert llm.call.await_count == 0 + assert llm.call_with_tools.await_count == 3 + # Uncapped transport; page length reached the model via the synthesis nudge. + final_kwargs = llm.call_with_tools.await_args_list[2].kwargs + assert final_kwargs["tool_choice"] is LLM_TOOL_CHOICE_NONE + assert final_kwargs["max_completion_tokens"] is None + assert "approximately 64 tokens" in final_kwargs["messages"][-1]["content"] finally: clear_config_cache() @@ -138,8 +155,10 @@ async def test_forced_synthesis_uses_config_cap_when_set(monkeypatch): **_mock_functions(), ) # The transport cap is the operator-set ceiling, still independent of the - # page budget (64). - assert llm.call.await_args.kwargs["max_completion_tokens"] == 12345 + # page budget (64). Synthesis is call_with_tools now (#3865). + assert llm.call.await_count == 0 + final_kwargs = llm.call_with_tools.await_args_list[2].kwargs + assert final_kwargs["max_completion_tokens"] == 12345 finally: clear_config_cache() From 55b5d710f940579b4dde240be30ed9c2b36113cb Mon Sep 17 00:00:00 2001 From: lei_lei <96427312+leilei3167@users.noreply.github.com> Date: Sat, 5 Sep 2026 01:38:40 +0000 Subject: [PATCH 3/4] ci: empty nudge to re-run free-threaded 3.14 flake From 6a067dae207dfa2942b134f4062f1bc7553f782e Mon Sep 17 00:00:00 2001 From: lei_lei <96427312+leilei3167@users.noreply.github.com> Date: Sat, 5 Sep 2026 06:06:32 +0000 Subject: [PATCH 4/4] ci: re-trigger free-threaded 3.14 flake check