fix(backtest): update replay harness for observe+verify ethflow strategy#346
Merged
Conversation
The old harness mocked submit_order and classified success by whether
the strategy POSTed an order. The redesigned ethflow-watcher strategy
GETs /api/v1/orders/{uid} instead — so every fixture previously
classified as RejectedUnexpected and the binary exited 1.
Now programs a 200 catch-all for cow_api_request, classifies success
by whether observed:{uid} was written to local store, and renames the
Submitted classification to Observed throughout.
Closes #45
- classify_ok now checks `observed:{uid}` (exact fixture UID) instead of
any `observed:*` prefix — prevents a compute_uid mismatch from producing
a false Observed classification
- report.rs intro: corrected "programs a 200 for GET /orders/{uid}" to
reflect the actual catch-all mock setup
- RejectedExpected variant doc: broadened from "404 only" to "any documented
skip path" so future skip paths are not misleadingly excluded
Review follow-ups on top of the rebase onto develop:
- classify_ok now demands the full observe contract, not just the
store marker: zero submit_order calls and exactly one
GET /api/v1/orders/{uid} against the fixture's UID. A regression
that writes the marker after a different call shape (or that also
submits) is classified RejectedUnexpected instead of Observed, so
the backtest stays an independent check on the module tests.
- Regenerated docs/operations/backtest-reports/backtest-7d-2026-06-22.md
from the committed fixtures: 240/240 (100.0%) Observed, replacing the
stale submit-era report (issue #45's "stale and unreproducible" item).
Verified: cargo run -p shepherd-backtest -- --fixtures
tools/backtest-collect/fixtures-2026-06-22.json exits 0 with 240/240.
The rebase onto develop removed replay.rs's last shepherd_sdk imports (the app_data mocks whose CowApiError/HttpFailure types needed it), leaving the declared dep unused - which the workspace's unused-crate-dependencies lint denies under CI's -D warnings. Verified: check --all-targets clean, harness still 240/240 Observed.
mfw78
added
the
base: develop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ethflow-watcherstrategy was redesigned from submit-based to observe+verify (GETs/api/v1/orders/{uid}, writesobserved:{uid}to local store). The backtest harness was never updated, so every fixture classified asRejectedUnexpectedand the binary exited 1.submit_order+app_datamocks; added a catch-all 200 response forcow_api_requestSubmitted→Observedthroughout (replay.rs,report.rs,main.rs)classify_oknow checks local store forobserved:*key instead ofsubmit_ordercall countsubmitted_bodyfield fromReplayOutcomeCloses #45
Test plan
cargo test -p ethflow-watcher— 11/11 pass (covers the full observe+verify strategy)cargo check -p shepherd-backtest— zero warningsbacktest_collect.py— 5/5 Observed, 100%, exit 0Observed + RejectedExpected