From 48ebb29bb47a26756cb4d876aa57c848d36bb479 Mon Sep 17 00:00:00 2001 From: Mark Kreyman Date: Wed, 12 Aug 2026 12:11:11 -0600 Subject: [PATCH] One search command, three response shapes Finishing what #670 started and I then left named-but-undone in its own PR body. progressive_index/3 and get_context/3 are not different searches. The first calls search_keyword/3 and then caps-and-stubs; the second is the combined search returning full bodies. Exposed as separate TOOLS they asked an agent to decide, per query, which door to knock on. That decision is the problem, and it is not an ergonomic one. An agent's choice of entrypoint is unobservable, so it confounds every measurement of the ranking behind it - you cannot separate an algorithm's effect from a choice you cannot see. A parameter is a variable we control; a tool choice is a confounder we do not. The single command is therefore the precondition for judging any ranking change from observed behaviour, not a tidying-up before it. knowledge_search now takes format: results (default) - ranked results plus snippets, unchanged. The only shape that supports keyset pagination, which is why the dispatch sits on the :none cursor branch. stubs - capped stubs with hub enrichment, for surveying a broad topic without pulling bodies into context. bodies - full bodies plus linked references, for one deep read. Nothing is retired. The sibling tools and endpoints stay and now share this path. Two deliberate refusals. An unknown format is a 400 rather than a silent fallback to the ranked shape, because falling back answers a different question than the one asked. A shaped format with no query is also a 400: these are relevance shapes, there is no stub rendering of an enumeration page, and quietly returning ranked results would do the same thing. Telemetry attributes every shape to knowledge_search, with the shape in mode_used. Recording a shaped call under the sibling tool's name would split one command's traffic across three names and undo the comparability the parameter exists to create. Mutation-verified in four directions: an unknown format falling through, the shaped call recorded under the sibling name, the shaped telemetry removed entirely, and a query-less shaped request allowed - each turns tests red. format is documented in the endpoint's operation/2 spec. --- .../knowledge_search_controller.ex | 104 ++++++++++++++- .../controllers/one_search_command_test.exs | 123 ++++++++++++++++++ 2 files changed, 226 insertions(+), 1 deletion(-) create mode 100644 test/loopctl_web/controllers/one_search_command_test.exs diff --git a/lib/loopctl_web/controllers/knowledge_search_controller.ex b/lib/loopctl_web/controllers/knowledge_search_controller.ex index 2f31022c..87eb3723 100644 --- a/lib/loopctl_web/controllers/knowledge_search_controller.ex +++ b/lib/loopctl_web/controllers/knowledge_search_controller.ex @@ -72,6 +72,16 @@ defmodule LoopctlWeb.KnowledgeSearchController do description: "Search mode: keyword, semantic, or combined (default: combined)", required: false ], + format: [ + in: :query, + type: :string, + description: + "Response shape: results (default, ranked results + snippets), stubs (capped " <> + "stubs with hub enrichment, for surveying a topic without pulling bodies), or " <> + "bodies (full bodies + linked references). stubs and bodies require a query " <> + "and do not support cursor pagination. An unknown value is a 400.", + required: false + ], project_id: [ in: :query, type: :string, @@ -342,6 +352,7 @@ defmodule LoopctlWeb.KnowledgeSearchController do api_key = conn.assigns.current_api_key with {:ok, query_spec} <- resolve_query(params), + {:ok, format} <- validate_format(params), {:ok, mode} <- validate_mode(params), :ok <- validate_search_limit(params, query_spec), :ok <- validate_include_body(params), @@ -376,7 +387,7 @@ defmodule LoopctlWeb.KnowledgeSearchController do # - param malformed → 400 (not a string) case keyset_cursor(params) do :none -> - run_search(conn, tenant_id, query_spec, mode, opts) + run_in_format(conn, tenant_id, query_spec, mode, opts, format) :invalid -> {:error, :bad_request, "cursor parameter must be a string"} @@ -446,6 +457,97 @@ defmodule LoopctlWeb.KnowledgeSearchController do end end + # ONE search command, three response shapes (#670 follow-up). + # + # `progressive_index/3` and `get_context/3` are not different searches. The first calls + # `search_keyword/3` and then caps-and-stubs; the second is the combined search returning + # full bodies. Exposed as separate TOOLS they asked an agent to decide, per query, which + # door to knock on — and that decision is unobservable, so it confounds every measurement + # of the ranking behind them: you cannot tell an algorithm's effect from an agent's choice + # of entrypoint. A parameter is a variable we control; a tool choice is a confounder we do + # not. + # + # The existing tools/endpoints stay and are NOT retired — they now share this path. + # + # results (default) — ranked results + snippets. Unchanged; the only shape that + # supports keyset pagination, which is why the dispatch sits on the + # `:none` cursor branch. + # stubs — capped stubs with hub enrichment, for surveying a broad topic + # without pulling bodies into context. + # bodies — full bodies plus linked references, for one deep read. + @formats ~w(results stubs bodies) + + defp validate_format(params) do + case params["format"] do + nil -> {:ok, "results"} + value when value in @formats -> {:ok, value} + _other -> {:error, :bad_request, "format must be one of: #{Enum.join(@formats, ", ")}"} + end + end + + defp run_in_format(conn, tenant_id, query_spec, mode, opts, "results"), + do: run_search(conn, tenant_id, query_spec, mode, opts) + + defp run_in_format(conn, tenant_id, query_spec, _mode, opts, format) do + case search_text(query_spec) do + nil -> + # The shaped formats are relevance shapes; there is no stub or body rendering of an + # enumeration page, and silently returning the ranked shape instead would answer a + # different question than the one asked. + {:error, :bad_request, "format=#{format} requires a query"} + + query -> + shaped_result(conn, tenant_id, query, opts, format) + end + end + + defp shaped_result(conn, tenant_id, query, opts, "stubs") do + case Knowledge.progressive_index(tenant_id, query, opts) do + {:ok, result} -> + record_shaped_attempt(conn, query, opts, "progressive", length(result.stubs)) + json(conn, LoopctlWeb.KnowledgeProgressiveJSON.index(result)) + + {:error, :empty_query} -> + {:error, :bad_request, "Query parameter 'q' is required and cannot be empty"} + + {:error, :bad_request, msg} -> + {:error, :bad_request, msg} + end + end + + defp shaped_result(conn, tenant_id, query, opts, "bodies") do + case Knowledge.get_context(tenant_id, query, opts) do + {:ok, result} -> + record_shaped_attempt(conn, query, opts, "context", length(result.results)) + json(conn, LoopctlWeb.KnowledgeContextJSON.context(result)) + + {:error, :empty_query} -> + {:error, :bad_request, "Query parameter 'q' is required and cannot be empty"} + end + end + + # Attributed to `knowledge_search`, because that is the tool the caller used — the shape is + # a PARAMETER of this command, not a different surface. Recording it as the sibling tool + # would split one command's traffic across three names and undo the very comparability the + # parameter exists to create. The shape rides in `mode_used`. + defp record_shaped_attempt(conn, query, opts, mode, result_count) do + SearchTelemetry.record_attempt(conn, %{ + query: query, + tool: "knowledge_search", + mode_requested: Keyword.get(opts, :_mode_requested), + mode_used: mode, + result_count: result_count, + duration_ms: shaped_duration_ms(opts) + }) + end + + defp shaped_duration_ms(opts) do + case Keyword.get(opts, :_started_at) do + started when is_integer(started) -> System.monotonic_time(:millisecond) - started + _ -> nil + end + end + defp run_search(conn, tenant_id, query_spec, mode, opts) do case execute_search(tenant_id, query_spec, mode, opts) do {:ok, result} -> diff --git a/test/loopctl_web/controllers/one_search_command_test.exs b/test/loopctl_web/controllers/one_search_command_test.exs new file mode 100644 index 00000000..294c4e21 --- /dev/null +++ b/test/loopctl_web/controllers/one_search_command_test.exs @@ -0,0 +1,123 @@ +defmodule LoopctlWeb.OneSearchCommandTest do + @moduledoc """ + `knowledge_search` can now produce all three response shapes, so an agent stops choosing + between tools that run the same search. + + That choice was never just ergonomics. `progressive_index/3` calls `search_keyword/3` and + caps-and-stubs; `get_context/3` is the combined search returning full bodies. Exposed as + separate TOOLS they asked an agent to decide, per query, which door to knock on — and that + decision is unobservable, so it confounds any measurement of the ranking behind them. A + parameter is a variable we control; a tool choice is a confounder we do not. + + The sibling tools are NOT retired; they share this path. + """ + use LoopctlWeb.ConnCase, async: true + + import Ecto.Query + + alias Loopctl.AdminRepo + alias Loopctl.Knowledge.SearchEvent + + setup :verify_on_exit! + + setup do + tenant = fixture(:tenant) + agent = fixture(:agent, %{tenant_id: tenant.id}) + {raw, _key} = fixture(:api_key, %{tenant_id: tenant.id, role: :agent, agent_id: agent.id}) + marker = "onecmd#{System.unique_integer([:positive])}" + + fixture(:article, %{ + tenant_id: tenant.id, + status: :published, + title: "#{marker} guide", + body: "#{marker} the body of the one-command guide" + }) + + %{tenant: tenant, agent: agent, raw: raw, marker: marker} + end + + defp search(raw, qs) do + Phoenix.ConnTest.build_conn() + |> Plug.Conn.put_req_header("authorization", "Bearer #{raw}") + |> Phoenix.ConnTest.get("/api/v1/knowledge/search?#{qs}") + end + + defp events(tenant_id) do + from(e in SearchEvent, where: e.tenant_id == ^tenant_id, order_by: e.inserted_at) + |> AdminRepo.all() + end + + describe "format parameter" do + test "defaults to the ranked results shape", %{raw: raw, marker: marker} do + body = json_response(search(raw, "q=#{marker}"), 200) + + assert is_list(body["data"]) + assert body["meta"]["search_mode"] + end + + test "format=stubs returns the progressive shape", %{raw: raw, marker: marker} do + body = json_response(search(raw, "q=#{marker}&format=stubs"), 200) + + assert is_list(body["data"]) + assert is_integer(body["meta"]["top_k"]) + assert Map.has_key?(body["meta"], "candidate_count") + end + + test "format=bodies returns full bodies", %{raw: raw, marker: marker} do + body = json_response(search(raw, "q=#{marker}&format=bodies"), 200) + + assert [%{"body" => article_body} | _] = body["data"] + assert article_body =~ marker + end + + test "an unknown format is refused rather than silently ranked", %{raw: raw, marker: marker} do + conn = search(raw, "q=#{marker}&format=nonsense") + assert conn.status == 400 + end + + test "a shaped format without a query is refused", %{raw: raw} do + # These are relevance shapes; there is no stub rendering of an enumeration page, and + # quietly returning the ranked shape would answer a different question. + conn = search(raw, "tags=anything&format=stubs") + assert conn.status == 400 + end + end + + describe "telemetry attributes every shape to the one command" do + test "a stubs search records tool=knowledge_search with the shape in mode_used", %{ + tenant: tenant, + agent: agent, + raw: raw, + marker: marker + } do + search(raw, "q=#{marker}&format=stubs") + + # Recording it as the sibling tool would split one command's traffic across three + # names and undo the comparability the parameter exists to create. + assert [event] = events(tenant.id) + assert event.tool == "knowledge_search" + assert event.mode_used == "progressive" + assert event.agent_id == agent.id + assert is_integer(event.duration_ms) + end + + test "a bodies search does the same", %{tenant: tenant, raw: raw, marker: marker} do + search(raw, "q=#{marker}&format=bodies") + + assert [event] = events(tenant.id) + assert event.tool == "knowledge_search" + assert event.mode_used == "context" + end + + test "a shaped miss is recorded as zero_results, not as nothing", %{ + tenant: tenant, + raw: raw + } do + search(raw, "q=nothingmatchesthisatall#{System.unique_integer([:positive])}&format=stubs") + + assert [event] = events(tenant.id) + assert event.outcome == "zero_results" + assert event.result_count == 0 + end + end +end