Skip to content

average_price edges; pin current IOC/GTD behavior at the level (#79)#97

Merged
joaquinbejar merged 2 commits into
mainfrom
issue-79-tif-expiry-avgprice-tests
Jun 23, 2026
Merged

average_price edges; pin current IOC/GTD behavior at the level (#79)#97
joaquinbejar merged 2 commits into
mainfrom
issue-79-tif-expiry-avgprice-tests

Conversation

@joaquinbejar

Copy link
Copy Markdown
Owner

Summary

Adds the non-gated parts of the TIF / expiry / average_price test coverage:
average_price edge cases, a pin of the level's de-facto IOC behavior, and a
test + doc note that time-in-force expiry is the caller's responsibility. The
parts that need taker-TIF enforcement (FOK all-or-nothing, true IOC discard) are
deferred to the gated #65.

Changes

  • average_price edges (match_result_trade.rs): zero-executed → Ok(None);
    exact small values; the f64 precision-loss case; and a never-NaN/Inf
    sweep over valid inputs.
  • IOC de-facto pin (level.rs tests):
    test_match_order_taker_exceeds_depth_fills_available_and_reports_remainder
    a larger-than-depth taker fills available, leaves remaining_quantity() > 0,
    is_complete() == false, and is never enqueued.
  • GTD/Day: test_match_order_does_not_enforce_gtd_maker_expiry — a Gtd maker
    with a past expiry still matches; added a match_order rustdoc note that TIF
    expiry is not enforced at the level (caller/order-book responsibility).
  • Clarifying comments on the existing test_match_fill_or_kill_order /
    test_match_immediate_or_cancel_order (maker TIF not enforced; matches like
    Gtc).

Technical decisions

  • average_price precision (no bug): for price = 2^53 + 1, quantity = 1
    the exact average is 9_007_199_254_740_993 but the f64 division yields
    …992.0 (off by 1) — 2^53 + 1 isn't representable in an f64 mantissa. The
    matching path keeps exact integer math; only average_price analytics drop to
    f64. Never NaN/Inf for valid inputs. Tests assert closeness + finiteness
    and document the limitation.
  • Deferred to Decide and document taker TIF / PostOnly / MarketToLimit responsibility at the level #65: FOK all-or-nothing "kill" and IOC discard enforcement
    match_order takes no taker TimeInForce and never enqueues the taker, so it
    can't distinguish fill-or-kill from fill-what-you-can. The IOC test pins only
    the observable behavior the engine already has.

Testing

  • make pre-push clean

cargo test: 380 (unit) + 1 (integration) + 9 (doctests) passed, 0 failed.
cargo clippy --all-targets --all-features -- -D warnings, cargo fmt --all --check, cargo build --release: all clean.

Checklist

Closes #79

…the level

- average_price: tests for zero-executed (Ok(None)), exact small values, the f64
  precision-loss case at 2^53+1 (finite, never NaN/Inf — documented intrinsic
  limitation), and a never-NaN/Inf sweep.
- IOC de-facto pin: taker exceeding depth fills available and reports the
  remainder; the taker is never enqueued. Real IOC/FOK taker enforcement is gated
  on #65.
- GTD/Day: test that match_order does NOT enforce maker expiry (a past-expiry Gtd
  maker still matches); added a match_order rustdoc note that TIF expiry is the
  caller's responsibility. Clarifying comments on the existing FOK/IOC maker
  tests. FOK all-or-nothing enforcement deferred to #65.
@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 19:39

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

Adds targeted test coverage and documentation to pin current PriceLevel::match_order semantics around time-in-force boundaries (IOC/FOK/GTD) and to extend MatchResult::average_price() coverage for numeric edge cases, without changing matching behavior.

Changes:

  • Adds average_price() edge-case tests (zero-executed → Ok(None), precision-loss case, and finiteness sweep).
  • Adds new PriceLevel tests to pin “taker exceeds depth” remainder behavior and to pin that maker GTD expiry is not enforced at this layer.
  • Updates PriceLevel::match_order rustdoc to explicitly state maker TIF expiry is caller/order-book responsibility, and clarifies existing IOC/FOK maker-TIF tests via comments.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/price_level/tests/level.rs Adds tests pinning taker-over-depth remainder behavior and maker GTD-expiry non-enforcement; clarifies existing IOC/FOK maker-TIF tests.
src/price_level/level.rs Documents that maker TIF expiry is not enforced in match_order (caller responsibility).
src/execution/tests/match_result_trade.rs Adds MatchResult::average_price() edge-case/precision/finite tests.

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

Comment thread src/execution/tests/match_result_trade.rs Outdated
Comment thread src/execution/tests/match_result_trade.rs Outdated
@codecov-commenter

codecov-commenter commented Jun 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

❌ Your project status has failed because the head coverage (72.71%) is below the target coverage (75.00%). You can increase the head coverage or adjust the target coverage.

Files with missing lines Coverage Δ
src/price_level/level.rs 93.37% <ø> (ø)

... and 1 file with indirect coverage changes

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

…fix comment

The precision test compared against EXACT_INT_AVG as f64, which rounds the same
way as the result (abs_err always 0), so it did not actually show the loss. Now
converts the f64 average back to u128 and asserts it is short of the exact
integer average by exactly 1. Also corrected a module comment that claimed
happy-path coverage lived in the price-level tests.
@joaquinbejar

Copy link
Copy Markdown
Owner Author

Thanks, both fixed. The off-by-1 is now shown in integer space (the f64 result converted back to u128 is short by exactly 1), and the module comment points to the right coverage.

@joaquinbejar joaquinbejar merged commit 8927e71 into main Jun 23, 2026
13 checks passed
@joaquinbejar joaquinbejar deleted the issue-79-tif-expiry-avgprice-tests branch June 23, 2026 19:51
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 TIF boundary, GTD/Day expiry, and average_price edge tests

3 participants