fix(mbp10): require explicit instrument_id in extract_trades_from_snapshots — no default
Removes the `instrument_id: 0` backward-compat default introduced in6c1ab8850. That default was nonsensical: a sentinel value for an identifier creates the exact half-state that future filtering code can't distinguish from real data. Now extract_trades_from_snapshots takes `instrument_id: u32` as an explicit required parameter. Caller must provide the contract id the snapshot stream belongs to (typically captured at the data-acquisition site alongside symbol). Tests: - test_extract_trades_from_snapshots updated to pass id=12345 + asserts every emitted trade carries that id (verifies the explicit-param contract). - New test_filter_front_month_mbp10_picks_highest_volume covers the helper added in6c1ab8850(no test before). - New test_filter_front_month_mbp10_empty for the empty-input edge case. 15/15 mbp10 tests pass. Workspace + examples compile clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -347,9 +347,9 @@ pub struct Mbp10Trade {
|
||||
/// Used by `mbp10_to_imbalance_bars` to filter to the front-month
|
||||
/// (highest-volume) contract per file, mirroring the per-file pattern
|
||||
/// in `precompute_features.rs:354`. Populated from `mbp10.hd.instrument_id`
|
||||
/// in `extract_trades_from_dbn_file`; left as 0 in the snapshot-diff
|
||||
/// path (`extract_trades_from_snapshots`) where the source records
|
||||
/// don't carry instrument IDs (mostly used by tests).
|
||||
/// in `extract_trades_from_dbn_file`; supplied by the caller in
|
||||
/// `extract_trades_from_snapshots` (snapshot records don't carry per-
|
||||
/// record IDs, so the caller passes the stream's contract id explicitly).
|
||||
pub instrument_id: u32,
|
||||
}
|
||||
|
||||
@@ -360,9 +360,19 @@ pub struct Mbp10Trade {
|
||||
/// when `trade_count` increases between consecutive snapshots, a trade occurred.
|
||||
/// Direction is inferred by comparing mid-prices (tick rule).
|
||||
///
|
||||
/// `instrument_id` is required — `Mbp10Snapshot` does NOT carry per-record
|
||||
/// instrument IDs, so the caller must supply the contract this snapshot
|
||||
/// stream belongs to (typically captured at the data-acquisition site
|
||||
/// alongside `symbol`). Stamped onto every emitted `Mbp10Trade` so downstream
|
||||
/// front-month filtering (`filter_front_month_mbp10`) works uniformly across
|
||||
/// both extraction paths.
|
||||
///
|
||||
/// For higher-fidelity trade extraction, use [`extract_trades_from_dbn_file`]
|
||||
/// which reads raw MBP-10 records with their action/side fields.
|
||||
pub fn extract_trades_from_snapshots(snapshots: &[Mbp10Snapshot]) -> Vec<Mbp10Trade> {
|
||||
pub fn extract_trades_from_snapshots(
|
||||
snapshots: &[Mbp10Snapshot],
|
||||
instrument_id: u32,
|
||||
) -> Vec<Mbp10Trade> {
|
||||
let mut trades = Vec::new();
|
||||
let mut prev_trade_count: u32 = 0;
|
||||
let mut prev_mid: f64 = 0.0;
|
||||
@@ -392,7 +402,7 @@ pub fn extract_trades_from_snapshots(snapshots: &[Mbp10Snapshot]) -> Vec<Mbp10Tr
|
||||
volume,
|
||||
timestamp: dt,
|
||||
is_buy,
|
||||
instrument_id: 0, // snapshot-diff path has no per-record id
|
||||
instrument_id,
|
||||
});
|
||||
}
|
||||
}
|
||||
@@ -1053,13 +1063,46 @@ mod tests {
|
||||
},
|
||||
];
|
||||
|
||||
let trades = extract_trades_from_snapshots(&snapshots);
|
||||
let trades = extract_trades_from_snapshots(&snapshots, 12345);
|
||||
// 3 transitions where trade_count increases (snap 0->1, 1->2, 2->3)
|
||||
assert_eq!(trades.len(), 3);
|
||||
// First trade: price went up => is_buy=true
|
||||
assert!(trades[0].is_buy);
|
||||
// Third trade: price went down => is_buy=false
|
||||
assert!(!trades[2].is_buy);
|
||||
// All trades carry the caller-supplied instrument_id (verifies the
|
||||
// explicit-param contract — no implicit 0 default).
|
||||
assert!(trades.iter().all(|t| t.instrument_id == 12345));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_filter_front_month_mbp10_picks_highest_volume() {
|
||||
let ts = DateTime::<Utc>::from_timestamp(1_700_000_000, 0).unwrap();
|
||||
let mk = |id: u32, vol: f64| Mbp10Trade {
|
||||
price: 100.0,
|
||||
volume: vol,
|
||||
timestamp: ts,
|
||||
is_buy: true,
|
||||
instrument_id: id,
|
||||
};
|
||||
// Three contracts with different total volumes; id=200 is the
|
||||
// highest-volume contract (10 + 5 = 15).
|
||||
let trades = vec![
|
||||
mk(100, 3.0),
|
||||
mk(200, 10.0),
|
||||
mk(300, 1.0),
|
||||
mk(200, 5.0),
|
||||
mk(100, 2.0),
|
||||
];
|
||||
let filtered = filter_front_month_mbp10(&trades);
|
||||
assert_eq!(filtered.len(), 2, "should keep only id=200 trades");
|
||||
assert!(filtered.iter().all(|t| t.instrument_id == 200));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_filter_front_month_mbp10_empty() {
|
||||
let filtered = filter_front_month_mbp10(&[]);
|
||||
assert!(filtered.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
Reference in New Issue
Block a user