Add detailed market data backend design document - #2
Conversation
Consolidates the interface, simulator, and Massive API design docs into one implementation-accurate reference, matching the as-built code in backend/app/market/ after code review fixes were applied. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VxEsuAL4E6q2LaTfu9BqPW
There was a problem hiding this comment.
🟡 Changes recommended
The document contains unresolved critical and moderate correctness, lifecycle, and integration issues.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds a consolidated, implementation-ready design document for the market data backend.
Changes:
- Documents the cache, interface, simulator, Massive client, factory, and SSE architecture.
- Adds lifecycle, watchlist, testing, configuration, and integration guidance.
- Supersedes the archived market-data drafts.
File summaries
| File | Description | Final findings |
|---|---|---|
planning/MARKET_DATA_DESIGN.md |
Unified market data backend design and integration reference. | 15 findings: 1 critical, 7 moderate, and 7 nits, covering timestamp handling, cache versioning, router reuse, shutdown races, ticker normalization, startup tracking, status-code consistency, dependency wording, PSD validation, complexity, and test setup. |
Review details
Suppressed comments (10)
planning/MARKET_DATA_DESIGN.md:44
- The simulator imports NumPy, and
numpyis a required backend dependency, so calling this path “no external deps” is misleading for anyone following this design to install/run it. Please say “no external API/service” (or explicitly mention the NumPy dependency).
no external deps) poller, needs API key)
planning/MARKET_DATA_DESIGN.md:911
- When a Massive poll fails, the cache retains stale values, but the SSE generator does not emit another event because the cache version is unchanged. Please change this row to say that the connection remains open and readers retain access to stale cache values, rather than claiming SSE keeps streaming stale events.
| **All tickers fail** | Cache retains last-known prices; SSE keeps streaming stale data (better than no data). |
planning/MARKET_DATA_DESIGN.md:1146
- The trade example maps every cache miss to 404, while §13.2 explicitly defines a not-yet-available cached price as a 400 condition. Since a newly added Massive ticker can be temporarily unavailable without the ticker being unknown, use one status-code contract in both examples.
raise HTTPException(404, f"No price available for {trade.ticker}")
planning/MARKET_DATA_DESIGN.md:1303
- Adding a ticker to
MassiveDataSourceonly appends it;_poll_loop()will not fetch it until the next interval. This is documented elsewhere as up topoll_interval, so the empty-watchlist section should not say tracking starts immediately for both implementations.
simply sends no events until a ticker is added, at which point tracking
starts immediately.
planning/MARKET_DATA_DESIGN.md:687
stop()clears the task but leaves_simpopulated, so a lateradd_ticker()still calls_cache.update(). This violates the interface contract above that no cache writes occur afterstop()and can mutate state during shutdown; clear_simor guard the mutation methods once stopped.
self._task = None
planning/MARKET_DATA_DESIGN.md:1107
- Startup loads only watchlist tickers, but §11 says a ticker removed from the watchlist while it has a nonzero position must remain tracked. After a process restart that ticker is absent from
initial_tickers, so portfolio valuation loses its price. Initialize the source with the union of watchlist tickers and held-position tickers.
initial_tickers = await load_watchlist_tickers()
planning/MARKET_DATA_DESIGN.md:696
- The two implementations are not behaviorally interchangeable for ticker inputs: Massive uppercases and strips symbols, while the simulator stores the input verbatim (and
start()also skips normalization). A lowercase or whitespace-padded watchlist value therefore creates a different cache key and an unknown simulated ticker. Normalize at a shared boundary or consistently in both implementations.
async def add_ticker(self, ticker: str) -> None:
if self._sim:
self._sim.add_ticker(ticker)
price = self._sim.get_price(ticker)
if price is not None:
self._cache.update(ticker=ticker, price=price)
logger.info("Simulator: added ticker %s", ticker)
planning/MARKET_DATA_DESIGN.md:835
- Cancelling the poller task does not cancel an already-running
asyncio.to_thread()worker.stop()then sets_client = None, so the worker can either dereferenceNoneor keep the network request running after shutdown. Keep the client alive until the synchronous call finishes (with a timeout), or otherwise coordinate worker completion before releasing it.
async def stop(self) -> None:
if self._task and not self._task.done():
self._task.cancel()
try:
await self._task
except asyncio.CancelledError:
pass
self._task = None
self._client = None
planning/MARKET_DATA_DESIGN.md:621
- The rebuild includes
np.linalg.cholesky(corr), which is O(n^3); only constructing the correlation matrix is O(n^2). Calling the whole operation O(n^2) understates the cost of dynamic watchlist changes and should be corrected in this implementation reference.
Called whenever tickers are added or removed. O(n^2) but n < 50.
"""
planning/MARKET_DATA_DESIGN.md:1264
- This example never initializes
source._client, but_poll_once()returns immediately when the client is unset (massive_client.py:91-92). As written,_fetch_snapshotsis never called and the cache assertions fail; set a mock client before invoking_poll_once().
source = MassiveDataSource(api_key="test-key", price_cache=cache, poll_interval=60.0)
source._tickers = ["AAPL", "BAD"]
- Files reviewed: 1/1 changed files
- Comments generated: 6
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| # The Massive RESTClient is synchronous — run in a thread to | ||
| # avoid blocking the event loop. | ||
| snapshots = await asyncio.to_thread(self._fetch_snapshots) |
| (direction='flat'). | ||
| """ | ||
| with self._lock: | ||
| ts = timestamp or time.time() |
| def remove(self, ticker: str) -> None: | ||
| """Remove a ticker from the cache (e.g., when removed from watchlist).""" | ||
| with self._lock: | ||
| self._prices.pop(ticker, None) |
| router = APIRouter(prefix="/api/stream", tags=["streaming"]) | ||
|
|
||
|
|
||
| def create_stream_router(price_cache: PriceCache) -> APIRouter: | ||
| """Create the SSE streaming router with a reference to the price cache. | ||
|
|
||
| This factory pattern lets us inject the PriceCache without globals. | ||
| """ | ||
|
|
||
| @router.get("/prices") |
| requires the matrix be positive semi-definite, which holds for any valid | ||
| correlation matrix (all diagonal 1s, off-diagonal in `[-1, 1]`, symmetric). |
| tech = CORRELATION_GROUPS["tech"] | ||
| finance = CORRELATION_GROUPS["finance"] | ||
|
|
||
| # TSLA is in the tech set but behaves independently |
Summary
planning/MARKET_DATA_DESIGN.md, a detailed, implementation-ready design document for the market data backend (unifiedMarketDataSourceinterface, GBM simulator, Massive/Polygon.io API client, thread-safe price cache, SSE streaming).planning/archive/(MARKET_INTERFACE.md,MARKET_SIMULATOR.md,MASSIVE_API.md, and the earlierMARKET_DATA_DESIGN.md) into one document that matches the as-built code inbackend/app/market/, including the fixes called out inplanning/archive/MARKET_DATA_REVIEW.md(publicGBMSimulator.get_tickers(), top-levelmassiveimports, correctAsyncGeneratorreturn type on the SSE generator, etc.).lifespanintegration pattern (backend/app/main.pydoes not exist yet) so the next agent wiring up portfolio/watchlist/chat routes has a concrete, ready-to-use pattern for starting/stopping the market data source and injecting thePriceCache.Test plan
backend/app/market/for accuracy.🤖 Generated with Claude Code
https://claude.ai/code/session_01VxEsuAL4E6q2LaTfu9BqPW
Generated by Claude Code