From cbf6b2d28c9c6afa9ed49d336bcb73135f5c86f7 Mon Sep 17 00:00:00 2001 From: ULookup Date: Mon, 22 Jun 2026 12:07:41 +0000 Subject: [PATCH 1/8] docs(spec): ISSUE #171 orphaned tool_result fix design Three-layer defense for tool_use/tool_result pairing: - Layer 1: MemoryStore::recent_history round-aware window + orphan expand/drop - Layer 2: ContextPipeline hard trim round-aware deletion - Layer 3: ContextSerializer sanitize_orphans safety net References mainstream Agent implementations (Claude Code, Codex, OpenCode) and includes three-layer regression test plan. --- ...2-issue-171-orphaned-tool-result-design.md | 448 ++++++++++++++++++ 1 file changed, 448 insertions(+) create mode 100644 docs/superpowers/specs/2026-06-22-issue-171-orphaned-tool-result-design.md diff --git a/docs/superpowers/specs/2026-06-22-issue-171-orphaned-tool-result-design.md b/docs/superpowers/specs/2026-06-22-issue-171-orphaned-tool-result-design.md new file mode 100644 index 00000000..d3490589 --- /dev/null +++ b/docs/superpowers/specs/2026-06-22-issue-171-orphaned-tool-result-design.md @@ -0,0 +1,448 @@ +# ISSUE #171 修复：孤儿 tool_result 导致 Anthropic API 400 + +**日期**：2026-06-22 +**ISSUE**：[#171](https://github.com/anthropics/merak/issues/171) — LLM API 400: orphaned tool_result blocks in Anthropic request — tool_use_id mismatch + +## 问题陈述 + +Anthropic API 返回 HTTP 400： + +``` +unexpected `messages.0.content.0: tool_use_id` found in `tool_result` blocks: +call_00_xxx. Each `tool_result` block must have a corresponding `tool_use` +block in the previous message. +``` + +### 根因 + +两个代码路径会破坏 `tool_use` / `tool_result` 配对不变量： + +1. **`MemoryStore::recent_history()`**（`libs/memory/src/memory_store.cpp:28-38`） + 用 `max_turns * 2` 估算窗口大小，假设每轮 2 条消息。含工具调用的轮次实际 4–10+ 条（`user → assistant(tool_use) → tool_result × N`）。窗口可能切在 `assistant(tool_use)` 与 `tool_result` 之间，返回的 history 以孤儿 `tool` 消息开头。 + +2. **`ContextPipeline::planned_assemble()` hard trim**（`libs/context/src/context_pipeline.cpp:85-101`） + 逐条 `erase` 消息，若删到含 `tool_use` 的 assistant 消息，后续 `tool_result` 变孤儿。 + +序列化器 `ContextSerializer::serialize()`（`libs/context/src/context_serializer.cpp`）将孤儿 `tool` 消息转为 Anthropic `tool_result` block，出现在 `messages[0]`，API 校验失败。 + +## 主流 Agent 实现参考 + +| Agent | 机制 | +|---|---| +| **Claude Code** | 对话以 turn 为单元存储（user + assistant + 所有 tool_use/tool_result）。截断时整轮原子删除。发送前校验 tool_use/tool_result 配对，孤儿 tool_result 会被丢弃或补占位 assistant tool_use。 | +| **Codex** | 对话存为带 parent 引用的 items（function_call + function_output 分别是 item）。截断按 (call, output) 对一起删。超 budget 时 rollback 到最近一致 checkpoint。 | +| **OpenCode** | thread 有显式 turn 标记；序列化前扫描 tool_use_id 索引，若 tool 消息的父 assistant 不在窗口内，要么扩展窗口包含父节点，要么丢弃孤儿。 | + +**共同模式**：① round-aware 截断（永不在 user → assistant → tool_result 中间切）+ ② 序列化前配对校验作为安全网 + ③ 检测到孤儿时修复（扩展窗口或丢弃孤儿）。 + +Merak 的 `ContextOptimizer::drop_rounds()`（`libs/context/src/context_optimizer.cpp:114-177`）已实现 ①，但 `recent_history` 和 hard trim 绕过了它，且无 ②/③ 防护。 + +## 设计方案 + +采用**方案 A：分层修复**。三层防线从源头到出口依次加固，纵深防御。 + +### 架构 + +``` +┌─────────────────────────────────────────────────────────┐ +│ Layer 1: MemoryStore::recent_history() │ +│ 源头修复 — 窗口切在 user 边界 + 孤儿扩展/丢弃 │ +└──────────────────────┬──────────────────────────────────┘ + │ vector + ▼ +┌─────────────────────────────────────────────────────────┐ +│ Layer 2: ContextPipeline::planned_assemble() │ +│ hard trim 改为按轮次删除（复用 drop_rounds 范式） │ +└──────────────────────┬──────────────────────────────────┘ + │ BoundContext.provider_messages + ▼ +┌─────────────────────────────────────────────────────────┐ +│ Layer 3: ContextSerializer::serialize() [安全网] │ +│ sanitize_orphans() 预处理 — 兜底任何上游漏过的孤儿 │ +└──────────────────────┬──────────────────────────────────┘ + │ anthropic_json + ▼ + Anthropic API +``` + +**职责边界**： +- **Layer 1** 负责"窗口选取正确"——送进 pipeline 的历史本身配对完整。 +- **Layer 2** 负责"压缩后仍配对"——hard trim 不破坏轮次原子性。 +- **Layer 3** 负责"序列化输出合法"——最后一道防线，检测到任何孤儿就修复，防御未来代码变更引入的新路径。 + +Layer 3 是纯函数、无副作用、不依赖前两层。即使前两层被误改，Layer 3 仍能保证发往 API 的 payload 合法。 + +--- + +## Layer 1：`MemoryStore::recent_history()` 修改 + +### 修改后逻辑 + +```cpp +std::vector MemoryStore::recent_history(int max_turns) const { + std::lock_guard lock(working_memory_mutex_); + int total = (int)working_memory_.size(); + if (total == 0) return {}; + + // 1. 收集所有 user 消息索引（轮次边界） + std::vector user_indices; + for (int i = 0; i < total; i++) { + if (working_memory_[i].role == "user") user_indices.push_back(i); + } + + // 2. 选取最后 max_turns 个轮次 + int keep_rounds = std::min(max_turns, (int)user_indices.size()); + if (keep_rounds <= 0) return {}; + int start = user_indices[(int)user_indices.size() - keep_rounds]; + + // 3. 扩展优先：若 start 之前有连续 tool 消息（孤儿），向前扩展含父 assistant + // 退而丢弃：若父 assistant 距离过远（> max_turns*2 + 4），改为从首个非孤儿开始 + start = adjust_for_orphan_tools(working_memory_, start, max_turns); + + std::vector result; + for (int i = start; i < total; i++) result.push_back(working_memory_[i]); + return result; +} +``` + +### `adjust_for_orphan_tools` 算法 + +``` +输入: messages, start, max_turns +输出: adjusted_start + +1. 扫描 messages[start..] 开头连续的 tool 消息 + - 记录 orphan_tool_ids = {tool_call_id of each} +2. 若无孤儿 tool 消息开头 → 返回 start +3. 向前扫描 messages[0..start-1] 找最近的 assistant 含 tool_calls + - 若该 assistant 的 tool_calls 覆盖所有 orphan_tool_ids 且距离 ≤ max_turns*2 + 4 + → 返回该 assistant 的索引 + - 否则 → 从 start 起跳过开头连续 tool 消息，返回首个非 tool 消息索引 +``` + +**"距离过远"阈值**：`max_turns * 2 + 4` 条消息。若父 assistant 在此距离内，扩展；否则丢弃孤儿。保证窗口最多膨胀一个轮次，不会失控。 + +### 边界 + +- `working_memory_` 为空 → 返回空。 +- 没有 user 消息（全是 assistant/tool）→ 返回最后 `max_turns*2` 条，并跑 `adjust_for_orphan_tools` 兜底。 +- `max_turns = 0` → 返回空。 + +### 接口不变 + +`recent_history(int max_turns) const` 签名不变。调用方 `AgentLoop` 无需改动。 + +--- + +## Layer 2：`ContextPipeline::planned_assemble()` hard trim 修改 + +### 当前问题 + +```cpp +while (opt_stats.tokens_after > model_max_tokens && msgs.size() > 2) { + size_t target = 1; + while (target < msgs.size() && msgs[target].role == "system") target++; + msgs.erase(msgs.begin() + target); // 逐条删除，可能拆散 tool_use/tool_result +} +``` + +### 修改后逻辑 + +复用 `ContextOptimizer::drop_rounds` 的轮次识别范式，但 hard trim 按 token 预算删（`drop_rounds` 按 `min_rounds_to_keep` 删），两者不能合并。每次循环重新扫描 `round_starts`，避免索引偏移陷阱。 + +```cpp +if (opt_stats.tokens_after > model_max_tokens) { + auto& msgs = bound.provider_messages; + int removed = 0; + + while (opt_stats.tokens_after > model_max_tokens) { + std::vector rs; + for (size_t i = 0; i < msgs.size(); i++) { + if (msgs[i].role == "user") rs.push_back(i); + } + if (rs.size() <= 1) break; // 至少保留 1 轮 + + size_t del_end = rs[1]; + int chars = 0; + for (size_t i = rs[0]; i < del_end; i++) { + chars += (int)msgs[i].content.size(); + } + opt_stats.tokens_after -= chars / 3.5; + msgs.erase(msgs.begin() + (long)rs[0], msgs.begin() + (long)del_end); + removed += (int)(del_end - rs[0]); + } + + stats_.hard_trims += removed; + spdlog::warn("ContextPipeline: hard trim removed {} messages (round-aware) " + "(tokens_after={}, max={})", removed, opt_stats.tokens_after, + model_max_tokens); +} +``` + +### 复杂度 + +每次循环重新扫描 O(n)，最坏 O(n²)。hard trim 是异常路径（正常 pipeline 不触发），可接受。 + +### 保留行为 + +- 跳过 system 消息：`round_starts` 不计入 system（role != "user"）。第一轮 user 前的 system 消息不会被删。 +- 至少保留 1 轮：`rs.size() <= 1` 时 break。 +- 统计：`stats_.hard_trims += removed` 保留。 + +### 不做的事 + +- 不合并 `drop_rounds` 和 hard trim：触发条件不同，合并会引入耦合。 +- 不抽 `round_utils.hpp`：避免跨库依赖。 +- 不处理 system 消息删除：保持现状。 + +--- + +## Layer 3：`ContextSerializer::serialize()` 安全网 + +### 职责 + +在 Anthropic 序列化前，对 `payload.messages` 做配对扫描，丢弃任何孤儿。纯函数，无副作用，不依赖 Layer 1/2 的正确性。 + +### 两种孤儿 + +1. **开头孤儿 tool_result**：`tool` 消息的 `tool_call_id` 在其之前的所有 `assistant` 消息的 `tool_calls` 中找不到匹配。 +2. **末尾孤儿 tool_use**：序列末尾的 `assistant` 消息含 `tool_calls`，但其后没有对应的 `tool` 消息（被截断或 loop 中途崩溃）。只处理最后一条 assistant——这是最常见的场景（loop 在 assistant 发出 tool_use 后崩溃，工具未执行）。中间或更早的孤儿 tool_use 不在安全网范围（见下方边界）。 + +### `sanitize_orphans()` 算法 + +```cpp +static std::vector sanitize_orphans(std::vector msgs) { + // 收集所有有匹配 tool_result 的 tool_call_id + std::set referenced_ids; + for (auto& m : msgs) { + if (m.role == "tool" && m.tool_call_id) { + referenced_ids.insert(*m.tool_call_id); + } + } + + std::set produced_ids; + for (auto& m : msgs) { + if (m.role == "assistant") { + for (auto& tc : m.tool_calls) produced_ids.insert(tc.id); + } + } + + // Pass 1: 移除开头连续的孤儿 tool 消息 + size_t i = 0; + while (i < msgs.size() && msgs[i].role == "tool") { + 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("")); + msgs.erase(msgs.begin() + (long)i); + } else { + break; + } + } + + // Pass 2: 移除末尾最后一条 assistant 的孤儿 tool_use + { + int last_assistant = -1; + for (int k = (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 = true; + for (auto& tc : last_a.tool_calls) { + if (referenced_ids.count(tc.id) > 0) { all_orphan = false; break; } + } + if (all_orphan && !last_a.tool_calls.empty()) { + 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() + (long)last_assistant); + } + } + } + } + + return msgs; +} +``` + +### 接入点 + +在 `ContextSerializer::serialize()` 中： + +```cpp +payload.messages = sanitize_orphans(std::move(ctx.provider_messages)); +``` + +放在 `payload.messages = ctx.provider_messages;` 之后，两个格式分支之前。**对 OpenAI 和 Anthropic 格式都生效**——OpenAI API 对孤儿 tool 消息同样报错。 + +### 不修改 `ctx.provider_messages` 本身 + +`payload.messages = ctx.provider_messages;` 是值拷贝，修改 payload 不影响 `BoundContext`。 + +### 边界 + +- 空消息列表 → 直接返回。 +- 无 tool 消息 → Pass 1 no-op。 +- 无 assistant tool_calls → Pass 2 no-op。 +- **中间孤儿（非开头非末尾）不在安全网范围**。中间孤儿意味着配对在序列中间被打乱，是 Layer 1/2 的责任。若中间出现孤儿，Anthropic API 仍会报错——这种状态只可能由未知严重 bug 产生，不应被安全网静默吞掉。 + +### 日志 + +所有丢弃操作记 `spdlog::warn`，含 `tool_use_id` 和位置（head/tail），便于线上排查。 + +--- + +## 错误处理、日志与可观测性 + +### 日志策略 + +| 层 | 触发场景 | 日志级别 | 内容 | +|---|---|---|---| +| Layer 1 | `recent_history` 扩展窗口含父 assistant | `debug` | `MemoryStore: expanded window from {} to {} to cover orphan tool_result (tool_use_id={})` | +| Layer 1 | `recent_history` 丢弃开头孤儿 tool 消息 | `warn` | `MemoryStore: dropped {} orphan tool messages at head (max_turns={})` | +| Layer 2 | hard trim 触发整轮删除 | `warn` | 已有，保留并补充 `round-aware` 标识 | +| Layer 3 | 序列化丢弃孤儿 tool_result | `warn` | `ContextSerializer: dropping orphan tool_result at head (tool_use_id={})` | +| Layer 3 | 序列化丢弃末尾孤儿 tool_use | `warn` | `ContextSerializer: dropping {} orphan tool_use at tail` | + +Layer 1 扩展是 debug（信息保留，预期内），丢弃是 warn（数据丢失，需关注）。 + +### 错误处理边界 + +**不抛异常**。三层都在热路径，抛异常会中断 AgentLoop。任何异常状态用 warn 日志 + 降级处理： + +- Layer 1 扩展失败 → 退化为丢弃孤儿，记 warn。 +- Layer 2 hard trim 无轮次可删（只剩 1 轮仍超预算）→ break，让 payload 发出去，由 provider 返回错误（单轮超 max_tokens 是配置问题，pipeline 救不了）。 +- Layer 3 `sanitize_orphans` 任何异常 → 返回原 messages 不处理，记 error。**绝不能让安全网自己崩掉**。 + +### 不做的事 + +- 不加 metrics 计数器（`PipelineStats` 无 orphan 字段，加字段污染统计语义）。 +- 不加告警（hard trim 触发已是 warn，再加噪音）。 +- 不修复历史数据（`working_memory_` 中可能已存不一致状态，安全网兜底即可，不回写）。 + +--- + +## 回归测试 + +三层测试，每层独立验证对应防线。 + +### 测试 1：`MemoryStore` 单测 + +**文件**：`libs/memory/tests/test_memory_history.cpp`（新建） +**可执行**：`merak-memory-history-test` + +构造 `MemoryConfig{.enabled = false}`，不连 DB。通过 `append_message()` 灌入历史，调 `recent_history()` 验证。 + +**用例**： + +1. `RecentHistory_NoToolCalls_ReturnsLastNTurns` — 5 轮纯文本，`max_turns=3`，断言返回最后 6 条。 +2. `RecentHistory_WithToolCalls_StartsOnUserBoundary` — 3 轮含工具调用，`max_turns=2`，断言首条是 `user`。 +3. `RecentHistory_OrphanToolHead_ExpandsToParent` — naive 窗口切在 tool 消息上，断言扩展含父 assistant。 +4. `RecentHistory_OrphanToolHead_TooFar_DropsOrphan` — 父 assistant 距离 > `max_turns*2 + 4`，断言丢弃开头孤儿 tool 消息。 +5. `RecentHistory_EmptyMemory_ReturnsEmpty` — 空 `working_memory_`，返回空。 +6. `RecentHistory_NoUserMessages_FallsBackToTail` — 只有 assistant/tool，返回最后若干条且开头无孤儿 tool。 + +### 测试 2：`ContextSerializer` 单测 + +**文件**：`libs/context/tests/test_serializer_orphans.cpp`（新建） +**可执行**：`merak-context-serializer-test` + +**用例**： + +1. `Serialize_Anthropic_OrphanToolResultHead_Dropped` — 开头孤儿 tool 消息，断言 `anthropic_json["messages"][0]["role"] == "user"`，无 `tool_result` block。 +2. `Serialize_Anthropic_OrphanToolUseTail_Dropped` — 末尾 assistant 含孤儿 tool_use，断言输出无 `tool_use` block（或整条移除）。 +3. `Serialize_Anthropic_PairedToolUse_Preserved` — 正常配对，断言 3 条消息含完整 `tool_use` / `tool_result`。 +4. `Serialize_Anthropic_MultipleOrphanToolsAtHead_AllDropped` — 开头连续 2 条孤儿 tool，断言都被丢弃。 +5. `Serialize_OpenAI_OrphanToolResultHead_Dropped` — 同样输入，断言 OpenAI 格式也无孤儿。 + +### 测试 3：`ContextPipeline` 集成测 + +**文件**：`libs/context/tests/test_pipeline_hard_trim.cpp`（新建） +**可执行**：`merak-context-pipeline-test` + +**用例**： + +1. `PlannedAssemble_HardTrim_KeepsRoundBoundaries` — 5 轮含工具调用，总 token 超 `model_max_tokens`，断言输出以 `user` 开头，所有 `tool` 消息的 `tool_call_id` 能在之前的 `assistant.tool_calls` 中找到匹配。 +2. `PlannedAssemble_HardTrim_PreservesAtLeastOneRound` — 极小 `model_max_tokens`，断言不会清空，至少保留 1 轮。 +3. `PlannedAssemble_HardTrim_DoesNotDeleteSystemMessages` — `msgs[0]` 是 system，触发 hard trim，断言 system 仍在。 +4. `PlannedAssemble_EndToEnd_NoOrphanToolResult` — 长历史含工具调用，调 `planned_assemble()`，对 `anthropic_json["messages"]` 做完整配对校验。**这是 ISSUE #171 的复现测试**。 + +### CMake 改动 + +在 `tests/CMakeLists.txt` 追加： + +```cmake +add_executable(merak-memory-history-test + ${CMAKE_SOURCE_DIR}/libs/memory/tests/test_memory_history.cpp +) +target_link_libraries(merak-memory-history-test PRIVATE merak-memory) +add_test(NAME merak-memory-history-test COMMAND merak-memory-history-test) + +add_executable(merak-context-serializer-test + ${CMAKE_SOURCE_DIR}/libs/context/tests/test_serializer_orphans.cpp +) +target_link_libraries(merak-context-serializer-test PRIVATE merak-context) +add_test(NAME merak-context-serializer-test COMMAND merak-context-serializer-test) + +add_executable(merak-context-pipeline-test + ${CMAKE_SOURCE_DIR}/libs/context/tests/test_pipeline_hard_trim.cpp +) +target_link_libraries(merak-context-pipeline-test PRIVATE merak-context) +add_test(NAME merak-context-pipeline-test COMMAND merak-context-pipeline-test) +``` + +### 测试覆盖矩阵 + +| ISSUE #171 根因 | 覆盖测试 | +|---|---| +| `recent_history` 窗口切在 tool 消息上 | 测试 1 用例 2、3 | +| hard trim 拆散 tool_use/tool_result | 测试 3 用例 1、4 | +| 序列化产生孤儿 tool_result | 测试 2 用例 1、4 + 测试 3 用例 4 | +| 末尾孤儿 tool_use | 测试 2 用例 2 | + +--- + +## 实施顺序与风险 + +### 实施顺序 + +1. **Layer 3 安全网先行**（ContextSerializer `sanitize_orphans`） + 纯函数、无外部依赖、可立即加测试。先部署安全网，即使 Layer 1/2 还没改，也能立即阻止 400 错误线上发生。**先止血**。 + +2. **Layer 1 修复**（MemoryStore `recent_history`） + 紧接着修源头，让窗口选取本身正确。 + +3. **Layer 2 修复**（ContextPipeline hard trim） + 最后修，依赖对轮次边界的理解，且是异常路径。 + +4. **测试与 CMakeLists 同步** + 每层改完立即加对应测试。 + +### 风险评估 + +| 风险 | 影响 | 缓解 | +|---|---|---| +| Layer 1 窗口扩展导致 token 超预算 | pipeline 后续阶段可能触发 hard trim | 扩展阈值 `max_turns*2+4` 限制膨胀；hard trim 本身也在修，能兜住 | +| Layer 2 重新扫描 O(n²) | 极长对话下 hard trim 慢 | hard trim 是异常路径；若未来成瓶颈再优化为索引偏移方案 | +| Layer 3 误删合法 tool_use（假阳性） | LLM 丢失工具调用上下文，可能重复调用 | `sanitize_orphans` 只删**确定无匹配**的孤儿，保守策略；Pass 2 只处理末尾 assistant，中间不碰 | +| 测试需要构造复杂消息序列 | 测试代码冗长 | 提取 `make_user_msg()` / `make_assistant_with_tools()` / `make_tool_result()` 辅助函数到测试文件顶部 | +| `merak-memory` 库目前没有 tests 目录 | CMake 改动需要创建目录 | 新建 `libs/memory/tests/` | + +### 回滚策略 + +三层修改彼此独立，三个 commit 分层提交，便于回滚和 bisect： +- Layer 3：删除 `sanitize_orphans` 调用即可回滚。 +- Layer 1：恢复 `max_turns * 2` 估算。 +- Layer 2：恢复逐条 erase。 + +--- + +## 不在本次范围 + +- **社区功能**（memory 中记录的下一步规划）——本次不做。 +- **`drop_rounds` 与 hard trim 逻辑统一**——方案 A 明确不抽公共 helper。 +- **中间孤儿检测**——Layer 3 只处理边界，中间孤儿是未知严重 bug 的信号，不应被安全网静默吞掉。 +- **`working_memory_` 持久化层修复**——ISSUE 只涉及内存中的 `recent_history`，不涉及 DB。 From 6429362b14b60443bd97f4fe1b30cbfc7d78bd56 Mon Sep 17 00:00:00 2001 From: ULookup Date: Mon, 22 Jun 2026 12:15:58 +0000 Subject: [PATCH 2/8] docs(plan): ISSUE #171 orphaned tool_result fix implementation plan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three-task TDD plan: - Task 1: Layer 3 safety net (ContextSerializer sanitize_orphans) - Task 2: Layer 1 source fix (MemoryStore recent_history round-aware) - Task 3: Layer 2 hard trim round-aware deletion - Task 4: Final verification Each task: failing test → CMake registration → implement → rebuild → regression check → commit. --- ...26-06-22-issue-171-orphaned-tool-result.md | 924 ++++++++++++++++++ 1 file changed, 924 insertions(+) create mode 100644 docs/superpowers/plans/2026-06-22-issue-171-orphaned-tool-result.md diff --git a/docs/superpowers/plans/2026-06-22-issue-171-orphaned-tool-result.md b/docs/superpowers/plans/2026-06-22-issue-171-orphaned-tool-result.md new file mode 100644 index 00000000..0ab4ac1c --- /dev/null +++ b/docs/superpowers/plans/2026-06-22-issue-171-orphaned-tool-result.md @@ -0,0 +1,924 @@ +# ISSUE #171 Orphaned tool_result Fix — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Eliminate Anthropic API HTTP 400 errors caused by orphaned `tool_result` blocks lacking matching `tool_use` blocks, via a three-layer defense (MemoryStore windowing + ContextPipeline round-aware hard trim + ContextSerializer safety net). + +**Architecture:** Three independent layers. Layer 3 (safety net) ships first as pure-function stopgap. Layer 1 (MemoryStore) fixes the windowing root cause. Layer 2 (ContextPipeline hard trim) makes the token-budget trim round-aware. Each layer has dedicated tests. Three commits, each independently revertable. + +**Tech Stack:** C++20, CMake, nlohmann::json, spdlog, pqxx (MemoryStore tests use `enabled=false` to skip DB). + +**Spec:** `docs/superpowers/specs/2026-06-22-issue-171-orphaned-tool-result-design.md` + +--- + +## File Structure + +| File | Action | Responsibility | +|---|---|---| +| `libs/context/src/context_serializer.cpp` | Modify | Add `sanitize_orphans()` static helper + call it in `serialize()` | +| `libs/context/tests/test_serializer_orphans.cpp` | Create | Layer 3 unit tests | +| `tests/CMakeLists.txt` | Modify | Register `merak-context-serializer-test` | +| `libs/memory/src/memory_store.cpp` | Modify | Rewrite `recent_history()` to be round-aware + `adjust_for_orphan_tools()` helper | +| `libs/memory/tests/test_memory_history.cpp` | Create | Layer 1 unit tests | +| `tests/CMakeLists.txt` | Modify | Register `merak-memory-history-test` | +| `libs/context/src/context_pipeline.cpp` | Modify | Replace per-message hard trim with round-aware deletion | +| `libs/context/tests/test_pipeline_hard_trim.cpp` | Create | Layer 2 integration tests | +| `tests/CMakeLists.txt` | Modify | Register `merak-context-pipeline-test` | + +--- + +## Task 1: Layer 3 Safety Net — `sanitize_orphans()` in ContextSerializer + +**Files:** +- Modify: `libs/context/src/context_serializer.cpp` (add static helper after `namespace merak {` line 4, call it in `serialize()` after line 27 `payload.messages = ctx.provider_messages;`) +- Create: `libs/context/tests/test_serializer_orphans.cpp` +- Modify: `tests/CMakeLists.txt` (append new target) + +- [ ] **Step 1: Write the failing test file** + +Create `libs/context/tests/test_serializer_orphans.cpp`: + +```cpp +#include +#include +#include +#include +#include +#include +#include + +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, const std::string& tool_name = "read_file") { + Message m; m.role = "assistant"; m.content = text; + ToolCall tc; tc.id = call_id; tc.name = tool_name; 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; +} + +// Walks anthropic_json["messages"] and returns true if every tool_result block +// has a matching tool_use block in a prior assistant message. +static bool no_orphan_tool_results(const nlohmann::json& msgs) { + std::vector produced_ids; + for (const auto& m : msgs) { + const std::string role = m.value("role", ""); + if (m.contains("content") && m["content"].is_array()) { + for (const auto& blk : m["content"]) { + if (blk.value("type", "") == "tool_use") { + produced_ids.push_back(blk.value("id", "")); + } + } + } + if (role == "user" && m.contains("content") && m["content"].is_array()) { + for (const auto& blk : m["content"]) { + if (blk.value("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) return false; + } + } + } + } + return true; +} + +// Returns true if no assistant message contains a tool_use block. +static bool no_tool_use_at_all(const nlohmann::json& msgs) { + for (const auto& m : msgs) { + if (m.contains("content") && m["content"].is_array()) { + for (const auto& blk : m["content"]) { + if (blk.value("type", "") == "tool_use") return false; + } + } + } + return true; +} + +int main() { + ContextSerializer serializer; + + // Test 1: Orphan tool_result at head is dropped (Anthropic) + { + BoundContext ctx; + ctx.provider_messages = { + make_tool_result("call_orphan_head", "stale result"), + make_user("hello"), + make_assistant_text("hi"), + }; + auto payload = serializer.serialize(ctx, "claude-sonnet-4-6", "", 1024); + assert(payload.is_anthropic); + const auto& msgs = payload.anthropic_json["messages"]; + assert(!msgs.empty()); + assert(msgs[0].value("role", "") == "user"); + assert(no_orphan_tool_results(msgs)); + std::cout << "Test 1 passed: orphan tool_result at head dropped (Anthropic)\n"; + } + + // Test 2: Orphan tool_use at tail is dropped (Anthropic) + { + BoundContext ctx; + ctx.provider_messages = { + make_user("do X"), + make_assistant_with_tool("", "call_orphan_tail"), + }; + auto payload = serializer.serialize(ctx, "claude-sonnet-4-6", "", 1024); + const auto& msgs = payload.anthropic_json["messages"]; + assert(no_tool_use_at_all(msgs)); + std::cout << "Test 2 passed: orphan tool_use at tail dropped (Anthropic)\n"; + } + + // Test 3: Paired tool_use preserved + { + BoundContext ctx; + ctx.provider_messages = { + make_user("do X"), + make_assistant_with_tool("", "call_ok"), + make_tool_result("call_ok", "result"), + }; + auto payload = serializer.serialize(ctx, "claude-sonnet-4-6", "", 1024); + const auto& msgs = payload.anthropic_json["messages"]; + assert(no_orphan_tool_results(msgs)); + // 3 logical messages: user, assistant(tool_use), user(tool_result) + assert(msgs.size() == 3); + std::cout << "Test 3 passed: paired tool_use preserved\n"; + } + + // Test 4: Multiple orphan tool_results at head all dropped + { + BoundContext ctx; + ctx.provider_messages = { + make_tool_result("orphan_1", "r1"), + make_tool_result("orphan_2", "r2"), + make_user("hello"), + make_assistant_text("hi"), + }; + auto payload = serializer.serialize(ctx, "claude-sonnet-4-6", "", 1024); + const auto& msgs = payload.anthropic_json["messages"]; + assert(msgs[0].value("role", "") == "user"); + assert(no_orphan_tool_results(msgs)); + std::cout << "Test 4 passed: multiple orphan tool_results at head all dropped\n"; + } + + // Test 5: OpenAI format also has no orphan tool messages + { + BoundContext ctx; + ctx.provider_messages = { + make_tool_result("call_orphan_openai", "stale"), + make_user("hello"), + make_assistant_text("hi"), + }; + auto payload = serializer.serialize(ctx, "gpt-4o", "", 1024); + assert(!payload.is_anthropic); + const auto& msgs = payload.openai_json["messages"]; + // First non-system message must not be a tool message + bool found_first_non_system = false; + for (const auto& m : msgs) { + if (m.value("role", "") == "system") continue; + assert(m.value("role", "") != "tool"); + found_first_non_system = true; + break; + } + assert(found_first_non_system); + std::cout << "Test 5 passed: OpenAI format also has no orphan tool at head\n"; + } + + std::cout << "All ContextSerializer orphan tests passed.\n"; + return 0; +} +``` + +- [ ] **Step 2: Register the test in CMakeLists** + +Append to `tests/CMakeLists.txt` (after line 22, the existing `merak-context-test` block): + +```cmake + +# Context serializer orphan safety net tests +add_executable(merak-context-serializer-test + ${CMAKE_SOURCE_DIR}/libs/context/tests/test_serializer_orphans.cpp +) +target_link_libraries(merak-context-serializer-test PRIVATE merak-context) +add_test(NAME merak-context-serializer-test COMMAND merak-context-serializer-test) +``` + +- [ ] **Step 3: Build the test to confirm it compiles** + +Run: `cmake --build build --target merak-context-serializer-test -j8` +Expected: Builds successfully. (It will fail at runtime because `sanitize_orphans` is not implemented yet — the test will fail on assertions.) + +- [ ] **Step 4: Run test to verify it fails** + +Run: `./build/tests/merak-context-serializer-test` +Expected: Assertion failure on Test 1 (orphan tool_result at head is NOT dropped — current serializer emits it as `messages[0]` with `tool_result` block). + +- [ ] **Step 5: Add `sanitize_orphans()` static helper in `context_serializer.cpp`** + +In `libs/context/src/context_serializer.cpp`, immediately after line 4 (`namespace merak {`) and before the `serialize` function, insert: + +```cpp +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 sanitize_orphans(std::vector msgs) { + std::set 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 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("")); + msgs.erase(msgs.begin() + static_cast(i)); + } else { + break; + } + } + + // Pass 2: last assistant's orphan tool_use + int last_assistant = -1; + for (int k = static_cast(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(last_assistant)); + } + } + } + + return msgs; +} + +} // anonymous namespace +``` + +Also add `#include ` at the top of the file with the other includes. + +- [ ] **Step 6: Call `sanitize_orphans()` in `serialize()`** + +In `libs/context/src/context_serializer.cpp`, replace the line `payload.messages = ctx.provider_messages;` (currently line 27) with: + +```cpp + payload.messages = sanitize_orphans(ctx.provider_messages); +``` + +`ctx` is `const BoundContext&`, so this copies `ctx.provider_messages` into `sanitize_orphans`, which returns a new sanitized vector assigned to `payload.messages`. `ctx` itself is unchanged. + +- [ ] **Step 7: Rebuild and run test** + +Run: `cmake --build build --target merak-context-serializer-test -j8 && ./build/tests/merak-context-serializer-test` +Expected: All 5 tests pass with "All ContextSerializer orphan tests passed." + +- [ ] **Step 8: Run existing context tests to ensure no regression** + +Run: `cmake --build build --target merak-context-test -j8 && ./build/tests/merak-context-test` +Expected: Existing tests still pass. + +- [ ] **Step 9: Commit** + +```bash +git add libs/context/src/context_serializer.cpp \ + libs/context/tests/test_serializer_orphans.cpp \ + tests/CMakeLists.txt +git commit -m "$(cat <<'EOF' +fix(context): add sanitize_orphans safety net in ContextSerializer + +Drops orphan tool_result at head and orphan tool_use at tail before +serialization. Prevents Anthropic API HTTP 400 errors when upstream +windowing or trimming produces unpaired tool messages. + +Layer 3 of ISSUE #171 fix. +EOF +)" +``` + +--- + +## Task 2: Layer 1 — `MemoryStore::recent_history()` round-aware windowing + +**Files:** +- Modify: `libs/memory/src/memory_store.cpp` (replace `recent_history` body at lines 28-39, add private helper `adjust_for_orphan_tools`) +- Modify: `libs/memory/include/merak/memory_store.hpp` (add private helper declaration) +- Create: `libs/memory/tests/test_memory_history.cpp` +- Modify: `tests/CMakeLists.txt` (append new target) + +- [ ] **Step 1: Write the failing test file** + +Create `libs/memory/tests/test_memory_history.cpp`: + +```cpp +#include +#include +#include +#include +#include + +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; +} + +// Builds a MemoryStore with disabled DB (no PostgreSQL needed). +static std::unique_ptr make_store() { + MemoryConfig cfg; + cfg.enabled = false; + return std::make_unique(cfg, nullptr); +} + +static bool starts_with_user(const std::vector& msgs) { + return !msgs.empty() && msgs.front().role == "user"; +} + +static bool no_orphan_tool_at_head(const std::vector& msgs) { + if (msgs.empty()) return true; + if (msgs.front().role != "tool") return true; + // If first is tool, its tool_call_id must be produced by an earlier assistant — + // but there's no earlier assistant, so it's always orphan. + return false; +} + +int main() { + // Test 1: No tool calls — returns last N turns + { + auto store = make_store(); + for (int i = 0; i < 5; i++) { + store->append_message(make_user("u" + std::to_string(i))); + store->append_message(make_assistant_text("a" + std::to_string(i))); + } + auto hist = store->recent_history(3); + assert(hist.size() == 6); + assert(hist[0].content == "u2"); + assert(hist[5].content == "a4"); + std::cout << "Test 1 passed: no tool calls, last N turns\n"; + } + + // Test 2: With tool calls — starts on user boundary + { + auto store = make_store(); + // Round 1: user → assistant(tool_use X) → tool(X) + store->append_message(make_user("u1")); + store->append_message(make_assistant_with_tool("", "call_X")); + store->append_message(make_tool_result("call_X", "r1")); + // Round 2: user → assistant(tool_use Y) → tool(Y) + store->append_message(make_user("u2")); + store->append_message(make_assistant_with_tool("", "call_Y")); + store->append_message(make_tool_result("call_Y", "r2")); + // Round 3: user → assistant(text) + store->append_message(make_user("u3")); + store->append_message(make_assistant_text("a3")); + + auto hist = store->recent_history(2); + assert(starts_with_user(hist)); + assert(hist[0].content == "u2"); + std::cout << "Test 2 passed: with tool calls, starts on user boundary\n"; + } + + // Test 3: Orphan tool at head — expands to parent assistant + { + auto store = make_store(); + // Old round: user → assistant(tool_use Z) → tool(Z) + store->append_message(make_user("u_old")); + store->append_message(make_assistant_with_tool("", "call_Z")); + store->append_message(make_tool_result("call_Z", "rZ")); + // Recent round: user → assistant(text) + store->append_message(make_user("u_recent")); + store->append_message(make_assistant_text("a_recent")); + + // max_turns=1 should pick the last user-led round: [u_recent, a_recent]. + // But if naive max_turns*2 slicing cut between assistant(tool_use Z) and tool(Z), + // we'd get [tool(Z), u_recent, a_recent] — orphan at head. + // For this test we want to trigger expansion: construct a case where the + // round-boundary window itself starts at a user (so no orphan). To force + // an orphan scenario, we manually need a history where the last user-led + // round boundary is preceded by tool messages without their parent assistant. + // + // Reconstruct: this happens when max_turns counting picks fewer rounds + // than the tool-result tail. We simulate by having a long tool-only tail. + // Since round-boundary logic always starts on user, the only way to get + // an orphan tool at head is if there's no user in the kept window at all — + // which we handle in Test 6. + // + // For Test 3, verify the normal case: no orphan when rounds are well-formed. + auto hist = store->recent_history(1); + assert(no_orphan_tool_at_head(hist)); + assert(hist[0].content == "u_recent"); + std::cout << "Test 3 passed: well-formed rounds, no orphan at head\n"; + } + + // Test 4: Orphan tool at head, parent too far — drops orphan + { + auto store = make_store(); + // Construct: no user messages at all, just assistant + orphan tools. + // The fallback path returns last max_turns*2 and must drop leading orphan tools. + store->append_message(make_assistant_with_tool("", "call_far")); + // Many filler messages to push distance beyond max_turns*2 + 4 + for (int i = 0; i < 20; i++) { + store->append_message(make_assistant_text("filler" + std::to_string(i))); + } + store->append_message(make_tool_result("call_far", "orphan_result")); + store->append_message(make_assistant_text("tail")); + + auto hist = store->recent_history(2); + assert(no_orphan_tool_at_head(hist)); + std::cout << "Test 4 passed: orphan tool at head with far parent, dropped\n"; + } + + // Test 5: Empty memory — returns empty + { + auto store = make_store(); + auto hist = store->recent_history(5); + assert(hist.empty()); + std::cout << "Test 5 passed: empty memory returns empty\n"; + } + + // Test 6: No user messages — fallback to tail with orphan handling + { + auto store = make_store(); + // Only assistant and tool messages, no user. + store->append_message(make_assistant_with_tool("", "call_nouser")); + store->append_message(make_tool_result("call_nouser", "r")); + store->append_message(make_assistant_text("tail")); + auto hist = store->recent_history(2); + assert(no_orphan_tool_at_head(hist)); + std::cout << "Test 6 passed: no user messages, fallback handles orphans\n"; + } + + std::cout << "All MemoryStore recent_history tests passed.\n"; + return 0; +} +``` + +- [ ] **Step 2: Register the test in CMakeLists** + +Append to `tests/CMakeLists.txt`: + +```cmake + +# MemoryStore recent_history tests (no DB) +add_executable(merak-memory-history-test + ${CMAKE_SOURCE_DIR}/libs/memory/tests/test_memory_history.cpp +) +target_link_libraries(merak-memory-history-test PRIVATE merak-memory) +add_test(NAME merak-memory-history-test COMMAND merak-memory-history-test) +``` + +- [ ] **Step 3: Build the test** + +Run: `cmake --build build --target merak-memory-history-test -j8` +Expected: Builds successfully. + +- [ ] **Step 4: Run test to verify it fails** + +Run: `./build/tests/merak-memory-history-test` +Expected: Test 2 (or another) fails — current `recent_history` uses `max_turns * 2` which will produce wrong slice for tool-call conversations. + +- [ ] **Step 5: Add `adjust_for_orphan_tools` declaration to header** + +In `libs/memory/include/merak/memory_store.hpp`, add to the private section (after `std::expected create_tables();`): + +```cpp + // Adjusts the start index forward if the window begins with orphan tool + // messages whose parent assistant is missing or too far away. + // - Expands window backward if parent assistant is within max_turns*2 + 4. + // - Otherwise drops the leading orphan tool messages. + static int adjust_for_orphan_tools(const std::vector& msgs, + int start, int max_turns); +``` + +- [ ] **Step 6: Implement `adjust_for_orphan_tools` and rewrite `recent_history` in `memory_store.cpp`** + +Replace the current `recent_history` body (lines 28-39 of `libs/memory/src/memory_store.cpp`) with: + +```cpp +std::vector MemoryStore::recent_history(int max_turns) const { + std::lock_guard lock(working_memory_mutex_); + int total = static_cast(working_memory_.size()); + if (total == 0 || max_turns <= 0) return {}; + + // 1. Collect user message indices (round boundaries) + std::vector user_indices; + for (int i = 0; i < total; i++) { + if (working_memory_[i].role == "user") user_indices.push_back(i); + } + + int start = 0; + if (!user_indices.empty()) { + int keep_rounds = std::min(max_turns, static_cast(user_indices.size())); + if (keep_rounds <= 0) return {}; + start = user_indices[static_cast(user_indices.size()) - keep_rounds]; + } else { + // No user messages — fall back to last max_turns*2 messages + start = std::max(0, total - max_turns * 2); + } + + start = adjust_for_orphan_tools(working_memory_, start, max_turns); + + std::vector result; + for (int i = start; i < total; i++) { + result.push_back(working_memory_[i]); + } + return result; +} + +int MemoryStore::adjust_for_orphan_tools(const std::vector& msgs, + int start, int max_turns) { + if (start >= static_cast(msgs.size())) return start; + if (msgs[start].role != "tool") return start; + + // Collect leading orphan tool ids + std::vector orphan_ids; + int probe = start; + while (probe < static_cast(msgs.size()) && msgs[probe].role == "tool") { + if (msgs[probe].tool_call_id) { + orphan_ids.push_back(*msgs[probe].tool_call_id); + } + probe++; + } + if (orphan_ids.empty()) return start; + + // Search backward for the most recent assistant with matching tool_calls + int parent_idx = -1; + for (int i = start - 1; i >= 0; i--) { + if (msgs[i].role != "assistant") continue; + bool covers_all = true; + for (const auto& oid : orphan_ids) { + bool found = false; + for (const auto& tc : msgs[i].tool_calls) { + if (tc.id == oid) { found = true; break; } + } + if (!found) { covers_all = false; break; } + } + if (covers_all) { parent_idx = i; break; } + } + + const int max_distance = max_turns * 2 + 4; + if (parent_idx >= 0 && (start - parent_idx) <= max_distance) { + spdlog::debug("MemoryStore: expanded window from {} to {} to cover " + "orphan tool_result ({} ids)", + start, parent_idx, orphan_ids.size()); + return parent_idx; + } + + // Drop leading orphan tool messages + int new_start = start; + while (new_start < static_cast(msgs.size()) && + msgs[new_start].role == "tool") { + new_start++; + } + spdlog::warn("MemoryStore: dropped {} orphan tool messages at head " + "(max_turns={}, parent_distance={})", + new_start - start, max_turns, + parent_idx >= 0 ? (start - parent_idx) : -1); + return new_start; +} +``` + +- [ ] **Step 7: Add necessary includes if missing** + +Verify `libs/memory/src/memory_store.cpp` includes `` (already present line 4) and `` (transitively via header). No new includes needed. + +- [ ] **Step 8: Rebuild and run test** + +Run: `cmake --build build --target merak-memory-history-test -j8 && ./build/tests/merak-memory-history-test` +Expected: All 6 tests pass. + +- [ ] **Step 9: Run existing memory-dependent tests for regression** + +Run: `cmake --build build --target merak-agent-loop-test merak-context-test -j8 && ./build/tests/merak-agent-loop-test && ./build/tests/merak-context-test` +Expected: Existing tests still pass. + +- [ ] **Step 10: Commit** + +```bash +git add libs/memory/src/memory_store.cpp \ + libs/memory/include/merak/memory_store.hpp \ + libs/memory/tests/test_memory_history.cpp \ + tests/CMakeLists.txt +git commit -m "$(cat <<'EOF' +fix(memory): make recent_history round-aware to prevent orphan tool_results + +Rewrite recent_history to slice on user-message boundaries instead of +naive max_turns*2 estimate. Adds adjust_for_orphan_tools helper that +expands the window to cover a parent assistant when nearby, or drops +leading orphan tool messages when the parent is too far. + +Layer 1 of ISSUE #171 fix. +EOF +)" +``` + +--- + +## Task 3: Layer 2 — `ContextPipeline::planned_assemble()` round-aware hard trim + +**Files:** +- Modify: `libs/context/src/context_pipeline.cpp` (replace lines 84-101, the `if (opt_stats.tokens_after > model_max_tokens)` block) +- Create: `libs/context/tests/test_pipeline_hard_trim.cpp` +- Modify: `tests/CMakeLists.txt` (append new target) + +- [ ] **Step 1: Write the failing test file** + +Create `libs/context/tests/test_pipeline_hard_trim.cpp`: + +```cpp +#include +#include +#include +#include +#include +#include + +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; +} + +// Verifies every tool message's tool_call_id has a matching assistant.tool_calls +// entry in a prior message. +static bool all_tool_messages_paired(const std::vector& msgs) { + std::vector 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 history; + // 5 rounds, each with tool calls and large content to force token overflow + 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'))); + } + // Final user message to trigger next turn + history.push_back(make_user("finalize")); + + BindSources sources; // empty tool specs + // Very small max_tokens to force hard trim + auto payload = pipeline.planned_assemble("system", "claude-sonnet-4-6", + 500, history, sources); + assert(all_tool_messages_paired(payload.messages)); + // Must start with a user message (round boundary) + 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 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; + // Absurdly small max_tokens — must still keep at least 1 round + auto payload = pipeline.planned_assemble("system", "claude-sonnet-4-6", + 10, history, sources); + assert(!payload.messages.empty()); + 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 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; + auto payload = pipeline.planned_assemble("system", "claude-sonnet-4-6", + 500, history, sources); + // Walk anthropic_json["messages"] — every tool_result block must have + // a matching tool_use block in a prior assistant message. + std::vector 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"; + } + + std::cout << "All ContextPipeline hard trim tests passed.\n"; + return 0; +} +``` + +- [ ] **Step 2: Register the test in CMakeLists** + +Append to `tests/CMakeLists.txt`: + +```cmake + +# ContextPipeline hard trim tests +add_executable(merak-context-pipeline-test + ${CMAKE_SOURCE_DIR}/libs/context/tests/test_pipeline_hard_trim.cpp +) +target_link_libraries(merak-context-pipeline-test PRIVATE merak-context) +add_test(NAME merak-context-pipeline-test COMMAND merak-context-pipeline-test) +``` + +- [ ] **Step 3: Build the test** + +Run: `cmake --build build --target merak-context-pipeline-test -j8` +Expected: Builds successfully. + +- [ ] **Step 4: Run test to verify it fails** + +Run: `./build/tests/merak-context-pipeline-test` +Expected: Test 1 or Test 3 fails — current hard trim per-message erase can break tool_use/tool_result pairing. + +- [ ] **Step 5: Replace hard trim block with round-aware version** + +In `libs/context/src/context_pipeline.cpp`, replace the entire block from line 84 (`// Hard trim: enforce model_max_tokens as hard ceiling`) through line 101 (the closing `}` of the `if (opt_stats.tokens_after > model_max_tokens)` block) with: + +```cpp + // 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. + if (opt_stats.tokens_after > model_max_tokens) { + auto& msgs = bound.provider_messages; + int removed = 0; + while (opt_stats.tokens_after > model_max_tokens) { + std::vector 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]; + int chars = 0; + for (size_t i = rs[0]; i < del_end; i++) { + chars += static_cast(msgs[i].content.size()); + } + opt_stats.tokens_after -= chars / 3.5; + msgs.erase(msgs.begin() + static_cast(rs[0]), + msgs.begin() + static_cast(del_end)); + removed += static_cast(del_end - rs[0]); + } + stats_.hard_trims += removed; + spdlog::warn("ContextPipeline: hard trim removed {} messages (round-aware) " + "(tokens_after={}, max={})", + removed, opt_stats.tokens_after, model_max_tokens); + } +``` + +- [ ] **Step 6: Rebuild and run test** + +Run: `cmake --build build --target merak-context-pipeline-test -j8 && ./build/tests/merak-context-pipeline-test` +Expected: All 3 tests pass. + +- [ ] **Step 7: Run full context test suite for regression** + +Run: `cmake --build build --target merak-context-test merak-context-serializer-test merak-context-pipeline-test -j8 && ctest --test-dir build -R "merak-context" --output-on-failure` +Expected: All context tests pass. + +- [ ] **Step 8: Run all tests for final regression check** + +Run: `cmake --build build -j8 && ctest --test-dir build --output-on-failure` +Expected: All tests pass. + +- [ ] **Step 9: Commit** + +```bash +git add libs/context/src/context_pipeline.cpp \ + libs/context/tests/test_pipeline_hard_trim.cpp \ + tests/CMakeLists.txt +git commit -m "$(cat <<'EOF' +fix(context): make hard trim round-aware to preserve tool_use/tool_result pairing + +Replace per-message erase in hard trim with whole-round deletion (user-led +boundaries). Prevents orphaned tool_result blocks when token-budget trim +removes an assistant message containing tool_use. Re-scans round starts +each iteration to avoid index drift. + +Layer 2 of ISSUE #171 fix. +EOF +)" +``` + +--- + +## Task 4: Final verification + +- [ ] **Step 1: Run the complete test suite** + +Run: `cmake --build build -j8 && ctest --test-dir build --output-on-failure` +Expected: All tests pass, including the 3 new test executables. + +- [ ] **Step 2: Verify three separate commits exist** + +Run: `git log --oneline -5` +Expected: Three commits visible — sanitize_orphans (Layer 3), recent_history (Layer 1), hard trim (Layer 2), plus the spec commit. + +- [ ] **Step 3: Manual smoke test (optional, if dev server available)** + +If the user wants end-to-end verification, run the agent with a tool-heavy conversation that previously triggered the 400 error and confirm the error no longer occurs. The Layer 3 safety net should log `dropping orphan tool_result` warnings if any upstream path still produces orphans. From 6a31f9b1558e640f287eab00caede0f4de8376b8 Mon Sep 17 00:00:00 2001 From: ULookup Date: Mon, 22 Jun 2026 12:24:31 +0000 Subject: [PATCH 3/8] fix(context): add sanitize_orphans safety net in ContextSerializer Drops orphan tool_result at head and orphan tool_use at tail before serialization. Prevents Anthropic API HTTP 400 errors when upstream windowing or trimming produces unpaired tool messages. Layer 3 of ISSUE #171 fix. --- libs/context/src/context_serializer.cpp | 67 +++++++- .../context/tests/test_serializer_orphans.cpp | 154 ++++++++++++++++++ tests/CMakeLists.txt | 7 + 3 files changed, 227 insertions(+), 1 deletion(-) create mode 100644 libs/context/tests/test_serializer_orphans.cpp diff --git a/libs/context/src/context_serializer.cpp b/libs/context/src/context_serializer.cpp index 068eaf4c..d0a79ffb 100644 --- a/libs/context/src/context_serializer.cpp +++ b/libs/context/src/context_serializer.cpp @@ -1,8 +1,73 @@ #include #include +#include +#include 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 sanitize_orphans(std::vector msgs) { + std::set 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 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("")); + msgs.erase(msgs.begin() + static_cast(i)); + } else { + break; + } + } + + // Pass 2: last assistant's orphan tool_use + int last_assistant = -1; + for (int k = static_cast(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(last_assistant)); + } + } + } + + 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 +89,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 ────────────────────────────────────────────── diff --git a/libs/context/tests/test_serializer_orphans.cpp b/libs/context/tests/test_serializer_orphans.cpp new file mode 100644 index 00000000..4233e667 --- /dev/null +++ b/libs/context/tests/test_serializer_orphans.cpp @@ -0,0 +1,154 @@ +#include +#include +#include +#include +#include +#include +#include + +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, const std::string& tool_name = "read_file") { + Message m; m.role = "assistant"; m.content = text; + ToolCall tc; tc.id = call_id; tc.name = tool_name; 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 no_orphan_tool_results(const nlohmann::json& msgs) { + std::vector produced_ids; + for (const auto& m : msgs) { + const std::string role = m.value("role", ""); + if (m.contains("content") && m["content"].is_array()) { + for (const auto& blk : m["content"]) { + if (blk.value("type", "") == "tool_use") { + produced_ids.push_back(blk.value("id", "")); + } + } + } + if (role == "user" && m.contains("content") && m["content"].is_array()) { + for (const auto& blk : m["content"]) { + if (blk.value("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) return false; + } + } + } + } + return true; +} + +static bool no_tool_use_at_all(const nlohmann::json& msgs) { + for (const auto& m : msgs) { + if (m.contains("content") && m["content"].is_array()) { + for (const auto& blk : m["content"]) { + if (blk.value("type", "") == "tool_use") return false; + } + } + } + return true; +} + +int main() { + ContextSerializer serializer; + + // Test 1: Orphan tool_result at head is dropped (Anthropic) + { + BoundContext ctx; + ctx.provider_messages = { + make_tool_result("call_orphan_head", "stale result"), + make_user("hello"), + make_assistant_text("hi"), + }; + auto payload = serializer.serialize(ctx, "claude-sonnet-4-6", "", 1024); + assert(payload.is_anthropic); + const auto& msgs = payload.anthropic_json["messages"]; + assert(!msgs.empty()); + assert(msgs[0].value("role", "") == "user"); + assert(no_orphan_tool_results(msgs)); + std::cout << "Test 1 passed: orphan tool_result at head dropped (Anthropic)\n"; + } + + // Test 2: Orphan tool_use at tail is dropped (Anthropic) + { + BoundContext ctx; + ctx.provider_messages = { + make_user("do X"), + make_assistant_with_tool("", "call_orphan_tail"), + }; + auto payload = serializer.serialize(ctx, "claude-sonnet-4-6", "", 1024); + const auto& msgs = payload.anthropic_json["messages"]; + assert(no_tool_use_at_all(msgs)); + std::cout << "Test 2 passed: orphan tool_use at tail dropped (Anthropic)\n"; + } + + // Test 3: Paired tool_use preserved + { + BoundContext ctx; + ctx.provider_messages = { + make_user("do X"), + make_assistant_with_tool("", "call_ok"), + make_tool_result("call_ok", "result"), + }; + auto payload = serializer.serialize(ctx, "claude-sonnet-4-6", "", 1024); + const auto& msgs = payload.anthropic_json["messages"]; + assert(no_orphan_tool_results(msgs)); + assert(msgs.size() == 3); + std::cout << "Test 3 passed: paired tool_use preserved\n"; + } + + // Test 4: Multiple orphan tool_results at head all dropped + { + BoundContext ctx; + ctx.provider_messages = { + make_tool_result("orphan_1", "r1"), + make_tool_result("orphan_2", "r2"), + make_user("hello"), + make_assistant_text("hi"), + }; + auto payload = serializer.serialize(ctx, "claude-sonnet-4-6", "", 1024); + const auto& msgs = payload.anthropic_json["messages"]; + assert(msgs[0].value("role", "") == "user"); + assert(no_orphan_tool_results(msgs)); + std::cout << "Test 4 passed: multiple orphan tool_results at head all dropped\n"; + } + + // Test 5: OpenAI format also has no orphan tool messages + { + BoundContext ctx; + ctx.provider_messages = { + make_tool_result("call_orphan_openai", "stale"), + make_user("hello"), + make_assistant_text("hi"), + }; + auto payload = serializer.serialize(ctx, "gpt-4o", "", 1024); + assert(!payload.is_anthropic); + const auto& msgs = payload.openai_json["messages"]; + bool found_first_non_system = false; + for (const auto& m : msgs) { + if (m.value("role", "") == "system") continue; + assert(m.value("role", "") != "tool"); + found_first_non_system = true; + break; + } + assert(found_first_non_system); + std::cout << "Test 5 passed: OpenAI format also has no orphan tool at head\n"; + } + + std::cout << "All ContextSerializer orphan tests passed.\n"; + return 0; +} diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index ec900544..a1545f91 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -21,6 +21,13 @@ add_executable(merak-context-test target_link_libraries(merak-context-test PRIVATE merak-context) add_test(NAME merak-context-test COMMAND merak-context-test) +# Context serializer orphan safety net tests +add_executable(merak-context-serializer-test + ${CMAKE_SOURCE_DIR}/libs/context/tests/test_serializer_orphans.cpp +) +target_link_libraries(merak-context-serializer-test PRIVATE merak-context) +add_test(NAME merak-context-serializer-test COMMAND merak-context-serializer-test) + add_executable(merak-storage-test ${CMAKE_SOURCE_DIR}/libs/storage/tests/test_storage.cpp ) From 001da48b0f84671b379904207a605dd42fc5615f Mon Sep 17 00:00:00 2001 From: ULookup Date: Mon, 22 Jun 2026 12:47:58 +0000 Subject: [PATCH 4/8] fix(memory): make recent_history round-aware to prevent orphan tool_results Rewrite recent_history to slice on user-message boundaries instead of naive max_turns*2 estimate. Adds adjust_for_orphan_tools helper that expands the window to cover a parent assistant when nearby, or drops leading orphan tool messages when the parent is too far. Layer 1 of ISSUE #171 fix. --- libs/memory/include/merak/memory_store.hpp | 7 ++ libs/memory/src/memory_store.cpp | 75 +++++++++++- libs/memory/tests/test_memory_history.cpp | 127 +++++++++++++++++++++ tests/CMakeLists.txt | 7 ++ 4 files changed, 213 insertions(+), 3 deletions(-) create mode 100644 libs/memory/tests/test_memory_history.cpp diff --git a/libs/memory/include/merak/memory_store.hpp b/libs/memory/include/merak/memory_store.hpp index 057af9fa..48f1413f 100644 --- a/libs/memory/include/merak/memory_store.hpp +++ b/libs/memory/include/merak/memory_store.hpp @@ -72,6 +72,13 @@ class MemoryStore { pqxx::connection open_conn() const; std::expected create_tables(); + + // Adjusts the start index forward if the window begins with orphan tool + // messages whose parent assistant is missing or too far away. + // - Expands window backward if parent assistant is within max_turns*2 + 4. + // - Otherwise drops the leading orphan tool messages. + static int adjust_for_orphan_tools(const std::vector& msgs, + int start, int max_turns); }; } // namespace merak diff --git a/libs/memory/src/memory_store.cpp b/libs/memory/src/memory_store.cpp index 13bab7e0..218116c6 100644 --- a/libs/memory/src/memory_store.cpp +++ b/libs/memory/src/memory_store.cpp @@ -27,9 +27,26 @@ void MemoryStore::append_message(const Message& msg) { std::vector MemoryStore::recent_history(int max_turns) const { std::lock_guard lock(working_memory_mutex_); - int msg_count = max_turns * 2; - int total = (int)working_memory_.size(); - int start = std::max(0, total - msg_count); + int total = static_cast(working_memory_.size()); + if (total == 0 || max_turns <= 0) return {}; + + // 1. Collect user message indices (round boundaries) + std::vector user_indices; + for (int i = 0; i < total; i++) { + if (working_memory_[i].role == "user") user_indices.push_back(i); + } + + int start = 0; + if (!user_indices.empty()) { + int keep_rounds = std::min(max_turns, static_cast(user_indices.size())); + if (keep_rounds <= 0) return {}; + start = user_indices[static_cast(user_indices.size()) - keep_rounds]; + } else { + // No user messages — fall back to last max_turns*2 messages + start = std::max(0, total - max_turns * 2); + } + + start = adjust_for_orphan_tools(working_memory_, start, max_turns); std::vector result; for (int i = start; i < total; i++) { @@ -38,6 +55,58 @@ std::vector MemoryStore::recent_history(int max_turns) const { return result; } +int MemoryStore::adjust_for_orphan_tools(const std::vector& msgs, + int start, int max_turns) { + if (start >= static_cast(msgs.size())) return start; + if (msgs[start].role != "tool") return start; + + // Collect leading orphan tool ids + std::vector orphan_ids; + int probe = start; + while (probe < static_cast(msgs.size()) && msgs[probe].role == "tool") { + if (msgs[probe].tool_call_id) { + orphan_ids.push_back(*msgs[probe].tool_call_id); + } + probe++; + } + if (orphan_ids.empty()) return start; + + // Search backward for the most recent assistant with matching tool_calls + int parent_idx = -1; + for (int i = start - 1; i >= 0; i--) { + if (msgs[i].role != "assistant") continue; + bool covers_all = true; + for (const auto& oid : orphan_ids) { + bool found = false; + for (const auto& tc : msgs[i].tool_calls) { + if (tc.id == oid) { found = true; break; } + } + if (!found) { covers_all = false; break; } + } + if (covers_all) { parent_idx = i; break; } + } + + const int max_distance = max_turns * 2 + 4; + if (parent_idx >= 0 && (start - parent_idx) <= max_distance) { + spdlog::debug("MemoryStore: expanded window from {} to {} to cover " + "orphan tool_result ({} ids)", + start, parent_idx, orphan_ids.size()); + return parent_idx; + } + + // Drop leading orphan tool messages + int new_start = start; + while (new_start < static_cast(msgs.size()) && + msgs[new_start].role == "tool") { + new_start++; + } + spdlog::warn("MemoryStore: dropped {} orphan tool messages at head " + "(max_turns={}, parent_distance={})", + new_start - start, max_turns, + parent_idx >= 0 ? (start - parent_idx) : -1); + return new_start; +} + int MemoryStore::message_count() const { std::lock_guard lock(working_memory_mutex_); return (int)working_memory_.size(); diff --git a/libs/memory/tests/test_memory_history.cpp b/libs/memory/tests/test_memory_history.cpp new file mode 100644 index 00000000..24d093f4 --- /dev/null +++ b/libs/memory/tests/test_memory_history.cpp @@ -0,0 +1,127 @@ +#include +#include +#include +#include +#include + +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; +} + +// Builds a MemoryStore with disabled DB (no PostgreSQL needed). +static std::unique_ptr make_store() { + MemoryConfig cfg; + cfg.enabled = false; + return std::make_unique(cfg, nullptr); +} + +static bool starts_with_user(const std::vector& msgs) { + return !msgs.empty() && msgs.front().role == "user"; +} + +static bool no_orphan_tool_at_head(const std::vector& msgs) { + if (msgs.empty()) return true; + if (msgs.front().role != "tool") return true; + return false; +} + +int main() { + // Test 1: No tool calls — returns last N turns + { + auto store = make_store(); + for (int i = 0; i < 5; i++) { + store->append_message(make_user("u" + std::to_string(i))); + store->append_message(make_assistant_text("a" + std::to_string(i))); + } + auto hist = store->recent_history(3); + assert(hist.size() == 6); + assert(hist[0].content == "u2"); + assert(hist[5].content == "a4"); + std::cout << "Test 1 passed: no tool calls, last N turns\n"; + } + + // Test 2: With tool calls — starts on user boundary + { + auto store = make_store(); + store->append_message(make_user("u1")); + store->append_message(make_assistant_with_tool("", "call_X")); + store->append_message(make_tool_result("call_X", "r1")); + store->append_message(make_user("u2")); + store->append_message(make_assistant_with_tool("", "call_Y")); + store->append_message(make_tool_result("call_Y", "r2")); + store->append_message(make_user("u3")); + store->append_message(make_assistant_text("a3")); + + auto hist = store->recent_history(2); + assert(starts_with_user(hist)); + assert(hist[0].content == "u2"); + std::cout << "Test 2 passed: with tool calls, starts on user boundary\n"; + } + + // Test 3: Orphan tool at head — expands to parent assistant + { + auto store = make_store(); + store->append_message(make_user("u_old")); + store->append_message(make_assistant_with_tool("", "call_Z")); + store->append_message(make_tool_result("call_Z", "rZ")); + store->append_message(make_user("u_recent")); + store->append_message(make_assistant_text("a_recent")); + + auto hist = store->recent_history(1); + assert(no_orphan_tool_at_head(hist)); + assert(hist[0].content == "u_recent"); + std::cout << "Test 3 passed: well-formed rounds, no orphan at head\n"; + } + + // Test 4: Orphan tool at head, parent too far — drops orphan + { + auto store = make_store(); + store->append_message(make_assistant_with_tool("", "call_far")); + for (int i = 0; i < 20; i++) { + store->append_message(make_assistant_text("filler" + std::to_string(i))); + } + store->append_message(make_tool_result("call_far", "orphan_result")); + store->append_message(make_assistant_text("tail")); + + auto hist = store->recent_history(2); + assert(no_orphan_tool_at_head(hist)); + std::cout << "Test 4 passed: orphan tool at head with far parent, dropped\n"; + } + + // Test 5: Empty memory — returns empty + { + auto store = make_store(); + auto hist = store->recent_history(5); + assert(hist.empty()); + std::cout << "Test 5 passed: empty memory returns empty\n"; + } + + // Test 6: No user messages — fallback to tail with orphan handling + { + auto store = make_store(); + store->append_message(make_assistant_with_tool("", "call_nouser")); + store->append_message(make_tool_result("call_nouser", "r")); + store->append_message(make_assistant_text("tail")); + auto hist = store->recent_history(2); + assert(no_orphan_tool_at_head(hist)); + std::cout << "Test 6 passed: no user messages, fallback handles orphans\n"; + } + + std::cout << "All MemoryStore recent_history tests passed.\n"; + return 0; +} diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index a1545f91..bae215b9 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -110,3 +110,10 @@ add_executable(merak-sub-agent-runner-test ) target_link_libraries(merak-sub-agent-runner-test PRIVATE merak-loop) add_test(NAME merak-sub-agent-runner-test COMMAND merak-sub-agent-runner-test) + +# MemoryStore recent_history tests (no DB) +add_executable(merak-memory-history-test + ${CMAKE_SOURCE_DIR}/libs/memory/tests/test_memory_history.cpp +) +target_link_libraries(merak-memory-history-test PRIVATE merak-memory) +add_test(NAME merak-memory-history-test COMMAND merak-memory-history-test) From b1afb05d75d1c0c56f24fce43ca1912eb69d3dfe Mon Sep 17 00:00:00 2001 From: ULookup Date: Mon, 22 Jun 2026 13:01:16 +0000 Subject: [PATCH 5/8] test(memory): fix Test 4 and Test 6 to actually exercise orphan paths Previous Test 4 and Test 6 passed trivially because the window never started on a tool message. Rewrote scenarios to force start onto a tool_result, triggering the drop path (Test 4) and expand path (Test 6). --- libs/memory/tests/test_memory_history.cpp | 45 ++++++++++++++++------- 1 file changed, 32 insertions(+), 13 deletions(-) diff --git a/libs/memory/tests/test_memory_history.cpp b/libs/memory/tests/test_memory_history.cpp index 24d093f4..d10521d2 100644 --- a/libs/memory/tests/test_memory_history.cpp +++ b/libs/memory/tests/test_memory_history.cpp @@ -73,7 +73,7 @@ int main() { std::cout << "Test 2 passed: with tool calls, starts on user boundary\n"; } - // Test 3: Orphan tool at head — expands to parent assistant + // Test 3: Well-formed rounds — no orphan at head { auto store = make_store(); store->append_message(make_user("u_old")); @@ -91,15 +91,24 @@ int main() { // Test 4: Orphan tool at head, parent too far — drops orphan { auto store = make_store(); - store->append_message(make_assistant_with_tool("", "call_far")); - for (int i = 0; i < 20; i++) { - store->append_message(make_assistant_text("filler" + std::to_string(i))); + // No user messages — fallback path will be used. + // Parent assistant at idx 0, then many fillers, then orphan tool_result at idx 11. + // With max_turns=1: start = max(0, 13 - 2) = 11 → tool_result! Orphan at head. + // Parent distance = 11. max_turns*2 + 4 = 6. 11 > 6 → DROP path triggered. + store->append_message(make_assistant_with_tool("", "call_far")); // idx 0 + for (int i = 0; i < 10; i++) { + store->append_message(make_assistant_text("filler" + std::to_string(i))); // idx 1-10 } - store->append_message(make_tool_result("call_far", "orphan_result")); - store->append_message(make_assistant_text("tail")); + store->append_message(make_tool_result("call_far", "orphan_result")); // idx 11 + store->append_message(make_assistant_text("tail")); // idx 12 + // total = 13, max_turns = 1, start = 13 - 2 = 11 (tool message) - auto hist = store->recent_history(2); + auto hist = store->recent_history(1); assert(no_orphan_tool_at_head(hist)); + // The orphan tool_result should be dropped; hist should start with "tail" + assert(!hist.empty()); + assert(hist[0].role == "assistant"); + assert(hist[0].content == "tail"); std::cout << "Test 4 passed: orphan tool at head with far parent, dropped\n"; } @@ -111,15 +120,25 @@ int main() { std::cout << "Test 5 passed: empty memory returns empty\n"; } - // Test 6: No user messages — fallback to tail with orphan handling + // Test 6: No user messages — fallback expands to nearby parent assistant { auto store = make_store(); - store->append_message(make_assistant_with_tool("", "call_nouser")); - store->append_message(make_tool_result("call_nouser", "r")); - store->append_message(make_assistant_text("tail")); - auto hist = store->recent_history(2); + // Parent assistant at idx 0, orphan tool_result at idx 1, tail at idx 2. + // max_turns=1: start = max(0, 3 - 2) = 1 → tool_result! Orphan at head. + // Parent distance = 1. max_turns*2 + 4 = 6. 1 <= 6 → EXPAND path triggered. + // Should expand to include parent assistant at idx 0. + store->append_message(make_assistant_with_tool("", "call_nouser")); // idx 0 + store->append_message(make_tool_result("call_nouser", "r")); // idx 1 + store->append_message(make_assistant_text("tail")); // idx 2 + + auto hist = store->recent_history(1); assert(no_orphan_tool_at_head(hist)); - std::cout << "Test 6 passed: no user messages, fallback handles orphans\n"; + // Should have expanded to include the parent assistant. + // hist[0] should be the parent assistant (idx 0). + assert(hist.size() == 3); + assert(hist[0].role == "assistant"); + assert(!hist[0].tool_calls.empty()); + std::cout << "Test 6 passed: no user messages, fallback expands to nearby parent\n"; } std::cout << "All MemoryStore recent_history tests passed.\n"; From 3ea265d8281dfed51229926625017ec206136668 Mon Sep 17 00:00:00 2001 From: ULookup Date: Mon, 22 Jun 2026 13:31:56 +0000 Subject: [PATCH 6/8] fix(context): make hard trim round-aware to preserve tool_use/tool_result pairing Replace per-message erase in hard trim with whole-round deletion (user-led boundaries). Prevents orphaned tool_result blocks when token-budget trim removes an assistant message containing tool_use. Re-scans round starts each iteration to avoid index drift. Layer 2 of ISSUE #171 fix. --- libs/context/src/context_pipeline.cpp | 39 ++++-- .../context/tests/test_pipeline_hard_trim.cpp | 127 ++++++++++++++++++ tests/CMakeLists.txt | 7 + 3 files changed, 159 insertions(+), 14 deletions(-) create mode 100644 libs/context/tests/test_pipeline_hard_trim.cpp diff --git a/libs/context/src/context_pipeline.cpp b/libs/context/src/context_pipeline.cpp index 97d2702d..8cddf48e 100644 --- a/libs/context/src/context_pipeline.cpp +++ b/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,38 @@ SerializedPayload ContextPipeline::planned_assemble( } opt_stats.tokens_after += static_cast(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(msgs[target].content.size() / 3.5); - msgs.erase(msgs.begin() + static_cast(target)); - removed++; + while (opt_stats.tokens_after > model_max_tokens) { + std::vector 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]; + int chars = 0; + for (size_t i = rs[0]; i < del_end; i++) { + chars += static_cast(msgs[i].content.size()); + } + opt_stats.tokens_after -= chars / 3.5; + msgs.erase(msgs.begin() + static_cast(rs[0]), + msgs.begin() + static_cast(del_end)); + removed += static_cast(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; diff --git a/libs/context/tests/test_pipeline_hard_trim.cpp b/libs/context/tests/test_pipeline_hard_trim.cpp new file mode 100644 index 00000000..a02fca39 --- /dev/null +++ b/libs/context/tests/test_pipeline_hard_trim.cpp @@ -0,0 +1,127 @@ +#include +#include +#include +#include +#include +#include + +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& msgs) { + std::vector 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 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(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 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(!payload.messages.empty()); + 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 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); + std::vector 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"; + } + + std::cout << "All ContextPipeline hard trim tests passed.\n"; + return 0; +} diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index bae215b9..1b3b2aa0 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -117,3 +117,10 @@ add_executable(merak-memory-history-test ) target_link_libraries(merak-memory-history-test PRIVATE merak-memory) add_test(NAME merak-memory-history-test COMMAND merak-memory-history-test) + +# ContextPipeline hard trim tests +add_executable(merak-context-pipeline-test + ${CMAKE_SOURCE_DIR}/libs/context/tests/test_pipeline_hard_trim.cpp +) +target_link_libraries(merak-context-pipeline-test PRIVATE merak-context) +add_test(NAME merak-context-pipeline-test COMMAND merak-context-pipeline-test) From a16aeb00438e033f73d1344b74b3af213837d6e4 Mon Sep 17 00:00:00 2001 From: ULookup Date: Mon, 22 Jun 2026 13:46:03 +0000 Subject: [PATCH 7/8] fix(context): correct tokens_after bookkeeping and assert hard trim triggered - Per-message token truncation in hard trim matches initial computation, eliminates negative tokens_after artifact in logs. - Add pipeline.stats().hard_trims > 0 assertion to all 3 test cases so they verify hard trim actually fired (not just that output is clean). - Strengthen Test 2 to also assert front message is a user (round boundary). --- libs/context/src/context_pipeline.cpp | 4 +--- libs/context/tests/test_pipeline_hard_trim.cpp | 4 ++++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/libs/context/src/context_pipeline.cpp b/libs/context/src/context_pipeline.cpp index 8cddf48e..e16fd914 100644 --- a/libs/context/src/context_pipeline.cpp +++ b/libs/context/src/context_pipeline.cpp @@ -94,11 +94,9 @@ SerializedPayload ContextPipeline::planned_assemble( if (rs.size() <= 1) break; // preserve at least one round size_t del_end = rs[1]; - int chars = 0; for (size_t i = rs[0]; i < del_end; i++) { - chars += static_cast(msgs[i].content.size()); + opt_stats.tokens_after -= static_cast(msgs[i].content.size() / 3.5); } - opt_stats.tokens_after -= chars / 3.5; msgs.erase(msgs.begin() + static_cast(rs[0]), msgs.begin() + static_cast(del_end)); removed += static_cast(del_end - rs[0]); diff --git a/libs/context/tests/test_pipeline_hard_trim.cpp b/libs/context/tests/test_pipeline_hard_trim.cpp index a02fca39..91f23bb8 100644 --- a/libs/context/tests/test_pipeline_hard_trim.cpp +++ b/libs/context/tests/test_pipeline_hard_trim.cpp @@ -59,6 +59,7 @@ int main() { 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"); @@ -78,7 +79,9 @@ int main() { 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"; } @@ -98,6 +101,7 @@ int main() { 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 produced_ids; const auto& msgs = payload.anthropic_json["messages"]; bool ok = true; From cddc570b5623dea879144c2aace84e2893372dcc Mon Sep 17 00:00:00 2001 From: ULookup Date: Mon, 22 Jun 2026 14:17:17 +0000 Subject: [PATCH 8/8] =?UTF-8?q?fix(context):=20close=20spec=20gaps=20?= =?UTF-8?q?=E2=80=94=20system=20msg=20test,=20sanitize=5Forphans=20try/cat?= =?UTF-8?q?ch,=20spec=20update?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add Test 4: DoesNotDeleteSystemMessages to test_pipeline_hard_trim.cpp (spec-required test case, was missing). - Wrap sanitize_orphans in try/catch per spec line 318: safety net must not crash AgentLoop on unexpected exceptions. - Document the serialize() move in spec: Layer 2 fix moves serialize() after hard trim to fix latent no-op bug where hard trim mutated bound.provider_messages after payload was already built. --- ...2-issue-171-orphaned-tool-result-design.md | 6 ++ libs/context/src/context_serializer.cpp | 86 ++++++++++--------- .../context/tests/test_pipeline_hard_trim.cpp | 29 +++++++ 3 files changed, 81 insertions(+), 40 deletions(-) diff --git a/docs/superpowers/specs/2026-06-22-issue-171-orphaned-tool-result-design.md b/docs/superpowers/specs/2026-06-22-issue-171-orphaned-tool-result-design.md index d3490589..9106e969 100644 --- a/docs/superpowers/specs/2026-06-22-issue-171-orphaned-tool-result-design.md +++ b/docs/superpowers/specs/2026-06-22-issue-171-orphaned-tool-result-design.md @@ -183,6 +183,12 @@ if (opt_stats.tokens_after > model_max_tokens) { 每次循环重新扫描 O(n)，最坏 O(n²)。hard trim 是异常路径（正常 pipeline 不触发），可接受。 +### 隐含修复：`serialize()` 调用位置 + +原代码中 `serializer_.serialize()` 在 hard trim **之前**调用（line 72），hard trim 在 **之后**修改 `bound.provider_messages`（lines 84-101）。由于 `payload` 已经构建，hard trim 对实际发往 API 的 payload 是 **no-op** —— 只更新 `stats_.hard_trims` 和 `opt_stats.tokens_after`，不影响序列化输出。 + +Layer 2 修复将 `serialize()` 移到 hard trim **之后**，使 hard trim 真正影响 payload。这是修复 latent bug 的必要改动，但意味着 hard trim 首次在生产中实际生效。Layer 3 安全网作为兜底，即使 round-aware 逻辑有边界情况 bug，也会在序列化前丢弃孤儿。 + ### 保留行为 - 跳过 system 消息：`round_starts` 不计入 system（role != "user"）。第一轮 user 前的 system 消息不会被删。 diff --git a/libs/context/src/context_serializer.cpp b/libs/context/src/context_serializer.cpp index d0a79ffb..6d5ffdae 100644 --- a/libs/context/src/context_serializer.cpp +++ b/libs/context/src/context_serializer.cpp @@ -14,56 +14,62 @@ namespace { // - 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 sanitize_orphans(std::vector msgs) { - std::set produced_ids; - for (const auto& m : msgs) { - if (m.role == "assistant") { - for (const auto& tc : m.tool_calls) produced_ids.insert(tc.id); + try { + std::set 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 referenced_ids; - for (const auto& m : msgs) { - if (m.role == "tool" && m.tool_call_id) { - referenced_ids.insert(*m.tool_call_id); + std::set 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("")); - msgs.erase(msgs.begin() + static_cast(i)); - } else { - break; + // 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("")); + msgs.erase(msgs.begin() + static_cast(i)); + } else { + break; + } } - } - // Pass 2: last assistant's orphan tool_use - int last_assistant = -1; - for (int k = static_cast(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; } + // Pass 2: last assistant's orphan tool_use + int last_assistant = -1; + for (int k = static_cast(msgs.size()) - 1; k >= 0; k--) { + if (msgs[k].role == "assistant") { last_assistant = k; 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(last_assistant)); + 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(last_assistant)); + } } } - } - return msgs; + return msgs; + } catch (const std::exception& e) { + spdlog::error("ContextSerializer: sanitize_orphans failed, returning " + "messages unchanged: {}", e.what()); + return msgs; + } } } // anonymous namespace diff --git a/libs/context/tests/test_pipeline_hard_trim.cpp b/libs/context/tests/test_pipeline_hard_trim.cpp index 91f23bb8..916d18e4 100644 --- a/libs/context/tests/test_pipeline_hard_trim.cpp +++ b/libs/context/tests/test_pipeline_hard_trim.cpp @@ -126,6 +126,35 @@ int main() { 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 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; }