diff --git a/.dialyzer_ignore.exs b/.dialyzer_ignore.exs index 8a97a8d9..5275c030 100644 --- a/.dialyzer_ignore.exs +++ b/.dialyzer_ignore.exs @@ -222,7 +222,7 @@ # Defensive fallbacks and MapSet opacity retained at the 0.3 cut. These are # individually pinned so a changed success type makes the gate ask again. {"bench/imp/benchmark_truth/multimodal_runner.ex", :pattern_match_cov, {341, 16}}, - {"lib/imp/adapter/chat.ex", :pattern_match_cov, {730, 8}}, + {"lib/imp/adapter/chat.ex", :pattern_match_cov, {742, 8}}, {"lib/imp/adapter/xml.ex", :pattern_match_cov, {675, 8}}, {"lib/imp/mcp.ex", :pattern_match_cov, {372, 8}}, {"lib/imp/optimizer/artifact.ex", :call_without_opaque, {745, 52}}, diff --git a/docs/TRAJECTORIES.md b/docs/TRAJECTORIES.md index 2187140d..da31c7cc 100644 --- a/docs/TRAJECTORIES.md +++ b/docs/TRAJECTORIES.md @@ -36,8 +36,10 @@ Capture is bounded separately from execution. `Imp.Run.start/3` accepts markers before sink delivery. Snapshot eviction adds a `capture_gap` marker; the sink can retain all bounded events independently of snapshot eviction. Applications can choose a larger event bound when persisting long prompts, but -must account for memory and sink throughput. A digest is not a recoverable -artifact. Truncated observations are never exported as complete model output. +must account for memory and sink throughput. Each of the three also accepts +`:infinity`, which removes that bound: a host that needs a complete record of a +run sets all three and accepts that the run holds every event in memory. A +digest is not a recoverable artifact. Truncated observations are never exported as complete model output. The projection uses ATIF-v1.8. It preserves actual initial context roles, marks that context as copied, and keeps later model request messages in metadata. diff --git a/lib/imp/adapter/chat.ex b/lib/imp/adapter/chat.ex index 78ff3fb1..b8f16647 100644 --- a/lib/imp/adapter/chat.ex +++ b/lib/imp/adapter/chat.ex @@ -13,8 +13,10 @@ defmodule Imp.Adapter.Chat do Options to `format/3`: `:demos`, `:response_instruction`, `:guidance`, `:omit_empty_request`, and the renderer seams `:output_renderer`, - `:input_section_renderer` and `:system_renderer`, which let another adapter - reuse this message assembly with its own dialect. Options outside that list + `:input_section_renderer`, `:system_renderer` and `:tool_result_renderer`, + which let another adapter reuse this message assembly with its own dialect + and let a host bound what a tool result costs in the prompt without changing + what the loop records. Options outside that list are ignored; anything that is not a keyword list raises `ArgumentError`. """ @@ -39,6 +41,13 @@ defmodule Imp.Adapter.Chat do # format options, so a renderer can read `:guidance`. Default: # `render_system/2`. Replacing it leaves parsing unchanged. system_renderer: [type: {:fun, 2}], + # Renderer for one TOOL result message: (result, call), where call is + # `%{id:, name:}` for the call that produced it. Default: + # `format_tool_result/1`. This is where a host bounds what the model reads + # of a large result: the loop still records the whole result in history and + # in run events, and only the prompt carries the bounded view. Errors reach + # it too, so a host decides how a failure reads. + tool_result_renderer: [type: {:fun, 2}], # Loop guidance a program passes as data rather than writing into # `signature.instructions`: `%{finish_tool:, input_names:, output_names:, # tool_names:}`. @@ -60,8 +69,11 @@ defmodule Imp.Adapter.Chat do input_renderer = Keyword.get(opts, :input_section_renderer) || (&chat_input_section/2) system_renderer = Keyword.get(opts, :system_renderer) || (&render_system/2) + tool_result_renderer = + Keyword.get(opts, :tool_result_renderer) || (&default_tool_result_renderer/2) + {history_messages, history_fields} = - extract_history(signature, inputs, output_renderer, input_renderer) + extract_history(signature, inputs, output_renderer, input_renderer, tool_result_renderer) request = %{ role: :user, @@ -790,9 +802,11 @@ defmodule Imp.Adapter.Chat do Successful results format like any other value. A failed result renders as one sentence instead of an Elixir term: a denied call says who declined it, - a crashed tool names itself and its message, and an atom reason is spelled - out. Hosts that relay Imp tool results over a protocol boundary should use - this so the same words reach the person that reached the model. + a crashed tool names itself and its message, an atom reason is spelled out, a + rejected `submit` says which outputs it needs, and a structured `:reason` map + reads as its reason and limit. Hosts that relay Imp tool results over a + protocol boundary should use this so the same words reach the person that + reached the model. iex> Imp.Adapter.Chat.format_tool_result({:error, {:tool_authorization_denied, :post, :client_denied}}) "Error: post was not allowed; the person declined it." @@ -808,14 +822,48 @@ defmodule Imp.Adapter.Chat do do: "#{name} was not allowed: #{error_prose(reason)}" defp error_prose({:tool_error, name, message}), do: "#{name} failed: #{error_prose(message)}" + + # A failed submit is the one tool error the model is expected to act on, so it + # says what is wrong with the call rather than naming an internal term. + defp error_prose({:missing_output_fields, names}) when is_list(names), + do: "submit is missing: " <> Enum.map_join(names, ", ", &to_string/1) + + defp error_prose({:invalid_submit_outputs, reason}), + do: "submit outputs were not accepted: #{error_prose(reason)}" + + defp error_prose({:invalid_submit_arguments, _arguments}), do: "submit needs a map of outputs" + defp error_prose(reason) when is_binary(reason), do: reason defp error_prose(reason) when is_atom(reason), do: reason |> Atom.to_string() |> String.replace("_", " ") defp error_prose(reason) when is_exception(reason), do: Exception.message(reason) + + # Adapters and tools carry structured failures as a map keyed on :reason. The + # reason is the sentence; a :limit is the number the reader needs with it. + defp error_prose(reason) when is_map(reason) and not is_struct(reason) do + case fetch_either(reason, :reason) do + {:ok, value} -> + case fetch_either(reason, :limit) do + {:ok, limit} -> "#{error_prose(value)} (limit #{format_value(limit)})" + :error -> error_prose(value) + end + + :error -> + inspect(reason, limit: 20) + end + end + defp error_prose(reason), do: inspect(reason, limit: 20) + defp fetch_either(map, key) do + case Map.fetch(map, key) do + {:ok, value} -> {:ok, value} + :error -> Map.fetch(map, Atom.to_string(key)) + end + end + # Scalars take Python's `str(...)` spelling: `None`, `True`, `False`, where # Elixir's `to_string/1` would give "", "true" and "false". Public as an # internal cross-adapter seam, so the other adapters format scalars @@ -947,7 +995,9 @@ defmodule Imp.Adapter.Chat do ) end - defp extract_history(signature, inputs, renderer, input_renderer) do + defp default_tool_result_renderer(result, _call), do: format_tool_result(result) + + defp extract_history(signature, inputs, renderer, input_renderer, tool_result_renderer) do signature.inputs |> Enum.reduce({[], MapSet.new()}, fn field, {messages, fields} -> case fetch_field(inputs, field.name) do @@ -957,7 +1007,8 @@ defmodule Imp.Adapter.Chat do signature, Imp.History.messages(history), renderer, - input_renderer + input_renderer, + tool_result_renderer ), MapSet.put(fields, field.name)} _other -> @@ -966,13 +1017,13 @@ defmodule Imp.Adapter.Chat do end) end - defp render_history_turns(signature, turns, renderer, input_renderer) do + defp render_history_turns(signature, turns, renderer, input_renderer, tool_result_renderer) do turns |> Enum.flat_map(fn turn -> turn = Imp.Example.new(turn) |> Imp.Example.to_map() if native_tool_history_turn?(turn) do - render_native_tool_history_turn(signature, turn) + render_native_tool_history_turn(signature, turn, tool_result_renderer) else [ %{ @@ -996,7 +1047,7 @@ defmodule Imp.Adapter.Chat do defp native_tool_history_turn?(turn), do: not is_nil(fetch_field(turn, :tool_calls)) - defp render_native_tool_history_turn(signature, turn) do + defp render_native_tool_history_turn(signature, turn, tool_result_renderer) do calls = normalize_history_tool_calls(fetch_field(turn, :tool_calls)) results = List.wrap(fetch_field(turn, :tool_call_results)) @@ -1014,10 +1065,11 @@ defmodule Imp.Adapter.Chat do tool_messages = Enum.map(results, fn result -> id = fetch_field(result, :id) + call = %{id: id, name: result |> fetch_field(:name) |> blank_to_empty()} %{ role: :tool, - content: result |> fetch_field(:result) |> format_tool_result(), + content: tool_result_renderer.(fetch_field(result, :result), call), tool_calls: [%{id: id}] } end) diff --git a/lib/imp/predict/react_v2.ex b/lib/imp/predict/react_v2.ex index a552d2b1..a9cd2462 100644 --- a/lib/imp/predict/react_v2.ex +++ b/lib/imp/predict/react_v2.ex @@ -4,7 +4,10 @@ defmodule Imp.Predict.ReActV2 do ReActV2 preserves parallel tool call IDs and results in `Imp.History`, keeps unknown and failed tool calls as observations, and forces a final `submit` - call when the loop ends without outputs. If a provider cannot + call when the loop ends without outputs. That forced request says nothing + about why by default; `:forced_submit_notice`, a string or a 1-arity function + of the termination reason, adds one user-visible turn saying so, which is kept + in the returned history like any other turn. If a provider cannot honor that tool contract, a tools-disabled typed extractor derives the task outputs from the original inputs and accumulated history. @@ -24,7 +27,14 @@ defmodule Imp.Predict.ReActV2 do @malformed_tool_call "__imp_malformed_tool_call__" - defstruct [:signature, :react, tools: %{}, max_iters: 20, tool_policy: :allow] + defstruct [ + :signature, + :react, + :forced_submit_notice, + tools: %{}, + max_iters: 20, + tool_policy: :allow + ] @type t :: %__MODULE__{} @@ -38,7 +48,11 @@ defmodule Imp.Predict.ReActV2 do adapter_opts: [type: :keyword_list, default: []], metadata: [type: {:map, :any, :any}, default: %{}], max_iters: [type: :non_neg_integer, default: 20], - tool_policy: [type: {:custom, Imp.ToolPolicy, :validate, []}, default: :allow] + tool_policy: [type: {:custom, Imp.ToolPolicy, :validate, []}, default: :allow], + # What to tell the model when the loop makes it submit. A 1-arity function + # of the termination reason, or a plain string; nil says nothing, which is + # what the loop did before this option existed. + forced_submit_notice: [type: {:or, [{:fun, 1}, :string, nil]}, default: nil] ] def new(signature, tools, opts \\ []) do @@ -88,7 +102,8 @@ defmodule Imp.Predict.ReActV2 do ), tools: tools, max_iters: opts[:max_iters], - tool_policy: opts[:tool_policy] + tool_policy: opts[:tool_policy], + forced_submit_notice: opts[:forced_submit_notice] } end @@ -222,6 +237,8 @@ defmodule Imp.Predict.ReActV2 do initial_error, execution ) do + history = append_forced_submit_notice(react, history, reason) + case forced_submit_prediction(react, history, pending) do {:ok, prediction, history} -> calls = prediction |> Imp.get(:tool_calls, []) |> normalize_calls(turn) @@ -256,6 +273,26 @@ defmodule Imp.Predict.ReActV2 do end end + # The notice is what the model is told, so it goes into the durable history + # rather than into one request: the record of the run carries it, and the + # prompt renders it as the last user message before the forced request. + defp append_forced_submit_notice(react, history, reason) do + case notice_text(react.forced_submit_notice, reason) do + text when is_binary(text) and text != "" -> + case Imp.Signature.input_names(react.signature) do + [first | _rest] -> append_history(history, %{first => text}) + [] -> history + end + + _none -> + history + end + end + + defp notice_text(nil, _reason), do: nil + defp notice_text(text, _reason) when is_binary(text), do: text + defp notice_text(fun, reason) when is_function(fun, 1), do: fun.(reason) + defp forced_submit_prediction(react, history, pending) do forced = forced_submit_program(react, %{type: "tool", name: "submit"}) diff --git a/lib/imp/run.ex b/lib/imp/run.ex index b83847ec..a60f43b8 100644 --- a/lib/imp/run.ex +++ b/lib/imp/run.ex @@ -25,9 +25,14 @@ defmodule Imp.Run do Capture defaults to 64 KiB per event and a 512-event, 4 MiB snapshot; `:max_event_bytes`, `:max_events` and `:max_snapshot_bytes` override them at - start. An oversized event payload becomes a digest and size marker before sink - delivery, and snapshot eviction adds a `:capture_gap` marker. A sink receives - every bounded event; the snapshot is a bounded recent window. + start. Each takes a positive integer or `:infinity`, which removes that bound + entirely: with `:max_event_bytes` set to `:infinity` an event reaches the sink + and the snapshot whole however large it is, and with `:max_events` and + `:max_snapshot_bytes` set to `:infinity` nothing is ever evicted. A host that + must keep a complete record of a run sets all three. An oversized event + payload becomes a digest and size marker before sink delivery, and snapshot + eviction adds a `:capture_gap` marker. A sink receives every bounded event; + the snapshot is a bounded recent window. """ alias Imp.Run.Control @@ -299,8 +304,8 @@ defmodule Imp.Run.Control do max_snapshot_bytes: Keyword.get(capture, :max_snapshot_bytes, 4_194_304) } - unless Enum.all?(limits, fn {_, n} -> is_integer(n) and n > 0 end), - do: raise(ArgumentError, "run capture limits must be positive integers") + unless Enum.all?(limits, fn {_, n} -> bound?(n) end), + do: raise(ArgumentError, "run capture limits must be positive integers or :infinity") {:ok, delivery} = EventDelivery.start_link(Keyword.fetch!(opts, :event_sink)) @@ -453,6 +458,13 @@ defmodule Imp.Run.Control do |> bound_snapshot() end + defp bound?(:infinity), do: true + defp bound?(n), do: is_integer(n) and n > 0 + + # `:infinity` is the absence of a bound, not a very large one: the event is + # never measured, so a host that wants the whole record pays no digest cost. + defp bound_event(event, :infinity), do: event + defp bound_event(event, max_bytes) do bytes = :erlang.external_size(event) @@ -476,8 +488,8 @@ defmodule Imp.Run.Control do end defp bound_snapshot(state) do - if length(state.events) > state.limits.max_events or - state.snapshot_bytes > state.limits.max_snapshot_bytes do + if over?(length(state.events), state.limits.max_events) or + over?(state.snapshot_bytes, state.limits.max_snapshot_bytes) do {last, events} = List.pop_at(state.events, -1) bound_snapshot(%{ @@ -491,6 +503,9 @@ defmodule Imp.Run.Control do end end + defp over?(_measured, :infinity), do: false + defp over?(measured, limit), do: measured > limit + defp safe_cancel(fun, reason) do _ = fun.(reason) :ok diff --git a/priv/public_api.json b/priv/public_api.json index 4cd98b42..52079151 100644 --- a/priv/public_api.json +++ b/priv/public_api.json @@ -7370,6 +7370,7 @@ }, "source": "lib/imp/predict/react_v2.ex", "struct_fields": [ + "forced_submit_notice", "max_iters", "react", "signature", diff --git a/test/adapter_chat_format_value_test.exs b/test/adapter_chat_format_value_test.exs index a936d326..09d6a8ba 100644 --- a/test/adapter_chat_format_value_test.exs +++ b/test/adapter_chat_format_value_test.exs @@ -30,4 +30,49 @@ defmodule Imp.Adapter.ChatFormatValueTest do refute rendered =~ "..." assert Chat.format_tool_result({:error, :nope}) == "Error: nope" end + + # A model reads the tool result. A failed submit told it `{:missing_output_fields, + # [:answer]}`, which is a term, not an instruction it can act on. + test "a failed submit says in words what the model has to do differently" do + assert Chat.format_tool_result({:error, {:missing_output_fields, [:answer, :note]}}) == + "Error: submit is missing: answer, note" + + assert Chat.format_tool_result({:error, {:missing_output_fields, ["answer"]}}) == + "Error: submit is missing: answer" + + assert Chat.format_tool_result({:error, {:invalid_submit_outputs, :answer_is_not_a_number}}) == + "Error: submit outputs were not accepted: answer is not a number" + + assert Chat.format_tool_result({:error, {:invalid_submit_arguments, ["answer"]}}) == + "Error: submit needs a map of outputs" + + assert Chat.format_tool_result({:error, {:invalid_submit_arguments, nil}}) == + "Error: submit needs a map of outputs" + end + + test "a map reason renders its reason, and its limit, as a sentence" do + assert Chat.format_tool_result({:error, %{reason: :context_window_exceeded}}) == + "Error: context window exceeded" + + assert Chat.format_tool_result({:error, %{reason: :too_many_results, limit: 20}}) == + "Error: too many results (limit 20)" + + assert Chat.format_tool_result({:error, %{"reason" => "the file was not found"}}) == + "Error: the file was not found" + + assert Chat.format_tool_result({:error, %{"reason" => "too long", "limit" => 4}}) == + "Error: too long (limit 4)" + end + + test "a map with no reason key still falls back to a term" do + assert Chat.format_tool_result({:error, %{status: 500}}) == "Error: %{status: 500}" + end + + test "the existing prose clauses are unchanged" do + assert Chat.format_tool_result({:error, {:tool_error, :read, "no such file"}}) == + "Error: read failed: no such file" + + assert Chat.format_tool_result({:error, {:tool_authorization_denied, :post, :client_denied}}) == + "Error: post was not allowed; the person declined it." + end end diff --git a/test/adapter_chat_tool_result_renderer_test.exs b/test/adapter_chat_tool_result_renderer_test.exs new file mode 100644 index 00000000..da3d6e56 --- /dev/null +++ b/test/adapter_chat_tool_result_renderer_test.exs @@ -0,0 +1,104 @@ +defmodule Imp.Adapter.ChatToolResultRendererTest do + use ExUnit.Case, async: true + + alias Imp.Adapter.Chat + + # A host that must bound what the model reads of a tool result used to wrap + # the tool's run function, which cut the value the loop recorded. Bounding + # belongs in rendering: the loop keeps the whole result in history and + # events, and the adapter renders a bounded view into the prompt. + + defp big, do: String.duplicate("x", 10_000) + + defp recording_lm(owner) do + counter = :counters.new(1, []) + + Imp.LM.Static.new( + handler: fn messages, opts -> + n = :counters.get(counter, 1) + 1 + :counters.put(counter, 1, n) + send(owner, {:request, n, messages, opts}) + + if n == 1, + do: %{ + next_thought: "look", + tool_calls: [%{id: "c1", name: "look", arguments: %{}}] + }, + else: %{tool_calls: [%{id: "s", name: "submit", arguments: %{answer: "ok"}}]} + end + ) + end + + defp request(i), do: receive(do: ({:request, ^i, messages, _opts} -> messages)) + + defp tool_contents(messages), + do: messages |> Enum.filter(&(&1[:role] == :tool)) |> Enum.map(& &1[:content]) + + test "the renderer bounds what the prompt carries while history keeps the whole result" do + owner = self() + look = Imp.tool(:look, "Look", fn _ -> big() end) + + program = + Imp.react_v2("intent -> answer", [look], + lm: recording_lm(owner), + adapter_opts: [ + tool_result_renderer: fn result, call -> + "#{call.name}:#{call.id}:#{String.length(to_string(result))}" + end + ] + ) + + assert {:ok, prediction} = Imp.call(program, %{intent: "hello"}) + assert tool_contents(request(2)) == ["look:c1:10000"] + + [turn | _] = Imp.History.messages(Imp.get(prediction, :history)) + assert [%{result: recorded}] = turn.tool_call_results + assert recorded == big() + end + + test "the default renderer is today's prose" do + owner = self() + look = Imp.tool(:look, "Look", fn _ -> "seen" end) + program = Imp.react_v2("intent -> answer", [look], lm: recording_lm(owner)) + + assert {:ok, _} = Imp.call(program, %{intent: "hello"}) + assert tool_contents(request(2)) == ["seen"] + end + + test "errors go through the renderer too, so a host decides how they read" do + history = + Imp.History.new([ + %{ + intent: "hello", + next_thought: "look", + tool_calls: %{tool_calls: [%{id: "c1", name: "look", arguments: %{}}]}, + tool_call_results: [%{id: "c1", name: "look", result: {:error, :nope}}] + } + ]) + + signature = %Imp.Signature{ + inputs: [ + Imp.Signature.Field.new(:intent, :input), + Imp.Signature.Field.new(%{name: :history, type: :history}, :input) + ], + outputs: [Imp.Signature.Field.new(:answer, :output)] + } + + messages = + Chat.format(signature, %{intent: "hello", history: history}, + tool_result_renderer: fn {:error, reason}, call -> + "#{call.name} said #{inspect(reason)}" + end + ) + + assert Enum.any?(messages, &(&1[:role] == :tool and &1[:content] == "look said :nope")) + end + + test "the option must be a 2-arity function" do + signature = Imp.Signature.ensure("intent -> answer") + + assert_raise ArgumentError, ~r/tool_result_renderer/, fn -> + Chat.format(signature, %{intent: "hello"}, tool_result_renderer: fn r -> r end) + end + end +end diff --git a/test/react_v2_forced_submit_notice_test.exs b/test/react_v2_forced_submit_notice_test.exs new file mode 100644 index 00000000..bcd915ff --- /dev/null +++ b/test/react_v2_forced_submit_notice_test.exs @@ -0,0 +1,125 @@ +defmodule ReActV2ForcedSubmitNoticeTest do + use ExUnit.Case, async: true + + # The forced submit re-asks the model with `tool_choice: submit` and says + # nothing about why. A host that wants the model told why it is being made to + # finish sets `:forced_submit_notice`; the notice is a user message in the + # forced request and stays in the returned history, because the record of the + # run has to contain what the model was told. + + defp look, do: Imp.tool(:look, "Look at a thing", fn _ -> %{"seen" => true} end) + + defp recording_lm(owner) do + counter = :counters.new(1, []) + + Imp.LM.Static.new( + handler: fn messages, opts -> + n = :counters.get(counter, 1) + 1 + :counters.put(counter, 1, n) + send(owner, {:request, n, messages, opts}) + + if forced?(opts) do + %{tool_calls: [%{id: "s", name: "submit", arguments: %{answer: "ok"}}]} + else + %{ + next_thought: "look first", + tool_calls: [%{id: "c", name: "look", arguments: %{}}] + } + end + end + ) + end + + defp forced?(opts), do: opts[:tool_choice] not in [nil, "auto"] + + defp requests(n), do: for(i <- 1..n, do: receive(do: ({:request, ^i, m, o} -> {m, o}))) + + defp user_contents(messages), + do: messages |> Enum.filter(&(&1[:role] == :user)) |> Enum.map(& &1[:content]) + + test "the notice is the last user message of the forced request and reaches the history" do + owner = self() + notice = "You have used every turn. Submit the answer you have now." + + program = + Imp.react_v2("intent -> answer", [look()], + lm: recording_lm(owner), + max_iters: 1, + forced_submit_notice: fn reason -> + send(owner, {:notice_asked, reason}) + notice + end + ) + + assert {:ok, prediction} = Imp.call(program, %{intent: "hello"}) + assert Imp.get(prediction, :answer) == "ok" + assert_receive {:notice_asked, :max_iters} + + [{_first, _}, {forced, forced_opts}] = requests(2) + assert forced_opts[:tool_choice] == %{type: "tool", name: "submit"} + + assert List.last(user_contents(forced)) =~ notice + + history = Imp.get(prediction, :history) + assert Enum.any?(Imp.History.messages(history), &(Map.get(&1, :intent) == notice)) + end + + test "the notice is a plain string too, and an empty trailing request is still omitted" do + owner = self() + + program = + Imp.react_v2("intent -> answer", [look()], + lm: recording_lm(owner), + max_iters: 1, + forced_submit_notice: "Submit now." + ) + + assert {:ok, _prediction} = Imp.call(program, %{intent: "hello"}) + [_first, {forced, _}] = requests(2) + + assert List.last(user_contents(forced)) =~ "Submit now." + # omit_empty_request: the forced request carries no new pending inputs, so + # there is no blank user message after the notice turn. + refute Enum.any?(forced, &(&1[:role] == :user and String.trim(&1[:content] || "") == "")) + end + + test "no notice leaves the forced request exactly as it was" do + owner = self() + + program = + Imp.react_v2("intent -> answer", [look()], lm: recording_lm(owner), max_iters: 1) + + assert {:ok, prediction} = Imp.call(program, %{intent: "hello"}) + [{first, _}, {forced, _}] = requests(2) + + # Only the first step's exchange separates the two requests: user inputs, + # assistant tool call, tool result. Nothing was added on the model's behalf. + assert length(forced) == length(first) + 2 + assert Enum.count(Imp.History.messages(Imp.get(prediction, :history))) == 2 + end + + test "a notice function returning nil is a no-op" do + owner = self() + + program = + Imp.react_v2("intent -> answer", [look()], + lm: recording_lm(owner), + max_iters: 1, + forced_submit_notice: fn _reason -> nil end + ) + + assert {:ok, _} = Imp.call(program, %{intent: "hello"}) + [{first, _}, {forced, _}] = requests(2) + assert length(forced) == length(first) + 2 + end + + test "the option rejects anything that is not a string or a 1-arity function" do + assert_raise ArgumentError, ~r/forced_submit_notice/, fn -> + Imp.react_v2("intent -> answer", [look()], forced_submit_notice: fn -> "no" end) + end + + assert_raise ArgumentError, ~r/forced_submit_notice/, fn -> + Imp.react_v2("intent -> answer", [look()], forced_submit_notice: 7) + end + end +end diff --git a/test/run_observation_test.exs b/test/run_observation_test.exs index ea90994b..d8725159 100644 --- a/test/run_observation_test.exs +++ b/test/run_observation_test.exs @@ -82,6 +82,61 @@ defmodule Imp.RunObservationTest do assert terminal.kind == :run_cancelled end + test "infinity capture keeps a megabyte tool result whole, in the sink and the snapshot" do + owner = self() + + {:ok, run} = + Imp.Run.start(%Wait{}, %{owner: owner}, + event_sink: fn event -> send(owner, {:sunk, event}) end, + max_events: :infinity, + max_event_bytes: :infinity, + max_snapshot_bytes: :infinity + ) + + assert_receive :waiting + payload = String.duplicate("lorem ipsum ", 90_000) + assert byte_size(payload) > 1_000_000 + + Imp.Run.with_context(run.control, fn -> + Imp.Run.emit(:tool_result, output: payload) + end) + + assert_receive {:sunk, %{kind: :tool_result} = sunk}, 5_000 + assert sunk.output == payload + refute Map.has_key?(sunk.metadata, :capture) + + assert [_started, retained] = Imp.Run.events(run) + assert retained.output == payload + Imp.Run.cancel(run) + end + + test "an infinite event bound still truncates when a byte bound is set" do + {:ok, run} = + Imp.Run.start(%Wait{}, %{owner: self()}, max_events: :infinity, max_event_bytes: 1000) + + assert_receive :waiting + + Imp.Run.with_context(run.control, fn -> + Imp.Run.emit(:tool_result, output: String.duplicate("large", 1000)) + end) + + assert [_started, large] = Imp.Run.events(run) + assert large.output == nil + assert large.metadata.capture.truncated + Imp.Run.cancel(run) + end + + test "capture limits still reject anything that is neither a positive integer nor infinity" do + ExUnit.CaptureLog.capture_log(fn -> + for bad <- [[max_events: 0], [max_event_bytes: :unbounded], [max_snapshot_bytes: -1]] do + assert {:error, {%ArgumentError{} = error, _stacktrace}} = + Imp.Run.start(%Wait{}, %{owner: self()}, bad) + + assert Exception.message(error) =~ "positive integers" + end + end) + end + test "task death is recorded once by control even when the task cannot emit" do {:ok, run} = Imp.Run.start(%Wait{}, %{owner: self()}) assert_receive :waiting diff --git a/test/task_supervision_test.exs b/test/task_supervision_test.exs index 6aed06f6..3136a93d 100644 --- a/test/task_supervision_test.exs +++ b/test/task_supervision_test.exs @@ -3,6 +3,16 @@ defmodule TaskSupervisionTest do @moduletag :capture_log + defmodule NeverReturns do + @behaviour Imp.Module + defstruct [:signature] + + def call(_program, %{owner: owner}) do + send(owner, :waiting) + Process.sleep(:infinity) + end + end + setup do Application.ensure_all_started(:imp) Imp.Settings.reset() @@ -334,6 +344,23 @@ defmodule TaskSupervisionTest do assert Imp.get(first, :answer) != Imp.get(second, :answer) end + # Imp.Run.start/3 admits its task. cancel/3 ends the run and releases that + # lease; stop/1 is documented only for a run that has already completed and + # releases the control process alone, so stopping a still-running task leaves + # its lease held for the life of the node. + test "cancelling a run releases the admission lease it took" do + assert wait_for_status(%{active: 0, queued: 0}) + + {:ok, run} = Imp.Run.start(%NeverReturns{}, %{owner: self()}) + assert_receive :waiting + assert wait_for_active(1) + + :ok = Imp.Run.cancel(run) + + assert wait_for_status(%{active: 0, queued: 0}) + refute Process.alive?(run.task.pid) + end + defp wait_for_active(expected, attempts \\ 100) defp wait_for_active(expected, attempts) when attempts > 0 do