perf(mcp): stop repeating the payload in structuredContent (#1375)

Every non-JSON tool result shipped its payload twice: content[0].text, and an
identical copy as structuredContent {"text": <entire payload>}. Measured on a
20k-node query_graph, the reply was 2.05x the payload it carried. Half of every
large answer was redundant bytes — half the 10 MiB transport budget, and double
the tokens billed to every LLM caller on every call.

Measured, same query, same fixture:

  before   8,529,990 bytes   content.text 4,154,932   structuredContent.text SAME
  after    4,265,047 bytes   content.text 4,154,932   structuredContent {}

Exactly 50.0% smaller with content.text byte-identical: the payload is fully
delivered, only the second copy is gone. The 10 MiB ceiling now also admits
roughly twice the rows before #1375's limit applies.

Nothing is lost. structuredContent exists to carry STRUCTURE, and a string
re-wrapped in a one-key object has none — a client reading
structuredContent.text learned exactly what content[0].text already told it. The
empty object still satisfies the declared outputSchema, which is
{"type":"object","additionalProperties":true} and never required a text field.

Two cases are deliberately NOT changed:

  * JSON payloads. structuredContent stays the PARSED object. That is the
    spec's structured+serialized pattern rather than waste, and it is what our
    own hook-augment consumes (structuredContent.projects from list_projects).
  * Errors. structuredContent.error is kept: bounded, small, and the only
    machine-readable form of a failure a client gets.

Guarded at both levels, because the defect was invisible per-tool — each result
looked reasonable alone, and only measuring the wire showed half of it was
redundant:

  * tests/test_mcp.c enumerates the TOOL TABLE itself, so a new tool is covered
    the moment it is registered, with no test edit. A guard pinned to
    query_graph would not have caught search_graph, and would not catch whatever
    is added next. It fails if no tool produced a non-JSON payload, so it cannot
    report a green it never earned — which it did on the first attempt, catching
    that {} args make every tool error out and assert nothing.
  * scripts/smoke-test.sh Phase 3z asserts the same property on the SHIPPED
    binary. This is a wire-format contract: what a real client receives from the
    real artifact is not something a from-source test can prove.

Both revert-checked: restoring the duplication reddens the unit guard at the
strcmp, the plain-text guard, the search_graph expectation, and the smoke phase
(which names search_graph and get_architecture).

tests/test_mcp.c expectations that pinned the old duplicating shape are updated
rather than relaxed — the format changed on purpose.

Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>

scripts/smoke-test.sh

@@ -390,6 +390,64 @@ if [ "$TOTAL" -lt 1 ]; then
   echo "FAIL: search_graph for 'compute' returned 0 results"
   exit 1
 fi
+
+echo ""
+echo "=== Phase 3z: no tool duplicates its payload into structuredContent (#1375) ==="
+# Asserts on the SHIPPED artifact what the unit suite asserts from source: a
+# non-JSON payload must travel ONCE. It used to appear twice — content[0].text
+# plus an identical structuredContent.text — costing 2.05x the bytes on a large
+# query_graph, i.e. half the 10 MiB transport budget and double the tokens billed
+# to every LLM caller.
+#
+# In smoke as well as the unit suite because this is a WIRE-FORMAT property: it
+# is what a real client actually receives from the real binary, and a from-source
+# test cannot prove the released artifact behaves the same way.
+DUP_TOOLS=0
+DUP_CHECKED=0
+for TOOL_ARGS in "search_graph --project $PROJECT --name-pattern compute" \
+                 "search_code --project $PROJECT --query compute" \
+                 "get_architecture --project $PROJECT" \
+                 "index_status --project $PROJECT"; do
+  # shellcheck disable=SC2086
+  ENVELOPE=$("$BINARY" cli $TOOL_ARGS --json 2>/dev/null || true)
+  [ -z "$ENVELOPE" ] && continue
+  VERDICT=$(printf '%s' "$ENVELOPE" | python3 -c '
+import json,sys
+try:
+    d = json.loads(sys.stdin.read())
+except Exception:
+    print("skip"); raise SystemExit
+if d.get("isError"):
+    print("skip"); raise SystemExit
+content = d.get("content") or []
+text = content[0].get("text", "") if content else ""
+sc = d.get("structuredContent")
+if not isinstance(sc, dict):
+    print("no-structured"); raise SystemExit
+try:
+    payload_is_object = isinstance(json.loads(text), dict)
+except Exception:
+    payload_is_object = False
+if payload_is_object:
+    print("skip"); raise SystemExit
+print("dup" if sc.get("text") == text and text else "ok")
+')
+  case "$VERDICT" in
+    dup) echo "FAIL: $(echo "$TOOL_ARGS" | cut -d" " -f1) repeats its payload in structuredContent (#1375)"; DUP_TOOLS=$((DUP_TOOLS+1)) ;;
+    ok) DUP_CHECKED=$((DUP_CHECKED+1)) ;;
+    no-structured) echo "FAIL: $(echo "$TOOL_ARGS" | cut -d" " -f1) has no structuredContent object (outputSchema requires one)"; DUP_TOOLS=$((DUP_TOOLS+1)) ;;
+  esac
+done
+if [ "$DUP_TOOLS" -ne 0 ]; then
+  echo "FAIL: $DUP_TOOLS tool(s) duplicate their payload on the shipped binary"
+  exit 1
+fi
+if [ "$DUP_CHECKED" -eq 0 ]; then
+  echo "FAIL: no tool produced a non-JSON payload — this check proved nothing"
+  exit 1
+fi
+echo "OK: $DUP_CHECKED tool(s) deliver their payload exactly once"
+
 echo "OK: search_graph found $TOTAL result(s) for 'compute'"
 
 # 3b: trace_path — verify compute has callers

src/mcp/mcp.c

@@ -302,12 +302,34 @@ char *cbm_mcp_text_result(const char *text, bool is_error) {
         }
     }
     if (!has_structured_content) {
-        /* Every advertised MCP tool has an object outputSchema, so even compact
-         * TOON/plain-text and error results must carry a conforming structured
-         * object. Keep the text Content block above for model visibility and
-         * backwards compatibility. */
+        /* Every advertised MCP tool declares an object outputSchema, so a
+         * conforming structuredContent object is mandatory even for compact
+         * TOON/plain-text and error results. What is NOT mandatory is putting
+         * the whole payload in it a second time.
+         *
+         * It used to. For any result that is not a JSON object — which is every
+         * TOON answer, i.e. the large ones — structuredContent was
+         * {"text": <the entire payload>} sitting beside an identical
+         * content[0].text. Measured at 2.05x the payload on a 20k-node
+         * query_graph, so half of every reply was redundant bytes: half the
+         * usable transport budget, and double the tokens billed to every LLM
+         * caller (#1375).
+         *
+         * Nothing is lost by dropping it. structuredContent exists to carry
+         * STRUCTURE, and a string re-wrapped in a one-key object has none —
+         * a client parsing structuredContent.text learns exactly what
+         * content[0].text already told it. The empty object still satisfies
+         * outputSchema ({"type":"object","additionalProperties":true}), and the
+         * JSON branch above is untouched: when a tool really does return an
+         * object, callers still get it parsed.
+         *
+         * Errors keep their payload. They are bounded and small, the duplication
+         * costs nothing measurable, and structuredContent.error is the only
+         * machine-readable form of a failure a client has. */
         yyjson_mut_val *structured = yyjson_mut_obj(doc);
-        yyjson_mut_obj_add_str(doc, structured, is_error ? "error" : "text", text ? text : "");
+        if (is_error) {
+            yyjson_mut_obj_add_str(doc, structured, "error", text ? text : "");
+        }
         yyjson_mut_obj_add_val(doc, root, "structuredContent", structured);
     }
     yyjson_mut_obj_add_bool(doc, root, "isError", is_error);

tests/test_mcp.c

@@ -1093,10 +1093,21 @@ TEST(mcp_text_result) {
     PASS();
 }
 
-TEST(mcp_text_result_wraps_plain_text_as_structured_content) {
+TEST(mcp_text_result_does_not_duplicate_plain_text_into_structured_content) {
+    /* A non-JSON payload used to be repeated verbatim as
+     * structuredContent {"text": <payload>} beside content[0].text — 2.05x the
+     * payload measured on a 20k-node query_graph, i.e. half the transport budget
+     * and double the tokens for every LLM caller (#1375).
+     *
+     * structuredContent carries STRUCTURE; a string rewrapped in a one-key
+     * object has none, so the empty object is the honest answer and still
+     * satisfies the permissive outputSchema. The payload stays in content. */
     char *json = cbm_mcp_text_result("plain text", false);
     ASSERT_NOT_NULL(json);
-    ASSERT_NOT_NULL(strstr(json, "\"structuredContent\":{\"text\":\"plain text\"}"));
+    ASSERT_NOT_NULL(strstr(json, "\"structuredContent\":{}"));
+    ASSERT_NULL(strstr(json, "\"structuredContent\":{\"text\""));
+    /* The payload is still delivered — exactly once. */
+    ASSERT_NOT_NULL(strstr(json, "\"text\":\"plain text\""));
     ASSERT_NOT_NULL(strstr(json, "\"isError\":false"));
     free(json);
     PASS();
@@ -1813,6 +1824,84 @@ TEST(tool_get_code_snippet_clips_whole_file_node) {
     PASS();
 }
 
+/* EVERY tool, not just the one that was reported.
+ *
+ * The duplication was invisible per-tool: each result looked reasonable on its
+ * own, and only measuring the wire showed half of it was redundant. A guard
+ * pinned to query_graph would not have caught it in search_graph, and would not
+ * catch it in whatever tool is added next. So this enumerates the tool table
+ * itself — a new tool is covered the moment it is registered, with no test edit.
+ *
+ * The invariant: for a NON-error result whose payload is not a JSON object,
+ * structuredContent must not carry the payload a second time. Errors are exempt
+ * and deliberately so — bounded, small, and structuredContent.error is the only
+ * machine-readable form of a failure a client gets. */
+TEST(mcp_every_tool_result_is_duplication_free) {
+    char tmp[256];
+    cbm_mcp_server_t *srv = setup_snippet_server(tmp, sizeof(tmp));
+    ASSERT_NOT_NULL(srv);
+
+    int tools = cbm_mcp_tool_count();
+    ASSERT_TRUE(tools > 0); /* an empty table would assert nothing at all */
+    int checked = 0;
+
+    for (int i = 0; i < tools; i++) {
+        const char *name = cbm_mcp_tool_name(i);
+        ASSERT_NOT_NULL(name);
+        /* Minimal args: most tools error out, which is fine — an error envelope
+         * is still an envelope, and the property must hold for it too. */
+        char *envelope = cbm_mcp_handle_tool(srv, name, "{\"project\":\"test-project\"}");
+        if (!envelope) {
+            continue;
+        }
+        yyjson_doc *doc = yyjson_read(envelope, strlen(envelope), 0);
+        ASSERT_NOT_NULL(doc);
+        yyjson_val *root = yyjson_doc_get_root(doc);
+        yyjson_val *content = yyjson_obj_get(root, "content");
+        yyjson_val *first = content ? yyjson_arr_get(content, 0) : NULL;
+        yyjson_val *text_val = first ? yyjson_obj_get(first, "text") : NULL;
+        const char *text = text_val ? yyjson_get_str(text_val) : NULL;
+        yyjson_val *structured = yyjson_obj_get(root, "structuredContent");
+
+        /* outputSchema is declared for every tool, so this stays mandatory. */
+        ASSERT_NOT_NULL(structured);
+        ASSERT_TRUE(yyjson_is_obj(structured));
+
+        yyjson_val *is_error = yyjson_obj_get(root, "isError");
+        bool errored = is_error && yyjson_is_true(is_error);
+
+        if (text && text[0] && !errored) {
+            /* If the payload is itself a JSON object, structuredContent is the
+             * PARSED form and legitimately holds the same data — that is the
+             * spec's structured+serialized pattern, not waste. Only the
+             * non-object case is checked here. */
+            yyjson_doc *as_json = yyjson_read(text, strlen(text), 0);
+            bool payload_is_object = as_json && yyjson_is_obj(yyjson_doc_get_root(as_json));
+            if (as_json) {
+                yyjson_doc_free(as_json);
+            }
+            if (!payload_is_object) {
+                yyjson_val *dup = yyjson_obj_get(structured, "text");
+                if (dup && yyjson_is_str(dup)) {
+                    const char *dup_str = yyjson_get_str(dup);
+                    /* The exact defect: same bytes, twice, in one reply. */
+                    ASSERT_TRUE(!(dup_str && strcmp(dup_str, text) == 0));
+                }
+                checked++;
+            }
+        }
+        yyjson_doc_free(doc);
+        free(envelope);
+    }
+
+    /* If no tool produced a non-JSON payload, this test proved nothing — fail
+     * rather than report a green that was never exercised. */
+    ASSERT_TRUE(checked > 0);
+    cbm_mcp_server_free(srv);
+    th_rmtree(tmp);
+    PASS();
+}
+
 TEST(tool_search_graph_includes_node_properties) {
     /* Node properties are OPT-IN columns in the default TOON output: the
      * default row is qn/label/file/lines/degrees only, `fields` adds the
@@ -1830,7 +1919,11 @@ TEST(tool_search_graph_includes_node_properties) {
              "\"arguments\":{\"project\":\"test-project\",\"label\":\"Function\","
              "\"name_pattern\":\"HandleRequest\",\"limit\":5}}}");
     ASSERT_NOT_NULL(resp);
-    ASSERT_NOT_NULL(strstr(resp, "\"structuredContent\":{\"text\":"));
+    /* TOON is not a JSON object, so structuredContent stays empty rather than
+     * repeating the whole table a second time (#1375). The payload travels once,
+     * in content. */
+    ASSERT_NOT_NULL(strstr(resp, "\"structuredContent\":{}"));
+    ASSERT_NULL(strstr(resp, "\"structuredContent\":{\"text\":"));
     char *inner = extract_text_content(resp);
     ASSERT_NOT_NULL(inner);
     ASSERT_NOT_NULL(strstr(inner, "results:")); /* TOON table header */
@@ -10358,7 +10451,8 @@ SUITE(mcp) {
     RUN_TEST(mcp_ingest_traces_items_disallow_additional_properties_issue731);
     RUN_TEST(mcp_get_architecture_aspects_schema_enum_pr560);
     RUN_TEST(mcp_text_result);
-    RUN_TEST(mcp_text_result_wraps_plain_text_as_structured_content);
+    RUN_TEST(mcp_text_result_does_not_duplicate_plain_text_into_structured_content);
+    RUN_TEST(mcp_every_tool_result_is_duplication_free);
     RUN_TEST(mcp_cancel_matches_request_id);
     RUN_TEST(mcp_text_result_error);