feat(hindsight): measure route decay over the blocks after a quote

[kayibal]

Adds a decay subcommand that quotes a sample of trade shapes at one block, then replays those
exact routes at each of the next --offsets blocks (default 5). Each offset records the total
move, the market drift a fresh solve at the same state also saw, and the difference — what holding
a stale route cost.

Why a separate measurement

monitor's existing slippage field solves a settled trade at N-1 and replays it at N. That state
already contains the trade's own price impact, so the number measures the route eating its own
shadow. The trades decay samples never execute: shapes are drawn from a monitor run's
comparison JSONL (pair and input amount only) and re-quoted at unrelated live blocks, which removes
that contamination.

Cost

Rounds do not overlap, so per-block work is --sample-size replays plus --sample-size solves
regardless of offset depth. A run warns when a block's work exceeds half the chain's block time.

Refactors carried along

• SteppingSolver moves to resolve/step.rs and the solver build/rebuild scaffolding to
  resolve/session.rs; both are now shared with monitor.
• StepAdapter gains an encoding opt-out. decay reads only amount_out, and requesting calldata
  would both cost work and drop routes an encoder rejects.
• RotatingWriter takes an explicit filename prefix, so decay output cannot be re-read as sampler
  input.
• The sampler tolerates reverted-transaction records, which carry null amounts by design, without
  counting them as malformed.

Note on the open hindsight stack

This branch sits directly on main, independent of the open decoding stack (#388 → #371 → #391).
Those PRs also touch resolve/jsonl.rs and resolve/monitor.rs, so whichever merges second will
need a rebase; there is no functional overlap.

🤖 Generated with Claude Code

[kayibal]

🤖 Review of the decay subcommand

I read the diff against origin/main rather than the commit message, and checked each of the PR's claims against the code.

Overall: the structure is good and the refactor is clean. SteppingSolver::current_block on the trait, StepAdapter/Session in their own modules, and the Encoding opt-out are all the right shape, and the extraction out of monitor.rs looks behaviour-preserving to me (details below). The decomposition math itself is correct and well tested. My concerns are concentrated in two places: measurements that silently disappear from the summary, and the run having no guard against sliding behind chain head.

I have left 8 inline comments. Ranked:

1. decay/record.rs:170 — a zero amount in any of the three legs produces a record with neither bps nor failure. This silently drops exactly the worst outcomes from p01_bps / tail_share.
2. decay/mod.rs:283 — the per-block budget check excludes quote_round, and one block in every --offsets does 50% more solver work than the PR's cost model says. The warning cannot fire on the busiest block.
3. decay/mod.rs:58 — --max-lag-blocks is accepted but never enforced by decay. Nothing stops the run drifting arbitrarily far behind head.
4. decay/mod.rs:289 — nothing checks that measured_block == quote_block + offset, so a gap or reorg puts a longer measurement into a shorter offset's bucket.
5. decay/record.rs:30 — PoolGone classification depends on the exact wording of a fynd-core error string, with no test guarding the wording.
6. decay/sample.rs:36 — the "monitor records reverted transactions" claim does not describe the code on this branch.
7. decay/mod.rs:267 — an all-unquotable round does not count toward --max-blocks.
8. decay/summary.rs:3 — the summary grows without bound for the life of the process.

---

Methodology: the market/execution split has no noise control

This is not a bug, but it affects the headline number the tool exists to produce, so I want to flag it.

execution_slippage_bps = (replayed - fresh) / quoted. replayed is deterministic given the state — same route, same pools. fresh is not: it comes from a time-boxed worker-pool solve, so two solves of the same order at the same state can return different routes. All of that variance lands in the split.

The direction matters. A fresh solve is never better than the true optimum, so when it underperforms, market_movement_bps reads more negative than it should and execution_slippage_bps reads less negative — which inflates market_share and deflates execution_share, the exact ratio the summary reports and the exact ratio PR #297 is being compared against.

There is a cheap control available: measure offset 0 as well. At offset 0 the replay must equal the quote exactly (same state, same route), so any nonzero market_movement_bps at offset 0 is pure solver noise, and you get a per-run number to subtract or at least to report alongside the split. It costs one extra measurement per round.

A related diagnostic that is currently invisible: how often execution_slippage_bps is positive — the stale route beating a fresh solve. That is physically impossible in a static market, so its frequency is a direct read on how noisy the reference is.

Monitor refactor

I diffed run and run_session line by line against origin/main. Everything moved verbatim: advance's barrier, timeouts, warn intervals, Pacing, the head-lag check, decode_block_when_available, the price snapshots, the JSONL write. Two things I specifically checked and they are fine:

• Encoding::Requested is passed at monitor.rs:187, so EncodingOptions::new(0.005) is still applied — ENCODING_SLIPPAGE is the same 0.005.
• load_pools_config reproduces the old inline logic exactly, including the default-path-only fallback.

The RotatingWriter prefix claim holds: is_comparisons_file requires the literal comparisons- prefix, and decay writes decay-, so a decay run cannot re-read its own output. (It could not anyway within a single run, since load_shapes runs before the writer is opened — but across runs the guard is real.)

Minor, take or leave

• --registry / HINDSIGHT_REGISTRY reach decay through ChainArgs, but decay never calls load_registry(). tools/hindsight/CLAUDE.md now says decay takes --registry "to load a custom address book", which is not true. Either drop the flag from the docs for this subcommand or note that it is unused.
• order_quote_to_outcome still builds quote_json via slim_quote for every solve, including all 2 * sample_size solves per block in decay, which never reads it. Since the point of Encoding::Skipped was to stop paying for output nobody consumes, the same reasoning applies here.
• decay reuses telemetry::record_block_seconds, but a decay "block" measures one offset (excluding the quote round) while a monitor "block" measures a full top/back pass. Two different quantities under one Prometheus series name.
• FEED_DEAD_TIMEOUT in resolve/step.rs:33 is pub(crate) but only used inside step.rs.
• decay/mod.rs::run_session is ~90 lines with roughly a dozen branch points. Under the 100-line limit but over the complexity-8 guidance; splitting the offset loop into its own function would bring both down.

Tests

Good coverage overall — the round-shape tests in decay/mod.rs are testing behaviour, not implementation, and the mock is well built. Of the edge cases worth having: empty sample ✅, malformed JSONL ✅, feed death mid-round ✅, offsets outside the summary's range ✅. Not covered: a block gap between offsets (there is no logic for it — see comment 4), and a zero amount in any leg (see comment 1).

[kayibal]

🤖 PoolGone depends on the exact wording of a fynd-core error message, with nothing guarding that wording.

The substring matched here comes from #[error("no simulation state for component {0}")] on ReplayError::MissingState (fynd-core/src/replay.rs:39). It matches today. But nothing connects the two: someone rewording that thiserror attribute — a change that looks entirely cosmetic and that no test in fynd-core covers — would silently reclassify every PoolGone as SimulationFailed.

That is a quiet failure. The counters still add up, the run still finishes, and the operator sees pool_gone = 0 and concludes routes are never being invalidated by pools leaving the feed — which, per this enum's own doc comment at line 11-13, is the signal being looked for.

ReplayError is pub, so the cheap guard is a test in this module that constructs the real error and asserts the classifier handles it:

let rendered = format!(
    "re-execution failed: {}",
    fynd_core::ReplayError::MissingState("0xabc".to_string())
);
assert_eq!(ReplayFailure::classify(&rendered), ReplayFailure::PoolGone);

The existing test at line 280-290 asserts against a hand-written string, which passes whether or not fynd-core still produces it.

Better still would be threading the typed error through SteppingSolver::reexecute instead of stringifying it at step.rs:155, but that is a bigger change than this PR needs.

[kayibal]

Yeah add the cheap test guard.

[kayibal]

🤖 The reverted-record rationale does not describe the code on this branch.

This comment states that "monitor also records reverted transactions (\"status\": \"reverted\"), where they are explicitly null". I could not find that anywhere in this branch. comparison_record (resolve/jsonl.rs:155-196) is the only producer of comparisons-*.jsonl, and it emits no status field at all; token_in, token_out and amount_in are non-optional there and always written. The fixture at line 262 also carries "cause" and "decoder":"reverted", neither of which comparison_record produces.

So the shape being defended against comes from the decoding stack (#388 / #371 / #391) that the PR description says is not merged. Against this codebase, the test at line 259 asserts the handling of a record no writer produces, and the PR description's claim that "the sampler tolerates reverted-transaction records" describes future behaviour as present behaviour.

The Option<String> fields themselves are fine and I would keep them — parsing JSONL from a long-running appender defensively is correct regardless. It is the justification that is wrong, and it is the kind of comment that sends the next reader looking for a writer that does not exist. Either reword to "records may carry null trade fields" without attributing it to monitor, or land this after the decoding stack so the claim becomes true.

[kayibal]

Reword the comment

[kayibal]

🤖 Correction to my earlier review comment on the market/execution split, plus two concrete proposals.

My summary comment argued that fresh-solve noise biases execution_slippage_bps toward zero and inflates market_share. After checking this against the two collected decay datasets (450k Ethereum records, 705k Base), that directional claim is wrong, and one of my supporting arguments was invalid. Walking both back.

What I got wrong

I treated the fact that a fresh solve usually returns more than the original quote as evidence of an artifact. It isn't. The fresh solve returns the best of many candidate routes, and the original route is one of the candidates, so random movement in the other routes can only push that maximum up. fresh >= replayed is the structural expectation, not a symptom.

I also framed cases where a better fresh route exists as contamination. They are not — if a better route genuinely exists at the later block, then holding the stale route did cost that difference, which is exactly what this subcommand sets out to measure.

What does hold up

Only the wrong-sign cases: execution_slippage_bps > 0, meaning the fresh solve came back with less than replaying the original route. The fresh solve was free to pick that route, so this is the solver failing to find something it had found a few blocks earlier.

	Ethereum	Base
wrong-sign share of all measured records	3.5%	5.9%
wrong-sign share of records where the number is non-zero	20.0%	22.4%
median magnitude of those	1.68 bps	1.21 bps

Why this matters more than I first thought — the sign, not the size

These records carry positive values, and positives cancel negatives inside an average. They were wiping out 49% of the genuine measured cost on Ethereum and 74% on Base. Removing them roughly doubles the reported decay on Ethereum and quadruples it on Base:

mean execution_slippage_bps	as computed today	wrong-sign cases removed
Ethereum	-0.2209	-0.4328 (1.96x)
Base	-0.0487	-0.1889 (3.88x)

So the current output understates the cost of holding a stale route.

Proposal 1 — clamp execution_slippage_bps at zero

A real in-block solver holds the original route and would take the better of the two, so its outcome is max(replayed, fresh) and a worse fresh solve gains nothing. Under that model the correct value is min(0, (replayed - fresh) / quoted).

Clamping is preferable to dropping the record (clamped -0.4328 vs dropped -0.4486 on Ethereum, -0.1889 vs -0.2007 on Base — nearly identical): clamping keeps the trade in the sample contributing exactly zero, which is what actually happens, whereas dropping shrinks the population and makes per-offset counts non-comparable.

Worth noting the published summary table is already effectively clamped — computing "average gain per quote" as total gains over all quotes is the same operation. It is DecayBps / Summary that are not, so the JSONL and the logged summary currently disagree with the table.

Proposal 2 — record the fresh solve's route identity

DecayRecord::algorithm stores only trade.quote.algorithm (record.rs:125, 189). Because the fresh solve's route is not recorded, two very different situations are indistinguishable in the data:

• the fresh solve found a genuinely different, better route (real, measurable gain), versus
• the fresh solve returned the same route with a different number (solver inconsistency).

Adding the fresh solve's algorithm — or better, its route — makes the remaining noise measurable instead of inferred. This is cheap now and unanswerable retroactively for data already collected.

One residual bias, in the opposite direction

Because the fresh solve takes a maximum over many route valuations, an alternative that merely looks better through estimation error wins that maximum and is reported as achievable gain. That inflates the measured cost. It is smaller than the sign flips, it cannot be sized without proposal 2, and it partly offsets the understatement above — so the corrected figures should be read as better, not exact.

My original inline comment about a zero amount silently dropping both bps and failure (record.rs:163-175) is unaffected by any of this and still stands.