Validate MatchResult invariants during decoding (#116)

[joaquinbejar]

Summary

Derived Deserialize wrote MatchResult's private fields directly and
FromStr constructed without validation, so malformed input could claim
is_complete with a positive remainder, attach trades to Rejected /
Killed results, disagree about executed quantity, or list filled makers
unrepresented by trades. Both decoders now route through one private
invariant validator; the wire format is byte-identical and every
previously-valid payload still decodes.

Stack

• Stack: orders: Reduce every supported order variant after a partial fill #118 → execution: Parse malformed UTF-8 without panicking #114 → execution: Validate MatchResult invariants during decoding #116 → price-level: Reject quantity overflow before mutating level state #111 → price-level: Reject duplicate order IDs atomically #113 → price-level: Enforce level admission and trade topology invariants #120 → price-level: Resequence quantity increases without exposing an absent order #119 → price-level: Apply UpdateQuantity to the live maker state #115 → price-level: Make execution-statistics failures non-partial and observable #117 → price-level: Make PostOnly and FOK decisions atomic with the sweep #112 (this is 3/10)
• Base branch: stack/2-114-utf8-safe-parser
• Stacked on: Parse malformed UTF-8 in MatchResult::from_str without panicking (#114) #122 — the diff vs main is cumulative; review against the base branch.

Changes

• Deserialize replaced with #[serde(try_from = "MatchResultWire")] — a
  private permissive mirror with identical field names and the outcome
  default preserved; Serialize untouched.
• Single validator enforcing: is_complete ⇔ remaining == 0; checked
  trade-quantity sum and sum + remaining fit u64; Killed / Rejected
  carry zero trades and zero filled ids; every filled_order_id appears as a
  trade maker (the exact match_order contract — ids recorded only on full
  consumption right after their trade).
• FromStr routes through the same validator, and its filled_order_ids
  bracket now rejects trailing content after ] (asymmetry flagged in Parse malformed UTF-8 in MatchResult::from_str without panicking (#114) #122's
  review).

Technical decisions

• NotFilled / PartiallyFilled vs trades is deliberately not
  constrained: legacy payloads default outcome to NotFilled with trades
  present and accessors correct it — enforcing it would break backward
  compatibility. Only the pre-sweep Killed / Rejected outcomes are
  constrained.
• Reuses existing InvalidOperation / InvalidFormat error variants — no new
  variant, no wire-format change, no snapshot impact.

Testing

• Negative serde JSON per invariant (structurally valid payloads only the
  validator rejects) + negative FromStr cases + trailing-content tightening
• Positive round-trips: filled ids + trades, killed, rejected
• 256-case proptest: arbitrary valid results built via the public API
  round-trip through both encodings with no field drift
• Pre-push checks clean locally

467 tests passed; 0 failed (448 lib + 9 + 1 + 9 doctests).

Checklist

• Code follows rules/global_rules.md and CLAUDE.md
• No .unwrap() / .expect() / unchecked indexing in production code
• Checked arithmetic in the sums — no saturating_* / wrapping_*
• Wire struct stays private — no public-surface change
• No new production unsafe, no new dependencies
• Module boundaries respected

Closes #116

• Blocks: Reject quantity overflow before mutating level state (#111) #124

[codecov-commenter]

Codecov Report

❌ Patch coverage is 89.32039% with 11 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/execution/match_result.rs	89.32%	11 Missing ⚠️

Files with missing lines	Coverage Δ
src/execution/match_result.rs	92.90% <89.32%> (+13.09%)	⬆️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[joaquinbejar]

The validator closes several malformed decode paths, but explicit outcomes can still contradict completion and trades can still reference a different taker. The intended effective verdict is REQUEST_CHANGES. GitHub does not accept that event from the PR author, so this review is submitted as COMMENT.

[joaquinbejar]

The existing review covers outcome and taker identity mismatches. One additional filled maker uniqueness and ordering gap remains. The intended effective verdict is REQUEST_CHANGES, submitted as COMMENT because the active account owns this PR.

[joaquinbejar]

Thanks for both passes. All three gaps are closed in 893d532, explicit outcomes, taker identity, and filled-id uniqueness and ordering are all validated now.

[joaquinbejar]

The prior invariant findings are fixed, but explicit null outcomes are still normalized as legacy absence and the public filled id mutator can create values the decoder rejects. The intended effective verdict was REQUEST_CHANGES before this head merged. GitHub requires COMMENT because this is a self review.

[joaquinbejar]

🟠 Warning: Option with #[serde(default)] maps both a missing legacy key and an explicit "outcome": null to None, so the latter is silently normalized even though the old decoder rejected it. Use a missing-field sentinel or custom deserializer so only omission triggers legacy derivation, and add a null regression test.

[joaquinbejar]

🟠 Warning: This validator rejects arbitrary, duplicate, or reordered filled IDs, but the public infallible add_filled_order_id still creates those states. Such a value serializes successfully and then fails to deserialize, so public API values no longer reliably round-trip. Enforce this invariant in the mutator or make it non-public, and cover API-built invalid sequences.