Skip to content

Fix orderless position cache indexing - #4688

Open
pengpengyi92 wants to merge 6 commits into
nautechsystems:developfrom
pengpengyi92:fix/orderless-position-index
Open

Fix orderless position cache indexing#4688
pengpengyi92 wants to merge 6 commits into
nautechsystems:developfrom
pengpengyi92:fix/orderless-position-index

Conversation

@pengpengyi92

@pengpengyi92 pengpengyi92 commented Aug 8, 2026

Copy link
Copy Markdown
Contributor
  • I have reviewed CONTRIBUTING.md and followed the established practices
  • I have not modified RELEASES.md (maintainers keep it current to avoid merge conflicts)

Summary

Synthetic spread leg fills intentionally create positions without backing orders, but the cache previously persisted and rebuilt order associations for the absent synthetic order.

This change adds an explicit orderless-position path which:

  • preserves position, strategy, venue, instrument, and account indexes without creating an invalid order_position mapping;
  • keeps an empty position_orders bucket so orderless position queries and index rebuilds remain valid;
  • filters legacy persisted mappings whose client order no longer exists during cache load; and
  • replays orderless flips by splitting the source fill consistently with live execution, closing the predecessor, and binding the opening fragment to the following PositionOpened replacement.

Related issues/PRs

Closes #4628

Type of change

  • Bug fix (non-breaking)
  • New feature (non-breaking)
  • Improvement (non-breaking)
  • Breaking change (impacts existing behavior)
  • Documentation update
  • Maintenance / chore

Breaking change details (if applicable)

Not applicable.

Documentation

  • Documentation changes follow the style guide (docs/developer_guide/docs.md)
  • For PyO3 binding or wrapped Rust doc changes, I ran make py-stubs and committed the generated output

No documentation or PyO3 binding changes.

Testing

  • Affected code paths are already covered by the test suite

  • I added/updated tests to cover new or changed logic

  • cargo test -p nautilus-model events::order::filled::tests (10 passed)

  • cargo test -p nautilus-common cache::tests (282 passed; 2 ignored)

  • cargo test -p nautilus-event-store replay::tests (44 passed)

  • cargo test -p nautilus-event-store (all test targets passed)

  • cargo test -p nautilus-execution --test exec_engine (222 passed)

  • cargo clippy -p nautilus-model -p nautilus-common -p nautilus-execution -p nautilus-event-store --lib --tests -- -D warnings

  • cargo fmt --all -- --check

Regression coverage includes live and replayed HEDGING flips, subsequent-fill correlation, legacy cache upgrade loading, empty orderless reverse-index buckets before and after clear_index() -> build_index(), and cache integrity.

Synthetic spread leg fills create positions without backing orders. Preserve the position indexes while skipping the invalid order-to-position mapping and its database entry.

Resolves nautechsystems#4628
@cjdsellers

Copy link
Copy Markdown
Member

Hi @pengpengyi92,

Thanks for the PR. The NETTING change correctly removes the invalid order_position entry.

I found three issues to address before merge:

  • With a strategy-level oms_type=HEDGING override, removing the mapping also removes the correlation used by determine_leg_fill_position_id. In a focused test, two fills for the same synthetic leg ID produced two positions instead of updating the first.
  • add_position_without_order is not covered by the event-store recovery contract, which causes the current Rust CI failure. Replay also assumes every OrderFilled has a cached order, so this path needs explicit recovery handling, not just a classification entry.
  • The rebuilt-index behavior needs a regression test covering clear_index(), build_index(), the absent order mapping, and check_integrity().

Note: the HEDGING correlation contract is not clearly documented, so I'll follow up on that policy and documentation work. Until then, I think this PR should preserve the existing behavior.

When you're able, could you address the recovery and index-rebuild gaps, and preserve HEDGING correlation for now.

@pengpengyi92
pengpengyi92 force-pushed the fix/orderless-position-index branch from e4b43c7 to c2cca4c Compare August 9, 2026 05:48
@pengpengyi92
pengpengyi92 force-pushed the fix/orderless-position-index branch from c2cca4c to 8df8df9 Compare August 9, 2026 07:03
@cjdsellers

Copy link
Copy Markdown
Member

Hi @pengpengyi92,

Thanks for addressing the earlier feedback.

I found one remaining HEDGING edge case:

When an orderless leg position flips, the closed position and the new open position both retain the same opening_order_id. orderless_hedging_leg_position_id searches all matching positions, so a later fill sees two matches and generates another position instead of updating the open flipped position.

When you're able, could you restrict that lookup to the open position, or otherwise select the current flipped position, and add a regression covering an initial fill, an opposite-side overfill that flips the position, and a subsequent fill for the same synthetic leg ID?

@pengpengyi92

pengpengyi92 commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for catching this, Chris. I restricted the orderless HEDGING correlation lookup to open positions, so after a virtual position flips, the closed predecessor is ignored and the current open flipped position is reused.

I added a regression covering an initial buy fill, an opposite-side sell overfill that flips it, and a subsequent sell fill with the same synthetic leg ID; it asserts that only the closed original and open flipped position remain, and that the later fill updates the flipped position. Verification: the focused regression passes, and cargo test -p nautilus-execution completed with 1031 passed, 0 failed, and 1 ignored.

This change is intentionally limited to this orderless HEDGING flip edge case and does not try to define the broader undocumented correlation policy. I would be very happy to keep discussing or refine the contract if you think the selection semantics should be expressed differently.

@cjdsellers

cjdsellers commented Aug 11, 2026

Copy link
Copy Markdown
Member

Hi @pengpengyi92,

Thanks for addressing the HEDGING flip case. Restricting the lookup to open positions fixes the live execution path.

I found three remaining issues, two blockers and one smaller cache issue:

  • Event-store replay does not recreate the new virtual position opened by an orderless HEDGING flip. It applies the unsplit fill to the original position, then ignores the PositionOpened event because the replacement position does not exist in the recovered cache.
  • Existing invalid order_position entries survive an upgrade. Cache startup reloads the persisted mapping after clearing the in-memory indexes, and index rebuilding does not remove entries whose orders do not exist.
  • The orderless path still puts the synthetic ID in position_orders, so orders_for_position panics for an otherwise valid orderless position.

When you are able, could you make replay reproduce the fill -> flip state, filter or migrate stale persisted mappings during load, and keep an empty position_orders bucket for orderless positions?

@pengpengyi92

Copy link
Copy Markdown
Contributor Author

Thanks Chris — I addressed all three points in e85c56f.

  1. Replay: snapshot-tail replay now detects an orderless overfill, uses the same fill-splitting helper as live execution, applies only the closing fragment to the predecessor, and defers the opening fragment until the matching PositionOpened supplies the replacement ID. The recovered fragment preserves proportional commission and causation to the original venue fill; a distinct replacement ID also restores the HEDGING OMS metadata. Ambiguous or unresolved matches fail explicitly, and the single-entry API rejects this cross-entry case before mutating the position.

  2. Upgrade loading: both cache_all() and cache_orders() filter persisted order_position entries whose client order is absent. This intentionally uses the filter option rather than expanding this PR into a cross-adapter physical migration; valid mappings backed by real orders remain intact.

  3. Reverse index: orderless positions now receive an empty position_orders bucket. build_index() only adds IDs backed by cached orders, so orders_for_position() returns an empty result before and after rebuild without weakening its strict behavior for genuine corruption.

Regression coverage includes the complete BUY 1 -> SELL 2 flip -> SELL 1 replay sequence, split quantity/commission/provenance, legacy load filtering through both entry points, and orderless queries across clear_index() -> build_index().

Verification:

  • model order-filled tests: 10 passed
  • cache module: 282 passed, 2 existing ignored
  • event-store replay module: 44 passed; full event-store package passed
  • execution engine integration: 222 passed
  • repository formatting and strict Clippy for the four affected crates passed

I kept the change scoped to the orderless position lifecycle and did not try to define the broader HEDGING correlation policy or add an adapter schema migration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Interactive Brokers] Synthetic spread leg fills create invalid order-position cache index

2 participants