Skip to content

feat: mark-to-market P&L with multi-provider quote feeds and ranked failover - #16

Merged
maxkuttner merged 8 commits into
mainfrom
feat/marks-valuation
Jul 18, 2026
Merged

feat: mark-to-market P&L with multi-provider quote feeds and ranked failover#16
maxkuttner merged 8 commits into
mainfrom
feat/marks-valuation

Conversation

@maxkuttner

Copy link
Copy Markdown
Owner

Turns MarkStore from a write-only cache into live valuation, unifies the three
hand-written stream tasks behind one feed abstraction, and adds a second crypto
data source with ranked failover.

What changed (phased; each commit ships standalone)

  • P0/positions returns mark, market_value, unrealized_pnl, mark_ts. MarkStore had no reader before this; unrealized P&L was the missing input all along.
  • P1 — one stream_supervisor for reconnect/backoff/health across all three streams; fixes a backoff-never-resets bug that was in all three.
  • P2LiveQuoteFeed capability trait in dataprovider; OPRA ported to it. Feeds emit Quote, never touch MarkStore.
  • review fixes — two regressions the refactor introduced (OPRA lost its health reporting; the router dropped legitimate zero-bid quotes), both caught by self-review and now tested.
  • P3 — subscriptions come from instrument_xref PROVIDER rows, not instrument.symbol. Parity proven: 27,252 = 27,252, zero mismatches.
  • P4 — Binance spot feed. Crypto goes from routable-but-unmarkable to valued against production prices.
  • P5 — Bybit feed + ranked failover (provider_feed_policy) + crypto symbol = the pair (fixes a (symbol, venue) collision).

Design

Feed (per vendor) -> Quote -> MarkRouter -> MarkStore -> /positions
     ^                            ^
StreamSupervisor          provider_feed_policy (rank + staleness)

A new vendor is one impl LiveQuoteFeed — proven twice (Binance, Bybit). Quote carries ts_recv (our clock, so a vendor can't fake freshness) and source_code (provenance for failover + audit).

Verified against live markets

  • Full chain end to end: SOL marked 74.605 vs avg_cost 77.82 → unrealized −0.3215, hand-checked.
  • Failover: primary killed (unreachable endpoint) → BINANCE/SPOT down, BYBIT/SPOT live, SOL still marked; primary recovered → promoted straight back.

Schema

  • 0017 (oms) — xref identity drops external_native_id from the unique key; deduped 26,411 rows.
  • 0015 (public) — crypto symbol = the pair (SOLUSDT).
  • 0018 (oms) — new provider_feed_policy table.

Known gaps (documented, not fixed here)

  • No temporal identity (valid_from/valid_to); renames still rewrite history.
  • symbology_resolver omits source_type from its xref lookup.
  • STALE_AFTER = 10s is safe only because crypto ticks constantly.
  • OPRA's populated path unproven (no option held during market hours yet).

84 tests.

Max added 8 commits July 17, 2026 08:42
MarkStore has been write-only since it was added: opra_stream was the only
writer and no get/get_all caller existed anywhere in src/, so every quote we
decoded was discarded. The module doc claimed "/positions valuation and
market-order pre-trade risk read" this store; neither consumer was ever built,
and 0011's comment ("unrealized P&L is intentionally absent — it needs a
market-data mark") has been waiting for exactly this.

Join the mark in at request time: /portfolios/:id/positions now returns mark,
market_value, unrealized_pnl and mark_ts. The query gains contract_size from
the master (LEFT JOIN, defaulting to 1) so options scale by 100 while spot
does not.

The mark-dependent fields are Option. A position we can hold but not value —
no provider covers it, or the feed is down — renders as null rather than 0,
because a zero would read as "flat P&L" instead of "unknown". This is not
hypothetical: the three Binance crypto instruments are routable but have no
PROVIDER xref row, so they are structurally unmarkable today.

The valuation itself is extracted to positions::value() rather than left inline
in the handler, because sign errors on shorts are the classic bug here and they
belong under test. Both figures are signed through net_qty: a short carries a
negative market value and gains when the mark falls below avg_cost.

Marks are snapshotted once per request so every row is valued against the same
instant rather than racing the feed mid-loop.
All three stream tasks had grown the same outer loop, verbatim: set the health
badge connecting, run a session, set it down, sleep, double the backoff, retry.
Only the session body differed. Extract the loop into stream_supervisor and give
each stream a Session impl holding what its session needs.

This fixes a bug all three shared. backoff_secs was initialized outside the loop
and never reset, so it only ever grew for the life of the process: a stream that
stayed up for hours and then dropped once would wait the last capped 30s before
retrying, as though it were flapping. A session that lasted STABLE_AFTER_SECS is
now treated as healthy and its next failure restarts the schedule from 1s. The
backoff rule is unit-tested, including that reset.

The streams are deliberately not unified further. Alpaca and Binance emit
execution reports; OPRA emits quotes. They share mechanics, not purpose, so a
single trait over both would be the wrong abstraction — Session is only the
reconnect seam.

Also drops binance_stream::run's _api_key/_api_secret parameters. They were dead:
the adapter already holds the credentials and signs with an Ed25519 key loaded at
construction, so main.rs was passing two String::new() to satisfy a signature that
existed only to mirror Alpaca's.

Streams now name themselves to the supervisor (ALPACA/PAPER, BINANCE/PAPER,
DATABENTO/OPRA), so reconnect logs say which stream without reading the module path.

Verified: all three connect and reach live/connecting as before; 66 tests pass.
crates/dataprovider has advertised a "quote source" capability since it was
written — provider.rs names it in the doc comment beside UniverseSource and
Enricher — but it never existed, so the OPRA feed grew in the app as one function
that connected a socket, queried the database, decoded DBN, and wrote MarkStore.
Adding Binance or Bybit meant copying all four concerns.

Split them along the seams that actually differ per vendor:

  LiveQuoteFeed (dataprovider)  wire protocol -> Quote. Nothing else.
  quote_feed::QuoteFeedSession  what to subscribe to; watches the doorbell and
                                pushes newly-held instruments mid-session.
  stream_supervisor             reconnect, backoff, health.
  mark_router                   the sole writer to MarkStore.

A new vendor is now one impl LiveQuoteFeed: no reconnect loop, no health wiring,
no SQL, no subscription bookkeeping. Vendor concepts stay contained — Databento's
session-scoped numeric ids never reach the trait, because they mean nothing to
Binance.

Quote carries ts_recv and source_code. ts_recv is deliberately our clock, so a
vendor with a skewed or missing timestamp cannot make a stale quote look fresh;
source_code is provenance, without which failover cannot arbitrate between sources
it cannot name and a mark cannot be audited.

Feeds emit rather than write. That makes a feed testable with nothing but a
channel, and lets marks/Kafka/cockpit fan out later without reopening any feed.
mark_router drops one-sided books: a mid computed from a missing side is silently
wrong, and a stale real mark beats a fabricated one.

Ranked failover is not built. No instrument currently has more than one provider,
so it would be machinery with nothing to arbitrate; mark_router is where it goes
when a second feed lands.

load_subscribable still reads instrument.symbol and filters to options — it works
only because Databento's raw symbol is byte-identical to ours. P3 replaces the body
with the instrument_xref PROVIDER join, which is what makes the driver genuinely
vendor-neutral.

Verified: 66 tests; router starts, feed idles correctly with nothing held, all
three streams reach live/connecting as before.
Self-review of the P0-P2 refactor caught two regressions it introduced, plus
three smaller defects.

Health reporting was lost. The pre-refactor opra_stream called health.set_live()
after client.start() and health.record_event() per record; the new path called
neither, because the feed no longer held a StreamHandle and the supervisor only
knows set_connecting/set_down. DATABENTO/OPRA could therefore never leave
"connecting" even while streaming, and last_event_at stayed null forever — which
is precisely the freshness signal the planned staleness-based failover reads, so
the source could never have been selected as active. Fixed with a FeedHealth port
in dataprovider (on_connected/on_event) that StreamHandle implements, keeping the
crate ignorant of our registry while restoring the exact prior behaviour.

The mark router rejected zero-bid quotes. `!(bid > 0.0 && ask > 0.0)` treats a
0.00 x 0.05 book as one-sided, but a zero bid is a real price — that is what a
deep-OTM option looks like near expiry, exactly where today's SPY 750 call is
headed. Those contracts would report mark=null, indistinguishable from having no
data source at all, when in fact the market is quoting them. The missing-side
sentinel is UNDEF_PRICE, which feeds already filter. Now rejects only what cannot
produce a mid: no offer, negative price, or a crossed book. is_usable() is
extracted and tested, since this is the second time a plausible-looking guard here
has been wrong.

Also:
- quote_feed no longer returns Ok(()) from run_once when nothing is held. The
  supervisor read that as a closed stream, so an idle feed logged a reconnect
  warning and flapped its badge down->connecting every 60s while behaving exactly
  as designed. It now waits on the doorbell (with a timer backstop), which also
  picks up a fill immediately instead of up to a minute later.
- binance_stream passed a hardcoded "BINANCE/PAPER" to supervise despite taking an
  `environment` parameter, so a LIVE stream's reconnects would be logged as paper.
- get_portfolio_positions cloned the entire MarkStore per request to read two or
  three entries; it now looks up per row. Cost was O(all marks), and the clone held
  the read lock against the router's writes.

Verified: 72 tests; no flapping over 20s (was 1 warning/min); positions endpoint
unchanged; all three streams reach live/connecting.
….symbol

The quote driver read instrument.symbol and filtered to instrument_class='OPTION'
in SQL. That worked only by coincidence: Databento's raw_symbol happens to be
byte-identical to our master symbol for OSI options, and for nothing else. Binance
calls it SOLUSDT where we call it SOL, so the coincidence ends at the first
non-Databento feed — the driver was vendor-neutral in shape only.

Subscriptions now come from oms.instrument_xref PROVIDER rows, so the provider's
own external_symbol drives the subscription and a feed is free to use whatever
strings its venue uses. Verified parity over the full option universe: old and new
queries return identical (symbol, id) sets — 27,252 rows, 0 mismatches — so this
changes the mechanism without changing behaviour.

Coverage needed a second half, which the data made obvious. source_code alone is
not coverage: the held SPY *equity* carries a PROVIDER/DATABENTO xref row
(external_symbol 'SPY' @ ARCX), so a source_code-only query hands it to the
OPRA.PILLAR session — an options-only dataset that cannot quote it. LiveQuoteFeed
therefore declares covers() -> &[instrument_class], and DatabentoOpraFeed declares
OPTION. Confirmed against the live DB: without the filter the held set is
[SPY/SPOT]; with it, correctly empty.

covers() is the minimum honest scope, not the final answer. One vendor running
several feeds (Databento equities + OPRA) wants a per-feed dataset/schema config;
that is what the planned provider_feed_cfg table is for. Until then, a feed
declaring its classes beats a class hardcoded in shared SQL.

An instrument with no xref row for a source is simply absent from that feed's
subscribe set — the "tradable but unmarkable" state, which is a fact about
coverage, not an error.

Verified: 72 tests; all three streams connect; no errors.
The three Binance crypto instruments were routable but had no PROVIDER xref row,
so nothing in the identity graph claimed to price them: /positions reported a null
mark for positions we can freely trade. This closes that gap and, in doing so, is
the first real test of the LiveQuoteFeed abstraction — the whole point of the
previous three commits was that a second vendor should cost one trait impl.

It did. BinanceFeed is protocol only: connect the public bookTicker stream, decode,
emit Quote. No reconnect loop, no health wiring, no SQL, no subscription
bookkeeping — quote_feed, stream_supervisor and mark_router already owned those,
unchanged.

Marks come from PRODUCTION Binance even though orders route to testnet. Testnet
runs its own synthetic book, so marking against it would value positions at prices
nobody can trade; P&L against a testnet fill is meaningless either way, so the
choice is between a useful number and a useless one. The stream is public — no key,
no signing — and shares nothing with the authenticated WS-API the order stream uses.

The doorbell had to fan out. An mpsc has exactly one consumer, so with two feeds
the fill path's single signal now goes to a relay task that try_sends to each feed's
own doorbell. Keeping one sender on the fill path means execution.rs never learns
how many feeds exist.

Verified end-to-end against the live market — the first time any quote has flowed
through this chain:
  feed -> Quote -> mark_router -> MarkStore -> /positions
  SOL: mark 74.605 vs avg_cost 77.82, qty 0.1  -> unrealized -0.3215
  BTC: mark 62772.90 vs avg_cost 64100.26      -> unrealized -3.9821
Hand-checked: (74.605 - 77.82) * 0.1 = -0.3215. Marks tick live (74.595 -> 74.615)
with mark_ts advancing. 75 tests pass.
…to symbols

Three interlocking changes: Bybit gives crypto a second source, which is the first
case the failover design has ever had to arbitrate, and both depend on crypto
identity being right first.

Symbol collision (0015_CRYPTO_SYMBOL_IS_THE_PAIR)

  The seed modelled crypto as symbol = base asset (SOL), currency = quote (USDT).
  That cannot represent SOL/USDT and SOL/BTC at once: both key to (SOL, BINANCE)
  under instrument_symbol_venue_key, so the model silently permitted exactly one
  quote currency per base per venue — not a property of the market. The symbol now
  names the pair (SOLUSDT), which is what every venue calls it and what our own
  xref already stored as external_symbol; instrument.symbol was the only place
  saying SOL. Matches Nautilus's raw_symbol convention and CCXT's rule that
  contracts which differ must have distinct symbols. currency stays USDT so the
  pair remains decomposable, and recon is untouched (it matches the base asset via
  external_native_id).

Bybit feed

  orderbook.1 on the v5 public spot stream. Confirmed against the live endpoint
  before writing the decoder — Bybit and Binance quote SOL within 0.15% of each
  other, which is what makes one a usable stand-in for the other. Unlike Binance's
  bookTicker, orderbook.1 is stateful: a snapshot replaces the book, a delta
  carries only the side that moved, and size "0" deletes a level. The feed tracks
  the last known top per pair and emits only once both sides are known, so a delta
  can never publish a half-quote. Needs an application-level {"op":"ping"} every
  20s — a protocol ping frame does not reset Bybit's timer.

Ranked failover (0018_CREATE_PROVIDER_FEED_POLICY + mark_router)

  Rank lives in policy keyed by (source_code, instrument_class), not per instrument
  — that would be ~42k copies of the same integer for a decision that is a property
  of the source. Binance outranks Bybit for SPOT because we route crypto orders to
  Binance: its book is the one our fills print against, and Bybit prices the same
  pair on a different book.

  The router now owns arbitration: best-ranked live source holds the mark, a better
  source preempts immediately, a worse one may take over only once the incumbent
  has been quiet for STALE_AFTER_SECS. Staleness rather than connection state is
  the trigger because it catches the failure a socket cannot report — up,
  subscribed, and silently not delivering. Every ownership change logs; a silent
  failover is one nobody can diagnose later.

  Deliberately not A/B line arbitration, which is per-message sequence
  reconciliation between redundant lines of the same feed at microsecond
  granularity. This is cross-vendor, at seconds, between different books.

Verified live, not just in tests:
  - both feeds up: Bybit connected first and took the mark, Binance preempted it
    0.5s later ("promoting better-ranked source")
  - primary dead (BINANCE_FEED_WS_URL pointed at an unreachable host): BINANCE/SPOT
    down, BYBIT/SPOT live, SOL still marked at 74.825 with a fresh timestamp
  - primary recovered: promoted straight back to Binance, mark 74.835
84 tests, including 5 that pin the arbitration state machine.
The Bybit market-data xref rows were created by a manual INSERT during failover
testing and never landed in a seed file, so a fresh install had a BINANCE
provider but no BYBIT one — failover could not be reproduced. Add them next to
the Binance PROVIDER rows, keyed the same way (same pair string,
external_exchange = BYBIT).
@maxkuttner
maxkuttner merged commit 434f98a into main Jul 18, 2026
1 check passed
@maxkuttner
maxkuttner deleted the feat/marks-valuation branch July 18, 2026 07:47
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.

1 participant