From ffcdad71abc24f7238145e9373c7ecbf2ec035c9 Mon Sep 17 00:00:00 2001 From: stevehome Date: Mon, 23 Mar 2026 11:29:18 +0000 Subject: [PATCH 1/6] Add README and update docs to use direct Cerebras API - Add project README.md with setup, architecture, and dev instructions - Update PLAN.md: switch LLM integration from OpenRouter to direct Cerebras API, add review notes and open questions section, clarify SSE/portfolio/chat details - Update cerebras SKILL.md to reflect direct Cerebras API via LiteLLM Co-Authored-By: Claude Sonnet 4.6 --- .claude/skills/cerebras/SKILL.md | 22 +++---- README.md | 108 ++++++++++++++++++++----------- planning/PLAN.md | 85 +++++++++++++++++++----- 3 files changed, 149 insertions(+), 66 deletions(-) diff --git a/.claude/skills/cerebras/SKILL.md b/.claude/skills/cerebras/SKILL.md index 9efd01a3..a7a1e29a 100644 --- a/.claude/skills/cerebras/SKILL.md +++ b/.claude/skills/cerebras/SKILL.md @@ -1,43 +1,39 @@ --- -name: cerebras-inference -description: Use this to write code to call an LLM using LiteLLM and OpenRouter with the Cerebras inference provider +name: Cerebras Inference +description: Use this to write code to call an LLM using LiteLLM with the direct Cerebras API --- # Calling an LLM via Cerebras -These instructions allow you write code to call an LLM with Cerebras specified as the inference provider. -This method uses LiteLLM and OpenRouter. +These instructions allow you to write code to call an LLM using the direct Cerebras API via LiteLLM. ## Setup -The OPENROUTER_API_KEY must be set in the .env file and loaded in as an environment variable. +The `CEREBRAS_API_KEY` must be set in the `.env` file and loaded as an environment variable. The uv project must include litellm and pydantic. `uv add litellm pydantic` ## Code snippets -Use code like these examples in order to use Cerebras. - ### Imports and constants ```python from litellm import completion -MODEL = "openrouter/openai/gpt-oss-120b" -EXTRA_BODY = {"provider": {"order": ["cerebras"]}} +MODEL = "cerebras/qwen-3-235b-a22b-instruct-2507" ``` -### Code to call via Cerebras for a text response +### Code to call Cerebras for a text response ```python -response = completion(model=MODEL, messages=messages, reasoning_effort="low", extra_body=EXTRA_BODY) +response = completion(model=MODEL, messages=messages) result = response.choices[0].message.content ``` -### Code to call via Cerebras for a Structured Outputs response +### Code to call Cerebras for a Structured Outputs response ```python -response = completion(model=MODEL, messages=messages, response_format=MyBaseModelSubclass, reasoning_effort="low", extra_body=EXTRA_BODY) +response = completion(model=MODEL, messages=messages, response_format=MyBaseModelSubclass) result = response.choices[0].message.content result_as_object = MyBaseModelSubclass.model_validate_json(result) ``` \ No newline at end of file diff --git a/README.md b/README.md index 3f2582ae..43c8dfd4 100644 --- a/README.md +++ b/README.md @@ -1,62 +1,98 @@ # FinAlly — AI Trading Workstation -A visually stunning AI-powered trading workstation that streams live market data, simulates portfolio trading, and integrates an LLM chat assistant that can analyze positions and execute trades via natural language. +An AI-powered trading workstation that streams live market data, supports simulated portfolio trading, and includes an LLM chat assistant that can analyze positions and execute trades on your behalf. Built to look and feel like a Bloomberg terminal with an AI copilot. -Built entirely by coding agents as a capstone project for an agentic AI coding course. +> **Course project**: Built entirely by orchestrated AI coding agents, demonstrating how agentic AI can produce a production-quality full-stack application. -## Features +--- -- **Live price streaming** via SSE with green/red flash animations -- **Simulated portfolio** — $10k virtual cash, market orders, instant fills -- **Portfolio visualizations** — heatmap (treemap), P&L chart, positions table -- **AI chat assistant** — analyzes holdings, suggests and auto-executes trades -- **Watchlist management** — track tickers manually or via AI -- **Dark terminal aesthetic** — Bloomberg-inspired, data-dense layout +## What You Get -## Architecture - -Single Docker container serving everything on port 8000: +- **Live price streaming** — prices flash green/red on every tick via SSE +- **Sparkline mini-charts** — per-ticker price action accumulated from the live stream +- **Buy/sell shares** — market orders, instant fill, no fees +- **Portfolio heatmap** — treemap sized by weight, colored by P&L +- **P&L chart** — total portfolio value over time, live-updating +- **Positions table** — quantity, avg cost, current price, unrealized P&L +- **AI chat** — ask questions, get analysis, have the AI execute trades and manage your watchlist +- **$10,000 virtual cash** — no login, no signup, start immediately -- **Frontend**: Next.js (static export) with TypeScript and Tailwind CSS -- **Backend**: FastAPI (Python/uv) with SSE streaming -- **Database**: SQLite with lazy initialization -- **AI**: LiteLLM → OpenRouter (Cerebras inference) with structured outputs -- **Market data**: Built-in GBM simulator (default) or Massive API (optional) +--- ## Quick Start ```bash -# Clone and configure +# Copy and edit environment variables cp .env.example .env -# Add your OPENROUTER_API_KEY to .env +# Add your CEREBRAS_API_KEY to .env -# Run with Docker -docker build -t finally . -docker run -v finally-data:/app/db -p 8000:8000 --env-file .env finally +# Start (macOS/Linux) +./scripts/start_mac.sh -# Open http://localhost:8000 +# Start (Windows) +./scripts/start_windows.ps1 ``` +Open [http://localhost:8000](http://localhost:8000). + +--- + ## Environment Variables | Variable | Required | Description | -|---|---|---| -| `OPENROUTER_API_KEY` | Yes | OpenRouter API key for AI chat | -| `MASSIVE_API_KEY` | No | Massive (Polygon.io) key for real market data; omit to use simulator | -| `LLM_MOCK` | No | Set `true` for deterministic mock LLM responses (testing) | +|----------|----------|-------------| +| `CEREBRAS_API_KEY` | Yes | LLM inference via Cerebras | +| `MASSIVE_API_KEY` | No | Real market data via Massive/Polygon. Uses simulator if absent. | +| `LLM_MOCK` | No | Set `true` for deterministic mock LLM responses (testing/CI) | -## Project Structure +--- + +## Architecture + +Single Docker container, single port (8000): + +- **Frontend**: Next.js (TypeScript), static export served by FastAPI +- **Backend**: FastAPI (Python/uv) +- **Database**: SQLite — zero config, auto-initialized on first run +- **Real-time**: Server-Sent Events (SSE) +- **AI**: LiteLLM → Cerebras (`qwen-3-235b-a22b-instruct-2507`) +- **Market data**: Built-in GBM simulator (default) or Massive REST API ``` finally/ -├── frontend/ # Next.js static export -├── backend/ # FastAPI uv project -├── planning/ # Project documentation and agent contracts -├── test/ # Playwright E2E tests -├── db/ # SQLite volume mount (runtime) -└── scripts/ # Start/stop helpers +├── frontend/ # Next.js TypeScript project +├── backend/ # FastAPI uv project +├── planning/ # Project documentation +├── scripts/ # Start/stop scripts +├── test/ # Playwright E2E tests +├── db/ # SQLite volume mount (runtime only) +├── Dockerfile +└── docker-compose.yml ``` -## License +--- + +## Development -See [LICENSE](LICENSE). +```bash +# Backend (requires uv) +cd backend +uv run uvicorn app.main:app --reload --port 8000 + +# Frontend +cd frontend +npm install +npm run dev +``` + +--- + +## Testing + +```bash +# Backend unit tests +cd backend && uv run pytest + +# E2E tests (requires Docker) +cd test && docker compose -f docker-compose.test.yml up --abort-on-container-exit +``` diff --git a/planning/PLAN.md b/planning/PLAN.md index bc1811b3..d73b2803 100644 --- a/planning/PLAN.md +++ b/planning/PLAN.md @@ -66,7 +66,7 @@ The user runs a single Docker command (or a provided start script). A browser op - **Backend**: FastAPI (Python), managed as a `uv` project - **Database**: SQLite, single file at `db/finally.db`, volume-mounted for persistence - **Real-time data**: Server-Sent Events (SSE) — simpler than WebSockets, one-way server→client push, works everywhere -- **AI integration**: LiteLLM → OpenRouter (Cerebras for fast inference), with structured outputs for trade execution +- **AI integration**: LiteLLM → Cerebras (direct API, fast inference), with structured outputs for trade execution - **Market data**: Environment-variable driven — simulator by default, real data via Massive API if key provided ### Why These Choices @@ -110,8 +110,8 @@ finally/ - **`frontend/`** is a self-contained Next.js project. It knows nothing about Python. It talks to the backend via `/api/*` endpoints and `/api/stream/*` SSE endpoints. Internal structure is up to the Frontend Engineer agent. - **`backend/`** is a self-contained uv project with its own `pyproject.toml`. It owns all server logic including database initialization, schema, seed data, API routes, SSE streaming, market data, and LLM integration. Internal structure is up to the Backend/Market Data agents. -- **`backend/db/`** contains schema SQL definitions and seed logic. The backend lazily initializes the database on first request — creating tables and seeding default data if the SQLite file doesn't exist or is empty. -- **`db/`** at the top level is the runtime volume mount point. The SQLite file (`db/finally.db`) is created here by the backend and persists across container restarts via Docker volume. +- **`backend/db/`** contains schema SQL definitions and seed logic. The backend lazily initializes the database on first request — creating tables and seeding default data if the SQLite file doesn't exist or is empty. *(Note: `backend/db/` holds source code — SQL schema and seed scripts. Do not confuse it with the top-level `db/` directory.)* +- **`db/`** at the top level is the **runtime volume mount point only**. The SQLite file (`db/finally.db`) is created here by the backend at runtime and persists across container restarts via Docker volume. This directory contains no source code. - **`planning/`** contains project-wide documentation, including this plan. All agents reference files here as the shared contract. - **`test/`** contains Playwright E2E tests and supporting infrastructure (e.g., `docker-compose.test.yml`). Unit tests live within `frontend/` and `backend/` respectively, following each framework's conventions. - **`scripts/`** contains start/stop scripts that wrap Docker commands. @@ -121,8 +121,8 @@ finally/ ## 5. Environment Variables ```bash -# Required: OpenRouter API key for LLM chat functionality -OPENROUTER_API_KEY=your-openrouter-api-key-here +# Required: Cerebras API key for LLM chat functionality +CEREBRAS_API_KEY=your-cerebras-api-key-here # Optional: Massive (Polygon.io) API key for real market data # If not set, the built-in market simulator is used (recommended for most users) @@ -138,6 +138,7 @@ LLM_MOCK=false - If `MASSIVE_API_KEY` is absent or empty → backend uses the built-in market simulator - If `LLM_MOCK=true` → backend returns deterministic mock LLM responses (for E2E tests) - The backend reads `.env` from the project root (mounted into the container or read via docker `--env-file`) +- `CEREBRAS_API_KEY` is required for LLM chat; the app starts without it but chat endpoints will return an error --- @@ -159,7 +160,7 @@ Both the simulator and the Massive client implement the same abstract interface. ### Massive API (Optional) - REST API polling (not WebSocket) — simpler, works on all tiers -- Polls for the union of all watched tickers on a configurable interval +- Polls for the union of all watchlist tickers and all tickers with open positions on a configurable interval (matches SSE scope — ensures held positions removed from the watchlist remain current) - Free tier (5 calls/min): poll every 15 seconds - Paid tiers: poll every 2-15 seconds depending on tier - Parses REST response into the same format as the simulator @@ -175,8 +176,8 @@ Both the simulator and the Massive client implement the same abstract interface. - Endpoint: `GET /api/stream/prices` - Long-lived SSE connection; client uses native `EventSource` API -- Server pushes price updates for all tickers known to the system at a regular cadence (~500ms) — in the single-user model this is equivalent to the user's watchlist -- Each SSE event contains ticker, price, previous price, timestamp, and change direction +- Server pushes price updates for the union of all watchlist tickers and all tickers with open positions, at a regular cadence (~500ms). This ensures portfolio P&L stays current even for tickers removed from the watchlist while still holding a position. +- Each SSE event contains ticker, price, previous price, seed price (price at session start, for computing session change %), timestamp, and change direction - Client handles reconnection automatically (EventSource has built-in retry) --- @@ -258,7 +259,7 @@ All tables include a `user_id` column defaulting to `"default"`. This is hardcod |--------|------|-------------| | GET | `/api/portfolio` | Current positions, cash balance, total value, unrealized P&L | | POST | `/api/portfolio/trade` | Execute a trade: `{ticker, quantity, side}` | -| GET | `/api/portfolio/history` | Portfolio value snapshots over time (for P&L chart) | +| GET | `/api/portfolio/history` | Portfolio value snapshots over time (for P&L chart); returns last 24 hours by default | ### Watchlist | Method | Path | Description | @@ -270,6 +271,7 @@ All tables include a `user_id` column defaulting to `"default"`. This is hardcod ### Chat | Method | Path | Description | |--------|------|-------------| +| GET | `/api/chat/history` | Load prior conversation messages (for restoring chat UI on page load) | | POST | `/api/chat` | Send a message, receive complete JSON response (message + executed actions) | ### System @@ -281,9 +283,9 @@ All tables include a `user_id` column defaulting to `"default"`. This is hardcod ## 9. LLM Integration -When writing code to make calls to LLMs, use cerebras-inference skill to use LiteLLM via OpenRouter to the `openrouter/openai/gpt-oss-120b` model with Cerebras as the inference provider. Structured Outputs should be used to interpret the results. +When writing code to make calls to LLMs, use the cerebras skill to call the Cerebras API directly via LiteLLM, using model `cerebras/qwen-3-235b-a22b-instruct-2507`. Structured Outputs should be used to interpret the results. -There is an OPENROUTER_API_KEY in the .env file in the project root. +There is a `CEREBRAS_API_KEY` in the `.env` file in the project root. ### How It Works @@ -292,7 +294,7 @@ When the user sends a chat message, the backend: 1. Loads the user's current portfolio context (cash, positions with P&L, watchlist with live prices, total portfolio value) 2. Loads recent conversation history from the `chat_messages` table 3. Constructs a prompt with a system message, portfolio context, conversation history, and the user's new message -4. Calls the LLM via LiteLLM → OpenRouter, requesting structured output, using the cerebras-inference skill +4. Calls the LLM via LiteLLM → Cerebras (direct API), requesting structured output, using the cerebras skill 5. Parses the complete structured JSON response 6. Auto-executes any trades or watchlist changes specified in the response 7. Stores the message and executed actions in `chat_messages` @@ -316,7 +318,7 @@ The LLM is instructed to respond with JSON matching this schema: - `message` (required): The conversational text shown to the user - `trades` (optional): Array of trades to auto-execute. Each trade goes through the same validation as manual trades (sufficient cash for buys, sufficient shares for sells) -- `watchlist_changes` (optional): Array of watchlist modifications +- `watchlist_changes` (optional): Array of watchlist modifications. Each entry must have `action` set to `"add"` or `"remove"` — no other values are valid; the backend rejects unknown actions. ### Auto-Execution @@ -339,7 +341,7 @@ The LLM should be prompted as "FinAlly, an AI trading assistant" with instructio ### LLM Mock Mode -When `LLM_MOCK=true`, the backend returns deterministic mock responses instead of calling OpenRouter. This enables: +When `LLM_MOCK=true`, the backend returns deterministic mock responses instead of calling Cerebras via LiteLLM. This enables: - Fast, free, reproducible E2E tests - Development without an API key - CI/CD pipelines @@ -352,10 +354,10 @@ When `LLM_MOCK=true`, the backend returns deterministic mock responses instead o The frontend is a single-page application with a dense, terminal-inspired layout. The specific component architecture and layout system is up to the Frontend Engineer, but the UI should include these elements: -- **Watchlist panel** — grid/table of watched tickers with: ticker symbol, current price (flashing green/red on change), daily change %, and a sparkline mini-chart (accumulated from SSE since page load) +- **Watchlist panel** — grid/table of watched tickers with: ticker symbol, current price (flashing green/red on change), session change % (labeled "Session %" — calculated as `(current - seed) / seed × 100` where `seed` is the price recorded when the simulator or Massive poller initialised), and a sparkline mini-chart (accumulated from SSE since page load; resets on reconnect — this is acceptable by design) - **Main chart area** — larger chart for the currently selected ticker, with at minimum price over time. Clicking a ticker in the watchlist selects it here. - **Portfolio heatmap** — treemap visualization where each rectangle is a position, sized by portfolio weight, colored by P&L (green = profit, red = loss) -- **P&L chart** — line chart showing total portfolio value over time, using data from `portfolio_snapshots` +- **P&L chart** — line chart showing total portfolio value over time. On load, seed with data from `GET /api/portfolio/history` (last 24 hours of snapshots). Then update live: on every SSE price tick, recalculate total portfolio value client-side (current prices × position quantities + cash), but append a new chart data point at most once per second to avoid unbounded memory growth and chart noise. - **Positions table** — tabular view of all positions: ticker, quantity, avg cost, current price, unrealized P&L, % change - **Trade bar** — simple input area: ticker field, quantity field, buy button, sell button. Market orders, instant fill. - **AI chat panel** — docked/collapsible sidebar. Message input, scrolling conversation history, loading indicator while waiting for LLM response. Trade executions and watchlist changes shown inline as confirmations. @@ -399,7 +401,7 @@ The SQLite database persists via a named Docker volume: docker run -v finally-data:/app/db -p 8000:8000 --env-file .env finally ``` -The `db/` directory in the project root maps to `/app/db` in the container. The backend writes `finally.db` to this path. +The container path `/app/db` is where the backend writes `finally.db`. The named volume `finally-data` is mounted there, providing persistence across container restarts. The project-root `db/` directory is **not** bind-mounted into the container — it exists only to satisfy Docker volume expectations in development and to hold `.gitkeep` in the repo. ### Start/Stop Scripts @@ -454,3 +456,52 @@ The container is designed to deploy to AWS App Runner, Render, or any container - Portfolio visualization: heatmap renders with correct colors, P&L chart has data points - AI chat (mocked): send a message, receive a response, trade execution appears inline - SSE resilience: disconnect and verify reconnection + +--- + +## 13. Open Questions + +*No open questions — all resolved.* + +--- + +## 14. Review Notes + +### Questions & Clarifications + +**Section 6 — SSE Streaming** +- The spec says the SSE event includes `seed_price` (price at session start for "Session %"), but the current implementation's `PriceUpdate.to_dict()` does not include this field. Who owns the fix — the market data agent or the backend API agent? Should this be treated as a blocker before frontend integration begins? +- "Session" is defined as when the simulator/poller initialised. Does a container restart reset "Session %"? If so, is that acceptable behaviour, or should the seed price persist to the database so session % survives restarts? +- The SSE stream pushes updates at ~500ms cadence. Under the Massive API free tier (poll every 15 seconds), 30 consecutive SSE events will carry identical prices. Should the SSE generator suppress duplicate events, or is the frontend expected to handle this gracefully? + +**Section 9 — LLM Integration** +- How many recent `chat_messages` rows are loaded as conversation history? There is no limit specified. Without a cap, long sessions will grow the prompt indefinitely, eventually exceeding the model's context window or slowing responses. +- If the LLM returns a `trades` array and one trade fails validation (e.g., insufficient cash), do the remaining trades in the array still execute, or does the whole batch abort? This is not specified. +- The mock mode (`LLM_MOCK=true`) needs to specify what the deterministic response actually contains — at minimum one trade and one watchlist change — so E2E tests can assert against known values. + +**Section 7 — Database** +- `portfolio_snapshots` are recorded every 30 seconds and on every trade. There is no retention or pruning policy. After weeks of continuous use, this table could grow very large. Is pruning out of scope, or should the background task enforce a rolling window (e.g., keep only the last 7 days)? +- The `positions` table has a UNIQUE constraint on `(user_id, ticker)`. When quantity reaches zero after a full sell, should the row be deleted or kept with `quantity = 0`? The spec is silent on this; it affects how "positions table" renders and whether the portfolio heatmap shows zero-size rectangles. + +**Section 10 — Frontend** +- The P&L chart appends at most one data point per second from live SSE ticks, but there is no cap on total in-memory data points. After hours of use, this could degrade browser performance. Should the frontend enforce a maximum number of in-memory points (e.g., 3,600 for one hour at 1/s)? +- "Recharts" renders via SVG and struggles with large datasets; "Lightweight Charts" uses canvas and is significantly more performant. The spec says "preferred" but leaves the choice open. For a course demo this matters — recommend standardising on Lightweight Charts for both the main chart and the P&L chart. +- The AI chat panel is described as "docked/collapsible". What is the default state — open or collapsed? This affects the initial layout and how much space the watchlist/chart area gets by default. + +**Section 11 — Docker** +- The start script says "Builds the Docker image if not already built (or if `--build` flag passed)". If the user edits the code and reruns the script without `--build`, they will be running stale code silently. Consider making `--build` the default, or at least printing a warning. +- The `.env` file is described as "gitignored, .env.example committed" but no `.gitignore` exists in the repo yet. This is a security risk — `CEREBRAS_API_KEY` could be accidentally committed. Creating `.gitignore` should be an early task. + +--- + +### Simplification Opportunities + +1. **`backend/db/` naming confusion** — The distinction between `backend/db/` (source code: schema SQL) and `db/` (runtime volume) is a frequent source of confusion, noted both in this document and in code review. Consider renaming `backend/db/` to `backend/schema/` or `backend/migrations/` to eliminate ambiguity entirely. + +2. **`watchlist` UUID primary key** — The `watchlist` table has an `id UUID` primary key, but the natural key is `(user_id, ticker)` which already has a UNIQUE constraint. The UUID is never referenced in any endpoint. Dropping `id` from this table (and possibly `positions`) removes a column that adds complexity without value in a single-user app. + +3. **`GET /api/chat/history` vs `POST /api/chat` response** — The chat history endpoint exists solely to restore the UI on page load. If `POST /api/chat` already returns the assistant message and executed actions, the frontend could maintain its own in-memory message list (seeded once on load from `GET /api/chat/history`) without needing a separate round-trip. This is already the implied design — just worth confirming that `GET /api/chat/history` is only called once on load, not polled. + +4. **`portfolio_snapshots` background task vs on-trade recording** — Recording a snapshot both every 30 seconds *and* after every trade means the P&L chart may have clusters of close-together points around active trading periods. A simpler approach: record only on trade execution and let the frontend interpolate with SSE-driven live recalculation (already specified). The 30-second background snapshots could be dropped, simplifying the background task. That said, the snapshots provide historical persistence across page reloads, so this is a trade-off worth discussing. + +5. **Section 13 "No open questions"** — This section now has content (above). The placeholder text can be removed. From 45de201af3da2d6a0c14fa08ed30705576290301 Mon Sep 17 00:00:00 2001 From: stevehome Date: Mon, 23 Mar 2026 14:44:27 +0000 Subject: [PATCH 2/6] "Update Claude PR Assistant workflow" From 9ec8869e02c7a7d0b0c80289af46b821006768fd Mon Sep 17 00:00:00 2001 From: stevehome Date: Mon, 23 Mar 2026 14:44:28 +0000 Subject: [PATCH 3/6] "Update Claude Code Review workflow" From ddefc8aeeaad6f6bb5bb33935e7f71640ea83eab Mon Sep 17 00:00:00 2001 From: stevehome Date: Thu, 26 Mar 2026 10:43:50 +0000 Subject: [PATCH 4/6] Fix PriceCache.remove() to increment version, add planning docs and commands - Increment _version in PriceCache.remove() so SSE clients detect ticker removals - Add planning review and API docs - Add Claude commands and plugin config Co-Authored-By: Claude Sonnet 4.6 --- .claude-plugin/marketplace.json | 21 ++++ .claude/commands/doc-review.md | 1 + backend/app/market/cache.py | 1 + planning/MARKET_API.md | 212 ++++++++++++++++++++++++++++++++ planning/MARKET_SIMULATOR.md | 210 +++++++++++++++++++++++++++++++ planning/MASSIVE_API.md | 195 +++++++++++++++++++++++++++++ planning/REVIEW.md | 12 ++ 7 files changed, 652 insertions(+) create mode 100644 .claude-plugin/marketplace.json create mode 100644 .claude/commands/doc-review.md create mode 100644 planning/MARKET_API.md create mode 100644 planning/MARKET_SIMULATOR.md create mode 100644 planning/MASSIVE_API.md create mode 100644 planning/REVIEW.md diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json new file mode 100644 index 00000000..a8ea357d --- /dev/null +++ b/.claude-plugin/marketplace.json @@ -0,0 +1,21 @@ +{ + "name": "steve-tools", + "owner":{ + "name": "Steve", + "email": "steve@example.com" + }, + "plugins": [ + { + "name": "independent-reviewer", + "version": "1.0.0", + "source": "./independent-reviewer", + "description": "Carry out an independent review of all changes since last commit.", + "author": + { + "name": "Steve", + "email": "steve@example.com" + } + } + + ] +} \ No newline at end of file diff --git a/.claude/commands/doc-review.md b/.claude/commands/doc-review.md new file mode 100644 index 00000000..e9627c72 --- /dev/null +++ b/.claude/commands/doc-review.md @@ -0,0 +1 @@ +Review the documentation file in the planning folder called $ARGUMENTS and add questions, clarifications or feedback to a new section at the end, along with any opportunities to simplify. \ No newline at end of file diff --git a/backend/app/market/cache.py b/backend/app/market/cache.py index 4d021577..896aa521 100644 --- a/backend/app/market/cache.py +++ b/backend/app/market/cache.py @@ -60,6 +60,7 @@ 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) + self._version += 1 @property def version(self) -> int: diff --git a/planning/MARKET_API.md b/planning/MARKET_API.md new file mode 100644 index 00000000..afde4898 --- /dev/null +++ b/planning/MARKET_API.md @@ -0,0 +1,212 @@ +# Market Data API — Unified Interface + +This document describes the unified market data API used throughout FinAlly. All code that needs stock prices interacts with `PriceCache` and `MarketDataSource` — never with the Massive client or simulator directly. + +--- + +## Architecture + +``` +MarketDataSource (ABC) +├── SimulatorDataSource ← default (no API key required) +└── MassiveDataSource ← when MASSIVE_API_KEY env var is set + │ + ▼ + PriceCache (thread-safe, in-memory) + │ + ├── SSE stream (/api/stream/prices) + ├── Portfolio valuation + └── Trade execution +``` + +**Producer**: one `MarketDataSource` runs a background task that writes prices to `PriceCache`. +**Consumers**: SSE endpoint, portfolio API, and trade execution all read from `PriceCache`. +Consumers never talk to the data source directly. + +--- + +## Module Layout + +``` +backend/app/market/ +├── models.py # PriceUpdate dataclass +├── interface.py # MarketDataSource ABC +├── cache.py # PriceCache +├── seed_prices.py # Seed prices and GBM params for the default watchlist +├── simulator.py # GBMSimulator + SimulatorDataSource +├── massive_client.py # MassiveDataSource +├── factory.py # create_market_data_source() +└── stream.py # FastAPI SSE router factory +``` + +--- + +## Public Imports + +```python +from app.market import ( + PriceCache, + PriceUpdate, + MarketDataSource, + create_market_data_source, + create_stream_router, +) +``` + +--- + +## `PriceUpdate` — Core Data Model + +Immutable frozen dataclass. Every price event is a `PriceUpdate`. + +```python +@dataclass(frozen=True, slots=True) +class PriceUpdate: + ticker: str + price: float + previous_price: float + timestamp: float # Unix seconds + + # Computed properties + @property + def change(self) -> float: ... # price - previous_price + @property + def change_percent(self) -> float: ... # % change + @property + def direction(self) -> str: ... # "up" | "down" | "flat" + + def to_dict(self) -> dict: ... # JSON-serialisable +``` + +--- + +## `PriceCache` — Shared State + +Thread-safe. One writer, many readers. + +```python +cache = PriceCache() + +# Write (done by the data source background task) +update: PriceUpdate = cache.update(ticker="AAPL", price=191.50) + +# Read +update: PriceUpdate | None = cache.get("AAPL") +price: float | None = cache.get_price("AAPL") +all: dict[str, PriceUpdate] = cache.get_all() + +# Remove (called when user removes ticker from watchlist) +cache.remove("AAPL") + +# Change detection for SSE +version: int = cache.version # increments on every update +``` + +--- + +## `MarketDataSource` — Abstract Interface + +Both `SimulatorDataSource` and `MassiveDataSource` implement this: + +```python +class MarketDataSource(ABC): + async def start(self, tickers: list[str]) -> None: ... + async def stop(self) -> None: ... + async def add_ticker(self, ticker: str) -> None: ... + async def remove_ticker(self, ticker: str) -> None: ... + def get_tickers(self) -> list[str]: ... +``` + +--- + +## Factory — Source Selection + +```python +from app.market import create_market_data_source + +source = create_market_data_source(cache) +# Returns MassiveDataSource if MASSIVE_API_KEY is set and non-empty. +# Returns SimulatorDataSource otherwise. +``` + +Selection logic in `factory.py`: + +```python +api_key = os.environ.get("MASSIVE_API_KEY", "").strip() +if api_key: + return MassiveDataSource(api_key=api_key, price_cache=cache) +else: + return SimulatorDataSource(price_cache=cache) +``` + +--- + +## Application Lifecycle + +Typically wired into FastAPI's lifespan: + +```python +@asynccontextmanager +async def lifespan(app: FastAPI): + cache = PriceCache() + source = create_market_data_source(cache) + tickers = load_watchlist_from_db() # e.g. ["AAPL", "GOOGL", ...] + await source.start(tickers) + app.state.cache = cache + app.state.source = source + yield + await source.stop() +``` + +--- + +## Watchlist Integration + +When the user adds or removes a ticker via the watchlist API: + +```python +# Add +await app.state.source.add_ticker("PYPL") + +# Remove +await app.state.source.remove_ticker("GOOGL") +# Also removes from PriceCache automatically +``` + +The SSE stream's ticker scope is determined by `source.get_tickers()` at stream time — it includes all active watchlist tickers plus any tickers with open positions. + +--- + +## SSE Streaming + +```python +from app.market import create_stream_router + +router = create_stream_router(price_cache) +app.include_router(router, prefix="/api") +# Registers: GET /api/stream/prices (text/event-stream) +``` + +The SSE endpoint uses `cache.version` to detect changes and only emits events when prices have actually updated, preventing redundant pushes (important for the Massive free tier where poll results may not change between ticks). + +--- + +## Environment Variables + +| Variable | Effect | +|----------|--------| +| `MASSIVE_API_KEY` (set) | Uses `MassiveDataSource` — real market data | +| `MASSIVE_API_KEY` (absent/empty) | Uses `SimulatorDataSource` — GBM simulation | + +--- + +## Seed Prices and Default Tickers + +Defined in `seed_prices.py`. Used by both the simulator (starting prices) and as the application's default watchlist: + +``` +AAPL $190 GOOGL $175 MSFT $420 AMZN $185 TSLA $250 +NVDA $800 META $500 JPM $195 V $280 NFLX $600 +``` + +For dynamically-added tickers not in the seed list, the simulator uses default params (`sigma=0.25, mu=0.05`) and a random starting price between $50–$300. diff --git a/planning/MARKET_SIMULATOR.md b/planning/MARKET_SIMULATOR.md new file mode 100644 index 00000000..f7434029 --- /dev/null +++ b/planning/MARKET_SIMULATOR.md @@ -0,0 +1,210 @@ +# Market Simulator — Design and Code Structure + +The simulator generates realistic-looking stock price movements without any external API. It is the default when `MASSIVE_API_KEY` is not set. + +--- + +## Design Goals + +- Visually convincing price movements for a demo/course environment +- Correlated moves across related stocks (tech sector, finance sector) +- Occasional dramatic events to keep the UI interesting +- Runs as an in-process asyncio background task — no external dependencies + +--- + +## Mathematical Model — Geometric Brownian Motion (GBM) + +Each ticker's price evolves as: + +``` +S(t+dt) = S(t) * exp( (mu - sigma²/2) * dt + sigma * sqrt(dt) * Z ) +``` + +| Symbol | Meaning | +|--------|---------| +| `S(t)` | Current price | +| `mu` | Annualised drift (expected return, e.g. 0.05 = 5%/year) | +| `sigma` | Annualised volatility (e.g. 0.25 = 25%/year) | +| `dt` | Time step as a fraction of a trading year | +| `Z` | Standard normal random variable (correlated across tickers) | + +**Time step**: ticks occur every 500ms. + +```python +TRADING_SECONDS_PER_YEAR = 252 * 6.5 * 3600 # = 5,896,800 +dt = 0.5 / TRADING_SECONDS_PER_YEAR # ≈ 8.48e-8 +``` + +This tiny `dt` means each tick produces sub-cent moves that accumulate naturally over time, matching how real intraday charts look. + +--- + +## Correlated Moves (Cholesky Decomposition) + +Real stocks in the same sector move together. The simulator replicates this using a Cholesky decomposition of a correlation matrix. + +**Correlation structure:** + +| Pair | Correlation | +|------|-------------| +| Two tech stocks (AAPL, GOOGL, MSFT, AMZN, META, NVDA, NFLX) | 0.6 | +| Two finance stocks (JPM, V) | 0.5 | +| TSLA with anything | 0.3 (behaves independently) | +| Cross-sector | 0.3 | +| Unknown tickers | 0.3 | + +**Algorithm per tick:** +1. Generate `n` independent standard normals: `Z_independent ~ N(0, I)` +2. Multiply by the Cholesky factor `L` of the correlation matrix: `Z_correlated = L @ Z_independent` +3. Apply each `Z_correlated[i]` to its ticker's GBM formula + +When tickers are added or removed, `_rebuild_cholesky()` recomputes `L` from scratch — O(n²) but n is always small (<50). + +--- + +## Random Shock Events + +Every tick, each ticker has a 0.1% chance of a sudden 2–5% shock (up or down). With 10 tickers at 2 ticks/sec, a shock occurs roughly every 50 seconds. This produces the kind of dramatic single-bar moves that make charts interesting. + +```python +if random.random() < self._event_prob: # default: 0.001 + magnitude = random.uniform(0.02, 0.05) + sign = random.choice([-1, 1]) + self._prices[ticker] *= 1 + magnitude * sign +``` + +--- + +## Per-Ticker Parameters + +Defined in `seed_prices.py`: + +| Ticker | Seed Price | Sigma (vol) | Mu (drift) | Notes | +|--------|-----------|-------------|------------|-------| +| AAPL | $190 | 0.22 | 0.05 | | +| GOOGL | $175 | 0.25 | 0.05 | | +| MSFT | $420 | 0.20 | 0.05 | Lowest vol in tech | +| AMZN | $185 | 0.28 | 0.05 | | +| TSLA | $250 | 0.50 | 0.03 | High vol, lower drift | +| NVDA | $800 | 0.40 | 0.08 | High vol, strong drift | +| META | $500 | 0.30 | 0.05 | | +| JPM | $195 | 0.18 | 0.04 | Low vol (bank) | +| V | $280 | 0.17 | 0.04 | Lowest vol overall | +| NFLX | $600 | 0.35 | 0.05 | | +| Unknown| $50–$300 | 0.25 | 0.05 | Random seed, default params | + +--- + +## Code Structure + +### `GBMSimulator` — Pure Math + +Lives in `simulator.py`. Stateful — holds current prices, params, Cholesky matrix. + +```python +class GBMSimulator: + def __init__(self, tickers: list[str], dt: float = DEFAULT_DT, + event_probability: float = 0.001) -> None: ... + + def step(self) -> dict[str, float]: + """Advance all prices by one dt. Returns {ticker: new_price}. + This is the hot path — called every 500ms.""" + + def add_ticker(self, ticker: str) -> None: + """Add ticker and rebuild Cholesky.""" + + def remove_ticker(self, ticker: str) -> None: + """Remove ticker and rebuild Cholesky.""" + + def get_price(self, ticker: str) -> float | None: ... + def get_tickers(self) -> list[str]: ... +``` + +`GBMSimulator` has no I/O and no asyncio — it's a pure math engine that can be tested synchronously. + +### `SimulatorDataSource` — Async Wrapper + +Also in `simulator.py`. Implements `MarketDataSource`. Owns the asyncio background task. + +```python +class SimulatorDataSource(MarketDataSource): + def __init__(self, price_cache: PriceCache, update_interval: float = 0.5, + event_probability: float = 0.001) -> None: ... + + async def start(self, tickers: list[str]) -> None: + # Creates GBMSimulator, seeds PriceCache with initial prices, + # starts _run_loop() as an asyncio Task + + async def stop(self) -> None: + # Cancels the background task + + async def add_ticker(self, ticker: str) -> None: + # Delegates to GBMSimulator, seeds PriceCache immediately + + async def remove_ticker(self, ticker: str) -> None: + # Delegates to GBMSimulator, removes from PriceCache + + async def _run_loop(self) -> None: + # Core loop: call sim.step(), write to cache, sleep(interval) +``` + +### `seed_prices.py` — Configuration + +Separate module to keep parameters easy to find and adjust: + +```python +SEED_PRICES: dict[str, float] # Starting price per ticker +TICKER_PARAMS: dict[str, dict[str, float]] # sigma and mu per ticker +DEFAULT_PARAMS: dict[str, float] # Fallback for unknown tickers +CORRELATION_GROUPS: dict[str, set[str]] # "tech" and "finance" groups +INTRA_TECH_CORR = 0.6 +INTRA_FINANCE_CORR = 0.5 +CROSS_GROUP_CORR = 0.3 +TSLA_CORR = 0.3 +``` + +--- + +## Data Flow + +``` +asyncio Task: _run_loop() + │ + ├── every 500ms: GBMSimulator.step() + │ ├── generate correlated Z via Cholesky + │ ├── apply GBM formula to each price + │ └── apply random shock (0.1% chance) + │ + └── for each new price: PriceCache.update(ticker, price) + └── creates PriceUpdate(ticker, price, previous_price, timestamp) + └── SSE stream reads from cache +``` + +--- + +## Initialisation Sequence + +``` +SimulatorDataSource.start(tickers): + 1. Create GBMSimulator(tickers) → initialises prices from SEED_PRICES + 2. For each ticker: PriceCache.update(ticker, price) ← so SSE has data immediately + 3. asyncio.create_task(_run_loop()) +``` + +This ensures the cache is populated before any SSE client connects. + +--- + +## Testing + +The `GBMSimulator` and `SimulatorDataSource` are tested in `backend/tests/market/`: + +- `test_simulator.py` — 17 tests covering GBM math, correlation, shock events, add/remove ticker +- `test_simulator_source.py` — 10 integration tests covering lifecycle, cache writes, add/remove + +Key test patterns: +- Call `step()` many times and assert price stays positive (GBM guarantee) +- Assert correlated tickers have higher return correlation than cross-sector pairs +- Assert `add_ticker` and `remove_ticker` correctly update `get_tickers()` +- Assert cache is seeded immediately on `start()` (not after first tick) diff --git a/planning/MASSIVE_API.md b/planning/MASSIVE_API.md new file mode 100644 index 00000000..52eca734 --- /dev/null +++ b/planning/MASSIVE_API.md @@ -0,0 +1,195 @@ +# Massive API (formerly Polygon.io) — Reference + +Massive.com rebranded from Polygon.io on October 30, 2025. The Python client package is `massive` (legacy: `polygon-api-client`, still supported). + +--- + +## Installation + +```bash +uv add massive +``` + +--- + +## Authentication + +```python +from massive import RESTClient + +# Explicit key +client = RESTClient(api_key="YOUR_API_KEY") + +# Or reads MASSIVE_API_KEY env var automatically +client = RESTClient() +``` + +--- + +## Batch Snapshot — Multiple Tickers + +The primary method for fetching prices for a set of tickers in one call. + +```python +from massive import RESTClient +from massive.rest.models import SnapshotMarketType + +client = RESTClient(api_key="...") +snapshots = client.get_snapshot_all( + market_type=SnapshotMarketType.Stocks, + tickers=["AAPL", "GOOGL", "MSFT", "TSLA"], +) +``` + +**Underlying endpoint:** `GET /v2/snapshot/locale/us/markets/stocks/tickers?tickers=AAPL,GOOGL,...` + +The method is **synchronous** (blocking). In an asyncio context, run it in a thread: + +```python +import asyncio + +snapshots = await asyncio.to_thread( + client.get_snapshot_all, + SnapshotMarketType.Stocks, + tickers, +) +``` + +--- + +## Response Model — `TickerSnapshot` + +Each element of the returned list is a `TickerSnapshot`: + +```python +for snap in snapshots: + snap.ticker # str — "AAPL" + snap.last_trade.price # float — latest trade price (aliased from .p) + snap.last_trade.size # float — shares in last trade + snap.last_trade.timestamp # int — nanosecond Unix timestamp + snap.last_quote.bid_price # float — best bid + snap.last_quote.ask_price # float — best ask + snap.day.open # float — today's open + snap.day.high # float — today's high + snap.day.low # float — today's low + snap.day.close # float — today's close / last price during trading + snap.day.volume # float — today's volume + snap.day.vwap # float — today's VWAP + snap.prev_day.close # float — yesterday's close + snap.todays_change # float — dollar change from prev_day.close + snap.todays_change_percent # float — % change from prev_day.close + snap.updated # int — nanosecond timestamp of last update +``` + +### Price Field Selection + +During market hours, `snap.day.close` holds the latest traded price. +Outside hours, fall back to `snap.last_trade.price`. + +```python +def best_price(snap) -> float | None: + try: + return snap.day.close or snap.last_trade.price + except AttributeError: + return None +``` + +--- + +## Previous Close / End-of-Day + +`snap.prev_day.close` is always present in the snapshot response — no separate call needed. + +For a dedicated end-of-day call: + +```python +prev = client.get_previous_close_agg("AAPL") +# Returns a list of OHLCV bars (usually one bar for yesterday) +``` + +--- + +## `SnapshotMarketType` Enum + +```python +from massive.rest.models import SnapshotMarketType + +SnapshotMarketType.Stocks # equities +SnapshotMarketType.Options +SnapshotMarketType.Forex +SnapshotMarketType.Crypto +SnapshotMarketType.Indices +``` + +String values (`"stocks"`) may be passed directly. + +--- + +## Rate Limits + +| Tier | Limit | Recommended poll interval | +|------|-------|--------------------------| +| Free | 5 requests/minute | 15 seconds | +| All paid tiers | Unlimited (soft cap ~100 req/s) | 2–5 seconds | + +Free tier data is **end-of-day only** — `last_trade` prices will be stale outside market hours. + +--- + +## Complete Working Example + +```python +import asyncio +import os +from massive import RESTClient +from massive.rest.models import SnapshotMarketType + +client = RESTClient(api_key=os.environ["MASSIVE_API_KEY"]) + +async def fetch_prices(tickers: list[str]) -> dict[str, float]: + """Return {ticker: price} for all tickers in one API call.""" + snapshots = await asyncio.to_thread( + client.get_snapshot_all, + SnapshotMarketType.Stocks, + tickers, + ) + result: dict[str, float] = {} + for snap in snapshots: + try: + price = snap.day.close or snap.last_trade.price + if price: + result[snap.ticker] = price + except AttributeError: + pass # Ticker had no data + return result + +# Run +prices = asyncio.run(fetch_prices(["AAPL", "GOOGL", "MSFT"])) +# {"AAPL": 190.50, "GOOGL": 175.20, "MSFT": 421.00} +``` + +--- + +## WebSocket Alternative (Real-Time Streaming) + +For continuous real-time data, Massive provides a `WebSocketClient` — but this requires a paid tier and adds complexity. The FinAlly project uses REST polling to keep the integration simple and free-tier compatible. + +```python +from massive import WebSocketClient + +def handle(msgs): + for msg in msgs: + print(f"{msg.symbol} @ ${msg.price}") + +ws = WebSocketClient(subscriptions=["T.AAPL", "T.MSFT"]) +ws.run(handle_msg=handle) +``` + +--- + +## Notes + +- The `RESTClient` has no async methods — always wrap in `asyncio.to_thread` for FastAPI/asyncio use +- Max ~250 tickers per `get_snapshot_all` call +- Free tier only returns end-of-day data; `last_trade.price` will be yesterday's close after hours +- Legacy import `from polygon import RESTClient` still works but `from massive import RESTClient` is preferred diff --git a/planning/REVIEW.md b/planning/REVIEW.md new file mode 100644 index 00000000..0b9fcd10 --- /dev/null +++ b/planning/REVIEW.md @@ -0,0 +1,12 @@ +# Review Findings + +1. High: The integration example in [planning/MARKET_API.md](/Users/steve/projects/finally/planning/MARKET_API.md#L185) registers the SSE router with `app.include_router(router, prefix="/api")`, but the router already has `prefix="/api/stream"` in [backend/app/market/stream.py](/Users/steve/projects/finally/backend/app/market/stream.py#L17). Following the doc literally would mount the endpoint at `/api/api/stream/prices`, not `/api/stream/prices` as claimed in [planning/MARKET_API.md](/Users/steve/projects/finally/planning/MARKET_API.md#L187). That is a broken setup example for anyone wiring this into FastAPI from the planning doc. + +2. High: Ticker removals are not observable by SSE clients. The docs say removing a ticker "also removes from `PriceCache` automatically" in [planning/MARKET_API.md](/Users/steve/projects/finally/planning/MARKET_API.md#L171), and both data sources do call `PriceCache.remove()` on removal in [backend/app/market/simulator.py](/Users/steve/projects/finally/backend/app/market/simulator.py#L233) and [backend/app/market/massive_client.py](/Users/steve/projects/finally/backend/app/market/massive_client.py#L72). But `PriceCache.remove()` does not increment `version` in [backend/app/market/cache.py](/Users/steve/projects/finally/backend/app/market/cache.py#L59), while the SSE loop only emits when `version` changes in [backend/app/market/stream.py](/Users/steve/projects/finally/backend/app/market/stream.py#L75). Result: a removed ticker disappears server-side but connected clients receive no event telling them to drop it, so the UI can keep showing stale data indefinitely. There is also no test covering this removal path. + +3. Medium: [planning/MASSIVE_API.md](/Users/steve/projects/finally/planning/MASSIVE_API.md#L33) documents API usage that conflicts with the shipped implementation. The doc examples use `SnapshotMarketType.Stocks` in three places ([planning/MASSIVE_API.md](/Users/steve/projects/finally/planning/MASSIVE_API.md#L39), [planning/MASSIVE_API.md](/Users/steve/projects/finally/planning/MASSIVE_API.md#L53), [planning/MASSIVE_API.md](/Users/steve/projects/finally/planning/MASSIVE_API.md#L153)), but the code uses `SnapshotMarketType.STOCKS` in [backend/app/market/massive_client.py](/Users/steve/projects/finally/backend/app/market/massive_client.py#L125). The same document labels `last_trade.timestamp` and `updated` as nanosecond values in [planning/MASSIVE_API.md](/Users/steve/projects/finally/planning/MASSIVE_API.md#L69) and [planning/MASSIVE_API.md](/Users/steve/projects/finally/planning/MASSIVE_API.md#L81), while the implementation and tests treat the timestamp as milliseconds and divide by `1000.0` before storing it in cache in [backend/app/market/massive_client.py](/Users/steve/projects/finally/backend/app/market/massive_client.py#L102) and [backend/tests/market/test_massive.py](/Users/steve/projects/finally/backend/tests/market/test_massive.py#L80). That makes the reference doc unsafe to implement from directly. + +## Open Questions / Assumptions + +- I treated untracked files as "changes since last commit" because `git diff HEAD` is empty and all current work is newly added files. +- I reviewed the new docs against the current checked-in backend implementation. I did not review archived planning files unless they affected an active document's correctness. From 7db64bf00d512daa74ba81427cf81898e088bc8f Mon Sep 17 00:00:00 2001 From: Sprite Date: Wed, 8 Apr 2026 18:08:39 +0000 Subject: [PATCH 5/6] Add market data code review Comprehensive review of backend/app/market/ covering test results (73/73 passing), coverage analysis (91% overall), and identified issues with recommendations. Co-Authored-By: Claude Sonnet 4.6 --- planning/MARKET_DATA_REVIEW.md | 191 +++++++++++++++++++++++++++++++++ 1 file changed, 191 insertions(+) create mode 100644 planning/MARKET_DATA_REVIEW.md diff --git a/planning/MARKET_DATA_REVIEW.md b/planning/MARKET_DATA_REVIEW.md new file mode 100644 index 00000000..0790c57a --- /dev/null +++ b/planning/MARKET_DATA_REVIEW.md @@ -0,0 +1,191 @@ +# Market Data Backend — Code Review + +**Date:** 2026-04-08 +**Reviewer:** Claude (Sonnet 4.6) +**Scope:** `backend/app/market/` (8 source modules) and `backend/tests/market/` (6 test modules) +**Environment:** Python 3.13.7, pytest-9.0.3, pytest-asyncio-1.3.0, pytest-cov-7.1.0 + +--- + +## 1. Test Results + +**73 tests collected, 73 passed. All green.** + +``` +tests/market/test_cache.py 13 passed +tests/market/test_factory.py 7 passed +tests/market/test_massive.py 13 passed +tests/market/test_models.py 11 passed +tests/market/test_simulator.py 17 passed +tests/market/test_simulator_source.py 10 passed +``` + +This is an improvement over the earlier archived review (which recorded 5 failures in `test_massive.py`). Those failures have been resolved — tests now use `patch.object(source, "_fetch_snapshots", ...)` rather than patching the `RESTClient` class at module level, making them robust regardless of whether the `massive` package is installed. + +**Lint (ruff):** All checks passed. No warnings in `app/` or `tests/`. + +--- + +## 2. Coverage + +``` +Name Stmts Miss Cover Missing lines +------------------------------------------------------------ +app/__init__.py 0 0 100% +app/market/__init__.py 6 0 100% +app/market/cache.py 39 0 100% +app/market/factory.py 15 0 100% +app/market/interface.py 13 0 100% +app/market/massive_client.py 67 4 94% 85-87, 125 +app/market/models.py 26 0 100% +app/market/seed_prices.py 8 0 100% +app/market/simulator.py 139 3 98% 149, 268-269 +app/market/stream.py 36 24 33% 26-48, 62-87 +------------------------------------------------------------ +TOTAL 349 31 91% +``` + +**Overall: 91%.** The uncovered lines are all explainable: + +- `massive_client.py:85-87` — the `_poll_loop` while-True body. Not covered because tests call `_poll_once` directly rather than letting the loop run. Acceptable: the loop logic is trivial (sleep + call `_poll_once`). +- `massive_client.py:125` — the actual `self._client.get_snapshot_all(...)` call inside `_fetch_snapshots`. Never reached because every test mocks `_fetch_snapshots` itself. Expected — this requires a real Polygon.io API key. +- `simulator.py:149` — the `if ticker in self._prices: return` guard inside `_add_ticker_internal`. Dead code in practice: `add_ticker` (line 122) already checks `if ticker in self._prices: return` before calling `_add_ticker_internal`. The inner guard can never be reached. +- `simulator.py:268-269` — the `logger.exception(...)` line inside the `except Exception` handler in `_run_loop`. Not covered because no test triggers an exception inside the simulation loop. Acceptable for a defensive handler. +- `stream.py:26-48, 62-87` — the entire SSE endpoint and generator. At 33%, this is the largest gap. No integration test exists for the SSE streaming path (requires a running ASGI server). + +--- + +## 3. Architecture Assessment + +The market data subsystem is clean and well-structured. It follows the strategy pattern correctly: + +``` +MarketDataSource (ABC) +├── SimulatorDataSource → GBM simulator (default, no API key needed) +└── MassiveDataSource → Polygon.io REST poller (when MASSIVE_API_KEY set) + │ + ▼ + PriceCache (thread-safe, in-memory, single point of truth) + │ + ├──→ SSE stream endpoint (/api/stream/prices) + ├──→ Portfolio valuation + └──→ Trade execution +``` + +All eight modules have focused, single-responsibility designs with clean boundaries. The public API documented in `__init__.py` and `CLAUDE.md` is accurate and sufficient for downstream code. + +--- + +## 4. Issues Found + +### 4.1 Dead Code in `_add_ticker_internal` (Severity: Low) + +`simulator.py:146-149`: + +```python +def _add_ticker_internal(self, ticker: str) -> None: + if ticker in self._prices: + return # <- dead code: outer add_ticker() already guards this + ... +``` + +`add_ticker()` at line 122 already calls `if ticker in self._prices: return` before calling `_add_ticker_internal`. The inner guard is unreachable via the public API. It would only be hit if `_add_ticker_internal` were called directly with a duplicate (which the `__init__` loop does not do — each ticker in the initial list is distinct by assumption). This is harmless but adds noise and suppresses a coverage warning for the wrong reason. + +**Fix:** Remove the guard from `_add_ticker_internal` and add a comment that this is an internal method assuming no duplicates, or keep it for defensive correctness and add `# pragma: no cover`. + +### 4.2 `version` Property Not Under Lock (Severity: Low) + +`cache.py:64-67`: + +```python +@property +def version(self) -> int: + return self._version # No lock acquired +``` + +All other reads in `PriceCache` acquire `self._lock`. Reading `_version` without a lock is safe on CPython (the GIL makes single-object reads atomic), but Python 3.13 introduced free-threaded mode (PEP 703, `python3.13t`). On a no-GIL build, a concurrent `update()` could produce a torn read of `_version`. Since `PriceCache` is explicitly designed for concurrent use (the docstring calls it thread-safe), this is inconsistent. + +**Fix:** Acquire the lock in the `version` property, or document the GIL assumption explicitly. + +### 4.3 No SSE Integration Test (Severity: Medium) + +`stream.py` is at 33% coverage. The SSE endpoint is the primary consumer of `PriceCache` and the real-time data delivery mechanism for the entire frontend. Yet it has no automated test. Testing it requires an ASGI test client (e.g., `httpx.AsyncClient` with the FastAPI app). Even a basic smoke test — connect, receive one event, verify JSON structure — would add meaningful confidence. + +**Fix:** Add at least one test in a new `tests/market/test_stream.py` using `httpx.AsyncClient(app=app, base_url="http://test")` to verify: +- Response content-type is `text/event-stream` +- Events contain the expected JSON keys +- Generator stops on client disconnect + +### 4.4 Module-Level Router Instance (Severity: Low) + +`stream.py:17`: + +```python +router = APIRouter(prefix="/api/stream", tags=["streaming"]) +``` + +`create_stream_router()` registers the `/prices` route on this module-level `router`. If the function were called twice (e.g., in multiple tests), the route would be registered twice on the same router object, producing duplicate route warnings or unexpected behavior. + +In production this won't happen (called once at startup), but it is a latent test footgun. A cleaner design would create a fresh `APIRouter` inside `create_stream_router()` and return it. + +### 4.5 `SimulatorDataSource` Not Started Before `add_ticker` (Severity: Low) + +`simulator.py:242-249`: + +```python +async def add_ticker(self, ticker: str) -> None: + if self._sim: # guarded + self._sim.add_ticker(ticker) + ... +``` + +If `add_ticker` is called before `start()`, the call is silently dropped (`self._sim` is `None`). The interface contract says nothing about this ordering requirement. A future caller could reasonably call `add_ticker` early and expect it to take effect once `start` is called. The same applies to `remove_ticker`. + +This is low severity for now (the PLAN.md lifecycle is clear: call `start` first), but documenting this constraint in the method docstring would prevent future confusion. + +### 4.6 Missing Test Scenarios (Severity: Low) + +- **All 10 default tickers together**: Tests use 1-2 tickers. There is no test confirming that initializing `GBMSimulator` with all 10 default tickers produces a valid Cholesky decomposition and correct `step()` output. A malformed correlation matrix would raise a `LinAlgError` at runtime. +- **No thread-safety stress test for `PriceCache`**: The locking logic looks correct from inspection, but a test with concurrent writes from multiple threads would verify it empirically. This is especially relevant given the free-threaded Python concern in 4.2. +- **No test for `_poll_loop`**: The polling loop in `MassiveDataSource` is not exercised. Even a short integration test (start, wait one interval, stop) would cover lines 85-87. + +--- + +## 5. Design Observations + +### 5.1 Things Done Well + +- **GBM math is correct.** `S(t+dt) = S(t) * exp((mu - 0.5*sigma^2)*dt + sigma*sqrt(dt)*Z)` is the standard log-normal GBM discretization. Using `math.exp` (not approximation) is correct. +- **Cholesky-correlated moves** via `np.linalg.cholesky` is the right approach. The sector-based structure (tech 0.6, finance 0.5, cross-sector 0.3) is realistic and well-parameterized. +- **GBM parameters are thoughtful.** TSLA at sigma=0.50 vs V at 0.17 reflects real-world volatility differences. NVDA's higher mu (0.08) adds appropriate drift. +- **Immutable `PriceUpdate`** with `frozen=True, slots=True` is the correct design — it's safe to pass around without defensive copies and slightly more memory-efficient. +- **Seed cache on `start()`** ensures the SSE client gets data on the very first poll, avoiding a blank screen on fresh page load. +- **Both data sources handle errors defensively** — exceptions in the simulator loop and the Massive poll loop are caught and logged, keeping the background task alive through transient failures. +- **SSE implementation uses version-based change detection** to avoid redundant payloads. The `retry: 1000\n\n` directive enables browser auto-reconnect. The `X-Accel-Buffering: no` header is a nice operational touch for nginx proxying. +- **`asyncio.to_thread`** is the correct way to run the synchronous Massive `RESTClient` without blocking the event loop. +- **Factory pattern** keeps the source selection logic in one place and downstream code fully source-agnostic. + +### 5.2 Minor Naming Observation + +`TSLA_CORR = 0.3` and `CROSS_GROUP_CORR = 0.3` in `seed_prices.py` are both 0.3 and semantically distinct but numerically identical. Code is correct — `_pairwise_correlation` uses `TSLA_CORR` for TSLA pairs and `CROSS_GROUP_CORR` as the default fallback. The names accurately express the intent. No change needed, but worth noting that these happen to share the same value. + +--- + +## 6. Verdict + +The market data backend is production-quality for the course scope. The code is well-structured, mathematically sound, correctly concurrent, and fully lint-clean. All 73 tests pass. + +**Must address before moving on:** +- Nothing is blocking. All previously reported critical issues (pyproject.toml build config, lazy imports, SSE return type, unused test imports, massive mock patches) have been resolved. + +**Should address:** +1. Add at least one SSE integration test (`test_stream.py`) to bring `stream.py` out of 33% coverage. +2. Add a test for `GBMSimulator` initialized with all 10 default tickers (Cholesky with full set). + +**Nice to have:** +3. Remove or `pragma: no cover` the dead guard in `_add_ticker_internal` (line 149). +4. Move `router = APIRouter(...)` inside `create_stream_router()` to eliminate the double-registration footgun. +5. Add a lock to the `version` property for free-threaded Python correctness. +6. Document the pre-condition that `start()` must be called before `add_ticker`/`remove_ticker`. + +**The subsystem is ready for integration** with the rest of the backend (portfolio API, trade execution, watchlist management, LLM chat). From 639ee2e7a705dfb38e22c2a20341cc42a7237b27 Mon Sep 17 00:00:00 2001 From: stevehome Date: Thu, 9 Apr 2026 19:21:11 +0100 Subject: [PATCH 6/6] Address all market data review findings: add SSE tests, fix thread safety and design issues MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - PriceCache.version: acquire lock (free-threaded Python safety, 4.2) - GBMSimulator._add_ticker_internal: remove dead guard, add caller contract note (4.1) - SimulatorDataSource add/remove_ticker: document start() pre-condition (4.5) - stream.py: move APIRouter creation inside create_stream_router() to prevent double-registration (4.4) - Add test_stream.py: 7 new tests covering _generate_events, router factory, and handler response (4.3) - Add test for all 10 default tickers Cholesky decomposition (4.6) - Add httpx to dev dependencies for async test client Coverage: 91% → 98%. All 81 tests pass, lint clean. Co-Authored-By: Claude Sonnet 4.6 --- backend/app/market/cache.py | 3 +- backend/app/market/simulator.py | 9 +- backend/app/market/stream.py | 5 +- backend/pyproject.toml | 1 + backend/tests/market/test_simulator.py | 21 +++- backend/tests/market/test_stream.py | 128 +++++++++++++++++++++++++ backend/uv.lock | 30 ++++++ 7 files changed, 189 insertions(+), 8 deletions(-) create mode 100644 backend/tests/market/test_stream.py diff --git a/backend/app/market/cache.py b/backend/app/market/cache.py index 896aa521..098a014d 100644 --- a/backend/app/market/cache.py +++ b/backend/app/market/cache.py @@ -65,7 +65,8 @@ def remove(self, ticker: str) -> None: @property def version(self) -> int: """Current version counter. Useful for SSE change detection.""" - return self._version + with self._lock: + return self._version def __len__(self) -> int: with self._lock: diff --git a/backend/app/market/simulator.py b/backend/app/market/simulator.py index b6803f59..fca429fc 100644 --- a/backend/app/market/simulator.py +++ b/backend/app/market/simulator.py @@ -144,9 +144,10 @@ def get_tickers(self) -> list[str]: # --- Internals --- def _add_ticker_internal(self, ticker: str) -> None: - """Add a ticker without rebuilding Cholesky (for batch initialization).""" - if ticker in self._prices: - return + """Add a ticker without rebuilding Cholesky (for batch initialization). + + Assumes no duplicates — callers must check before calling. + """ self._tickers.append(ticker) self._prices[ticker] = SEED_PRICES.get(ticker, random.uniform(50.0, 300.0)) self._params[ticker] = TICKER_PARAMS.get(ticker, dict(DEFAULT_PARAMS)) @@ -240,6 +241,7 @@ async def stop(self) -> None: logger.info("Simulator stopped") async def add_ticker(self, ticker: str) -> None: + """Add a ticker. Must be called after start().""" if self._sim: self._sim.add_ticker(ticker) # Seed cache immediately so the ticker has a price right away @@ -249,6 +251,7 @@ async def add_ticker(self, ticker: str) -> None: logger.info("Simulator: added ticker %s", ticker) async def remove_ticker(self, ticker: str) -> None: + """Remove a ticker. Must be called after start().""" if self._sim: self._sim.remove_ticker(ticker) self._cache.remove(ticker) diff --git a/backend/app/market/stream.py b/backend/app/market/stream.py index 7fd974b7..e5c5aa6c 100644 --- a/backend/app/market/stream.py +++ b/backend/app/market/stream.py @@ -14,14 +14,13 @@ logger = logging.getLogger(__name__) -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. + Returns a fresh APIRouter each call — safe to call multiple times (e.g., in tests). """ + router = APIRouter(prefix="/api/stream", tags=["streaming"]) @router.get("/prices") async def stream_prices(request: Request) -> StreamingResponse: diff --git a/backend/pyproject.toml b/backend/pyproject.toml index e172cca2..40dcabf2 100644 --- a/backend/pyproject.toml +++ b/backend/pyproject.toml @@ -18,6 +18,7 @@ dev = [ "pytest-asyncio>=0.24.0", "pytest-cov>=5.0.0", "ruff>=0.7.0", + "httpx>=0.27.0", ] [build-system] diff --git a/backend/tests/market/test_simulator.py b/backend/tests/market/test_simulator.py index 1845ec16..1b9210a0 100644 --- a/backend/tests/market/test_simulator.py +++ b/backend/tests/market/test_simulator.py @@ -1,6 +1,6 @@ """Tests for GBMSimulator.""" -from app.market.seed_prices import SEED_PRICES +from app.market.seed_prices import SEED_PRICES, TICKER_PARAMS from app.market.simulator import GBMSimulator @@ -116,6 +116,25 @@ def test_pairwise_correlation_cross_sector(self): corr = GBMSimulator._pairwise_correlation("AAPL", "JPM") assert corr == 0.3 + def test_all_ten_default_tickers(self): + """GBMSimulator initializes cleanly with all 10 default tickers. + + Verifies that the 10x10 correlation matrix produces a valid Cholesky + decomposition (i.e., is positive-definite) and that step() returns + correct prices for every ticker. + """ + all_tickers = list(TICKER_PARAMS.keys()) + assert len(all_tickers) == 10 + + sim = GBMSimulator(tickers=all_tickers) + assert sim._cholesky is not None + assert sim._cholesky.shape == (10, 10) + + result = sim.step() + assert set(result.keys()) == set(all_tickers) + for ticker, price in result.items(): + assert price > 0, f"{ticker} price should be positive" + def test_default_dt_is_reasonable(self): """Test that default dt is a reasonable small value.""" assert 0 < GBMSimulator.DEFAULT_DT < 0.0001 diff --git a/backend/tests/market/test_stream.py b/backend/tests/market/test_stream.py new file mode 100644 index 00000000..25ecfd0b --- /dev/null +++ b/backend/tests/market/test_stream.py @@ -0,0 +1,128 @@ +"""Integration tests for SSE streaming endpoint.""" + +import json +from unittest.mock import AsyncMock, MagicMock + +import pytest +from fastapi.responses import StreamingResponse + +from app.market.cache import PriceCache +from app.market.stream import _generate_events, create_stream_router + + +class TestGenerateEvents: + """Tests for the _generate_events async generator.""" + + @pytest.mark.asyncio + async def test_retry_directive_is_first(self): + """First yielded chunk should be the SSE retry directive.""" + cache = PriceCache() + mock_request = MagicMock() + mock_request.client.host = "test" + mock_request.is_disconnected = AsyncMock(return_value=True) + + events = [] + async for event in _generate_events(cache, mock_request, interval=0.0): + events.append(event) + + assert events[0] == "retry: 1000\n\n" + + @pytest.mark.asyncio + async def test_data_event_json_structure(self): + """Data events should contain valid JSON with expected fields.""" + cache = PriceCache() + cache.update("AAPL", 190.0) + + mock_request = MagicMock() + mock_request.client.host = "test" + # First check: not disconnected (yields data); second: disconnected (stops) + mock_request.is_disconnected = AsyncMock(side_effect=[False, True]) + + events = [] + async for event in _generate_events(cache, mock_request, interval=0.0): + events.append(event) + + data_events = [e for e in events if e.startswith("data:")] + assert len(data_events) >= 1 + + payload = json.loads(data_events[0][len("data: "):]) + assert "AAPL" in payload + aapl = payload["AAPL"] + for key in ("ticker", "price", "previous_price", "timestamp", "direction"): + assert key in aapl, f"Expected key '{key}' in AAPL payload" + + @pytest.mark.asyncio + async def test_no_data_event_when_cache_empty(self): + """No data event should be emitted when the cache is empty.""" + cache = PriceCache() + mock_request = MagicMock() + mock_request.client.host = "test" + mock_request.is_disconnected = AsyncMock(side_effect=[False, True]) + + events = [] + async for event in _generate_events(cache, mock_request, interval=0.0): + events.append(event) + + data_events = [e for e in events if e.startswith("data:")] + assert len(data_events) == 0 + + @pytest.mark.asyncio + async def test_stops_on_immediate_disconnect(self): + """Generator stops immediately when client is already disconnected.""" + cache = PriceCache() + cache.update("AAPL", 190.0) + + mock_request = MagicMock() + mock_request.client.host = "test" + mock_request.is_disconnected = AsyncMock(return_value=True) + + events = [] + async for event in _generate_events(cache, mock_request, interval=0.0): + events.append(event) + + # Only the retry directive should be emitted; no data loop runs + data_events = [e for e in events if e.startswith("data:")] + assert len(data_events) == 0 + + +class TestStreamRouterFactory: + """Tests for create_stream_router.""" + + def test_fresh_router_each_call(self): + """Each call returns a distinct APIRouter instance (no double-registration risk).""" + cache = PriceCache() + router1 = create_stream_router(cache) + router2 = create_stream_router(cache) + assert router1 is not router2 + + def test_router_has_prices_route(self): + """Router must expose the /prices route (path includes the prefix).""" + cache = PriceCache() + router = create_stream_router(cache) + paths = [r.path for r in router.routes] + assert "/api/stream/prices" in paths + + +class TestSSEEndpoint: + """Tests for the /api/stream/prices route handler.""" + + @pytest.mark.asyncio + async def test_handler_returns_streaming_response(self): + """Route handler must return a StreamingResponse with correct headers.""" + cache = PriceCache() + cache.update("AAPL", 190.0) + + router = create_stream_router(cache) + # Extract the registered route handler + route = router.routes[0] + + mock_request = MagicMock() + mock_request.client.host = "test" + mock_request.is_disconnected = AsyncMock(return_value=True) + + response = await route.endpoint(mock_request) + + assert isinstance(response, StreamingResponse) + assert response.media_type == "text/event-stream" + assert response.headers.get("cache-control") == "no-cache" + assert response.headers.get("x-accel-buffering") == "no" diff --git a/backend/uv.lock b/backend/uv.lock index 67d471b2..fd497795 100644 --- a/backend/uv.lock +++ b/backend/uv.lock @@ -177,6 +177,7 @@ dependencies = [ [package.optional-dependencies] dev = [ + { name = "httpx" }, { name = "pytest" }, { name = "pytest-asyncio" }, { name = "pytest-cov" }, @@ -186,6 +187,7 @@ dev = [ [package.metadata] requires-dist = [ { name = "fastapi", specifier = ">=0.115.0" }, + { name = "httpx", marker = "extra == 'dev'", specifier = ">=0.27.0" }, { name = "massive", specifier = ">=1.0.0" }, { name = "numpy", specifier = ">=2.0.0" }, { name = "pytest", marker = "extra == 'dev'", specifier = ">=8.3.0" }, @@ -206,6 +208,19 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/04/4b/29cac41a4d98d144bf5f6d33995617b185d14b22401f75ca86f384e87ff1/h11-0.16.0-py3-none-any.whl", hash = "sha256:63cf8bbe7522de3bf65932fda1d9c2772064ffb3dae62d55932da54b31cb6c86", size = 37515, upload-time = "2025-04-24T03:35:24.344Z" }, ] +[[package]] +name = "httpcore" +version = "1.0.9" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "certifi" }, + { name = "h11" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/06/94/82699a10bca87a5556c9c59b5963f2d039dbd239f25bc2a63907a05a14cb/httpcore-1.0.9.tar.gz", hash = "sha256:6e34463af53fd2ab5d807f399a9b45ea31c3dfa2276f15a2c3f00afff6e176e8", size = 85484, upload-time = "2025-04-24T22:06:22.219Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/7e/f5/f66802a942d491edb555dd61e3a9961140fd64c90bce1eafd741609d334d/httpcore-1.0.9-py3-none-any.whl", hash = "sha256:2d400746a40668fc9dec9810239072b40b4484b640a8c38fd654a024c7a1bf55", size = 78784, upload-time = "2025-04-24T22:06:20.566Z" }, +] + [[package]] name = "httptools" version = "0.7.1" @@ -235,6 +250,21 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/53/cf/878f3b91e4e6e011eff6d1fa9ca39f7eb17d19c9d7971b04873734112f30/httptools-0.7.1-cp314-cp314-win_amd64.whl", hash = "sha256:cfabda2a5bb85aa2a904ce06d974a3f30fb36cc63d7feaddec05d2050acede96", size = 88205, upload-time = "2025-10-10T03:55:00.389Z" }, ] +[[package]] +name = "httpx" +version = "0.28.1" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "anyio" }, + { name = "certifi" }, + { name = "httpcore" }, + { name = "idna" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/b1/df/48c586a5fe32a0f01324ee087459e112ebb7224f646c0b5023f5e79e9956/httpx-0.28.1.tar.gz", hash = "sha256:75e98c5f16b0f35b567856f597f06ff2270a374470a5c2392242528e3e3e42fc", size = 141406, upload-time = "2024-12-06T15:37:23.222Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/2a/39/e50c7c3a983047577ee07d2a9e53faf5a69493943ec3f6a384bdc792deb2/httpx-0.28.1-py3-none-any.whl", hash = "sha256:d909fcccc110f8c7faf814ca82a9a4d816bc5a6dbfea25d6591d6985b8ba59ad", size = 73517, upload-time = "2024-12-06T15:37:21.509Z" }, +] + [[package]] name = "idna" version = "3.11"