fix(context): orphaned tool_result HTTP 400 — three-layer defense (ISSUE #171)

libs/context/src/context_pipeline.cpp

@@ -69,8 +69,6 @@ SerializedPayload ContextPipeline::planned_assemble(
   }
   prev_split_ = split;
 
-  auto payload = serializer_.serialize(bound, model, system_prompt);
-
   // Compute tokens_after from final state (post-drop/microcompact/spill)
   opt_stats.tokens_after = 0;
   for (auto& sec : bound.sections) {
@@ -81,25 +79,36 @@ SerializedPayload ContextPipeline::planned_assemble(
   }
   opt_stats.tokens_after += static_cast<int>(system_prompt.size() / 3.5);
 
-  // Hard trim: enforce model_max_tokens as hard ceiling
+  // Hard trim: enforce model_max_tokens as hard ceiling.
+  // Round-aware: deletes whole rounds (user-led) to preserve tool_use/tool_result
+  // pairing. Re-scans round_starts each iteration to avoid index drift.
+  // Runs BEFORE serialize() so the trimmed message list is what gets serialized.
   if (opt_stats.tokens_after > model_max_tokens) {
       auto& msgs = bound.provider_messages;
       int removed = 0;
-      while (opt_stats.tokens_after > model_max_tokens && msgs.size() > 2) {
-          // Skip system messages
-          size_t target = 1;
-          while (target < msgs.size() && msgs[target].role == "system") target++;
-          if (target >= msgs.size()) break;
-
-          opt_stats.tokens_after -= static_cast<int>(msgs[target].content.size() / 3.5);
-          msgs.erase(msgs.begin() + static_cast<long>(target));
-          removed++;
+      while (opt_stats.tokens_after > model_max_tokens) {
+          std::vector<size_t> rs;
+          for (size_t i = 0; i < msgs.size(); i++) {
+              if (msgs[i].role == "user") rs.push_back(i);
+          }
+          if (rs.size() <= 1) break;  // preserve at least one round
+
+          size_t del_end = rs[1];
+          for (size_t i = rs[0]; i < del_end; i++) {
+              opt_stats.tokens_after -= static_cast<int>(msgs[i].content.size() / 3.5);
+          }
+          msgs.erase(msgs.begin() + static_cast<long>(rs[0]),
+                     msgs.begin() + static_cast<long>(del_end));
+          removed += static_cast<int>(del_end - rs[0]);
       }
       stats_.hard_trims += removed;
-      spdlog::warn("ContextPipeline: hard trim removed {} messages to fit budget "
-                   "(tokens_after={}, max={})", removed, opt_stats.tokens_after, model_max_tokens);
+      spdlog::warn("ContextPipeline: hard trim removed {} messages (round-aware) "
+                   "(tokens_after={}, max={})",
+                   removed, opt_stats.tokens_after, model_max_tokens);
   }
 
+  auto payload = serializer_.serialize(bound, model, system_prompt);
+
   // Record feedback for next-turn planning
   ContextFeedback fb{};
   fb.schema_count = schema_count;

libs/context/src/context_serializer.cpp

@@ -1,8 +1,79 @@
 #include <merak/context_serializer.hpp>
 #include <sstream>
+#include <set>
+#include <spdlog/spdlog.h>
 
 namespace merak {
 
+namespace {
+
+// Safety net: drop orphaned tool messages so the serialized payload never
+// violates the tool_use/tool_result pairing invariant.
+// - Pass 1: drop leading tool messages whose tool_call_id has no matching
+//   tool_use in any prior assistant message (orphan tool_result at head).
+// - Pass 2: drop tool_calls from the last assistant message if none of its
+//   ids have a matching tool_result afterwards (orphan tool_use at tail).
+std::vector<Message> sanitize_orphans(std::vector<Message> msgs) {
+    try {
+        std::set<std::string> produced_ids;
+        for (const auto& m : msgs) {
+            if (m.role == "assistant") {
+                for (const auto& tc : m.tool_calls) produced_ids.insert(tc.id);
+            }
+        }
+
+        std::set<std::string> referenced_ids;
+        for (const auto& m : msgs) {
+            if (m.role == "tool" && m.tool_call_id) {
+                referenced_ids.insert(*m.tool_call_id);
+            }
+        }
+
+        // Pass 1: leading orphan tool messages
+        size_t i = 0;
+        while (i < msgs.size() && msgs[i].role == "tool") {
+            const auto& id = msgs[i].tool_call_id;
+            if (!id.has_value() || produced_ids.count(*id) == 0) {
+                spdlog::warn("ContextSerializer: dropping orphan tool_result "
+                             "(tool_use_id={}) at head",
+                             id.value_or("<none>"));
+                msgs.erase(msgs.begin() + static_cast<long>(i));
+            } else {
+                break;
+            }
+        }
+
+        // Pass 2: last assistant's orphan tool_use
+        int last_assistant = -1;
+        for (int k = static_cast<int>(msgs.size()) - 1; k >= 0; k--) {
+            if (msgs[k].role == "assistant") { last_assistant = k; break; }
+        }
+        if (last_assistant >= 0) {
+            auto& last_a = msgs[last_assistant];
+            bool all_orphan = !last_a.tool_calls.empty();
+            for (const auto& tc : last_a.tool_calls) {
+                if (referenced_ids.count(tc.id) > 0) { all_orphan = false; break; }
+            }
+            if (all_orphan) {
+                spdlog::warn("ContextSerializer: dropping {} orphan tool_use at tail",
+                             last_a.tool_calls.size());
+                last_a.tool_calls.clear();
+                if (last_a.content.empty()) {
+                    msgs.erase(msgs.begin() + static_cast<long>(last_assistant));
+                }
+            }
+        }
+
+        return msgs;
+    } catch (const std::exception& e) {
+        spdlog::error("ContextSerializer: sanitize_orphans failed, returning "
+                      "messages unchanged: {}", e.what());
+        return msgs;
+    }
+}
+
+} // anonymous namespace
+
 SerializedPayload ContextSerializer::serialize(
     const BoundContext& ctx, const std::string& model,
     const std::string& system_prompt_full, int max_output_tokens) const {
@@ -24,7 +95,7 @@ SerializedPayload ContextSerializer::serialize(
   }
   payload.system_text = system_text;
 
-  payload.messages = ctx.provider_messages;
+  payload.messages = sanitize_orphans(ctx.provider_messages);
   payload.tool_schemas = ctx.tool_schemas;
 
   // ── OpenAI format ──────────────────────────────────────────────

libs/context/tests/test_pipeline_hard_trim.cpp

@@ -0,0 +1,160 @@
+#include <merak/context_pipeline.hpp>
+#include <merak/message.hpp>
+#include <merak/tool_spec.hpp>
+#include <cassert>
+#include <iostream>
+#include <string>
+
+using namespace merak;
+
+static Message make_user(const std::string& text) {
+    Message m; m.role = "user"; m.content = text; return m;
+}
+static Message make_assistant_text(const std::string& text) {
+    Message m; m.role = "assistant"; m.content = text; return m;
+}
+static Message make_assistant_with_tool(const std::string& text, const std::string& call_id) {
+    Message m; m.role = "assistant"; m.content = text;
+    ToolCall tc; tc.id = call_id; tc.name = "read_file"; tc.arguments = "{}";
+    m.tool_calls.push_back(tc);
+    return m;
+}
+static Message make_tool_result(const std::string& call_id, const std::string& output) {
+    Message m; m.role = "tool"; m.content = output; m.tool_call_id = call_id;
+    return m;
+}
+
+static bool all_tool_messages_paired(const std::vector<Message>& msgs) {
+    std::vector<std::string> produced;
+    for (const auto& m : msgs) {
+        if (m.role == "assistant") {
+            for (const auto& tc : m.tool_calls) produced.push_back(tc.id);
+        }
+        if (m.role == "tool") {
+            if (!m.tool_call_id) return false;
+            bool found = false;
+            for (const auto& pid : produced) {
+                if (pid == *m.tool_call_id) { found = true; break; }
+            }
+            if (!found) return false;
+        }
+    }
+    return true;
+}
+
+int main() {
+    // Test 1: Hard trim keeps round boundaries (no orphan tool_result)
+    {
+        ContextPipeline pipeline;
+        std::vector<Message> history;
+        for (int r = 0; r < 5; r++) {
+            std::string rid = "r" + std::to_string(r);
+            history.push_back(make_user(std::string(2000, 'u') + rid));
+            history.push_back(make_assistant_with_tool("", "call_" + rid));
+            history.push_back(make_tool_result("call_" + rid, std::string(2000, 't')));
+        }
+        history.push_back(make_user("finalize"));
+
+        BindSources sources;
+        sources.conversation_messages = history;
+        auto payload = pipeline.planned_assemble("system", "claude-sonnet-4-6",
+                                                  500, history, sources);
+        assert(pipeline.stats().hard_trims > 0);
+        assert(all_tool_messages_paired(payload.messages));
+        assert(!payload.messages.empty());
+        assert(payload.messages.front().role == "user");
+        std::cout << "Test 1 passed: hard trim keeps round boundaries\n";
+    }
+
+    // Test 2: Hard trim preserves at least one round
+    {
+        ContextPipeline pipeline;
+        std::vector<Message> history;
+        for (int r = 0; r < 5; r++) {
+            std::string rid = "r" + std::to_string(r);
+            history.push_back(make_user(std::string(2000, 'u') + rid));
+            history.push_back(make_assistant_text(std::string(2000, 'a')));
+        }
+        BindSources sources;
+        sources.conversation_messages = history;
+        auto payload = pipeline.planned_assemble("system", "claude-sonnet-4-6",
+                                                  10, history, sources);
+        assert(pipeline.stats().hard_trims > 0);
+        assert(!payload.messages.empty());
+        assert(payload.messages.front().role == "user");
+        std::cout << "Test 2 passed: hard trim preserves at least one round\n";
+    }
+
+    // Test 3: End-to-end — no orphan tool_result in anthropic_json
+    {
+        ContextPipeline pipeline;
+        std::vector<Message> history;
+        for (int r = 0; r < 5; r++) {
+            std::string rid = "r" + std::to_string(r);
+            history.push_back(make_user(std::string(2000, 'u') + rid));
+            history.push_back(make_assistant_with_tool("", "call_" + rid));
+            history.push_back(make_tool_result("call_" + rid, std::string(2000, 't')));
+        }
+        history.push_back(make_user("go"));
+
+        BindSources sources;
+        sources.conversation_messages = history;
+        auto payload = pipeline.planned_assemble("system", "claude-sonnet-4-6",
+                                                  500, history, sources);
+        assert(pipeline.stats().hard_trims > 0);
+        std::vector<std::string> produced_ids;
+        const auto& msgs = payload.anthropic_json["messages"];
+        bool ok = true;
+        for (const auto& m : msgs) {
+            if (m.contains("content") && m["content"].is_array()) {
+                for (const auto& blk : m["content"]) {
+                    const std::string type = blk.value("type", "");
+                    if (type == "tool_use") produced_ids.push_back(blk.value("id", ""));
+                    if (type == "tool_result") {
+                        const std::string use_id = blk.value("tool_use_id", "");
+                        bool found = false;
+                        for (const auto& pid : produced_ids) {
+                            if (pid == use_id) { found = true; break; }
+                        }
+                        if (!found) { ok = false; break; }
+                    }
+                }
+            }
+            if (!ok) break;
+        }
+        assert(ok);
+        std::cout << "Test 3 passed: end-to-end no orphan tool_result (ISSUE #171 repro)\n";
+    }
+
+    // Test 4: Hard trim does not delete system messages
+    {
+        ContextPipeline pipeline;
+        std::vector<Message> history;
+        // Leading system message
+        Message sys; sys.role = "system"; sys.content = "system prompt";
+        history.push_back(sys);
+        // 3 rounds with large content to force hard trim.
+        // Uses <4 rounds so drop_rounds (min_rounds_to_keep=4 default) is a
+        // no-op (drop_count <= 0 returns immediately). This isolates the spec
+        // behavior under test: hard trim erases from rs[0] (first user index)
+        // to rs[1], so a leading system message at index 0 is never touched.
+        for (int r = 0; r < 3; r++) {
+            std::string rid = "r" + std::to_string(r);
+            history.push_back(make_user(std::string(2000, 'u') + rid));
+            history.push_back(make_assistant_text(std::string(2000, 'a')));
+        }
+        BindSources sources;
+        sources.conversation_messages = history;
+        auto payload = pipeline.planned_assemble("system", "claude-sonnet-4-6",
+                                                  500, history, sources);
+        assert(pipeline.stats().hard_trims > 0);
+        // System message must survive — check payload.messages
+        assert(!payload.messages.empty());
+        assert(payload.messages.front().role == "system");
+        assert(payload.messages.front().content == "system prompt");
+        std::cout << "Test 4 passed: hard trim does not delete system messages\n";
+    }
+
+    std::cout << "All ContextPipeline hard trim tests passed.\n";
+    return 0;
+}