Skip to content

test(trace): bind replay results by exact tool call #6659

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
serrrfirat merged 1 commit into from
Jul 27, 2026
Merged

test(trace): bind replay results by exact tool call #6659

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions crates/ironclaw_llm/CLAUDE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,3 +286,6 @@ Providers in this crate import it as `use ironclaw_common::llm_costs as costs;`
## Trace Recording

Set `IRONCLAW_RECORD_TRACE=1` to enable live trace recording via `RecordingLlm`. Traces are JSON files containing: memory snapshot, HTTP exchanges from tools, and LLM steps (user inputs, text responses, tool call responses). Replay these in E2E tests via `TraceLlm`. Configure output path with `IRONCLAW_TRACE_OUTPUT` (default: `trace_{timestamp}.json`).

Arguments derived from an earlier tool result use an exact `$trace_result`
marker with the original `tool_call_id` and an RFC 6901 JSON Pointer.
Comment on lines +289 to +291

@coderabbitai coderabbitai Bot Jul 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Soften the absolute claim to mention the legacy fallback.

This states unqualified that tool-result-derived arguments "use an exact $trace_result marker," but parameterize_value in recording.rs still falls back to the legacy {{call_id.field}} template when a match is ambiguous or the source is a sanitized user message without a call ID. As written, this overstates the guarantee.

As per coding guidelines, **/*.{rs,md}: "Comments promising cross-layer guarantees must be enforced by code or tests, or softened to describe intent."

📝 Proposed doc fix 
-Arguments derived from an earlier tool result use an exact `$trace_result`
-marker with the original `tool_call_id` and an RFC 6901 JSON Pointer.
+Arguments that uniquely match an earlier native tool result use an exact
+`$trace_result` marker with the original `tool_call_id` and an RFC 6901 JSON
+Pointer. Ambiguous matches and sanitized (call-ID-less) tool results still
+fall back to the legacy `{{call_id.field}}` template.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Arguments derived from an earlier tool result use an exact `$trace_result`
marker with the original `tool_call_id` and an RFC 6901 JSON Pointer.
Arguments that uniquely match an earlier native tool result use an exact
`$trace_result` marker with the original `tool_call_id` and an RFC 6901 JSON
Pointer. Ambiguous matches and sanitized (call-ID-less) tool results still
fall back to the legacy `{{call_id.field}}` template.
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/ironclaw_llm/CLAUDE.md` around lines 289 - 291, Update the
documentation statement around tool-result-derived arguments to acknowledge the
legacy `{{call_id.field}}` fallback used by `parameterize_value` in
`recording.rs` when matching is ambiguous or no call ID is available. Soften the
absolute guarantee about the exact `$trace_result` marker while preserving the
documented preferred behavior and RFC 6901 pointer details.

Source: Coding guidelines

1 change: 1 addition & 0 deletions crates/ironclaw_llm/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ pub mod runtime;
pub mod session;
pub mod smart_routing;
mod token_refreshing;
pub mod trace_binding;
// arch-exempt: scaffolding, Phase A helpers awaiting first per-provider caller, plan #4522
// Remove the allow once any production call site references these items.
#[allow(dead_code)]
Expand Down
Loading
Loading