Skip to content

Add proptest harness for the nine price-level invariants (#80)#101

Merged
joaquinbejar merged 2 commits into
mainfrom
issue-80-proptest-harness
Jun 24, 2026
Merged

Add proptest harness for the nine price-level invariants (#80)#101
joaquinbejar merged 2 commits into
mainfrom
issue-80-proptest-harness

Conversation

@joaquinbejar

Copy link
Copy Markdown
Owner

Summary

Adds a property-test harness (proptest) covering the nine named price-level
invariants, driving a single PriceLevel through its public API with orders
built from the validated newtypes. User-approved dev-dependency.

Changes

  • proptest added to [dev-dependencies] (default-features = false,
    features = ["std","bit-set"]); dev-only, no runtime dep.
  • New tests/proptest target ([[test]], 256 cases, deterministic — fixed
    trade-id namespace + caller-supplied timestamps, no clock/RNG in the matcher).
  • strategies.rs: maker_strategy (Standard / Iceberg / auto-reserve),
    book_strategy, taker_tif_strategy, all via newtypes at one level price.

Properties

  1. Quantity conservation (+ counters == queue contents)
  2. No zero-quantity orders rest or trade
  3. FIFO / price-time priority (first-touch oldest-first; partial keeps front)
  4. MatchResult agreement (is_complete ⇔ remaining==0; executed_quantity == Σ trade qty; executed_value; unique filled_order_ids)
  5. Trade fields (maker≠taker, price==level, taker_side==maker_side.opposite())
  6. IOC fills available + discards remainder (taker never rests)
  7. FOK all-or-nothing (Filled vs Killed with zero trades + untouched queue)
  8. Iceberg / reserve replenish (visible refreshed from hidden; totals conserved)
  9. Snapshot round-trip + tampered checksum → ChecksumMismatch

Notes

  • Three initial counterexamples were over-strict test assumptions, not engine
    bugs
    (confirmed via the minimized cases): a non-auto reserve correctly
    discards its stranded hidden when visible is consumed, and a replenished maker
    correctly loses time priority (re-queued at tail). Assertions adjusted; the
    generic book strategy uses auto_replenish: true.
  • The committed properties.proptest-regressions records those seeds so they're
    retried in CI (they pass now). No production code changed.

Testing

cargo test: lib 408, proptest 9, integration 10, doctests 9 — 0 failed.
cargo clippy --all-targets --all-features -- -D warnings, cargo fmt --all --check, cargo build --release: clean.

Checklist

Closes #80

Adds proptest (dev-dependency) and a tests/proptest target (256 cases,
deterministic) covering: quantity conservation, no zero-qty orders, FIFO/
price-time priority, MatchResult field agreement, Trade field invariants, IOC
fill-and-discard, FOK all-or-nothing (Filled vs Killed), iceberg/reserve
replenish, and snapshot round-trip + tampered-checksum.

Strategies build orders through the validated newtypes and drive a single
PriceLevel via match_order (post-#65 signature). Three initial counterexamples
were over-strict test assumptions, not engine bugs (non-auto reserve discards
stranded hidden; a replenished maker correctly loses time priority); assertions
adjusted accordingly. No production code changed.
@joaquinbejar joaquinbejar added the price-level Engine: level/order_queue/snapshot/statistics + execution results label Jun 23, 2026
@joaquinbejar joaquinbejar self-assigned this Jun 23, 2026
@joaquinbejar joaquinbejar requested a review from Copilot June 23, 2026 23:58
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

❌ Your project status has failed because the head coverage (73.97%) is below the target coverage (75.00%). You can increase the head coverage or adjust the target coverage.
see 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds a deterministic proptest-based integration test target that exercises nine single-price-level invariants against PriceLevel via the public API, improving confidence in matching correctness (FIFO, conservation, TIF behaviors, snapshot integrity) without changing production code.

Changes:

  • Add proptest as a dev-dependency and register a dedicated [[test]] target (tests/proptest/mod.rs).
  • Introduce shared generation strategies for makers/books/taker TIF (tests/proptest/strategies.rs).
  • Implement nine property tests plus persisted regression seeds (tests/proptest/properties.rs, tests/proptest/properties.proptest-regressions).

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
Cargo.toml Adds proptest dev-dependency and a dedicated proptest integration test target.
tests/proptest/mod.rs Wires the proptest integration test modules and documents harness behavior.
tests/proptest/strategies.rs Defines proptest strategies for sides, makers, and books at a single level price.
tests/proptest/properties.rs Implements the nine invariants as proptest properties over PriceLevel::match_order and snapshots.
tests/proptest/properties.proptest-regressions Checks in regression seeds for CI replay of previously-found counterexamples.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/proptest/properties.rs
Comment thread tests/proptest/mod.rs Outdated
Comment thread tests/proptest/properties.proptest-regressions Outdated
Comment thread tests/proptest/strategies.rs Outdated
…gy fixes

- prop_fifo_price_time_priority's partial-fill-keeps-front check was unreachable
  (required !is_complete && remaining==0). Now guarded to a STANDARD front maker
  with is_complete + empty filled_order_ids, so it actually exercises the
  invariant. Root issue: snapshot order is (timestamp, seq) while the sweep pops
  by seq, so random maker timestamps made "the front" ambiguous; book_strategy
  now stamps insertion-order-monotonic timestamps (with_id_and_ts) so the two
  orderings coincide.
- Corrected the mod.rs doc to point at tests/proptest/properties.proptest-regressions.
- Removed the stale regressions file (annotations reflected the old
  auto_replenish:false strategy; no engine bug, regenerated only on real failure).
- book_strategy doc no longer claims "non-empty" (it honors the len range).
@joaquinbejar

Copy link
Copy Markdown
Owner Author

Thanks, all four addressed: the FIFO partial-fill check is now reachable and real (Standard front maker + monotonic book timestamps so snapshot/sweep ordering coincide), the regression-file doc path is corrected, the stale regressions file removed, and the book_strategy doc no longer overclaims non-empty.

@joaquinbejar joaquinbejar merged commit 1b79572 into main Jun 24, 2026
13 checks passed
@joaquinbejar joaquinbejar deleted the issue-80-proptest-harness branch June 24, 2026 00:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

price-level Engine: level/order_queue/snapshot/statistics + execution results

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add a proptest harness for the nine named price-level invariants

3 participants