Market data fixes - #12
Merged
Merged
Conversation
Reviews backend/app/market/ as of b45d882. Records the 127-test/97% coverage baseline and what this branch resolved since main (per-call APIRouter, SSE tests, symbol normalization, ensure_priced). Findings still open, each verified by execution: - No FastAPI app exists; create_stream_router has no production caller - add_ticker bypasses the simulated universe that ensure_priced enforces, so the watchlist path can stream invented prices - GBM dt is not derived from update_interval - The stream emits nothing on an empty priced set, contradicting its documented "silence means disconnected" contract - daily change % (PLAN.md:381) cannot be sourced from current data - The {seq, ts, prices} SSE envelope is undocumented in PLAN.md Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Addresses every item in planning/MARKET_DATA_REVIEW.md. Suite goes from
127 to 182 tests, coverage 97% -> 98%, ruff check and format clean.
High severity:
- Add backend/app/main.py. The market data layer had no host: create_stream_router
had no production caller, nothing read .env, /api/health did not exist, and
logging was never configured so every logger.info in app.market was discarded.
Adds lifespan wiring, DI accessors for downstream routers, and LOG_LEVEL.
- Enforce the simulated universe in SimulatorDataSource.add_ticker. Previously
only ensure_priced checked it, so the watchlist path could add an unrecognized
symbol and stream an invented price indefinitely. start() skips-and-warns
instead of raising, so a stale watchlist cannot stop the app booting.
- Derive the GBM time step from update_interval. dt was pinned to a 500ms
constant, so any other tick rate mis-scaled volatility (10x at 50ms).
Medium:
- Emit an SSE frame every tick even when nothing is priced. The empty-watchlist
case produced total silence, which is the same signal the client uses for a
dead connection.
- Add daily change %. PLAN.md:381 requires it and nothing could supply it:
change_percent is a tick delta that varies with connection age. PriceUpdate
now carries reference_price (session open in simulator mode, previous close
from Massive's prev_day) with daily_change/daily_change_percent derived.
- Document the {seq, ts, prices} envelope in PLAN.md, including that seq is
per-connection and restarts on reconnect.
Low:
- PriceCache.remove now bumps version; counter purpose documented.
- Honour an explicit timestamp=0.0 instead of treating it as unset.
- Copy the ticker list before handing it to the Massive worker thread.
- Stop treating HTTP 400 as unknown-symbol; only 404 is permanent.
- Drop the hop-by-hop Connection header from the SSE response.
- Seed the simulator RNG for reproducible runs; derive unseeded universe prices
from a stable hash so a restart reproduces the same market.
- Raise instead of silently no-op when add_ticker precedes start().
- Correct the is_simulated docstring; delete the no-op conftest fixture.
Verified against a running server: /api/health reports 10 priced tickers,
SSE streams 10 frames in 5s with daily_change_percent populated, and
application logs are now visible.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.