Skip to content

Batch: optimize JSON output, scenario tag validation, replay schema check, host-host design doc - #1344

Closed
davedumto wants to merge 4 commits into
Timi16:mainfrom
davedumto:fix/optimize-scenario-replay-trace-validation
Closed

Batch: optimize JSON output, scenario tag validation, replay schema check, host-host design doc#1344
davedumto wants to merge 4 commits into
Timi16:mainfrom
davedumto:fix/optimize-scenario-replay-trace-validation

Conversation

@davedumto

@davedumto davedumto commented May 27, 2026

Copy link
Copy Markdown
Contributor

Description

Batched contribution targeting four recently-assigned issues in cli,
scenario, compare, and the architecture docs. Each change is small,
scoped, and verified by tests. Commits are split one-per-issue so they
can be reviewed (or reverted) independently.

Related Issue

closes #927
closes #1280
closes #1282
closes #1288

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Documentation update
  • Test / CI improvement

Changes

  • No design document explains the decision to use soroban-env-host directly rather than higher-level Soroban SDK abstractions #927 — Architecture: direct soroban-env-host design note.
    New docs/design-soroban-env-host.md explains why the runtime,
    inspector, and protocol layers depend on soroban-env-host directly
    rather than going through soroban-sdk testutils, the tradeoffs we
    accept (tighter version coupling, lower-level error surfaces), and the
    rule that keeps SDK-level dependencies appropriate for fixtures and
    sample tests. ARCHITECTURE.md's existing one-line mention now links
    to the new doc.

  • Optimize: add JSON output option for optimization reports #1280optimize --format json for structured reports.
    Adds a format: OutputFormat field to OptimizeArgs (pretty is the
    default and preserves prior behavior) and a render_optimize_report_json
    helper that emits a stable JSON document keyed by schema_version,
    kind, contract, metadata, hotspots, and suggestions.
    --output semantics are unchanged: it still controls destination
    (stdout vs. file) independently of --format. JSON is built with
    serde_json::json! so the existing report types do not need new
    Serialize derives — keeps the change surface minimal. Unit test in
    commands.rs pins the JSON shape (kind, metadata keys, hotspots,
    suggestions, priority spelling). docs/optimization-guide.md
    documents how --format composes with --output. The optimize man
    page is regenerated by build.rs to reflect the new flag.

  • Scenario: validate include and exclude tag conflicts #1282 — Scenario: reject --tags / --exclude-tags overlap with
    a summary.
    The runner previously accepted overlapping tag sets and
    silently dropped every step carrying the conflicting tag — almost
    always a CLI typo. Now overlap is rejected fast with an actionable
    message that names every conflicting tag (sorted, de-duplicated), and
    the effective include/exclude lists are echoed before any steps run.
    Tag normalization is consolidated in a single parse_tag_list helper
    that trims whitespace, drops empty entries, and dedupes while
    preserving first-appearance ordering (matters for message stability).
    Tests cover whitespace, duplicates, disjoint sets (allowed),
    single-sided selection (allowed), overlap detection, and dedupe in
    the error message.

  • Replay: validate trace schema before replay execution #1288 — Replay: validate trace schema and required fields before
    execution.
    Adds a schema_version: Option<String> field to
    ExecutionTrace (#[serde(default, skip_serializing_if = "Option::is_none")]
    so the on-disk format stays backward-compatible with pre-versioning
    traces). New validate_for_replay(contract_override) method runs at
    the top of cli::commands::replay and:

    • accepts traces with no schema_version (legacy);
    • accepts traces whose major matches SUPPORTED_TRACE_SCHEMA_MAJORS;
    • rejects unparseable or unsupported versions with a remediation hint
      (re-capture, or upgrade/downgrade the debugger);
    • requires a non-empty function field;
    • requires a contract path either in the trace or via --contract.
      from_file parse-error message is improved to mention the file may
      be truncated, written by an incompatible version, or not a trace.
      Newly-written traces are stamped with output::SCHEMA_VERSION in
      build_execution_trace. The test-fixture trace builders in
      compare/engine.rs are updated for the new field. Adds four pinned
      fixtures under tests/schemas/replay_traces/ (valid current, valid
      legacy with no version, malformed missing function, unsupported
      future major) plus an integration test that loads them from disk and
      hits the same validation path the CLI uses.

CI/Test Behavior Changes

What changed in CI or test behavior?

Adds a new integration-test crate (tests/replay_validation_tests.rs)
and new unit tests in src/cli/commands.rs, src/compare/trace.rs, and
src/scenario.rs. No existing tests were modified except the two
ExecutionTrace literal-constructors in src/compare/engine.rs tests,
which gained the new schema_version field.

Migration notes

N/A. The new schema_version field on ExecutionTrace uses
#[serde(default, skip_serializing_if = "Option::is_none")], so old
trace files on disk continue to load and round-trip cleanly.

Test plan

  • cargo build --lib — clean
  • cargo check --tests — clean
  • cargo fmt --all -- --check — clean
  • cargo clippy --lib --tests -- -D warnings — clean
  • cargo test --lib — 516 passed, 0 failed (full lib suite)
  • cargo test --test replay_validation_tests — 4 passed
  • New unit tests pass:
    • scenario::tests::parse_tag_list_*
    • scenario::tests::validate_tag_selection_*
    • compare::trace::validate_for_replay_tests::* (7 cases)
    • cli::commands::tests::optimize_report_json_shape_is_stable
  • Man page for optimize regenerated by build.rs; the regenerated
    .1 file is committed.

Checklist

  • All tests pass locally
  • Code is formatted
  • Clippy is clean
  • Commit messages follow Conventional Commits (one commit per issue)
  • PR description mentions related issues with closes #N lines
  • CI/test behavior changes documented above
  • Regenerated man page committed (only optimize was affected)

davedumto and others added 4 commits May 27, 2026 18:24
…sion (Timi16#927)

Add docs/design-soroban-env-host.md explaining why the runtime, inspector,
and protocol layers depend on soroban-env-host directly rather than going
through the higher-level soroban-sdk testutils, what tradeoffs that choice
imposes (tighter version coupling, lower-level error surfaces), and the
rule that keeps SDK-level dependencies appropriate for fixture and sample
test code. Link the new doc from ARCHITECTURE.md so the existing one-line
mention of "Direct soroban-env-host Integration" now points at the full
rationale.

Closes the documentation gap tracked as backlog item I-016.

Co-Authored-By: Claude <noreply@anthropic.com>
…Timi16#1280)

The `optimize` command previously emitted only a human-readable markdown
report. Add a first-class `--format pretty|json` flag (pretty remains the
default, preserving existing behavior) and a new
`render_optimize_report_json` helper that produces a stable JSON document
keyed by schema_version, kind, contract, metadata, hotspots, and
suggestions. The schema_version is sourced from output::SCHEMA_VERSION so
consumers (CI gates, IDE plugins, dashboards) can pin to a version.

`--output` semantics are unchanged: `--format` chooses content, `--output`
chooses destination (stdout vs. file).

- src/cli/args.rs: add `format: OutputFormat` field to OptimizeArgs.
- src/cli/commands.rs: branch on args.format; render JSON via serde_json::json!
  so the report types do not need new Serialize derives. Add a unit test
  pinning the JSON shape (kind, metadata keys, hotspots, suggestions,
  priority spelling).
- docs/optimization-guide.md: document the format choice and how it
  composes with --output.
- man/man1/soroban-debug-optimize.1: regenerated by build.rs.

Co-Authored-By: Claude <noreply@anthropic.com>
…summary (Timi16#1282)

The scenario runner accepted overlapping include/exclude tag sets and
silently dropped every step that carried the conflicting tag — almost
always a CLI typo. Now overlap is rejected fast with an actionable error
naming each conflicting tag, and the effective include/exclude lists are
echoed before any steps run so the user can confirm the filter.

Tag normalization moved into a single `parse_tag_list` helper that trims
whitespace, drops empty entries, and deduplicates while preserving
first-appearance ordering (matters for error message stability).

Unit tests cover: whitespace trimming, duplicate handling, empty inputs,
disjoint sets (allowed), single-sided selection (allowed), overlap
detection, and de-duplication of conflicting tags in the error message.

Co-Authored-By: Claude <noreply@anthropic.com>
…Timi16#1288)

Make `replay` fail fast on malformed traces and on traces written against
schema versions this build does not understand, instead of crashing
partway through executor setup with an opaque error.

Changes:
- src/compare/trace.rs: add a `schema_version: Option<String>` field to
  ExecutionTrace (#[serde(default, skip_serializing_if = "Option::is_none")]
  so the JSON wire format stays backward-compatible with pre-versioning
  traces). Add a `validate_for_replay(contract_override)` method that:
  * accepts traces with no schema_version (legacy);
  * accepts traces whose major matches SUPPORTED_TRACE_SCHEMA_MAJORS;
  * rejects unparseable or unsupported versions with a remediation hint
    (re-capture, or upgrade/downgrade the debugger);
  * requires a non-empty `function` field;
  * requires a contract path either in the trace or via --contract.
  Improve the `from_file` parse-error message to tell the user the file
  may be truncated, incompatible, or not a trace at all.
- src/cli/commands.rs: call `validate_for_replay` at the top of `replay`
  before reading the WASM; stamp newly-written traces with
  output::SCHEMA_VERSION in `build_execution_trace`.
- src/compare/engine.rs: thread the new field through the two
  test-fixture trace builders.
- tests/schemas/replay_traces/: four JSON fixtures covering the matrix
  (valid current, valid legacy with no schema_version, malformed missing
  function, unsupported future major).
- tests/replay_validation_tests.rs: integration tests that load the
  fixtures from disk and exercise the same validation path the CLI uses.
- Unit tests in src/compare/trace.rs cover the in-memory matrix.

Co-Authored-By: Claude <noreply@anthropic.com>
@davedumto

Copy link
Copy Markdown
Contributor Author

Closing as duplicate of #1338 (opened earlier; covers the same four issues). The /team-autopilot run that opened this PR missed the prior one because it used a different branch name. #1338 is the canonical PR for #927 / #1280 / #1282 / #1288 — please review there.

@davedumto davedumto closed this May 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant