Files
ironclaw/tests/support/replay_outcome.rs
Illia Polosukhin ff119531d4 test(replay): promote engine traces to insta-backed snapshot gate (#2621)
* test(replay): promote engine replay traces to insta-backed snapshot gate

Adds a ReplayOutcome snapshot type, a replay-gate CI workflow, and a
developer script wrapper for cargo-insta. Replaces unreviewable 3,000-line
JSON diffs on engine changes with a YAML snapshot of the observable run
shape (tool sequence, final state, retrospective analyzer issues).

Why: engine v2 live-fixture traces had grown past reviewability. A single
prompt-wording change could move the whole fixture, and reviewers had no
way to see which behaviour actually changed. Splitting the fixture into a
"replay driver" (JSON stays in tests/fixtures/) and a "regression
snapshot" (YAML in tests/snapshots/) gives reviewers a narrow, stable diff
to approve, while keeping the full recorded context for deterministic
replay.

Changes:
- `tests/support/replay_outcome.rs` — ReplayOutcome + assert_replay_snapshot!
  macro; snapshots include retrospective analyzer output (TraceIssue
  severity/category) via a new `ironclaw::bridge::engine_retrospectives_for_test()`
  helper that runs `build_trace()` over engine threads
- `tests/e2e_engine_v2.rs` — three POC snapshot tests
  (single_tool_echo, tool_error_recovery, zizmor_scan_v2)
- `tests/e2e_bug_bash_snapshots.rs` + `tests/fixtures/llm_traces/bug_bash/`
  — bug-regression fixture template, mapped to open issues in the README
- `.github/workflows/replay-gate.yml` — cargo insta test --check on
  engine/agent/LLM/tools/bridge path changes; rejects committed .snap.new
- `scripts/replay-snap.sh` — review/accept/test/record wrappers around
  cargo-insta and IRONCLAW_RECORD_TRACE
- `scripts/trace-coverage.sh` — reports EventKind variants with
  snapshot coverage; `--strict` mode for future CI promotion
- `tests/e2e_live.rs` — `#[ignore]` swapped for
  `cfg_attr(not(feature="replay"), ignore)` so the replay CI job can
  run the scenarios without `-- --ignored`
- `Cargo.toml` — new `replay = ["libsql"]` feature; insta gains
  the `yaml` feature
- `tests/fixtures/llm_traces/README.md` — documents the two-role
  driver/snapshot split

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(replay): address PR #2621 review + swap cargo-insta installer

Review fixes:

- Replay gate was missing the bug-bash snapshot suite. Adds
  `tests/e2e_bug_bash_snapshots.rs` to the workflow paths trigger and the
  `cargo insta test --check` invocation so bug-regression snapshots are
  actually gated. (copilot-pull-request-reviewer)

- `cargo install cargo-insta --locked` added ~40s of cold-cache compile
  to the gate. Swapped for `taiki-e/install-action@v2`, which downloads
  a precompiled binary in a few seconds. Also updated
  `scripts/replay-snap.sh` to *fail closed* when cargo-insta is missing
  instead of silently auto-installing it. (gemini-code-assist)

- `engine_retrospectives_for_test` was `pub` and re-exported under the
  default-enabled `libsql` feature, contradicting its "not part of any
  public API" doc. Split the re-export, kept `reset_engine_state` as a
  plain `pub use`, and hid `engine_retrospectives_for_test` behind
  `#[doc(hidden)]` — it still needs to cross the crate boundary for
  integration tests (which live in a separate crate, so `#[cfg(test)]`
  doesn't reach them), but no longer appears in published docs.
  (copilot-pull-request-reviewer)

- Added an explicit "caller must serialize" note on
  `engine_retrospectives_for_test` explaining the `ENGINE_STATE`
  singleton and pointing new callers at `engine_v2_test_lock()` /
  `reset_engine_state()`. Matches what the existing snapshot tests
  already do. (gemini-code-assist)

Doc corrections:

- `snapshot_zizmor_scan_v2` doc claimed the snapshot pinned
  `ApprovalNeeded` events and response wording — it doesn't. Rewrote to
  describe what the snapshot actually asserts (tool order, step count,
  retrospective issues, final state). (copilot-pull-request-reviewer)

- `llm_call_count` was documented as "bucketed" but passed through
  verbatim. Updated the field doc to reflect the raw value. Bucketing
  wasn't needed because fixtures are deterministic. (copilot-pull-request-reviewer)

- `src/bridge/router.rs` doc referenced a non-existent
  `ReplayOutcome.trace_issues` field — the struct uses `engine_threads`.
  Fixed the reference. (copilot-pull-request-reviewer)

- `scripts/trace-coverage.sh` header claimed CI runs it with `--strict`;
  the workflow runs it in advisory mode. Rewrote the header to match,
  with a pointer for when to promote to strict. (copilot-pull-request-reviewer)

No-change replies (rationale commented in the code):

- `event_kind_name` uses an exhaustive `match` on `EventKind` rather
  than `Debug` or a `strum` derive. The compile-time exhaustiveness
  check is the point — adding a new engine event should force a
  conscious decision about how the snapshot represents it, not a silent
  fallthrough. Added a comment making that intent explicit.

- `trace-coverage.sh` awk parser of `event.rs` is fragile — agreed, but
  the script is advisory and its failure mode is false negatives
  (uncovered variants simply aren't gated). Documented the tradeoff and
  the rewrite-in-Rust escape hatch in the script header.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* ci(replay-gate): prime cache on staging, restrict PR runs to read-only

The second run on PR #2621 missed the cache ("No cache found" in the
rust-cache restore step) even though the workflow is wired correctly.
Root cause: the repo sits close to GitHub's 10 GB per-repo cache quota
(~59 entries, many >500 MB), and the LRU policy evicts PR-scoped caches
before they get reused.

Fix:
- Add `push: [staging, main]` so the gate runs (and saves a ~1.2 GB
  cache under the `replay-gate` key) on every merge to the branches
  PRs actually target. Subsequent PRs restore from that base-branch
  cache — GitHub Actions permits cross-ref restore when the restoring
  ref's base matches the saved ref.
- Set `save-if: ${{ github.event_name == 'push' }}` so PR runs only
  *read* the cache. Without this gate, each PR push would save its
  own copy and crowd out the primed base-branch cache, putting us
  right back in the eviction loop.

Expected effect: cold-cache 9m → warm ~2-3m once staging has a run with
the new workflow. Base-branch prime run still pays 9m (no regression).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(replay): drop bug-bash fixture scaffolding

Replay fixtures can't reproduce the Phase 3 target bugs because the
fixture *is* the LLM's output — handwriting a trace where the LLM
emits a tool call doesn't test whether the real LLM would have emitted
that call, only that the harness dispatches a scripted one. What
`summarization_uses_tools.json` actually pinned was the happy path,
not the #2541 bug.

Of the 7 open bug-bash issues, only #2544 ("plans and delegates but
never executes") is catchable by replay, and only via a live-recorded
fixture. The other six are LLM-behavior or infra-timing bugs outside
replay's reach. Rather than ship regression theater, tear out the
scaffolding.

Removed:
- tests/e2e_bug_bash_snapshots.rs
- tests/fixtures/llm_traces/bug_bash/
- tests/snapshots/replay__bug_bash_summarization_uses_tools.snap

Unwired:
- Replay-gate workflow paths + test list no longer mention bug_bash
- scripts/replay-snap.sh test command drops the extra --test flag

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* ci: switch to cargo-nextest with per-test timeouts

Nextest runs each integration test in its own process and runs test
binaries in parallel, which is a big unlock for this repo:

- Engine v2 tests share a process-global `ENGINE_STATE` singleton
  (OnceLock), which the current test lock serialises inside a single
  test binary. Nextest's process-per-test model gives each test a
  clean state automatically, so the 16 engine_v2 tests stop running
  one-by-one.

- Cross-binary parallelism: `cargo test --test A --test B` runs
  binaries in sequence; nextest runs them concurrently.

Measured locally: the replay-gate test set (3 binaries, 21 tests)
went from ~30s sequential to **2.7s parallel**.

Adds `.config/nextest.toml` with:
- `slow-timeout = 60s / terminate-after 3` in the default profile so
  a hung test fails fast instead of blocking the workflow-level 25-
  minute cap.
- A `ci` profile with `fail-fast = false` (one flake shouldn't mask
  other failures), `failure-output = immediate-final`,
  `success-output = never` for readable Actions logs.
- Per-test 300s override for the handful of genuinely slow scenarios
  (zizmor scan, e2e_thread_scheduling).

Workflows updated:
- `replay-gate.yml`: installs cargo-nextest via taiki-e/install-action
  alongside cargo-insta (one step), runs `cargo insta test
  --test-runner nextest` with `NEXTEST_PROFILE=ci`.
- `test.yml`: all five `cargo test` invocations swapped for
  `cargo nextest run --profile ci`. Nextest doesn't execute doctests,
  so every nextest step is paired with a `cargo test --doc` follow-up
  to preserve coverage.

Local dev is unchanged — `cargo test` still works; nextest is only
required in CI.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* ci: re-trigger replay-gate workflow after nextest migration

Previous push only modified workflow files and `.config/nextest.toml`;
GitHub skipped the `pull_request` workflow events for that sync, so
the nextest migration didn't actually get exercised in CI. Empty
commit forces re-evaluation.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(replay): note nextest wiring in the fixtures README

Also forces a CI re-run: the previous empty commit had no matching
paths, so the `pull_request.paths` filters skipped every workflow
including replay-gate. Touching a file under
`tests/fixtures/llm_traces/**` re-matches the filter and runs the
nextest-based gate.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* ci(test): defer test.yml nextest migration

Staging restructured test.yml significantly while this PR was open
(matrix-config dynamic matrix, `changes` code-detection job,
composite install-cargo-component action, save-if restricted to
base-branch pushes). The merge into staging had heavy conflicts for
every nextest-swap hunk.

Rather than force a re-layering of the new staging structure on top
of the nextest migration in this PR, revert test.yml to staging's
current version. This PR now scopes the nextest change to just the
replay-gate workflow (where it cleanly demonstrates the value) plus
the shared `.config/nextest.toml` profile. Migrating the rest of
test.yml to nextest is a follow-up that can rebase on the new
structure without the heavy conflict surface.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Henry Park <henrypark133@gmail.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-19 17:34:01 +09:00

385 lines
14 KiB
Rust

//! Snapshot-oriented view of what a replay produced.
//!
//! A trace fixture plays two roles. The `.json` file is the **replay driver**:
//! recorded LLM responses and HTTP exchanges the harness uses to stub real
//! calls deterministically. The `.snap` file generated from this struct is the
//! **regression snapshot**: the observable output of replaying that fixture —
//! what tools fired, in what order, the final state, and any issues the
//! retrospective analyzer flagged.
//!
//! Reviewers diff the snapshot, not the raw JSON. Keep the struct narrow so
//! small prompt-wording changes don't force snapshot churn.
#![allow(dead_code)] // Consumed by snapshot tests gated by features.
use std::collections::BTreeMap;
use serde::Serialize;
use ironclaw::channels::{OutgoingResponse, StatusUpdate};
use crate::support::test_rig::TestRig;
/// Short summary of a single status event, ordered and typed for snapshot
/// review. Excludes wall-clock fields, request IDs, and full tool output —
/// those live in the replay driver JSON, not here.
#[derive(Debug, Serialize)]
#[serde(tag = "kind", rename_all = "snake_case")]
pub enum EventSummary {
Thinking {
message: String,
},
ToolStarted {
name: String,
},
ToolCompleted {
name: String,
success: bool,
error: Option<String>,
},
ToolResultPreview {
name: String,
/// Character length of the preview the UI received. The full preview
/// text lives in the replay driver and is sensitive to prompt drift —
/// using a bucketed length keeps snapshots stable.
preview_len_bucket: usize,
},
Status {
message: String,
},
ApprovalNeeded {
tool_name: String,
},
AuthRequired {
extension_name: String,
},
AuthCompleted {
extension_name: String,
success: bool,
},
Suggestions {
count: usize,
},
/// Anything else we haven't explicitly modelled. Kept as a bucketed
/// variant so unrelated new status kinds don't spam snapshot diffs.
Other {
variant: &'static str,
},
}
/// Summary of a single tool invocation that the rig observed.
#[derive(Debug, Serialize)]
pub struct ToolCallSummary {
pub name: String,
pub success: bool,
}
/// Summary of a retrospective issue the engine flagged.
#[derive(Debug, Serialize)]
pub struct TraceIssueSummary {
pub severity: String,
pub category: String,
}
/// Summary of one engine thread's post-run state.
#[derive(Debug, Serialize)]
pub struct ThreadSummary {
pub final_state: String,
pub step_count: usize,
pub message_roles: Vec<String>,
pub event_kinds: Vec<String>,
pub issues: Vec<TraceIssueSummary>,
}
/// Regression snapshot of a replay run.
///
/// Serialized as YAML by [`assert_replay_snapshot`]. Stable under LLM prompt
/// drift: it captures shape (tool sequence, final state, issue categories)
/// rather than model text.
#[derive(Debug, Serialize)]
pub struct ReplayOutcome {
/// Number of outbound text responses the channel received.
pub response_count: usize,
/// Whether any final response was produced. Cheap for reviewers to
/// interpret — `false` means the scenario hit a dead-end.
pub has_final_response: bool,
/// Tool invocations the channel observed, in order.
pub tool_calls: Vec<ToolCallSummary>,
/// Ordered status events, bucketed and trimmed for stability.
pub events: Vec<EventSummary>,
/// Histogram of status event kinds — makes coverage assertions cheap.
pub event_kind_counts: BTreeMap<String, usize>,
/// Raw number of LLM calls observed during the replay. Not bucketed —
/// the fixture pins each step's response, so drift here reflects a real
/// change in how many times the engine called the provider.
pub llm_call_count: u32,
/// Number of safety-warning status events observed.
pub safety_warning_count: usize,
/// Per-thread retrospective analyzer output. Empty for engine v1 replays.
pub engine_threads: Vec<ThreadSummary>,
}
impl ReplayOutcome {
/// Capture the outcome of a just-completed replay from the rig.
///
/// Call this after `wait_for_responses` / `run_trace`, before `shutdown`.
pub async fn capture(rig: &TestRig, responses: &[OutgoingResponse]) -> Self {
let status_events = rig.captured_status_events();
let mut events: Vec<EventSummary> = Vec::with_capacity(status_events.len());
let mut kind_counts: BTreeMap<String, usize> = BTreeMap::new();
let mut safety_warning_count = 0usize;
for event in status_events {
let summary = match event {
StatusUpdate::Thinking(msg) => {
*kind_counts.entry("Thinking".into()).or_default() += 1;
EventSummary::Thinking {
message: bucket_text(&msg),
}
}
StatusUpdate::ToolStarted { name, .. } => {
*kind_counts.entry("ToolStarted".into()).or_default() += 1;
EventSummary::ToolStarted {
name: strip_tool_params(&name),
}
}
StatusUpdate::ToolCompleted {
name,
success,
error,
..
} => {
*kind_counts.entry("ToolCompleted".into()).or_default() += 1;
EventSummary::ToolCompleted {
name: strip_tool_params(&name),
success,
error: error.map(|e| bucket_text(&e)),
}
}
StatusUpdate::ToolResult { name, preview, .. } => {
*kind_counts.entry("ToolResult".into()).or_default() += 1;
EventSummary::ToolResultPreview {
name: strip_tool_params(&name),
preview_len_bucket: bucket_usize(preview.chars().count(), 100),
}
}
StatusUpdate::StreamChunk(_) => {
*kind_counts.entry("StreamChunk".into()).or_default() += 1;
continue;
}
StatusUpdate::Status(msg) => {
*kind_counts.entry("Status".into()).or_default() += 1;
if is_safety_warning(&msg) {
safety_warning_count += 1;
}
EventSummary::Status {
message: bucket_text(&msg),
}
}
StatusUpdate::JobStarted { .. } => {
*kind_counts.entry("JobStarted".into()).or_default() += 1;
EventSummary::Other {
variant: "JobStarted",
}
}
StatusUpdate::ApprovalNeeded { tool_name, .. } => {
*kind_counts.entry("ApprovalNeeded".into()).or_default() += 1;
EventSummary::ApprovalNeeded { tool_name }
}
StatusUpdate::AuthRequired { extension_name, .. } => {
*kind_counts.entry("AuthRequired".into()).or_default() += 1;
EventSummary::AuthRequired { extension_name }
}
StatusUpdate::AuthCompleted {
extension_name,
success,
..
} => {
*kind_counts.entry("AuthCompleted".into()).or_default() += 1;
EventSummary::AuthCompleted {
extension_name,
success,
}
}
StatusUpdate::ImageGenerated { .. } => {
*kind_counts.entry("ImageGenerated".into()).or_default() += 1;
EventSummary::Other {
variant: "ImageGenerated",
}
}
StatusUpdate::Suggestions { suggestions } => {
*kind_counts.entry("Suggestions".into()).or_default() += 1;
EventSummary::Suggestions {
count: suggestions.len(),
}
}
StatusUpdate::ReasoningUpdate { .. } => {
*kind_counts.entry("ReasoningUpdate".into()).or_default() += 1;
EventSummary::Other {
variant: "ReasoningUpdate",
}
}
_ => {
*kind_counts.entry("Other".into()).or_default() += 1;
EventSummary::Other { variant: "Other" }
}
};
events.push(summary);
}
let tool_calls: Vec<ToolCallSummary> = rig
.tool_calls_completed()
.into_iter()
.map(|(name, success)| ToolCallSummary {
name: strip_tool_params(&name),
success,
})
.collect();
let has_final_response = !responses.is_empty();
let engine_threads = capture_engine_threads().await;
Self {
response_count: responses.len(),
has_final_response,
tool_calls,
events,
event_kind_counts: kind_counts,
llm_call_count: rig.llm_call_count(),
safety_warning_count,
engine_threads,
}
}
}
/// Engine v2 formats tool names as `"echo(hello...)"`. The parameter summary
/// is useful for logs but depends on model wording and is a churn source —
/// strip it before snapshotting.
fn strip_tool_params(name: &str) -> String {
match name.find('(') {
Some(i) => name[..i].to_string(),
None => name.to_string(),
}
}
/// Bucket free-form message text to something stable under small rewording.
/// Keeps leading ~40 chars, lowercased, to help reviewers recognize which
/// event fired without comparing full model output.
fn bucket_text(s: &str) -> String {
let trimmed: String = s.chars().take(40).collect();
trimmed.to_lowercase()
}
fn bucket_usize(value: usize, bucket: usize) -> usize {
if bucket == 0 {
return value;
}
(value / bucket) * bucket
}
fn is_safety_warning(msg: &str) -> bool {
let lower = msg.to_lowercase();
lower.contains("sanitiz") || lower.contains("inject") || lower.contains("warning")
}
#[cfg(feature = "libsql")]
async fn capture_engine_threads() -> Vec<ThreadSummary> {
let traces = ironclaw::bridge::engine_retrospectives_for_test().await;
traces.into_iter().map(thread_summary_from).collect()
}
#[cfg(not(feature = "libsql"))]
async fn capture_engine_threads() -> Vec<ThreadSummary> {
Vec::new()
}
#[cfg(feature = "libsql")]
fn thread_summary_from(trace: ironclaw_engine::executor::trace::ExecutionTrace) -> ThreadSummary {
use ironclaw_engine::executor::trace::IssueSeverity;
let issues = trace
.issues
.into_iter()
.map(|issue| {
let severity = match issue.severity {
IssueSeverity::Error => "error",
IssueSeverity::Warning => "warning",
IssueSeverity::Info => "info",
}
.to_string();
TraceIssueSummary {
severity,
category: issue.category,
}
})
.collect();
let message_roles = trace.messages.iter().map(|m| m.role.clone()).collect();
let event_kinds = trace
.events
.iter()
.map(|e| event_kind_name(&e.kind).to_string())
.collect();
ThreadSummary {
final_state: format!("{:?}", trace.final_state),
step_count: trace.step_count,
message_roles,
event_kinds,
issues,
}
}
/// Exhaustive `match` on `EventKind` — not pulled from `Debug` or a `strum`
/// derive on purpose. Adding a variant upstream breaks this match, which is
/// the signal we want: a new engine event must be consciously classified as
/// either worth snapshotting or explicitly ignored, not silently swallowed
/// under the default `Debug` string. The duplication is the enforcement.
#[cfg(feature = "libsql")]
fn event_kind_name(kind: &ironclaw_engine::EventKind) -> &'static str {
use ironclaw_engine::EventKind;
match kind {
EventKind::StateChanged { .. } => "StateChanged",
EventKind::StepStarted { .. } => "StepStarted",
EventKind::StepCompleted { .. } => "StepCompleted",
EventKind::StepFailed { .. } => "StepFailed",
EventKind::ActionExecuted { .. } => "ActionExecuted",
EventKind::ActionFailed { .. } => "ActionFailed",
EventKind::LeaseGranted { .. } => "LeaseGranted",
EventKind::LeaseRevoked { .. } => "LeaseRevoked",
EventKind::LeaseExpired { .. } => "LeaseExpired",
EventKind::MessageAdded { .. } => "MessageAdded",
EventKind::ChildSpawned { .. } => "ChildSpawned",
EventKind::ChildCompleted { .. } => "ChildCompleted",
EventKind::ApprovalRequested { .. } => "ApprovalRequested",
EventKind::ApprovalReceived { .. } => "ApprovalReceived",
EventKind::SelfImprovementStarted => "SelfImprovementStarted",
EventKind::SelfImprovementComplete { .. } => "SelfImprovementComplete",
EventKind::SelfImprovementFailed { .. } => "SelfImprovementFailed",
EventKind::SkillActivated { .. } => "SkillActivated",
EventKind::CodeExecutionFailed { .. } => "CodeExecutionFailed",
EventKind::OrchestratorRollback { .. } => "OrchestratorRollback",
EventKind::Unknown => "Unknown",
}
}
/// Assert that `outcome` matches the saved YAML snapshot for `name`.
///
/// Snapshots live at `tests/snapshots/replay__{name}.snap`. Use
/// `cargo insta review` to accept snapshot diffs interactively.
#[macro_export]
macro_rules! assert_replay_snapshot {
($name:expr, $outcome:expr) => {{
let mut settings = ::insta::Settings::clone_current();
settings.set_snapshot_path(concat!(env!("CARGO_MANIFEST_DIR"), "/tests/snapshots"));
settings.set_prepend_module_to_snapshot(false);
settings.set_sort_maps(true);
settings.set_omit_expression(true);
settings.bind(|| {
::insta::assert_yaml_snapshot!(format!("replay__{}", $name), $outcome);
});
}};
}