diff --git a/.claude/agents/reviewer.md b/.claude/agents/reviewer.md new file mode 100644 index 000000000..23206312e --- /dev/null +++ b/.claude/agents/reviewer.md @@ -0,0 +1,18 @@ +--- +name: reviewer +description: Carry out a comprehensive review of planning/PLAN.md when requested. Use when the user asks for a plan review, spec review, or "run the reviewer". +tools: Read, Grep, Glob, Write, Edit +--- + +You review the project specification and record your feedback. + +## Task + +1. Read `planning/PLAN.md` in full, plus supporting docs in `planning/` (e.g. `MARKET_DATA_SUMMARY.md`, `planning/archive/`) for context on what is already built. +2. Assess the plan for: + - Internal consistency and contradictions + - Ambiguities or underspecified areas that would block an implementing agent + - Architectural risks and questionable design choices + - Gaps — things the plan should address but doesn't +3. Write the review to `planning/REVIEW.md` as well-structured markdown, with concrete, actionable recommendations. Build on the existing Section 13 (Design Decisions Log) rather than repeating it. +4. Report a concise summary of the key findings. diff --git a/.claude/skills/cerebras/SKILL.md b/.claude/skills/cerebras/SKILL.md index 9efd01a38..19d5ec717 100644 --- a/.claude/skills/cerebras/SKILL.md +++ b/.claude/skills/cerebras/SKILL.md @@ -1,5 +1,5 @@ --- -name: cerebras-inference +name: cerebras description: Use this to write code to call an LLM using LiteLLM and OpenRouter with the Cerebras inference provider --- diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml index b5e8cfd4d..37e66f3fd 100644 --- a/.github/workflows/claude-code-review.yml +++ b/.github/workflows/claude-code-review.yml @@ -38,7 +38,8 @@ jobs: claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} plugin_marketplaces: 'https://github.com/anthropics/claude-code.git' plugins: 'code-review@claude-code-plugins' - prompt: '/code-review:code-review ${{ github.repository }}/pull/${{ github.event.pull_request.number }}' + prompt: '/code-review:code-review --comment ${{ github.repository }}/pull/${{ github.event.pull_request.number }}' + claude_args: '--allowedTools "mcp__github_inline_comment__create_inline_comment"' # See https://github.com/anthropics/claude-code-action/blob/main/docs/usage.md # or https://code.claude.com/docs/en/cli-reference for available options diff --git a/.github/workflows/claude.yml b/.github/workflows/claude.yml index d300267f1..6b15fac7a 100644 --- a/.github/workflows/claude.yml +++ b/.github/workflows/claude.yml @@ -46,5 +46,5 @@ jobs: # Optional: Add claude_args to customize behavior and configuration # See https://github.com/anthropics/claude-code-action/blob/main/docs/usage.md # or https://code.claude.com/docs/en/cli-reference for available options - # claude_args: '--allowed-tools Bash(gh pr:*)' + # claude_args: '--allowed-tools Bash(gh pr *)' diff --git a/planning/MARKET_DATA_DESIGN.md b/planning/MARKET_DATA_DESIGN.md new file mode 100644 index 000000000..2104fcee8 --- /dev/null +++ b/planning/MARKET_DATA_DESIGN.md @@ -0,0 +1,1293 @@ +# Market Data Backend — Detailed Design + +> **Status.** The core subsystem below (`backend/app/market/`) is **built, tested, and reviewed** +> — see `planning/MARKET_DATA_SUMMARY.md`. This document has two jobs: +> 1. Describe that shipped code accurately, with real snippets (not the pre-build sketch archived +> at `planning/archive/MARKET_DATA_DESIGN.md`, which has since drifted from what was actually +> implemented). +> 2. Give an implementation-ready design for the pieces PLAN.md §6 and the Design Decisions Log +> call for that are **not** in the shipped code yet: the day-change **anchor**, ticker-format +> **validation**, the SSE **keepalive**, and the **watchlist ∪ positions** tracked-ticker +> invariant. These are called out explicitly wherever they touch already-shipped, already-tested +> files — this is a modification of reviewed code, not a greenfield add. +> +> Every section states which category it's in. + +--- + +## Table of Contents + +1. [Architecture Recap](#1-architecture-recap) +2. [File Structure](#2-file-structure) +3. [Data Model — `models.py`](#3-data-model--modelspy) +4. [Price Cache — `cache.py`](#4-price-cache--cachepy) +5. [Abstract Interface — `interface.py`](#5-abstract-interface--interfacepy) +6. [Ticker Validation — `validation.py` (new)](#6-ticker-validation--validationpy-new) +7. [Seed Prices & Correlation — `seed_prices.py`](#7-seed-prices--correlation--seed_pricespy) +8. [GBM Simulator — `simulator.py`](#8-gbm-simulator--simulatorpy) +9. [Massive API Client — `massive_client.py`](#9-massive-api-client--massive_clientpy) +10. [Factory — `factory.py`](#10-factory--factorypy) +11. [SSE Streaming Endpoint — `stream.py`](#11-sse-streaming-endpoint--streampy) +12. [FastAPI Lifespan Integration](#12-fastapi-lifespan-integration) +13. [Watchlist ↔ Positions Reconciliation](#13-watchlist--positions-reconciliation) +14. [Concurrency & Deployment Constraints](#14-concurrency--deployment-constraints) +15. [Testing Plan for the New Pieces](#15-testing-plan-for-the-new-pieces) +16. [Error Handling & Edge Cases](#16-error-handling--edge-cases) +17. [Configuration Summary](#17-configuration-summary) + +--- + +## 1. Architecture Recap + +``` +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, holds price + day-change anchor) + │ + ├──→ SSE stream endpoint (/api/stream/prices) — pushes anchor + keepalive + ├──→ Portfolio valuation + └──→ Trade execution +``` + +Both data sources implement one interface (`MarketDataSource`) and write into one shared +`PriceCache`; everything downstream (SSE, portfolio, trades) is source-agnostic and only ever +touches the cache, never the source directly (except to call `add_ticker` / `remove_ticker`). + +--- + +## 2. File Structure + +``` +backend/ + app/ + market/ + __init__.py # Re-exports the public API + models.py # PriceUpdate dataclass [MODIFY: + anchor] + cache.py # PriceCache (thread-safe in-memory store) [MODIFY: + anchor map] + interface.py # MarketDataSource ABC [unchanged] + validation.py # validate_ticker() [NEW] + seed_prices.py # SEED_PRICES, TICKER_PARAMS, correlation constants [unchanged] + simulator.py # GBMSimulator + SimulatorDataSource [unchanged] + massive_client.py # MassiveDataSource [MODIFY: + anchor] + factory.py # create_market_data_source() [unchanged] + stream.py # SSE endpoint (FastAPI router) [MODIFY: + keepalive, anchor] + reconcile.py # reconcile_tracked_tickers() helper [NEW — thin, platform-facing] +``` + +`reconcile.py` is placed in `app/market/` (rather than a future `app/portfolio/` or +`app/watchlist/` module) because it only orchestrates calls into `MarketDataSource` — it has no +DB code of its own, just a documented call contract the platform/watchlist/trade routes use. See +§13. + +--- + +## 3. Data Model — `models.py` + +**Currently shipped** (`backend/app/market/models.py`): + +```python +@dataclass(frozen=True, slots=True) +class PriceUpdate: + ticker: str + price: float + previous_price: float + timestamp: float = field(default_factory=time.time) # Unix seconds + + @property + def change(self) -> float: ... + @property + def change_percent(self) -> float: ... + @property + def direction(self) -> str: ... # "up" | "down" | "flat" + def to_dict(self) -> dict: ... +``` + +No `anchor` field exists. `change`/`change_percent`/`direction` are all **tick-to-tick** (vs. the +immediately preceding price), not day-over-day. PLAN.md §6 / Decision #1 need a separate +**day-change anchor** — the previous close (Massive) or first-observed price (simulator) — so the +UI can show "day change %" that doesn't reset every 500ms. + +### 3.1 Design: add `anchor` [MODIFY] + +```python +from __future__ import annotations + +import time +from dataclasses import dataclass, field + + +@dataclass(frozen=True, slots=True) +class PriceUpdate: + """Immutable snapshot of a single ticker's price at a point in time.""" + + ticker: str + price: float + previous_price: float + anchor: float # NEW — day-change baseline + timestamp: float = field(default_factory=time.time) # Unix seconds + + @property + def change(self) -> float: + """Absolute price change from the previous tick.""" + return round(self.price - self.previous_price, 4) + + @property + def change_percent(self) -> float: + """Percentage change from the previous tick.""" + if self.previous_price == 0: + return 0.0 + return round((self.price - self.previous_price) / self.previous_price * 100, 4) + + @property + def direction(self) -> str: + """'up', 'down', or 'flat' — tick-to-tick, drives the flash animation.""" + if self.price > self.previous_price: + return "up" + elif self.price < self.previous_price: + return "down" + return "flat" + + @property + def day_change(self) -> float: + """Absolute change vs. the day-change anchor (previous close / first observed).""" + return round(self.price - self.anchor, 4) + + @property + def day_change_percent(self) -> float: + """Percentage change vs. the day-change anchor. This is the '% change' shown + next to each ticker in the watchlist — NOT change_percent, which is per-tick.""" + if self.anchor == 0: + return 0.0 + return round((self.price - self.anchor) / self.anchor * 100, 4) + + def to_dict(self) -> dict: + """Serialize for JSON / SSE transmission.""" + return { + "ticker": self.ticker, + "price": self.price, + "previous_price": self.previous_price, + "anchor": self.anchor, + "timestamp": self.timestamp, + "change": self.change, + "change_percent": self.change_percent, + "direction": self.direction, + "day_change": self.day_change, + "day_change_percent": self.day_change_percent, + } +``` + +**Why a required field, not `Optional[float] = None`:** every `PriceUpdate` the cache ever +constructs has an anchor by construction (§4.1 — the cache captures it on first write). Making it +required means every consumer (frontend, tests) can rely on it always being a number, never +`null`. + +**Naming note:** keep `change` / `change_percent` (tick-to-tick, drives the flash) distinct from +`day_change` / `day_change_percent` (vs. anchor, drives the watchlist % column). The frontend +needs both — flashing on every tick uses `direction`; the persistent "% change" label uses +`day_change_percent`. Don't collapse these into one field. + +### 3.2 Timestamp format — explicit exception to Decision #19 [DOCUMENT] + +Decision #19 says "UTC ISO-8601 with `Z` everywhere," but the market layer has always used **Unix +epoch seconds** (`float`) end to end, and this document keeps it that way rather than converting +at the SSE boundary: + +- It's what `time.time()` and Massive's `last_trade.timestamp / 1000.0` naturally produce. +- The frontend plots timestamps on a numeric x-axis (Recharts) — epoch floats need no parsing; + ISO strings would require `Date.parse()` on every one of ~2 points/sec/ticker. +- This is a price-stream-only exception. Every *persisted* timestamp (`trades.executed_at`, + `portfolio_snapshots.recorded_at`, `chat_messages.created_at`, …) is still UTC ISO-8601 with + `Z`, per §7. Only the live SSE payload and in-memory `PriceUpdate.timestamp` are epoch seconds. + +--- + +## 4. Price Cache — `cache.py` + +**Currently shipped**: `PriceCache` stores `dict[str, PriceUpdate]` behind a `threading.Lock`, +with `update() / get() / get_all() / get_price() / remove()` and a `version` counter for SSE +change detection. Full current source: + +```python +class PriceCache: + def __init__(self) -> None: + self._prices: dict[str, PriceUpdate] = {} + self._lock = Lock() + self._version: int = 0 + + def update(self, ticker: str, price: float, timestamp: float | None = None) -> PriceUpdate: + with self._lock: + ts = timestamp or time.time() + prev = self._prices.get(ticker) + previous_price = prev.price if prev else price + update = PriceUpdate( + ticker=ticker, + price=round(price, 2), + previous_price=round(previous_price, 2), + timestamp=ts, + ) + self._prices[ticker] = update + self._version += 1 + return update + # get / get_all / get_price / remove / version / __len__ / __contains__ +``` + +There is **no** concept of an anchor anywhere in this file. + +### 4.1 Design: anchor capture [MODIFY] + +The cache is the natural owner of the anchor because it already owns the "have we seen this +ticker before?" check (`prev = self._prices.get(ticker)`). Add a parallel `_anchors` map, captured +once per ticker and never mutated by the tick loop: + +```python +from __future__ import annotations + +import time +from threading import Lock + +from .models import PriceUpdate + + +class PriceCache: + """Thread-safe in-memory cache of the latest price for each ticker. + + Writers: SimulatorDataSource or MassiveDataSource (one at a time). + Readers: SSE streaming endpoint, portfolio valuation, trade execution. + """ + + def __init__(self) -> None: + self._prices: dict[str, PriceUpdate] = {} + self._anchors: dict[str, float] = {} # NEW — ticker -> day-change baseline + self._lock = Lock() + self._version: int = 0 + + def update( + self, + ticker: str, + price: float, + timestamp: float | None = None, + anchor: float | None = None, # NEW + ) -> PriceUpdate: + """Record a new price for a ticker. Returns the created PriceUpdate. + + `anchor`, when given, is the day-change baseline for this ticker (e.g. Massive's + previous close). It is captured only on the *first* update seen for a ticker — later + calls ignore the argument and keep whatever anchor was captured first, so day-change + stays stable across a session (Decision #1 / #11: re-anchors only on process restart, + because the cache itself is rebuilt then). + + If no anchor is given (simulator mode, or Massive's previous close is briefly + unavailable), the anchor defaults to this call's `price` — "first observed price." + """ + with self._lock: + ts = timestamp or time.time() + prev = self._prices.get(ticker) + previous_price = prev.price if prev else price + + if ticker not in self._anchors: + self._anchors[ticker] = round(anchor if anchor is not None else price, 2) + + update = PriceUpdate( + ticker=ticker, + price=round(price, 2), + previous_price=round(previous_price, 2), + anchor=self._anchors[ticker], + timestamp=ts, + ) + self._prices[ticker] = update + self._version += 1 + return update + + def get_anchor(self, ticker: str) -> float | None: # NEW + """The captured day-change baseline for a ticker, or None if untracked.""" + with self._lock: + return self._anchors.get(ticker) + + def remove(self, ticker: str) -> None: + """Remove a ticker from the cache (e.g., when removed from watchlist). + + Also drops its anchor: if the ticker is re-tracked later (re-added to the watchlist, + or a new position reopens it), it re-anchors fresh from that moment — consistent with + "first observed price after tracking started" (Decision #1). + """ + with self._lock: + self._prices.pop(ticker, None) + self._anchors.pop(ticker, None) # NEW + + # get / get_all / get_price / version / __len__ / __contains__ — unchanged +``` + +`get / get_all / get_price / version / __len__ / __contains__` are untouched — they already +return `PriceUpdate` objects, which now simply carry the extra `anchor` field for free. + +### 4.2 Test impact + +`cache.update(...)` gains one optional kwarg; every existing call site (simulator, Massive, +all 13 tests in `test_cache.py`) keeps working unchanged because `anchor` defaults to `None` → +first-observed-price behavior, which is exactly what the simulator needs. New tests are listed in +§15. + +--- + +## 5. Abstract Interface — `interface.py` + +**Unchanged.** `MarketDataSource` stays exactly as shipped — `start / stop / add_ticker / +remove_ticker / get_tickers`. The anchor is a `PriceCache` concern; sources don't need new methods, +they just optionally pass `anchor=` into `cache.update()` (Massive does, simulator doesn't). + +```python +class MarketDataSource(ABC): + @abstractmethod + async def start(self, tickers: list[str]) -> None: ... + @abstractmethod + async def stop(self) -> None: ... + @abstractmethod + async def add_ticker(self, ticker: str) -> None: ... + @abstractmethod + async def remove_ticker(self, ticker: str) -> None: ... + @abstractmethod + def get_tickers(self) -> list[str]: ... +``` + +--- + +## 6. Ticker Validation — `validation.py` (new) + +PLAN.md §6 / Decision #3: "Tickers are validated as 1–5 uppercase letters before being accepted." +This does not exist anywhere in the shipped market code today — `GBMSimulator` happily accepts any +string as a ticker (`SEED_PRICES.get(ticker, random.uniform(50, 300))` works for `"nvda"`, +`"NOT-A-TICKER"`, `""`, anything), and `MassiveDataSource.add_ticker` just +`.upper().strip()`s its input with no rejection. + +Per §1.5 of `planning/REVIEW.md`, this is correctly scoped as **API-boundary validation**, not a +simulator change — the simulator's "synthesize a seed price for anything" behavior is already +correct and should stay permissive internally (defense in depth is cheap, but the single source of +truth for the format rule belongs at the edge, called from every write path once). + +```python +"""Ticker symbol validation shared by every write path that accepts a ticker.""" + +from __future__ import annotations + +import re + +_TICKER_RE = re.compile(r"^[A-Z]{1,5}$") + + +class InvalidTickerError(ValueError): + """Raised when a ticker does not match the 1-5 uppercase letter format.""" + + +def validate_ticker(raw: str) -> str: + """Normalize and validate a ticker symbol. + + Uppercases and strips whitespace, then requires 1-5 letters A-Z (Decision #3). + Returns the normalized ticker on success; raises InvalidTickerError otherwise. + + Callers (all four write paths, per REVIEW.md §5.13): + - POST /api/watchlist (manual watchlist add) + - POST /api/portfolio/trade (manual trade) + - LLM `trades[].ticker` (chat-initiated trade) + - LLM `watchlist_changes[].ticker` (chat-initiated watchlist change) + """ + ticker = raw.strip().upper() + if not _TICKER_RE.match(ticker): + raise InvalidTickerError( + f"Invalid ticker '{raw}': must be 1-5 letters (A-Z)." + ) + return ticker +``` + +### Usage at the API boundary + +```python +from fastapi import APIRouter, HTTPException +from app.market.validation import InvalidTickerError, validate_ticker + +router = APIRouter(prefix="/api") + + +@router.post("/watchlist") +async def add_to_watchlist(payload: WatchlistAdd): + try: + ticker = validate_ticker(payload.ticker) + except InvalidTickerError as e: + raise HTTPException(status_code=400, detail={"error": str(e)}) from e + # ... insert `ticker` into the watchlist table, call reconcile helper (§13) ... +``` + +The LLM-triggered paths reuse the *same* `validate_ticker` call inside trade/watchlist execution +— per §8 / Decision #16, a chat-initiated failure is **not** an HTTP error, it's folded into the +chat response's per-action result: + +```python +try: + ticker = validate_ticker(trade_spec.ticker) +except InvalidTickerError as e: + results.append({"ticker": trade_spec.ticker, "status": "error", "error": str(e)}) + continue +``` + +--- + +## 7. Seed Prices & Correlation — `seed_prices.py` + +**Unchanged — shipped and correct.** Pure constants, no logic: + +```python +SEED_PRICES: dict[str, float] = { + "AAPL": 190.00, "GOOGL": 175.00, "MSFT": 420.00, "AMZN": 185.00, "TSLA": 250.00, + "NVDA": 800.00, "META": 500.00, "JPM": 195.00, "V": 280.00, "NFLX": 600.00, +} + +TICKER_PARAMS: dict[str, dict[str, float]] = { + "AAPL": {"sigma": 0.22, "mu": 0.05}, + "GOOGL": {"sigma": 0.25, "mu": 0.05}, + "MSFT": {"sigma": 0.20, "mu": 0.05}, + "AMZN": {"sigma": 0.28, "mu": 0.05}, + "TSLA": {"sigma": 0.50, "mu": 0.03}, # High volatility + "NVDA": {"sigma": 0.40, "mu": 0.08}, # High volatility, strong drift + "META": {"sigma": 0.30, "mu": 0.05}, + "JPM": {"sigma": 0.18, "mu": 0.04}, # Low volatility (bank) + "V": {"sigma": 0.17, "mu": 0.04}, # Low volatility (payments) + "NFLX": {"sigma": 0.35, "mu": 0.05}, +} + +DEFAULT_PARAMS: dict[str, float] = {"sigma": 0.25, "mu": 0.05} + +CORRELATION_GROUPS: dict[str, set[str]] = { + "tech": {"AAPL", "GOOGL", "MSFT", "AMZN", "META", "NVDA", "NFLX"}, + "finance": {"JPM", "V"}, +} + +INTRA_TECH_CORR = 0.6 # Tech stocks move together +INTRA_FINANCE_CORR = 0.5 # Finance stocks move together +CROSS_GROUP_CORR = 0.3 # Between sectors / unknown tickers +TSLA_CORR = 0.3 # TSLA does its own thing +``` + +These 10 tickers must stay the single source of truth for the DB's default watchlist seed too +(REVIEW.md §6.6) — when the backend platform agent writes the schema seed logic, import +`SEED_PRICES.keys()` (or a small `DEFAULT_WATCHLIST: list[str]` re-export) rather than +hardcoding the 10 symbols a second time in SQL/seed code. + +--- + +## 8. GBM Simulator — `simulator.py` + +**Unchanged — shipped and correct**, including the unknown-ticker fallback. Full design rationale +(already implemented, not a new build): + +### 8.1 The math + +``` +S(t+dt) = S(t) * exp((mu - sigma^2/2) * dt + sigma * sqrt(dt) * Z) +``` + +`dt` is 500ms expressed as a fraction of a trading year (`0.5 / (252 * 6.5 * 3600) ≈ 8.48e-8`), +producing sub-cent moves per tick that accumulate naturally. `Z` is a **correlated** standard +normal, generated once per tick for all tickers via `Cholesky(corr_matrix) @ independent_normals` +— tech names at ρ=0.6, finance at ρ=0.5, everything else (including TSLA, deliberately excluded +from its own tech bucket) at ρ=0.3. + +```python +class GBMSimulator: + TRADING_SECONDS_PER_YEAR = 252 * 6.5 * 3600 # 5,896,800 + DEFAULT_DT = 0.5 / TRADING_SECONDS_PER_YEAR # ~8.48e-8 + + def __init__(self, tickers, dt=DEFAULT_DT, event_probability=0.001): + self._dt = dt + self._event_prob = event_probability + self._tickers: list[str] = [] + self._prices: dict[str, float] = {} + self._params: dict[str, dict[str, float]] = {} + self._cholesky: np.ndarray | None = None + for ticker in tickers: + self._add_ticker_internal(ticker) + self._rebuild_cholesky() + + def step(self) -> dict[str, float]: + n = len(self._tickers) + if n == 0: + return {} + z_independent = np.random.standard_normal(n) + z_correlated = self._cholesky @ z_independent if self._cholesky is not None else z_independent + + result = {} + for i, ticker in enumerate(self._tickers): + mu, sigma = self._params[ticker]["mu"], self._params[ticker]["sigma"] + drift = (mu - 0.5 * sigma ** 2) * self._dt + diffusion = sigma * math.sqrt(self._dt) * z_correlated[i] + self._prices[ticker] *= math.exp(drift + diffusion) + + if random.random() < self._event_prob: # ~0.1%/tick/ticker + shock = random.uniform(0.02, 0.05) * random.choice([-1, 1]) + self._prices[ticker] *= 1 + shock + + result[ticker] = round(self._prices[ticker], 2) + return result +``` + +### 8.2 Unknown-ticker synthesis (already built) + +```python +def _add_ticker_internal(self, ticker: str) -> None: + if ticker in self._prices: + return + 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)) +``` + +Any ticker not in `SEED_PRICES` gets a random seed in `[$50, $300]` and `DEFAULT_PARAMS` +(`sigma=0.25, mu=0.05`) with correlation `CROSS_GROUP_CORR` (0.3) to everything else. This already +satisfies Decision #3's "simulator synthesizes a seed price + default GBM params" clause — the +**only** missing piece was format validation, which now lives one layer up (§6), not in the +simulator. The simulator intentionally stays permissive; `validate_ticker()` at the API boundary +is what actually enforces "1-5 uppercase letters." + +### 8.3 `SimulatorDataSource` — async wrapper (unchanged) + +```python +class SimulatorDataSource(MarketDataSource): + def __init__(self, price_cache, update_interval=0.5, event_probability=0.001): + self._cache = price_cache + self._interval = update_interval + self._event_prob = event_probability + self._sim: GBMSimulator | None = None + self._task: asyncio.Task | None = None + + async def start(self, tickers: list[str]) -> None: + self._sim = GBMSimulator(tickers=tickers, event_probability=self._event_prob) + for ticker in tickers: # seed cache immediately + price = self._sim.get_price(ticker) + if price is not None: + self._cache.update(ticker=ticker, price=price) # anchor defaults to this price + self._task = asyncio.create_task(self._run_loop(), name="simulator-loop") + + 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 + + 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) # re-anchors fresh here + + async def remove_ticker(self, ticker: str) -> None: + if self._sim: + self._sim.remove_ticker(ticker) + self._cache.remove(ticker) # drops anchor too (§4.1) + + def get_tickers(self) -> list[str]: + return self._sim.get_tickers() if self._sim else [] + + async def _run_loop(self) -> None: + while True: + try: + if self._sim: + for ticker, price in self._sim.step().items(): + self._cache.update(ticker=ticker, price=price) # no anchor= passed + except Exception: + logger.exception("Simulator step failed") + await asyncio.sleep(self._interval) +``` + +No changes needed here for the anchor work: the simulator never passes `anchor=` — every call +relies on `PriceCache.update()`'s default ("first observed price"), which is exactly the +simulator's documented anchor semantics (Decision #1). + +### 8.4 Concurrency note (existing constraint, not a change) + +`add_ticker` / `remove_ticker` mutate `self._tickers` / `self._params` and rebuild the Cholesky +factor with **no lock**, while `step()` iterates the same structures every 500ms on the event-loop +task. This is safe only if every caller of `add_ticker`/`remove_ticker` runs on the event loop +thread — i.e., watchlist and trade routes that call `source.add_ticker()` **must be `async def`**, +never a sync route FastAPI would dispatch to its thread pool. See §14. + +--- + +## 9. Massive API Client — `massive_client.py` + +**Shipped, needs one addition**: plumbing the previous-close anchor through to the cache. + +### 9.1 Current shape (unchanged parts) + +```python +class MassiveDataSource(MarketDataSource): + def __init__(self, api_key: str, price_cache: PriceCache, poll_interval: float = 15.0): + self._api_key = api_key + self._cache = price_cache + self._interval = poll_interval + self._tickers: list[str] = [] + self._task: asyncio.Task | None = None + self._client: RESTClient | None = None + + async def start(self, tickers: list[str]) -> None: + self._client = RESTClient(api_key=self._api_key) + self._tickers = list(tickers) + await self._poll_once() # immediate first poll + self._task = asyncio.create_task(self._poll_loop(), name="massive-poller") + + async def stop(self) -> None: ... # cancel task, clear client — unchanged + async def add_ticker(self, ticker: str) -> None: ... # append to list — unchanged + async def remove_ticker(self, ticker: str) -> None: ... # remove + cache.remove — unchanged + def get_tickers(self) -> list[str]: ... + + async def _poll_loop(self) -> None: + while True: + await asyncio.sleep(self._interval) + await self._poll_once() + + def _fetch_snapshots(self) -> list: + return self._client.get_snapshot_all( + market_type=SnapshotMarketType.STOCKS, + tickers=self._tickers, + ) +``` + +### 9.2 Design: anchor plumbing in `_poll_once` [MODIFY] + +The Massive/Polygon snapshot object exposes `snap.day.previous_close` alongside +`snap.last_trade.price` (confirmed against the `massive` package's snapshot schema — see +`planning/archive/MASSIVE_API.md` §1 for the full response shape). Read it defensively, since a +malformed or partial snapshot must not break price updates — the price is more important than the +anchor: + +```python +async def _poll_once(self) -> None: + """Execute one poll cycle: fetch snapshots, update cache.""" + if not self._tickers or not self._client: + return + + try: + snapshots = await asyncio.to_thread(self._fetch_snapshots) + processed = 0 + for snap in snapshots: + try: + price = snap.last_trade.price + timestamp = snap.last_trade.timestamp / 1000.0 # ms -> seconds + + # Best-effort previous-close anchor. If the field is missing/None on this + # snapshot (pre-market, a thin plan tier, a transient partial response), + # fall back silently to PriceCache's own "first observed" default by + # passing anchor=None — never let a missing anchor drop the price update. + anchor = getattr(getattr(snap, "day", None), "previous_close", None) + + self._cache.update( + ticker=snap.ticker, + price=price, + timestamp=timestamp, + anchor=anchor, + ) + processed += 1 + except (AttributeError, TypeError) as e: + logger.warning("Skipping snapshot for %s: %s", getattr(snap, "ticker", "???"), e) + logger.debug("Massive poll: updated %d/%d tickers", processed, len(self._tickers)) + + except Exception as e: + logger.error("Massive poll failed: %s", e) + # Don't re-raise — retried on the next interval. +``` + +Recall `PriceCache.update()` only *uses* the `anchor` argument on a ticker's first write (§4.1); +every poll after that passes `anchor=snap.day.previous_close` again but it's a no-op — the cache +already locked in the value. This matters because Polygon's `day` object resets at market open, so +by mid-session `previous_close` is stable and matches what was captured on the first poll anyway. + +**Outside market hours** (§6 of PLAN.md: "prices are the last known close and simply stop +changing"): `last_trade.price` continues to reflect the last trade (which may be during +after-hours or literally the prior session's close), and the anchor stays whatever was captured on +the first poll of this process's lifetime — consistent with Decision #11 (re-anchor only on +restart). + +### 9.3 Lazy import — no longer applicable + +The archived pre-build design (`planning/archive/MARKET_DATA_DESIGN.md` §7) described a lazy +`from massive import RESTClient` inside `start()`. The shipped code imports `massive` at module +top level instead (see `planning/MARKET_DATA_SUMMARY.md`, review fix #2) because `massive` is a +core dependency of `pyproject.toml`, not optional — this document reflects the shipped choice, not +the archived one. + +--- + +## 10. Factory — `factory.py` + +**Unchanged.** + +```python +def create_market_data_source(price_cache: PriceCache) -> MarketDataSource: + """MASSIVE_API_KEY set and non-empty -> MassiveDataSource; otherwise -> SimulatorDataSource.""" + api_key = os.environ.get("MASSIVE_API_KEY", "").strip() + if api_key: + return MassiveDataSource(api_key=api_key, price_cache=price_cache) + return SimulatorDataSource(price_cache=price_cache) +``` + +```python +price_cache = PriceCache() +source = create_market_data_source(price_cache) +await source.start(initial_tickers) # e.g. watchlist ∪ open positions, see §13 +``` + +--- + +## 11. SSE Streaming Endpoint — `stream.py` + +### 11.1 Shipped behavior + +A single FastAPI route (`GET /api/stream/prices`) returns a `StreamingResponse` over an async +generator. Every ~500ms it checks `PriceCache.version`; if it changed since the last send, it +serializes **all** tracked tickers into one JSON object keyed by ticker and yields one `data:` +line. There is currently **no** periodic keepalive — the loop only ever yields when the version +changes. + +```python +async def _generate_events(price_cache, request, interval: float = 0.5) -> AsyncGenerator[str, None]: + yield "retry: 1000\n\n" + last_version = -1 + try: + while True: + if await request.is_disconnected(): + break + current_version = price_cache.version + if current_version != last_version: + last_version = current_version + prices = price_cache.get_all() + if prices: + data = {ticker: update.to_dict() for ticker, update in prices.items()} + yield f"data: {json.dumps(data)}\n\n" + await asyncio.sleep(interval) + except asyncio.CancelledError: + pass +``` + +Wire format (unchanged by this design — this envelope shape is correct and stays): + +``` +data: {"AAPL":{"ticker":"AAPL","price":190.50,"previous_price":190.42,"anchor":188.30,"timestamp":1707580800.5,"change":0.08,"change_percent":0.042,"direction":"up","day_change":2.20,"day_change_percent":1.168},"GOOGL":{...}} + +``` + +It's a **whole-snapshot object per event**, not one event per ticker — this is what the frontend +agent's `EventSource.onmessage` handler must parse (`JSON.parse(event.data)` → `{ticker: {...}}`). + +### 11.2 Design: keepalive [MODIFY] + +PLAN.md §6 / Decision #14: "The server emits a `: keepalive` comment every ~15s so the client can +distinguish an idle stream from a dropped connection." In Massive mode with a 15s free-tier poll +interval, or outside market hours when prices "simply stop changing," `PriceCache.version` can go +tens of seconds without changing — the loop above would then send nothing at all, and the client +has no way to tell "idle" from "connection silently died." + +Add a wall-clock timer, independent of `version`, that fires a comment line (SSE comments start +with `:` and are ignored by `EventSource.onmessage`, but keep the underlying TCP stream alive and +observable): + +```python +import time + +KEEPALIVE_INTERVAL = 15.0 # seconds + + +async def _generate_events( + price_cache: PriceCache, + request: Request, + interval: float = 0.5, + keepalive_interval: float = KEEPALIVE_INTERVAL, +) -> AsyncGenerator[str, None]: + yield "retry: 1000\n\n" + + last_version = -1 + last_send = time.monotonic() # NEW — tracks the last time *anything* was yielded + client_ip = request.client.host if request.client else "unknown" + logger.info("SSE client connected: %s", client_ip) + + try: + while True: + if await request.is_disconnected(): + logger.info("SSE client disconnected: %s", client_ip) + break + + now = time.monotonic() + current_version = price_cache.version + + if current_version != last_version: + last_version = current_version + prices = price_cache.get_all() + if prices: + data = {ticker: update.to_dict() for ticker, update in prices.items()} + yield f"data: {json.dumps(data)}\n\n" + last_send = now + elif now - last_send >= keepalive_interval: # NEW + yield ": keepalive\n\n" + last_send = now + + await asyncio.sleep(interval) + except asyncio.CancelledError: + logger.info("SSE stream cancelled for: %s", client_ip) +``` + +`last_send` is updated on **both** a real data event and a keepalive, so the two never compound — +worst case the client goes `keepalive_interval` (15s) between any bytes at all, whether that's +because the market genuinely hasn't moved or Massive's poll hasn't landed yet. The frontend +connection-status dot (green/yellow/red, §2) should treat "no bytes, including no keepalive, for +> ~2x keepalive_interval" as the "reconnecting" signal, and rely on `EventSource`'s own `onerror` +/ readyState for the hard-disconnect case. + +### 11.3 `to_dict()` now includes the anchor automatically + +No change needed in `stream.py` beyond the keepalive — `update.to_dict()` (§3.1) already emits +`anchor`, `day_change`, `day_change_percent` once `models.py` is updated, so the SSE payload gets +the new fields for free. + +### 11.4 `request.is_disconnected()` reliability [NOTE, not a required change] + +`REVIEW.md` §4.1 flags that polling `request.is_disconnected()` can be unreliable behind some +proxies/uvicorn versions and can leak generator tasks. Low priority for the local Docker demo (no +proxy in front of uvicorn per PLAN.md §11); worth revisiting only if the optional cloud deployment +(§11's App Runner stretch goal) is pursued behind a reverse proxy. + +--- + +## 12. FastAPI Lifespan Integration + +Not shipped yet (no `app/main.py` exists) — this is the forward-looking contract the backend +platform agent implements against. Ordering matters (REVIEW.md §2.2): DB must be ready and the +tracked-ticker set computed **before** `source.start()` is called, and the SSE router must only be +mounted once the cache exists. + +```python +from contextlib import asynccontextmanager + +from fastapi import FastAPI + +from app.market import PriceCache, create_market_data_source, create_stream_router +from app.market.reconcile import get_tracked_tickers # see §13 + + +@asynccontextmanager +async def lifespan(app: FastAPI): + # --- STARTUP, in order --- + + # 1. DB: open, create schema if missing, seed defaults (unconditionally — not "lazy on + # first request", since the market task and snapshot writer both need it immediately; + # see REVIEW.md §2.1). + await init_db() + + # 2. Create the shared price cache. + price_cache = PriceCache() + app.state.price_cache = price_cache + + # 3. Compute the tracked set = watchlist ∪ open positions (§13) and start the source with + # it — never just the watchlist, so an open position is never left unpriced even before + # the first watchlist mutation happens. + initial_tickers = await get_tracked_tickers(db) + source = create_market_data_source(price_cache) + app.state.market_source = source + await source.start(initial_tickers) + + # 4. Mount the SSE router. + app.include_router(create_stream_router(price_cache)) + + # 5. Start the portfolio-snapshot / housekeeping background task (owned by the platform + # layer, not this module — mentioned here only for ordering: it must start after the + # cache has data, so the first snapshot isn't valuing against an empty cache). + app.state.housekeeping_task = asyncio.create_task(housekeeping_loop(app.state)) + + yield # App is running + + # --- SHUTDOWN, in order --- + app.state.housekeeping_task.cancel() + await source.stop() + + +app = FastAPI(title="FinAlly", lifespan=lifespan) +``` + +### Dependency injection for routes + +```python +def get_price_cache(request: Request) -> PriceCache: + return request.app.state.price_cache + + +def get_market_source(request: Request) -> MarketDataSource: + return request.app.state.market_source +``` + +```python +@router.post("/portfolio/trade") +async def execute_trade( + trade: TradeRequest, + price_cache: PriceCache = Depends(get_price_cache), +): + current_price = price_cache.get_price(trade.ticker) + if current_price is None: + raise HTTPException(status_code=400, detail={"error": f"No price available for {trade.ticker}"}) + # ... execute at current_price ... +``` + +--- + +## 13. Watchlist ↔ Positions Reconciliation + +**New — not shipped.** REVIEW.md §3.1 flags this as a [BLOCKER]: nothing in the shipped market +code enforces "the tracked set is always `watchlist ∪ open positions`" (PLAN.md §6, Decision #4). +Both `SimulatorDataSource.remove_ticker` and `MassiveDataSource.remove_ticker` unconditionally +evict from the cache — the invariant is **entirely the caller's responsibility**. This section +specs the one helper that all three call sites (startup, watchlist routes, trade routes) share, so +the rule is enforced in exactly one place. + +### 13.1 The helper + +```python +"""Keeps the market data source's tracked ticker set in sync with +watchlist ∪ open positions (PLAN.md §6, Decision #4).""" + +from __future__ import annotations + +from .interface import MarketDataSource + + +async def get_tracked_tickers(db) -> list[str]: + """The tracked set at any point in time: every watchlist ticker, plus every ticker + with a non-zero open position, deduplicated. Used at startup and anywhere the full + set needs recomputing from scratch.""" + watchlist = await db.get_watchlist_tickers() # e.g. SELECT ticker FROM watchlist + positions = await db.get_open_position_tickers() # e.g. SELECT ticker FROM positions WHERE quantity > 0 + return sorted(set(watchlist) | set(positions)) + + +async def on_watchlist_add(source: MarketDataSource, ticker: str) -> None: + """Call after inserting a new watchlist row. Idempotent — add_ticker() on both + sources is already a no-op if the ticker is already tracked (e.g. via an open position).""" + await source.add_ticker(ticker) + + +async def on_watchlist_remove(source: MarketDataSource, db, ticker: str) -> None: + """Call after deleting a watchlist row. Only stops tracking if there is no open + position for this ticker — an open position keeps it priced even off the watchlist + (PLAN.md §8: "the ticker stays priced while the position is open").""" + position = await db.get_position(ticker) + if position is None or position.quantity == 0: + await source.remove_ticker(ticker) + + +async def on_trade_executed(source: MarketDataSource, db, ticker: str) -> None: + """Call after every trade commits (buy or sell), regardless of whether the trade + succeeded on a ticker already tracked. Covers two edge cases the plan states as an + invariant but never assigns an owner for (REVIEW.md §3.1): + + 1. A buy opens a *new* ticker not on the watchlist -> it must start being tracked. + 2. A sell reduces a ticker's quantity to 0, and that ticker had already been removed + from the watchlist earlier while the position was still open -> now that nothing + references it, stop tracking it. + """ + position = await db.get_position(ticker) + on_watchlist = await db.is_on_watchlist(ticker) + + if position and position.quantity > 0: + await source.add_ticker(ticker) # covers case 1; no-op if already tracked + elif not on_watchlist: + await source.remove_ticker(ticker) # covers case 2 +``` + +### 13.2 Call sites + +```python +@router.post("/watchlist") +async def add_to_watchlist( + payload: WatchlistAdd, + source: MarketDataSource = Depends(get_market_source), +): + ticker = validate_ticker(payload.ticker) # §6 — reject bad format before touching DB + await db.insert_watchlist_row(ticker) # persist first + await on_watchlist_add(source, ticker) # then reconcile the tracked set + return {"ticker": ticker, "price": price_cache.get_price(ticker)} + + +@router.delete("/watchlist/{ticker}") +async def remove_from_watchlist( + ticker: str, + source: MarketDataSource = Depends(get_market_source), +): + await db.delete_watchlist_row(ticker) # persist first + await on_watchlist_remove(source, db, ticker) # then reconcile (checks open position) + return {"status": "ok"} + + +@router.post("/portfolio/trade") +async def execute_trade( + trade: TradeRequest, + source: MarketDataSource = Depends(get_market_source), +): + # ... validate, run the trade in one DB transaction (§7/§8/Decision #15) ... + await on_trade_executed(source, db, trade.ticker) # then reconcile + return result +``` + +**Ordering rule, stated once:** in every write path, the **DB transaction commits first**, then +the market-source reconciliation call runs. If the reconciliation call fails or the process +crashes between the two, the tracked set self-heals on the next `get_tracked_tickers()` call — +which only happens at startup today. This is an acceptable v1 gap (a ticker might go briefly +unpriced until restart) but is worth a one-line note in the platform agent's error handling: a +failed `add_ticker`/`remove_ticker` call should be logged, not raised, so it never rolls back an +already-committed trade or watchlist change. + +--- + +## 14. Concurrency & Deployment Constraints + +These aren't new market-data *code*, but they're constraints the market layer's design depends on +and that the platform/Docker agents must honor, so they're recorded here where the reasoning lives +(REVIEW.md §1.7, §3.4). + +1. **Single uvicorn worker.** Everything hinges on one in-process `PriceCache` and one + simulator/poller task. Multiple workers means N independent simulators producing divergent + prices for the same tickers, N snapshot writers, N `PriceCache`s that never agree. The + Dockerfile's `CMD` must pin `--workers 1`: + ``` + CMD ["uvicorn", "app.main:app", "--host", "0.0.0.0", "--port", "8000", "--workers", "1"] + ``` + +2. **Watchlist/trade routes that touch the market source must be `async def`, not sync.** FastAPI + dispatches `def` (non-async) route handlers to a thread pool. `GBMSimulator.add_ticker` / + `remove_ticker` mutate shared state with no lock (§8.4), assuming everything runs on the event + loop. A sync route calling `source.add_ticker()` (itself `async def`, called via + `asyncio.run_coroutine_threadsafe` or similar) would race with the simulator's own `step()` + task. Keep every route in §13.2 `async def` end to end. + +3. **The market data task must be running before the housekeeping/snapshot task starts** (§12, + step 3 before step 5) — the snapshot writer values the portfolio using + `price_cache.get_price()`, and an empty cache would either write a wrong first snapshot or + need special-casing. Starting the source first avoids that entirely. + +--- + +## 15. Testing Plan for the New Pieces + +The 73 existing tests in `backend/tests/market/` (§ summary above) cover the shipped code and stay +green — `anchor` is purely additive with a defaulting kwarg. New tests needed for this design: + +### 15.1 `test_cache.py` — anchor behavior + +```python +class TestPriceCacheAnchor: + def test_first_update_sets_anchor_to_price(self): + cache = PriceCache() + update = cache.update("AAPL", 190.00) + assert update.anchor == 190.00 + + def test_explicit_anchor_used_on_first_update(self): + cache = PriceCache() + update = cache.update("AAPL", 190.00, anchor=185.50) + assert update.anchor == 185.50 + + def test_anchor_is_sticky_across_updates(self): + cache = PriceCache() + cache.update("AAPL", 190.00, anchor=185.50) + update = cache.update("AAPL", 192.00, anchor=999.00) # later anchor arg ignored + assert update.anchor == 185.50 + + def test_day_change_percent(self): + cache = PriceCache() + cache.update("AAPL", 190.00, anchor=100.00) + update = cache.update("AAPL", 200.00) + assert update.day_change == 100.00 + assert update.day_change_percent == 100.0 + + def test_remove_drops_anchor(self): + cache = PriceCache() + cache.update("AAPL", 190.00, anchor=185.50) + cache.remove("AAPL") + update = cache.update("AAPL", 300.00) # re-tracked later + assert update.anchor == 300.00 # re-anchored fresh, not 185.50 + + def test_get_anchor(self): + cache = PriceCache() + cache.update("AAPL", 190.00, anchor=185.50) + assert cache.get_anchor("AAPL") == 185.50 + assert cache.get_anchor("NOPE") is None +``` + +### 15.2 `test_models.py` — `PriceUpdate` day-change properties + +```python +def test_day_change_percent_zero_anchor_is_safe(): + update = PriceUpdate(ticker="X", price=10.0, previous_price=10.0, anchor=0.0) + assert update.day_change_percent == 0.0 # no ZeroDivisionError + +def test_to_dict_includes_anchor_fields(): + update = PriceUpdate(ticker="AAPL", price=200.0, previous_price=198.0, anchor=190.0) + d = update.to_dict() + assert d["anchor"] == 190.0 + assert d["day_change"] == 10.0 + assert d["day_change_percent"] == pytest.approx(5.2632, rel=1e-3) +``` + +### 15.3 `test_massive.py` — previous-close plumbing + +```python +def _make_snapshot(ticker, price, timestamp_ms, previous_close=None): + snap = MagicMock() + snap.ticker = ticker + snap.last_trade.price = price + snap.last_trade.timestamp = timestamp_ms + snap.day.previous_close = previous_close + return snap + +async def test_poll_captures_previous_close_as_anchor(self): + cache = PriceCache() + source = MassiveDataSource(api_key="test-key", price_cache=cache, poll_interval=60.0) + source._tickers = ["AAPL"] + snap = _make_snapshot("AAPL", 190.50, 1707580800000, previous_close=185.00) + with patch.object(source, "_fetch_snapshots", return_value=[snap]): + await source._poll_once() + assert cache.get_anchor("AAPL") == 185.00 + +async def test_missing_previous_close_falls_back_to_first_observed(self): + cache = PriceCache() + source = MassiveDataSource(api_key="test-key", price_cache=cache, poll_interval=60.0) + source._tickers = ["AAPL"] + snap = _make_snapshot("AAPL", 190.50, 1707580800000, previous_close=None) + with patch.object(source, "_fetch_snapshots", return_value=[snap]): + await source._poll_once() + assert cache.get_anchor("AAPL") == 190.50 # falls back to price itself +``` + +### 15.4 `test_validation.py` — new file + +```python +import pytest +from app.market.validation import InvalidTickerError, validate_ticker + +@pytest.mark.parametrize("raw,expected", [ + ("aapl", "AAPL"), + (" TSLA ", "TSLA"), + ("V", "V"), + ("GOOGL", "GOOGL"), +]) +def test_valid_tickers_normalize(raw, expected): + assert validate_ticker(raw) == expected + +@pytest.mark.parametrize("raw", ["", "TOOLONG", "AB3", "AB-C", "aapl$", "123", "A B"]) +def test_invalid_tickers_raise(raw): + with pytest.raises(InvalidTickerError): + validate_ticker(raw) +``` + +### 15.5 `test_stream.py` — keepalive (new file, or added to an existing stream test module) + +```python +@pytest.mark.asyncio +async def test_keepalive_sent_when_cache_idle(monkeypatch): + cache = PriceCache() + cache.update("AAPL", 190.0) # one real event, then silence + + request = FakeRequest(disconnect_after=3) # helper: is_disconnected() False for N calls + events = [ + e async for e in _generate_events(cache, request, interval=0.01, keepalive_interval=0.02) + ] + assert any(e.startswith(": keepalive") for e in events) +``` + +### 15.6 `test_reconcile.py` — new file, against a fake DB/source + +```python +@pytest.mark.asyncio +async def test_remove_watchlist_keeps_open_position(fake_db, fake_source): + fake_db.set_position("AAPL", quantity=10) + await on_watchlist_remove(fake_source, fake_db, "AAPL") + assert not fake_source.remove_called + +@pytest.mark.asyncio +async def test_remove_watchlist_no_position_removes(fake_db, fake_source): + fake_db.set_position("AAPL", quantity=0) + await on_watchlist_remove(fake_source, fake_db, "AAPL") + assert fake_source.remove_called + +@pytest.mark.asyncio +async def test_trade_opens_new_ticker_not_on_watchlist(fake_db, fake_source): + fake_db.set_position("PYPL", quantity=5) + fake_db.set_on_watchlist("PYPL", False) + await on_trade_executed(fake_source, fake_db, "PYPL") + assert fake_source.add_called_with == "PYPL" + +@pytest.mark.asyncio +async def test_sell_to_zero_off_watchlist_removes(fake_db, fake_source): + fake_db.set_position("PYPL", quantity=0) + fake_db.set_on_watchlist("PYPL", False) + await on_trade_executed(fake_source, fake_db, "PYPL") + assert fake_source.remove_called_with == "PYPL" +``` + +--- + +## 16. Error Handling & Edge Cases + +| Scenario | Behavior | +|---|---| +| Empty watchlist at startup | `get_tracked_tickers()` returns `[]`; both sources handle an empty ticker list gracefully (simulator produces no prices, Massive skips its poll). SSE sends nothing until a ticker is added. | +| Trade on a ticker with no cached price yet | `price_cache.get_price(ticker)` returns `None` → trade route responds `400 {"error": "Price not yet available for X"}`. The simulator avoids this by seeding synchronously in `add_ticker()`; Massive may have a brief gap between `add_ticker()` (appends to the poll list) and the next poll landing. | +| Massive API key invalid | First poll 401s, logged, poller keeps retrying every `poll_interval`. Cache stays empty for those tickers; SSE streams (with keepalives) but no data for them. Not currently escalated to `/api/health` — see REVIEW.md §2.6 if a `market_data: "stale"` health field is added later. | +| Massive snapshot missing `day.previous_close` | Anchor falls back to first-observed price (§9.2) — the *price* update still succeeds; only the anchor degrades. Never let an anchor problem drop a price. | +| Ticker removed from watchlist while a position is open | Stays tracked and priced (§13); its anchor is untouched (no `remove()` call happens). | +| Ticker re-added after being fully evicted (`remove()` called, position and watchlist both empty) | Re-anchors fresh from whatever price it starts at when re-tracked — this is correct per Decision #1's "first observed price after tracking started," not a bug. | +| SSE client behind a slow network | `request.is_disconnected()` may lag; worst case a generator keeps running slightly past actual disconnect, self-terminating on the next check. Acceptable for local Docker; see §11.4 for the cloud-deploy caveat. | +| Ticker format rejected (`validate_ticker` raises) | Manual API paths: `400 {"error": "..."}`. Chat-initiated: folded into that action's `status: "error"` entry in the chat response, per Decision #16 — never an HTTP error for an LLM-issued action. | + +--- + +## 17. Configuration Summary + +| Parameter | Location | Default | Description | +|---|---|---|---| +| `MASSIVE_API_KEY` | Environment variable | `""` (empty) | If set, use Massive API; otherwise use simulator. | +| `update_interval` | `SimulatorDataSource.__init__` | `0.5`s | Time between simulator ticks. | +| `poll_interval` | `MassiveDataSource.__init__` | `15.0`s | Time between Massive API polls (free tier). | +| `event_probability` | `GBMSimulator.__init__` | `0.001` | Chance of a random shock event per ticker per tick. | +| `dt` | `GBMSimulator.__init__` | `~8.5e-8` | GBM time step (fraction of a trading year). | +| SSE push interval | `_generate_events()` | `0.5`s | Cache poll cadence inside the SSE loop. | +| SSE keepalive interval | `_generate_events()` | `15.0`s (**new**) | Max gap between bytes sent to an idle SSE client. | +| SSE retry directive | `_generate_events()` | `1000`ms | Browser `EventSource` reconnection delay. | +| Ticker format | `validate_ticker()` (**new**) | `^[A-Z]{1,5}$` | Enforced at every write path (§6), not inside the simulator. | +| Uvicorn workers | Dockerfile `CMD` | `1` (**required**) | One `PriceCache` / one source per process — see §14. | + +### `__init__.py` — public exports (add the two new modules) + +```python +"""Market data subsystem for FinAlly.""" + +from .cache import PriceCache +from .factory import create_market_data_source +from .interface import MarketDataSource +from .models import PriceUpdate +from .reconcile import get_tracked_tickers, on_trade_executed, on_watchlist_add, on_watchlist_remove +from .stream import create_stream_router +from .validation import InvalidTickerError, validate_ticker + +__all__ = [ + "PriceUpdate", + "PriceCache", + "MarketDataSource", + "create_market_data_source", + "create_stream_router", + "get_tracked_tickers", + "on_watchlist_add", + "on_watchlist_remove", + "on_trade_executed", + "validate_ticker", + "InvalidTickerError", +] +``` diff --git a/planning/PLAN.md b/planning/PLAN.md index bc1811b33..ee3b3c9bf 100644 --- a/planning/PLAN.md +++ b/planning/PLAN.md @@ -1,6 +1,8 @@ # FinAlly — AI Trading Workstation -## Project Specification +> **Project Specification.** Section 13 (Design Decisions Log) records choices made after the +> initial draft during a documentation review; where it conflicts with an earlier section, the +> log wins and the section text has been updated to match. ## 1. Vision @@ -8,6 +10,8 @@ FinAlly (Finance Ally) is a visually stunning AI-powered trading workstation tha This is the capstone project for an agentic AI coding course. It is built entirely by Coding Agents demonstrating how orchestrated AI agents can produce a production-quality full-stack application. Agents interact through files in `planning/`. +--- + ## 2. User Experience ### First Launch @@ -22,11 +26,12 @@ The user runs a single Docker command (or a provided start script). A browser op ### What the User Can Do - **Watch prices stream** — prices flash green (uptick) or red (downtick) with subtle CSS animations that fade -- **View sparkline mini-charts** — price action beside each ticker in the watchlist, accumulated on the frontend from the SSE stream since page load (sparklines fill in progressively) -- **Click a ticker** to see a larger detailed chart in the main chart area +- **View sparkline mini-charts** — price action beside each ticker in the watchlist, accumulated on the frontend from the SSE stream since page load (sparklines fill in progressively). Price history is **not** persisted server-side in v1: a page reload restarts every sparkline and the detail chart from empty. This is an accepted trade-off (see Section 13). +- **Click a ticker** to see a larger detailed chart in the main chart area (also accumulated from the SSE stream since page load) - **Buy and sell shares** — market orders only, instant fill at current price, no fees, no confirmation dialog -- **Monitor their portfolio** — a heatmap (treemap) showing positions sized by weight and colored by P&L, plus a P&L chart tracking total portfolio value over time -- **View a positions table** — ticker, quantity, average cost, current price, unrealized P&L, % change +- **Monitor their portfolio** — a heatmap (treemap) showing positions sized by weight and colored by unrealized P&L, plus a P&L chart tracking total portfolio value over time +- **View a positions table** — ticker, quantity, average cost, current price, unrealized P&L, % change. Percent change is measured against average cost. +- **See realized P&L** — cumulative realized gain/loss from closed and reduced positions, computed from the trade log and shown alongside cash and total value - **Chat with the AI assistant** — ask about their portfolio, get analysis, and have the AI execute trades and manage the watchlist through natural language - **Manage the watchlist** — add/remove tickers manually or via the AI chat @@ -43,6 +48,8 @@ The user runs a single Docker command (or a provided start script). A browser op - Blue Primary: `#209dd7` - Purple Secondary: `#753991` (submit buttons) +--- + ## 3. Architecture Overview ### Single Container, Single Port @@ -97,11 +104,11 @@ finally/ │ ├── stop_mac.sh # Stop Docker container (macOS/Linux) │ ├── start_windows.ps1 # Launch Docker container (Windows PowerShell) │ └── stop_windows.ps1 # Stop Docker container (Windows PowerShell) -├── test/ # Playwright E2E tests + docker-compose.test.yml +├── test/ # Playwright E2E tests (run on host against the compose stack) ├── db/ # Volume mount target (SQLite file lives here at runtime) │ └── .gitkeep # Directory exists in repo; finally.db is gitignored ├── Dockerfile # Multi-stage build (Node → Python) -├── docker-compose.yml # Optional convenience wrapper +├── docker-compose.yml # Single entrypoint — start/stop scripts wrap this ├── .env # Environment variables (gitignored, .env.example committed) └── .gitignore ``` @@ -113,7 +120,7 @@ finally/ - **`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. - **`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. +- **`test/`** contains Playwright E2E tests, run on the host against the running compose stack. Unit tests live within `frontend/` and `backend/` respectively, following each framework's conventions. - **`scripts/`** contains start/stop scripts that wrap Docker commands. --- @@ -147,6 +154,8 @@ LLM_MOCK=false Both the simulator and the Massive client implement the same abstract interface. The backend selects which to use based on the environment variable. All downstream code (SSE streaming, price cache, frontend) is agnostic to the source. +> The market data subsystem is already implemented (`backend/app/market/`, see `MARKET_DATA_SUMMARY.md`). The **day-change anchor** and the unknown-ticker fallback below are follow-up additions to that subsystem, not yet built. + ### Simulator (Default) - Generates prices using geometric Brownian motion (GBM) with configurable drift and volatility per ticker @@ -155,6 +164,7 @@ Both the simulator and the Massive client implement the same abstract interface. - Occasional random "events" — sudden 2-5% moves on a ticker for drama - Starts from realistic seed prices (e.g., AAPL ~$190, GOOGL ~$175, etc.) - Runs as an in-process background task — no external dependencies +- **Unknown tickers**: when a ticker not in the seed table is added, the simulator synthesizes a seed price (a bounded random value in a plausible range) and assigns default GBM drift/volatility with no correlation group. Tickers are validated as 1–5 uppercase letters before being accepted. ### Massive API (Optional) @@ -163,20 +173,25 @@ Both the simulator and the Massive client implement the same abstract interface. - 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 +- Uses the API's previous-close value as the day-change anchor (see below); outside market hours prices are the last known close and simply stop changing ### Shared Price Cache - A single background task (simulator or Massive poller) writes to an in-memory price cache -- The cache holds the latest price, previous price, and timestamp for each ticker +- The cache holds the latest price, previous price, timestamp, and a **day-change anchor** for each ticker +- The anchor is the previous close (Massive mode) or the first price observed after the ticker started being tracked (simulator mode). "Day change %" everywhere in the UI is `(price − anchor) / anchor`. +- The set of tracked tickers is always `watchlist ∪ tickers with an open position` — a position is never left unpriced, even if its ticker is removed from the watchlist - SSE streams read from this cache and push updates to connected clients +- Prices are in-memory only. On restart the simulator re-anchors from fresh seed prices while positions/cash persist in SQLite, so unrealized P&L can jump — accepted demo behavior (see Section 13) - This architecture supports future multi-user scenarios without changes to the data layer ### SSE Streaming - 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 all tracked tickers (`watchlist ∪ open positions`) whenever the cache changes, at up to ~500ms cadence. In Massive mode updates are as sparse as the poll interval. +- Each SSE event contains ticker, price, previous price, day-change anchor, timestamp, and change direction (`"up" | "down" | "flat"`) +- The server emits a `: keepalive` comment every ~15s so the client can distinguish an idle stream from a dropped connection and keep the status indicator accurate - Client handles reconnection automatically (EventSource has built-in retry) --- @@ -195,6 +210,10 @@ The backend checks for the SQLite database on startup (or first request). If the All tables include a `user_id` column defaulting to `"default"`. This is hardcoded for now (single-user) but enables future multi-user support without schema migration. +All timestamps are stored as **UTC ISO-8601 strings with a `Z` suffix** so lexicographic ordering matches chronological ordering. + +**`trades` is the source of truth.** `users_profile.cash_balance` and the `positions` table are a cache maintained *in the same transaction* as each trade insert. Cash and positions could be replayed from `trades` alone if the cache is ever lost. + **users_profile** — User state (cash balance) - `id` TEXT PRIMARY KEY (default: `"default"`) - `cash_balance` REAL (default: `10000.0`) @@ -213,9 +232,12 @@ All tables include a `user_id` column defaulting to `"default"`. This is hardcod - `ticker` TEXT - `quantity` REAL (fractional shares supported) - `avg_cost` REAL +- `realized_pnl` REAL (default: `0.0`) — cumulative realized gain/loss for this ticker - `updated_at` TEXT (ISO timestamp) - UNIQUE constraint on `(user_id, ticker)` +Cost basis is **weighted average cost**. A buy recomputes `avg_cost`; a sell leaves `avg_cost` unchanged, reduces `quantity`, and adds `(sell_price − avg_cost) × sold_qty` to `realized_pnl`. When `quantity` reaches ~0 the row is **kept** (so `realized_pnl` history survives) with `quantity = 0`; zero-quantity rows are excluded from the heatmap and, by default, the positions table. Total realized P&L is `SUM(realized_pnl)` across rows. + **trades** — Trade history (append-only log) - `id` TEXT PRIMARY KEY (UUID) - `user_id` TEXT (default: `"default"`) @@ -225,12 +247,14 @@ All tables include a `user_id` column defaulting to `"default"`. This is hardcod - `price` REAL - `executed_at` TEXT (ISO timestamp) -**portfolio_snapshots** — Portfolio value over time (for P&L chart). Recorded every 30 seconds by a background task, and immediately after each trade execution. +**portfolio_snapshots** — Portfolio value over time (for P&L chart). Recorded every 30 seconds by a background task (only while the market data task is running and the value has changed since the last snapshot), and immediately after each trade execution. - `id` TEXT PRIMARY KEY (UUID) - `user_id` TEXT (default: `"default"`) - `total_value` REAL - `recorded_at` TEXT (ISO timestamp) +A background job trims this table to the most recent 7 days on each snapshot write. + **chat_messages** — Conversation history with LLM - `id` TEXT PRIMARY KEY (UUID) - `user_id` TEXT (default: `"default"`) @@ -248,6 +272,11 @@ All tables include a `user_id` column defaulting to `"default"`. This is hardcod ## 8. API Endpoints +### Bootstrap +| Method | Path | Description | +|--------|------|-------------| +| GET | `/api/bootstrap` | Everything needed for first paint in one round trip: portfolio (positions, cash, realized/unrealized P&L, total value), watchlist with latest prices + anchors, portfolio history, and recent chat history. Individual endpoints below remain for later refreshes. | + ### Market Data | Method | Path | Description | |--------|------|-------------| @@ -256,45 +285,59 @@ All tables include a `user_id` column defaulting to `"default"`. This is hardcod ### Portfolio | Method | Path | Description | |--------|------|-------------| -| GET | `/api/portfolio` | Current positions, cash balance, total value, unrealized P&L | +| GET | `/api/portfolio` | Current positions, cash balance, total value, unrealized P&L, cumulative realized 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). Optional `?limit=` (default 500, newest N) | ### Watchlist | Method | Path | Description | |--------|------|-------------| -| GET | `/api/watchlist` | Current watchlist tickers with latest prices | +| GET | `/api/watchlist` | Current watchlist tickers with latest prices and day-change anchors | | POST | `/api/watchlist` | Add a ticker: `{ticker}` | -| DELETE | `/api/watchlist/{ticker}` | Remove a ticker | +| DELETE | `/api/watchlist/{ticker}` | Remove a ticker (allowed even if a position is held; the ticker stays priced while the position is open) | ### Chat | Method | Path | Description | |--------|------|-------------| | POST | `/api/chat` | Send a message, receive complete JSON response (message + executed actions) | +| GET | `/api/chat/history` | Prior conversation messages (optional `?limit=`, default 50) so a page reload restores the conversation | ### System | Method | Path | Description | |--------|------|-------------| -| GET | `/api/health` | Health check (for Docker/deployment) | +| GET | `/api/health` | Health check: returns 200 only if the process is up **and** a trivial SQLite query succeeds | + +### Trade Validation + +`quantity` is always in **shares** (fractional allowed), and must be `> 0`. There are no notional/dollar orders — the AI converts a dollar amount to shares using the live price in its context. Share quantities are rounded to 6 decimals, cash to cents. Buys require sufficient cash; sells require sufficient shares. Each trade executes in a single DB transaction; multiple trades from one chat turn execute sequentially. + +### Error Contract + +Errors return `{ "error": "" }` with: +- `400` — validation failure (bad quantity, insufficient cash/shares, malformed ticker) +- `404` — unknown resource (e.g. removing a ticker not on the watchlist) +- `502` — upstream failure (LLM/market data provider) + +A trade requested via chat that fails validation is **not** an HTTP error — the failure message is returned inside the chat response so the LLM can relay it. --- ## 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 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. -There is an OPENROUTER_API_KEY in the .env file in the project root. +There is an OPENROUTER_API_KEY in the .env file in the project root. The app must still start and serve every non-chat route when the key is absent (chat then returns a `502` with a clear message, unless `LLM_MOCK=true`). ### How It Works 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 +2. Loads the last 20 messages of 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 -5. Parses the complete structured JSON response -6. Auto-executes any trades or watchlist changes specified in the response +4. Calls the LLM via LiteLLM → OpenRouter, requesting structured output, using the `cerebras` skill +5. Parses and validates the structured JSON response. If parsing/validation fails, retries once; if it fails again, falls back to returning the raw text as `message` with no actions +6. Auto-executes any trades or watchlist changes specified in the response, collecting per-action success/error results 7. Stores the message and executed actions in `chat_messages` 8. Returns the complete JSON response to the frontend (no token-by-token streaming — Cerebras inference is fast enough that a loading indicator is sufficient) @@ -315,8 +358,10 @@ 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 +- `trades` (optional): Array of `{ticker, side: "buy"|"sell", quantity}` 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 `{ticker, action: "add"|"remove"}` + +Note on structured-output support: if the Cerebras provider path does not enforce the JSON schema, the parse-validate-retry-once flow in "How It Works" step 5 is the safety net. Confirm actual behavior via the `cerebras` skill during implementation. ### Auto-Execution @@ -354,19 +399,20 @@ The frontend is a single-page application with a dense, terminal-inspired layout - **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) - **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) +- **Portfolio heatmap** — treemap visualization where each rectangle is a position, sized by portfolio weight, colored by unrealized P&L % (green = profit, red = loss); zero-quantity rows excluded - **P&L chart** — line chart showing total portfolio value over time, using data from `portfolio_snapshots` - **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. -- **Header** — portfolio total value (updating live), connection status indicator, cash balance +- **AI chat panel** — docked/collapsible sidebar. Message input, scrolling conversation history (restored from `/api/chat/history` on load), loading indicator while waiting for LLM response. Trade executions and watchlist changes shown inline as confirmations, including failures. +- **Header** — portfolio total value, connection status indicator, cash balance. Total value updates live: recomputed client-side on every SSE tick as `cash + Σ(quantity × latest price)`, not re-fetched. ### Technical Notes - Use `EventSource` for SSE connection to `/api/stream/prices` -- Canvas-based charting library preferred (Lightweight Charts or Recharts) for performance +- **Recharts** for all four visuals (watchlist sparkline, detail chart, P&L line, portfolio treemap) — one dependency covers every case. Revisit only if the detail chart's render performance proves inadequate, in which case swap just that one chart for a canvas library. - Price flash effect: on receiving a new price, briefly apply a CSS class with background color transition, then remove it -- All API calls go to the same origin (`/api/*`) — no CORS configuration needed +- All API calls go to the same origin (`/api/*`). In production FastAPI serves the static export, so there is no CORS. For local `next dev` on :3000, use `next.config` `rewrites` to proxy `/api/*` to `http://localhost:8000`. +- `output: 'export'` disables Next.js image optimization, route handlers, middleware, and server-side dynamic routes — design within those limits (single route, client-side data fetching) - Tailwind CSS for styling with a custom dark theme --- @@ -378,7 +424,7 @@ The frontend is a single-page application with a dense, terminal-inspired layout ``` Stage 1: Node 20 slim - Copy frontend/ - - npm install && npm run build (produces static export) + - npm ci && npm run build (produces static export; npm ci for reproducible builds from the lockfile) Stage 2: Python 3.12 slim - Install uv @@ -391,36 +437,37 @@ Stage 2: Python 3.12 slim FastAPI serves the static frontend files and all API routes on port 8000. -### Docker Volume +### Docker Compose — the single entrypoint -The SQLite database persists via a named Docker volume: +`docker-compose.yml` is the one source of truth for the port mapping, the named volume, and the `.env` file. The SQLite database persists via a named Docker volume mounted at `/app/db`; the backend writes `finally.db` there. -```bash -docker run -v finally-data:/app/db -p 8000:8000 --env-file .env finally +```yaml +# docker-compose.yml (sketch) +services: + app: + build: . + ports: ["8000:8000"] + env_file: .env + volumes: ["finally-data:/app/db"] +volumes: + finally-data: ``` -The `db/` directory in the project root maps to `/app/db` in the container. The backend writes `finally.db` to this path. - ### Start/Stop Scripts -**`scripts/start_mac.sh`** (macOS/Linux): -- Builds the Docker image if not already built (or if `--build` flag passed) -- Runs the container with the volume mount, port mapping, and `.env` file -- Prints the URL to access the app -- Optionally opens the browser - -**`scripts/stop_mac.sh`** (macOS/Linux): -- Stops and removes the running container -- Does NOT remove the volume (data persists) +The scripts are thin wrappers so there is no second copy of the run configuration: -**`scripts/start_windows.ps1`** / **`scripts/stop_windows.ps1`**: PowerShell equivalents for Windows. +- **`scripts/start_mac.sh`** / **`scripts/start_windows.ps1`** — run `docker compose up -d --build`, print the URL, optionally open the browser +- **`scripts/stop_mac.sh`** / **`scripts/stop_windows.ps1`** — run `docker compose down` (the volume is **not** removed, so data persists) -All scripts should be idempotent — safe to run multiple times. +Compose handles "rebuild only if needed", so the scripts carry no build-detection logic. All scripts are idempotent — safe to run multiple times. ### Optional Cloud Deployment The container is designed to deploy to AWS App Runner, Render, or any container platform. A Terraform configuration for App Runner may be provided in a `deploy/` directory as a stretch goal, but is not part of the core build. +**Security note for any public deployment:** the app has no auth and the portfolio is a single global `user_id="default"`. A public URL means anyone can drive unlimited LLM calls against your `OPENROUTER_API_KEY`. Do not deploy publicly without at least basic auth / a shared secret and request rate limiting. + --- ## 12. Testing Strategy @@ -442,7 +489,7 @@ The container is designed to deploy to AWS App Runner, Render, or any container ### E2E Tests (in `test/`) -**Infrastructure**: A separate `docker-compose.test.yml` in `test/` that spins up the app container plus a Playwright container. This keeps browser dependencies out of the production image. +**Infrastructure**: Playwright runs on the host (its deps live in `test/`, never in the Dockerfile, so the production image stays lean). The test runner starts the app with `docker compose up -d` (overriding `LLM_MOCK=true` via env), waits for `/api/health`, runs the specs against `http://localhost:8000`, then `docker compose down`. No dedicated test compose file. **Environment**: Tests run with `LLM_MOCK=true` by default for speed and determinism. @@ -450,7 +497,57 @@ The container is designed to deploy to AWS App Runner, Render, or any container - Fresh start: default watchlist appears, $10k balance shown, prices are streaming - Add and remove a ticker from the watchlist - Buy shares: cash decreases, position appears, portfolio updates -- Sell shares: cash increases, position updates or disappears +- Sell part of a position: cash increases, quantity and avg cost behave correctly +- Sell an entire position: row leaves the table/heatmap, realized P&L reflects the gain/loss - 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 +- AI chat (mocked): send a message, receive a response, trade execution appears inline; failed trade shows an error inline +- Chat history persists across a page reload +- SSE resilience: disconnect and verify reconnection; status indicator reacts + +--- + +## 13. Design Decisions Log + +_Resolved 2026-09-08 during a documentation review. These decisions have been folded into the sections above; this log is the rationale record. The pre-review draft is archived at `planning/archive/PLAN.pre-review-backup-2026-09-08.md`._ + +### Decisions made + +| # | Topic | Decision | +|---|-------|----------| +| 1 | Day-change baseline | Each tracked ticker has a **day-change anchor** in the price cache: previous close (Massive) or first observed price (simulator). "Day change %" = `(price − anchor) / anchor`. Anchor ships in the SSE payload and `/api/watchlist`. (§6) | +| 2 | Price history | **Client-only accumulation** from SSE for v1. A reload restarts every chart from empty. No server-side history buffer, no `/api/history/{ticker}`. (§2) | +| 3 | Unknown tickers | Simulator synthesizes a seed price + default GBM params for any ticker not in the seed table. Tickers validated as 1–5 uppercase letters. (§6) | +| 4 | Watchlist ↔ positions | Tracked set is always `watchlist ∪ open positions`. `DELETE /api/watchlist/{ticker}` is allowed with an open position; the ticker stays priced. (§6, §8) | +| 5 | Realized P&L | Tracked. `positions.realized_pnl` per ticker, accumulated on sells; total shown alongside cash/unrealized. Zero-quantity rows are kept (not deleted) so history survives. (§2, §7) | +| 6 | Accounting method | Weighted average cost. Buy recomputes `avg_cost`; sell leaves it, reduces `quantity`, books realized P&L. % change is vs. `avg_cost`. (§7) | +| 7 | Trade semantics | Shares only (fractional, `> 0`); no notional orders — the LLM converts dollars using its live-price context. Shares rounded to 6 dp, cash to cents. (§8) | +| 8 | Structured outputs | Parse-validate-**retry once**, then fall back to raw text with no actions. Confirm actual Cerebras enforcement via the `cerebras` skill. (§9) | +| 9 | Chat history window | Last **20 messages** into the prompt; `GET /api/chat/history` (default 50) for reload restore. (§8, §9) | +| 10 | Bootstrap endpoint | `GET /api/bootstrap` returns portfolio + watchlist + history + recent chat in one call for first paint. Individual endpoints remain. (§8) | +| 11 | Restart P&L jump | Accepted demo behavior — simulator re-anchors from seed prices on restart while positions persist. Documented, not fixed. (§6) | +| 12 | Snapshot growth | Snapshot only when value changed; trim table to 7 days on each write; `/api/portfolio/history?limit=` (default 500). (§7, §8) | +| 13 | Public-deploy security | Documented warning: no auth + global portfolio + exposed `OPENROUTER_API_KEY` spend. Needs basic auth + rate limiting before any public deploy. (§11) | +| 14 | SSE keepalive | Server emits `: keepalive` every ~15s so the status indicator can tell idle from disconnected. (§6) | +| 15 | Trade atomicity | Each trade = one DB transaction; multiple trades from a chat turn run sequentially. `trades` is the source of truth; `cash_balance`/`positions` are a same-transaction cache. (§7, §8) | +| 16 | Error contract | `{ "error": "..." }` with `400` validation / `404` unknown / `502` upstream. Chat-initiated trade failures ride inside the chat response, not as HTTP errors. (§8) | +| 17 | Dev proxy | `next.config` `rewrites` proxy `/api/*` → `:8000` for `next dev`. (§10) | +| 18 | `output: 'export'` limits | Noted for the frontend agent: no image optimization, route handlers, middleware, or dynamic routes. (§10) | +| 19 | Timestamps | UTC ISO-8601 with `Z` everywhere. (§7) | +| 20 | Charting library | **Recharts** for all four visuals (sparkline, detail, P&L line, treemap). Swap only the detail chart for canvas if perf demands. (§10) | +| 21 | Compose as entrypoint | `docker-compose.yml` is the single source of truth; start/stop scripts are thin wrappers around `docker compose up -d --build` / `down`. (§11) | +| 22 | E2E infra | Playwright runs on the host against the compose stack; no `docker-compose.test.yml`, no Playwright container. (§12) | +| 23 | `npm ci` | Dockerfile stage 1 uses `npm ci`, not `npm install`. (§11) | +| 24 | Editorial | Skill name is `cerebras` (not "cerebras-inference"); `watchlist_changes[].action` ∈ `{add, remove}`; SSE `direction` ∈ `{up, down, flat}`; `/api/health` also runs a trivial DB query; header/separator cleanup. | + +### Deferred / not doing (v1) + +- **Full event-sourcing** (dropping `positions` / `cash_balance` entirely and replaying `trades` on every request). Kept the cache tables for backend simplicity; `trades` is still the authoritative log if a rebuild is ever needed. +- **Server-side price history** and per-ticker history endpoint — revisit if reload-loses-charts proves annoying in the demo. +- **Trimming `change`/`direction` from the SSE payload** — already built into the market data layer; not worth the churn. +- **Removing unused audit columns** (`users_profile.created_at`) — harmless, left in place. +- **Notional trades** in the trade API — the LLM converts to shares instead. + +### Still open (non-blocking) + +- **Agent Roles & Build Order.** The plan refers to "the Frontend Engineer agent" and "Backend/Market Data agents" coordinating via `planning/` files, but there's no index of which agent owns which deliverable or the build sequence. Worth a short subsection or a separate `planning/ROLES.md` before the next agent starts. +- **Treemap color scale.** "Colored by unrealized P&L %" needs concrete buckets or a continuous diverging scale + domain clamp — leave to the Frontend Engineer, but flag in review. diff --git a/planning/REVIEW.md b/planning/REVIEW.md new file mode 100644 index 000000000..68e381745 --- /dev/null +++ b/planning/REVIEW.md @@ -0,0 +1,365 @@ +# PLAN.md — Comprehensive Review + +_Reviewer pass, 2026-09-08. Builds on Section 13 (Design Decisions Log); does not re-litigate +decisions already recorded there. Focus: readiness of the plan for the **next** implementing +agents (backend platform, LLM, frontend, Docker/E2E), and consistency between the plan and the +**already-built** market data subsystem in `backend/app/market/`._ + +Severity legend: **[BLOCKER]** stops an agent cold · **[RISK]** likely to cause rework or a +production defect · **[GAP]** missing spec, should be added · **[NIT]** editorial / low stakes. + +--- + +## 1. Plan vs. what was actually built (`backend/app/market/`) + +The plan already flags the day-change anchor and unknown-ticker fallback as "follow-up +additions, not yet built." Reading the code, the divergence is wider than those two items and +should be spelled out so the platform agent knows the market layer is **not** a finished +dependency it can just import. + +### 1.1 [BLOCKER] Day-change anchor does not exist anywhere in the built code +Decision #1 and §6 require the price cache to hold a per-ticker **anchor** and for it to ship in +the SSE payload, `/api/watchlist`, and `/api/bootstrap`. Reality: +- `PriceUpdate` (`models.py`) is a `frozen=True, slots=True` dataclass with fields + `ticker, price, previous_price, timestamp` only. No anchor, and it cannot be added at runtime. +- `PriceCache.update()` (`cache.py`) takes `(ticker, price, timestamp)` — no anchor concept, no + place to store one. +- `stream.py` `to_dict()` emits `change`, `change_percent`, `direction` — no anchor. +- `MassiveDataSource` never reads a previous-close field; `_poll_once` only pulls + `last_trade.price`. + +This is real rework on "done, tested, reviewed" code (`PriceUpdate` +/- field, `PriceCache` +anchor map + first-write capture, `to_dict`, Massive previous-close plumbing, plus 73 tests to +update). The plan should (a) acknowledge the anchor is a modification of the shipped subsystem, +not a greenfield add, (b) name the owning agent, and (c) specify where the anchor is captured in +Massive mode (Polygon snapshot `prevDay.c` / `todaysChange` — confirm the `massive` package +surfaces it; if not, the "previous close" anchor is not achievable and simulator-style +first-observed is the only option in both modes). + +### 1.2 [BLOCKER] SSE payload shape in §6 does not match `stream.py` +§6: "Each SSE event contains ticker, price, previous price, day-change anchor, timestamp, and +change direction" — i.e. one event per ticker. +Built: a **single** `data:` line per tick carrying a JSON **object keyed by every ticker** +(`{"AAPL": {...}, "GOOGL": {...}}`), pushed only when `PriceCache.version` changed, plus a +one-time `retry: 1000`. Extra field `change_percent` is present; there is no `event:` type. +The frontend agent will build the `EventSource` handler straight from §6 and get it wrong. +Fix §6 to document the actual envelope (whole-snapshot object, version-gated, ~500ms poll) and +list the actual per-ticker keys, then add `anchor` to that list. + +### 1.3 [RISK] SSE `: keepalive` (Decision #14 / §6) is not implemented +`_generate_events` sleeps and only yields when `version` changes. In Massive mode between polls +(15s free tier) or outside market hours ("prices ... simply stop changing"), the stream is +silent and the client cannot distinguish idle from dead — exactly the case Decision #14 exists +to solve. Needs a `yield ": keepalive\n\n"` on a ~15s timer inside the loop. Cheap, but +currently absent. + +### 1.4 [RISK] Timestamp format contradiction — Decision #19 vs. the price stream +Decision #19 / §7: "UTC ISO-8601 with `Z` **everywhere**." The market layer uses **Unix epoch +floats** end to end (`PriceUpdate.timestamp = time.time()`, SSE ships the float, Massive divides +ms by 1000). Either carve an explicit exception into Decision #19 ("DB timestamps are ISO-8601 +Z; the price stream uses epoch seconds") or convert at the SSE boundary. The frontend needs to +know which it is getting for the sparkline/detail-chart x-axis. + +### 1.5 [NIT] Unknown-ticker synthesis is already partly built +§6 / Decision #3 describe this as not-yet-built, but `simulator.py` +(`SEED_PRICES.get(ticker, random.uniform(50.0, 300.0))` + `DEFAULT_PARAMS`) already synthesizes +a price and GBM params for unseen tickers. What is **missing** is the ticker-format validation +("1–5 uppercase letters") — it exists nowhere in the market layer, and `MassiveDataSource` +silently `.upper().strip()`s input. Reassign this as "add validation at the API boundary" +rather than "build simulator fallback." + +### 1.6 [RISK] `remove_ticker` unconditionally evicts from the cache +Both sources' `remove_ticker` call `self._cache.remove(ticker)`. The §6/§8/Decision #4 +invariant "a position is never left unpriced" is therefore **entirely the caller's +responsibility**: `DELETE /api/watchlist/{ticker}` must NOT call `source.remove_ticker` when an +open position exists, and something must re-add a ticker when a trade opens a position for a +symbol that is not on the watchlist. The plan states the invariant but never says who maintains +the tracked set. See §3.1 below. + +### 1.7 [RISK] Simulator ticker mutation is not concurrency-safe with the step loop +`GBMSimulator.add_ticker/remove_ticker` mutate `self._tickers` / `self._params` and rebuild the +Cholesky factor with no lock; `step()` iterates those same structures every 500ms. This is only +safe if every caller runs on the event-loop thread. Therefore the watchlist mutation routes +**must be `async def`** (not sync routes dispatched to the threadpool). Worth an explicit note +for the backend agent, since FastAPI makes the sync path easy to reach by accident. + +--- + +## 2. Internal consistency & contradictions + +### 2.1 [RISK] "Lazy init on first request" vs. startup background tasks +§7: DB is initialized "on startup (or first request)." But §3/§7 also describe a market-data +task and a 30-second `portfolio_snapshots` writer that need the schema and the watchlist +**before** any HTTP request arrives. The snapshot writer will crash on an empty DB. Resolve to: +initialize + seed on startup (lifespan), unconditionally, before starting background tasks. +Drop "or first request." + +### 2.2 [RISK] Startup ordering is unspecified +Several things must happen in order and the plan never sequences them: +1. open DB, create schema if missing, seed defaults; +2. read `watchlist ∪ open positions` from DB; +3. `create_market_data_source(cache)` → `await source.start(tracked)`; +4. start the snapshot-writer task; +5. mount routers. +Add a short "Application lifespan" subsection. Without it, three agents will invent three +different orderings. + +### 2.3 [RISK] Realized P&L — two descriptions of the source of truth +§2: "cumulative realized gain/loss ... computed from the trade log." §7: stored in +`positions.realized_pnl`, accumulated in the trade transaction. These are reconcilable (§7 says +the cache tables are replayable from `trades`), but §2's wording will send an agent to compute +realized P&L by scanning `trades` at request time while another reads the column. State once: +**`positions.realized_pnl` is the read path; `trades` is the rebuild path.** + +### 2.4 [GAP] `/api/bootstrap` payload is undefined +It is described only prose-wise ("everything needed for first paint"). The frontend agent needs +the concrete JSON: which keys, nesting, whether it reuses the exact shapes of `/api/portfolio` + +`/api/watchlist` + `/api/portfolio/history` + `/api/chat/history`, and what limits apply to the +embedded history and chat arrays (500 / 50 to match the standalone endpoints?). Specify it as +"the four responses under four keys" or write the schema out. + +### 2.5 [GAP] Error contract vs. FastAPI defaults +Decision #16 / §8 mandate `{"error": "..."}` with 400 / 404 / 502. FastAPI emits +`{"detail": ...}` and — critically — **422** for request-body validation, not 400. The backend +agent must install a custom exception handler and override `RequestValidationError`. Say so, or +the contract silently won't hold for "bad quantity / malformed ticker." + +### 2.6 [GAP] `502 — upstream failure (LLM/market data provider)` — which endpoint? +Chat clearly returns 502 when `OPENROUTER_API_KEY` is missing/failing. But no endpoint surfaces +a market-data upstream failure: the source is chosen once at startup, Massive poll failures are +swallowed (`massive_client.py` logs and retries), and SSE just serves a stale cache. Either drop +"market data provider" from the 502 line or define the behavior (e.g. `/api/health` degrades, or +SSE emits an `event: error`). + +### 2.7 [NIT] `chat_messages` write — "Stores the message" (singular) +§9 step 7 should say it stores **both** the user row and the assistant row (the user row has +`actions = null` per §7). As written it reads like only one row is persisted. + +### 2.8 [NIT] Two paths to restore chat on load +§10 says the chat panel restores from `/api/chat/history`; §8/Decision #10 says `/api/bootstrap` +already returns "recent chat history." Pick one for first paint (bootstrap) and reserve +`/api/chat/history` for pagination, to avoid a double fetch and a flash of two states. + +### 2.9 [NIT] Start scripts `--build`, E2E runner does not +§11 scripts run `docker compose up -d --build`; §12 E2E runner runs `docker compose up -d` +(no `--build`), so tests can run against a stale image after a code change. Align them. + +--- + +## 3. Ambiguities that will block or mislead an implementing agent + +### 3.1 [BLOCKER] Ownership of the "tracked set = watchlist ∪ open positions" invariant +No component is assigned to keep the market data source's ticker set in sync. Concretely +unspecified: +- On `POST /api/watchlist`: persist row **and** `await source.add_ticker()` — in which order, + and what if `add_ticker` is a no-op because a position already tracks it? +- On `DELETE /api/watchlist/{ticker}`: skip `source.remove_ticker()` iff `positions.quantity > 0`. +- On a buy that opens a brand-new ticker not on the watchlist: who calls `add_ticker`? +- On a sell that takes `quantity` to 0 for a ticker that was removed from the watchlist earlier: + who calls `remove_ticker` now that nothing references it? +- On startup: seed the source from the union, not just the watchlist. +This is a single helper ("reconcile_tracked_tickers") but three agents touch it. Spec it. + +### 3.2 [BLOCKER] Database path / configuration +§4 shows `db/finally.db` (repo root) and `/app/db` (container volume). No env var in §5 controls +it. Tests need a throwaway DB. Add `DATABASE_PATH` (or similar) to §5 with its default and note +that E2E / unit tests point it at a temp file. + +### 3.3 [RISK] SQLite concurrency model is unspecified +Concurrent writers in one process: the trade endpoint, the 30s snapshot task, lazy init, and +"immediately after each trade" snapshot. FastAPI async + `sqlite3` needs a deliberate choice: +WAL mode, `check_same_thread`, one connection vs. connection-per-request, and a write lock or +`BEGIN IMMEDIATE` so a snapshot write doesn't collide with a trade transaction. `SQLITE_BUSY` +under the E2E suite is a real risk. Add a "Database access" paragraph. + +### 3.4 [BLOCKER] Uvicorn must run a single worker — not stated anywhere +Everything hinges on one in-process `PriceCache` and one simulator/poller. Multiple uvicorn +workers → N independent simulators producing divergent prices, N snapshot writers, N lazy inits +racing. §11's `CMD` must pin `--workers 1` and the plan should say why. This is easy to miss and +catastrophic for the demo. + +### 3.5 [GAP] `chat_messages.actions` JSON schema +§7 says "JSON — trades executed, watchlist changes made"; §10 says the chat panel renders these +"inline as confirmations, including failures." The shape is never defined. Propose: +```json +{ + "trades": [{"ticker":"AAPL","side":"buy","quantity":10,"status":"ok","price":190.12,"error":null}], + "watchlist_changes": [{"ticker":"PYPL","action":"add","status":"error","error":"..."}] +} +``` +Both the backend (writer) and frontend (renderer) need this frozen before either starts. + +### 3.6 [GAP] Valuing a position whose price is not yet in the cache +A just-added ticker has no cache entry for a tick or two (`get_price` → `None`). How is total +value / unrealized P&L computed then — skip the leg, use `avg_cost`, use last trade price? +Affects `/api/portfolio`, `/api/bootstrap`, snapshots, and the client-side header recompute. +Pick a rule. + +### 3.7 [GAP] "Day change %" is a misnomer in simulator mode +The anchor is "first observed price after tracking started" and never re-anchors except on +process restart (Decision #11). A container up for days shows an ever-growing "day change." Note +that the label means "since session/tracking start" in simulator mode, and decide whether the +UI should hedge the wording. + +### 3.8 [GAP] `LLM_MOCK=true` override mechanism for E2E +§12: the runner starts compose "overriding `LLM_MOCK=true` via env." Compose reads `.env` via +`env_file`; `docker compose up` has no `-e`. Overriding requires either `environment: [LLM_MOCK]` +in `docker-compose.yml` (passthrough from the host shell) or the runner writing `.env`. Decision +#22 forbids a test compose file, so the passthrough must be baked into the main compose file — +say so explicitly. + +### 3.9 [GAP] Mock LLM response contract +§9 says mock mode returns "deterministic mock responses" but never defines them. E2E scenarios +require specific behavior: a message that triggers a successful trade, one that triggers a +failed trade, one plain chat. Define the mock's input→output mapping (e.g. keyed on substrings +in the user message) so the E2E agent and the LLM agent agree. + +### 3.10 [GAP] Concurrent chat / trade requests +Single user, but the frontend can fire a manual trade while a chat turn is mid-execution, or a +user double-sends. No guidance on serialization. At minimum note that trades are serialized by +the DB transaction and that a second concurrent chat request is allowed to queue / 409 / proceed. + +--- + +## 4. Architectural risks & questionable choices + +### 4.1 [RISK] `request.is_disconnected()` as the SSE liveness check +`stream.py` polls `await request.is_disconnected()` every 500ms. Under uvicorn this is known to +be unreliable behind some proxies and can miss disconnects, leaking generator tasks that keep +touching the cache. Consider also breaking on write failure and capping stream lifetime. Low +priority for a local demo, real for the "Optional Cloud Deployment." + +### 4.2 [RISK] Whole-snapshot SSE payload on every version change +The simulator bumps `version` once per ticker per tick, so `version` jumps by ~N each 500ms and +the stream re-serializes **all** tickers every time. Fine at 10–20 tickers; if the AI or user +piles on symbols it grows O(N) per 500ms per client. Acceptable for scope — just acknowledge the +ceiling (say, soft-cap the watchlist at ~30). + +### 4.3 [RISK] Unbounded client-side price history +§2/§10: sparklines and the detail chart accumulate from SSE "since page load" with no cap. A +tab left open for hours grows arrays without bound (10–20 tickers × 2 Hz). Specify a ring buffer +/ max points per series (e.g. last 1,800 = 15 min) — this is a frontend spec item, not just an +optimization. + +### 4.4 [RISK] Recharts for the detail chart at 2 Hz +Decision #20 mandates Recharts for all four visuals, with a canvas escape hatch only for the +detail chart. Recharts (SVG) re-rendering a growing line at 2 Hz will get janky within minutes. +Recommend pre-emptively: throttle chart updates to ~1 Hz and downsample to a fixed width, +regardless of library. Flag so the frontend agent budgets for it rather than discovering it late. + +### 4.5 [RISK] No schema migration path with a persisted volume +Decision explicitly defers migrations, and lazy "create if missing" does not `ALTER` for new +columns. Since the volume persists across rebuilds (§11), the first schema change after anyone +has run the app will hit a stale DB with no upgrade path. At least document "delete the volume +to reset" and consider a `schema_version` row now so a future migration hook has a hook point. + +### 4.6 [RISK] Auto-executed trades from unschema'd LLM output +§9 notes Cerebras may not enforce the JSON schema (Decision #8 retry-once fallback). Combined +with auto-execution and no confirmation, a hallucinated `{"ticker":"APPL","quantity":9999}` goes +straight to validation. Validation catches insufficient cash, but not a wrong-but-affordable +ticker/qty. Consider a sanity clamp (e.g. reject trades whose notional > current total value, or +> some multiple of cash) as defense in depth. Cheap, and a good demo-safety story. + +### 4.7 [NIT] `.env` "read by the backend" wording +§5: "The backend reads `.env` from the project root." In the container, compose's `env_file` +injects real environment variables — the backend should read `os.environ` only. `python-dotenv` +(if used) is a local-dev convenience. Clarify to avoid an agent shipping a container that tries +to open a non-existent `.env`. + +--- + +## 5. Gaps — things the plan should cover but doesn't + +| # | Gap | Why it matters | +|---|-----|----------------| +| 5.1 | **Agent roles & build order** (already "still open" in §13) | Four agents, shared files, hard ordering deps (§2.2, §3.1). This is now the single biggest blocker to starting. Write `planning/ROLES.md`. | +| 5.2 | **Application lifespan / startup sequence** | §2.1, §2.2. | +| 5.3 | **`DATABASE_PATH` env var + test DB strategy** | §3.2. | +| 5.4 | **`--workers 1` requirement** | §3.4. | +| 5.5 | **SQLite concurrency (WAL, connection strategy, write serialization)** | §3.3. | +| 5.6 | **`/api/bootstrap` concrete schema** | §2.4. | +| 5.7 | **`chat_messages.actions` schema** | §3.5. | +| 5.8 | **Custom exception handler for the `{"error"}` contract + 422→400** | §2.5. | +| 5.9 | **Mock LLM response contract** | §3.9. | +| 5.10 | **`LLM_MOCK` compose passthrough** | §3.8. | +| 5.11 | **Missing-price valuation rule** | §3.6. | +| 5.12 | **Client-side history cap** | §4.3. | +| 5.13 | **Ticker validation location + regex** (`^[A-Z]{1,5}$`) applied at every write path: manual watchlist add, manual trade, LLM trade, LLM watchlist change | §1.5. | +| 5.14 | **`GET /api/trades` (blotter)** — §2 promises the user "sees realized P&L ... from the trade log" and E2E checks avg-cost behavior, but no endpoint exposes trades and bootstrap omits them. Either add the endpoint or state explicitly that trades are not surfaced in v1. | UI completeness / testability. | +| 5.15 | **SSE through `next dev` rewrites** — Next.js rewrites proxying `text/event-stream` can buffer and break flush semantics in dev. Note that devs should hit `:8000` directly for the stream, or test SSE only against the built export. | §10 dev-proxy usability. | +| 5.16 | **Number/currency formatting & rounding at the display layer** — §8 fixes storage rounding (6dp shares, cents cash) but not display (price decimals, P&L %, treemap thresholds). Minor, but "still open" already lists the treemap scale. | Consistency across panels. | +| 5.17 | **Health check semantics for the market task** — `/api/health` checks process + DB. It does not report whether prices are actually streaming. Consider a `/api/health` field `market_data: "ok"|"stale"` (last cache update age). | Ops / E2E "prices are streaming" assertion. | +| 5.18 | **Graceful shutdown** — lifespan must `await source.stop()` and cancel the snapshot task; SSE generators must exit. Not mentioned. | Clean `docker compose down`, no orphaned tasks in tests. | + +--- + +## 6. Opportunities to simplify + +- **6.1 Collapse `/api/portfolio` and the portfolio slice of `/api/bootstrap` into one serializer.** + Same for watchlist and history. State in §8 that bootstrap composes the other responses + verbatim — removes any chance of drift between the two shapes. +- **6.2 Drop `change` / `change_percent` from the SSE `to_dict()`.** The UI shows day-change % + (vs. anchor) and a flash driven by `direction`; per-tick absolute change and per-tick percent + are unused. (Decision "not doing: trimming change/direction" — but `change_percent` isn't even + mentioned in that decision and is pure noise. Re-open just that sub-item.) +- **6.3 One background task, not two.** The 30s `portfolio_snapshots` writer and any future + cache-maintenance work can share a single "housekeeping" loop rather than separate tasks — + fewer things to order, start, and stop in the lifespan. +- **6.4 Skip the standalone `GET /api/watchlist` / `GET /api/portfolio/history` for v1?** After + bootstrap, the watchlist only changes via calls the client itself makes (it can update state + locally from the mutation response), and history is only appended by the server (the client + already recomputes total value live from SSE; the P&L chart can extend from SSE ticks too). + If that holds, bootstrap + the mutation endpoints + SSE cover the whole app and two GETs go + away. Worth a hard look before building them. +- **6.5 `previous_price` in the SSE payload is redundant with client state.** The client sees + every tick, so it already knows the prior price for the flash. Keeping it is harmless (1 + number) but if trimming, this is a candidate. +- **6.6 Seed prices live in code twice conceptually.** `seed_prices.py` has the 10 tickers; + §7 seed data has the same 10 in the `watchlist` table. Make the DB seed import the list from + the market module (single source) rather than hardcoding it in schema SQL. + +--- + +## 7. Top recommendations (do these before the next agent starts) + +1. **Write `planning/ROLES.md`** — agent ownership + build order. Nominate: (a) Backend Platform + (DB, lifespan, portfolio/watchlist/trade APIs, error contract), (b) LLM (chat endpoint, mock, + structured output), (c) Frontend, (d) Docker/E2E. Sequence: A → (B ∥ C) → D. +2. **Add an "Application lifespan & configuration" subsection to §3 or §7**: startup ordering + (§2.2), `--workers 1` (§3.4), `DATABASE_PATH` (§3.2), SQLite WAL + connection strategy + (§3.3), graceful shutdown (§5.18). +3. **Reconcile §6 with the shipped market code**: real SSE envelope (§1.2), keepalive as a + to-do with an owner (§1.3), timestamp format exception (§1.4), and an explicit note that the + **anchor is a modification of already-tested code** with a named owner and a Massive + previous-close feasibility check (§1.1). +4. **Specify the tracked-ticker reconciliation helper** (§3.1) and require async watchlist + routes (§1.7). +5. **Freeze two schemas**: `/api/bootstrap` response (§2.4) and `chat_messages.actions` (§3.5). +6. **Nail the error contract**: custom handler, 422→400 mapping, and which endpoints can 502 + (§2.5, §2.6). +7. **Define the mock-LLM contract and the `LLM_MOCK` compose passthrough** (§3.8, §3.9) so E2E + isn't blocked on the LLM agent. +8. **Add ticker validation** (`^[A-Z]{1,5}$`) at all four write paths (§5.13). + +--- + +## 8. Smaller notes + +- §4 directory tree lists `test/` for Playwright and says unit tests live in `frontend/` and + `backend/`. `backend/tests/` already exists; confirm the frontend testing library choice + (§12 says "React Testing Library or similar" — pick one so lockfiles are deterministic). +- §7 `portfolio_snapshots` "trims to 7 days on each write" while `/api/portfolio/history` + defaults to `limit=500`. At one snapshot / 30s while active, 7 days ≈ 20k rows but realistic + active use is far less; 500 is ~4 hours. Fine, just confirm the chart is meant to show + "recent" not "all 7 days" by default. +- §9 "last 20 messages" — clarify whether that is 20 rows (10 turns) or 20 turns. §8 history + default is 50. +- §10 header "Total value updates live ... recomputed client-side" — this diverges from the + server's snapshot valuation whenever the client is missing a tick or a price (§3.6). Note the + client value is an estimate that reconciles to the server on the next bootstrap/portfolio + fetch. +- Decision #24 says skill name is `cerebras`; the repo has `.claude/skills/cerebras/` — matches. +- §11 Dockerfile "Node 20" vs. §11 stage-1 "Node 20 slim" vs. common Next.js 14+ needing Node + 18.17+/20 — fine, just pin the exact tag in the Dockerfile. +- No mention of `favicon`, page ``, or basic branding for the served SPA — trivial, but + add a line so it isn't forgotten in the "visually stunning" product. diff --git a/planning/archive/PLAN.pre-review-backup-2026-09-08.md b/planning/archive/PLAN.pre-review-backup-2026-09-08.md new file mode 100644 index 000000000..7c686b0af --- /dev/null +++ b/planning/archive/PLAN.pre-review-backup-2026-09-08.md @@ -0,0 +1,531 @@ +# FinAlly — AI Trading Workstation + +## Project Specification + +## 1. Vision + +FinAlly (Finance Ally) is a visually stunning AI-powered trading workstation that streams live market data, lets users trade a simulated portfolio, and integrates an LLM chat assistant that can analyze positions and execute trades on the user's behalf. It looks and feels like a modern Bloomberg terminal with an AI copilot. + +This is the capstone project for an agentic AI coding course. It is built entirely by Coding Agents demonstrating how orchestrated AI agents can produce a production-quality full-stack application. Agents interact through files in `planning/`. + +## 2. User Experience + +### First Launch + +The user runs a single Docker command (or a provided start script). A browser opens to `http://localhost:8000`. No login, no signup. They immediately see: + +- A watchlist of 10 default tickers with live-updating prices in a grid +- $10,000 in virtual cash +- A dark, data-rich trading terminal aesthetic +- An AI chat panel ready to assist + +### What the User Can Do + +- **Watch prices stream** — prices flash green (uptick) or red (downtick) with subtle CSS animations that fade +- **View sparkline mini-charts** — price action beside each ticker in the watchlist, accumulated on the frontend from the SSE stream since page load (sparklines fill in progressively) +- **Click a ticker** to see a larger detailed chart in the main chart area +- **Buy and sell shares** — market orders only, instant fill at current price, no fees, no confirmation dialog +- **Monitor their portfolio** — a heatmap (treemap) showing positions sized by weight and colored by P&L, plus a P&L chart tracking total portfolio value over time +- **View a positions table** — ticker, quantity, average cost, current price, unrealized P&L, % change +- **Chat with the AI assistant** — ask about their portfolio, get analysis, and have the AI execute trades and manage the watchlist through natural language +- **Manage the watchlist** — add/remove tickers manually or via the AI chat + +### Visual Design + +- **Dark theme**: backgrounds around `#0d1117` or `#1a1a2e`, muted gray borders, no pure black +- **Price flash animations**: brief green/red background highlight on price change, fading over ~500ms via CSS transitions +- **Connection status indicator**: a small colored dot (green = connected, yellow = reconnecting, red = disconnected) visible in the header +- **Professional, data-dense layout**: inspired by Bloomberg/trading terminals — every pixel earns its place +- **Responsive but desktop-first**: optimized for wide screens, functional on tablet + +### Color Scheme +- Accent Yellow: `#ecad0a` +- Blue Primary: `#209dd7` +- Purple Secondary: `#753991` (submit buttons) + +## 3. Architecture Overview + +### Single Container, Single Port + +``` +┌─────────────────────────────────────────────────┐ +│ Docker Container (port 8000) │ +│ │ +│ FastAPI (Python/uv) │ +│ ├── /api/* REST endpoints │ +│ ├── /api/stream/* SSE streaming │ +│ └── /* Static file serving │ +│ (Next.js export) │ +│ │ +│ SQLite database (volume-mounted) │ +│ Background task: market data polling/sim │ +└─────────────────────────────────────────────────┘ +``` + +- **Frontend**: Next.js with TypeScript, built as a static export (`output: 'export'`), served by FastAPI as static files +- **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 +- **Market data**: Environment-variable driven — simulator by default, real data via Massive API if key provided + +### Why These Choices + +| Decision | Rationale | +|---|---| +| SSE over WebSockets | One-way push is all we need; simpler, no bidirectional complexity, universal browser support | +| Static Next.js export | Single origin, no CORS issues, one port, one container, simple deployment | +| SQLite over Postgres | No auth = no multi-user = no need for a database server; self-contained, zero config | +| Single Docker container | Students run one command; no docker-compose for production, no service orchestration | +| uv for Python | Fast, modern Python project management; reproducible lockfile; what students should learn | +| Market orders only | Eliminates order book, limit order logic, partial fills — dramatically simpler portfolio math | + +--- + +## 4. Directory Structure + +``` +finally/ +├── frontend/ # Next.js TypeScript project (static export) +├── backend/ # FastAPI uv project (Python) +│ └── db/ # Schema definitions, seed data, migration logic +├── planning/ # Project-wide documentation for agents +│ ├── PLAN.md # This document +│ └── ... # Additional agent reference docs +├── scripts/ +│ ├── start_mac.sh # Launch Docker container (macOS/Linux) +│ ├── stop_mac.sh # Stop Docker container (macOS/Linux) +│ ├── start_windows.ps1 # Launch Docker container (Windows PowerShell) +│ └── stop_windows.ps1 # Stop Docker container (Windows PowerShell) +├── test/ # Playwright E2E tests + docker-compose.test.yml +├── db/ # Volume mount target (SQLite file lives here at runtime) +│ └── .gitkeep # Directory exists in repo; finally.db is gitignored +├── Dockerfile # Multi-stage build (Node → Python) +├── docker-compose.yml # Optional convenience wrapper +├── .env # Environment variables (gitignored, .env.example committed) +└── .gitignore +``` + +### Key Boundaries + +- **`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. +- **`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. + +--- + +## 5. Environment Variables + +```bash +# Required: OpenRouter API key for LLM chat functionality +OPENROUTER_API_KEY=your-openrouter-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) +MASSIVE_API_KEY= + +# Optional: Set to "true" for deterministic mock LLM responses (testing) +LLM_MOCK=false +``` + +### Behavior + +- If `MASSIVE_API_KEY` is set and non-empty → backend uses Massive REST API for market data +- 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`) + +--- + +## 6. Market Data + +### Two Implementations, One Interface + +Both the simulator and the Massive client implement the same abstract interface. The backend selects which to use based on the environment variable. All downstream code (SSE streaming, price cache, frontend) is agnostic to the source. + +### Simulator (Default) + +- Generates prices using geometric Brownian motion (GBM) with configurable drift and volatility per ticker +- Updates at ~500ms intervals +- Correlated moves across tickers (e.g., tech stocks move together) +- Occasional random "events" — sudden 2-5% moves on a ticker for drama +- Starts from realistic seed prices (e.g., AAPL ~$190, GOOGL ~$175, etc.) +- Runs as an in-process background task — no external dependencies + +### 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 +- 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 + +### Shared Price Cache + +- A single background task (simulator or Massive poller) writes to an in-memory price cache +- The cache holds the latest price, previous price, and timestamp for each ticker +- SSE streams read from this cache and push updates to connected clients +- This architecture supports future multi-user scenarios without changes to the data layer + +### SSE Streaming + +- 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 +- Client handles reconnection automatically (EventSource has built-in retry) + +--- + +## 7. Database + +### SQLite with Lazy Initialization + +The backend checks for the SQLite database on startup (or first request). If the file doesn't exist or tables are missing, it creates the schema and seeds default data. This means: + +- No separate migration step +- No manual database setup +- Fresh Docker volumes start with a clean, seeded database automatically + +### Schema + +All tables include a `user_id` column defaulting to `"default"`. This is hardcoded for now (single-user) but enables future multi-user support without schema migration. + +**users_profile** — User state (cash balance) +- `id` TEXT PRIMARY KEY (default: `"default"`) +- `cash_balance` REAL (default: `10000.0`) +- `created_at` TEXT (ISO timestamp) + +**watchlist** — Tickers the user is watching +- `id` TEXT PRIMARY KEY (UUID) +- `user_id` TEXT (default: `"default"`) +- `ticker` TEXT +- `added_at` TEXT (ISO timestamp) +- UNIQUE constraint on `(user_id, ticker)` + +**positions** — Current holdings (one row per ticker per user) +- `id` TEXT PRIMARY KEY (UUID) +- `user_id` TEXT (default: `"default"`) +- `ticker` TEXT +- `quantity` REAL (fractional shares supported) +- `avg_cost` REAL +- `updated_at` TEXT (ISO timestamp) +- UNIQUE constraint on `(user_id, ticker)` + +**trades** — Trade history (append-only log) +- `id` TEXT PRIMARY KEY (UUID) +- `user_id` TEXT (default: `"default"`) +- `ticker` TEXT +- `side` TEXT (`"buy"` or `"sell"`) +- `quantity` REAL (fractional shares supported) +- `price` REAL +- `executed_at` TEXT (ISO timestamp) + +**portfolio_snapshots** — Portfolio value over time (for P&L chart). Recorded every 30 seconds by a background task, and immediately after each trade execution. +- `id` TEXT PRIMARY KEY (UUID) +- `user_id` TEXT (default: `"default"`) +- `total_value` REAL +- `recorded_at` TEXT (ISO timestamp) + +**chat_messages** — Conversation history with LLM +- `id` TEXT PRIMARY KEY (UUID) +- `user_id` TEXT (default: `"default"`) +- `role` TEXT (`"user"` or `"assistant"`) +- `content` TEXT +- `actions` TEXT (JSON — trades executed, watchlist changes made; null for user messages) +- `created_at` TEXT (ISO timestamp) + +### Default Seed Data + +- One user profile: `id="default"`, `cash_balance=10000.0` +- Ten watchlist entries: AAPL, GOOGL, MSFT, AMZN, TSLA, NVDA, META, JPM, V, NFLX + +--- + +## 8. API Endpoints + +### Market Data +| Method | Path | Description | +|--------|------|-------------| +| GET | `/api/stream/prices` | SSE stream of live price updates | + +### Portfolio +| Method | Path | Description | +|--------|------|-------------| +| 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) | + +### Watchlist +| Method | Path | Description | +|--------|------|-------------| +| GET | `/api/watchlist` | Current watchlist tickers with latest prices | +| POST | `/api/watchlist` | Add a ticker: `{ticker}` | +| DELETE | `/api/watchlist/{ticker}` | Remove a ticker | + +### Chat +| Method | Path | Description | +|--------|------|-------------| +| POST | `/api/chat` | Send a message, receive complete JSON response (message + executed actions) | + +### System +| Method | Path | Description | +|--------|------|-------------| +| GET | `/api/health` | Health check (for Docker/deployment) | + +--- + +## 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. + +There is an OPENROUTER_API_KEY in the .env file in the project root. + +### How It Works + +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 +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` +8. Returns the complete JSON response to the frontend (no token-by-token streaming — Cerebras inference is fast enough that a loading indicator is sufficient) + +### Structured Output Schema + +The LLM is instructed to respond with JSON matching this schema: + +```json +{ + "message": "Your conversational response to the user", + "trades": [ + {"ticker": "AAPL", "side": "buy", "quantity": 10} + ], + "watchlist_changes": [ + {"ticker": "PYPL", "action": "add"} + ] +} +``` + +- `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 + +### Auto-Execution + +Trades specified by the LLM execute automatically — no confirmation dialog. This is a deliberate design choice: +- It's a simulated environment with fake money, so the stakes are zero +- It creates an impressive, fluid demo experience +- It demonstrates agentic AI capabilities — the core theme of the course + +If a trade fails validation (e.g., insufficient cash), the error is included in the chat response so the LLM can inform the user. + +### System Prompt Guidance + +The LLM should be prompted as "FinAlly, an AI trading assistant" with instructions to: +- Analyze portfolio composition, risk concentration, and P&L +- Suggest trades with reasoning +- Execute trades when the user asks or agrees +- Manage the watchlist proactively +- Be concise and data-driven in responses +- Always respond with valid structured JSON + +### LLM Mock Mode + +When `LLM_MOCK=true`, the backend returns deterministic mock responses instead of calling OpenRouter. This enables: +- Fast, free, reproducible E2E tests +- Development without an API key +- CI/CD pipelines + +--- + +## 10. Frontend Design + +### Layout + +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) +- **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` +- **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. +- **Header** — portfolio total value (updating live), connection status indicator, cash balance + +### Technical Notes + +- Use `EventSource` for SSE connection to `/api/stream/prices` +- Canvas-based charting library preferred (Lightweight Charts or Recharts) for performance +- Price flash effect: on receiving a new price, briefly apply a CSS class with background color transition, then remove it +- All API calls go to the same origin (`/api/*`) — no CORS configuration needed +- Tailwind CSS for styling with a custom dark theme + +--- + +## 11. Docker & Deployment + +### Multi-Stage Dockerfile + +``` +Stage 1: Node 20 slim + - Copy frontend/ + - npm install && npm run build (produces static export) + +Stage 2: Python 3.12 slim + - Install uv + - Copy backend/ + - uv sync (install Python dependencies from lockfile) + - Copy frontend build output into a static/ directory + - Expose port 8000 + - CMD: uvicorn serving FastAPI app +``` + +FastAPI serves the static frontend files and all API routes on port 8000. + +### Docker Volume + +The SQLite database persists via a named Docker volume: + +```bash +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. + +### Start/Stop Scripts + +**`scripts/start_mac.sh`** (macOS/Linux): +- Builds the Docker image if not already built (or if `--build` flag passed) +- Runs the container with the volume mount, port mapping, and `.env` file +- Prints the URL to access the app +- Optionally opens the browser + +**`scripts/stop_mac.sh`** (macOS/Linux): +- Stops and removes the running container +- Does NOT remove the volume (data persists) + +**`scripts/start_windows.ps1`** / **`scripts/stop_windows.ps1`**: PowerShell equivalents for Windows. + +All scripts should be idempotent — safe to run multiple times. + +### Optional Cloud Deployment + +The container is designed to deploy to AWS App Runner, Render, or any container platform. A Terraform configuration for App Runner may be provided in a `deploy/` directory as a stretch goal, but is not part of the core build. + +--- + +## 12. Testing Strategy + +### Unit Tests (within `frontend/` and `backend/`) + +**Backend (pytest)**: +- Market data: simulator generates valid prices, GBM math is correct, Massive API response parsing works, both implementations conform to the abstract interface +- Portfolio: trade execution logic, P&L calculations, edge cases (selling more than owned, buying with insufficient cash, selling at a loss) +- LLM: structured output parsing handles all valid schemas, graceful handling of malformed responses, trade validation within chat flow +- API routes: correct status codes, response shapes, error handling + +**Frontend (React Testing Library or similar)**: +- Component rendering with mock data +- Price flash animation triggers correctly on price changes +- Watchlist CRUD operations +- Portfolio display calculations +- Chat message rendering and loading state + +### E2E Tests (in `test/`) + +**Infrastructure**: A separate `docker-compose.test.yml` in `test/` that spins up the app container plus a Playwright container. This keeps browser dependencies out of the production image. + +**Environment**: Tests run with `LLM_MOCK=true` by default for speed and determinism. + +**Key Scenarios**: +- Fresh start: default watchlist appears, $10k balance shown, prices are streaming +- Add and remove a ticker from the watchlist +- Buy shares: cash decreases, position appears, portfolio updates +- Sell shares: cash increases, position updates or disappears +- 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. Review — Questions, Clarifications & Simplification Opportunities + +_Added 2026-09-08 during a documentation review. Organized by priority. Nothing here blocks starting work, but the items in "Open Questions" should be resolved before the relevant component is built. Market data is already implemented (see `MARKET_DATA_SUMMARY.md`); items touching it are noted as such._ + +### A. Open Questions (decide before building the affected component) + +1. **What is the baseline for "daily change %"?** The watchlist and positions table both show a percent change, but the simulator has no concept of a previous close or session open — it starts from a seed price and drifts. `PriceUpdate.change` is tick-over-tick, not day-over-day. We need to define an anchor price per ticker (e.g. "first price seen this server session" or a stored `open_price`) and expose it (in `/api/watchlist` and the SSE payload, or a separate field). In Massive mode Polygon provides a previous-close value; the simulator needs an equivalent. **Affects: market data (already built — may need a small addition), frontend.** + +2. **Is chart/price history purely client-accumulated from SSE?** Section 2 says sparklines accumulate on the frontend since page load. Does the same apply to the main detail chart and the watchlist sparklines after a reload — i.e. all history is lost on refresh? If that's acceptable, state it explicitly. If not, we need a server-side rolling price-history buffer (last N minutes per ticker) and a `GET /api/history/{ticker}` endpoint. Recommendation for simplicity: accept client-only accumulation for v1 and document it. + +3. **Adding a ticker that the simulator doesn't know.** The LLM schema example literally adds `PYPL`, which is presumably not in `seed_prices.py`. What happens when an unknown ticker is added in simulator mode? We need a defined fallback (generate a plausible seed price + default GBM drift/volatility, assign to a correlation group or none). Also define ticker validation: format check only, or reject unknown symbols? In Massive mode, what if Polygon returns no data for the symbol? **Affects: market data (already built — confirm behavior), watchlist API.** + +4. **Watchlist vs. positions coupling.** If a user removes a ticker from the watchlist while holding a position in it, its price must keep updating (portfolio valuation, P&L, heatmap all depend on it). Confirm the tracked-ticker set is always `watchlist ∪ position tickers`, and that `DELETE /api/watchlist/{ticker}` is allowed (or blocked) when a position exists. Section 6 says SSE pushes "all tickers known to the system" — make explicit that this union is what's tracked. + +5. **Realized P&L — tracked or not?** The schema stores `trades` (enough to compute it) but no table or field holds realized P&L, and Section 2/10 only mention *unrealized* P&L. When a position is fully sold, does its row get deleted (Section 12 says "updates or disappears")? If it disappears, realized gains vanish from the UI. Decide: (a) ignore realized P&L for v1, or (b) show a realized P&L figure in the header/portfolio, computed from `trades`. + +6. **Position accounting method.** State explicitly that cost basis is **weighted average cost** (not FIFO/LIFO): buys recompute `avg_cost`, sells leave `avg_cost` unchanged and reduce `quantity`. Define what happens at `quantity == 0` (delete row vs. keep with zeroed quantity). + +7. **Trade quantity semantics.** `POST /api/portfolio/trade` takes `{ticker, quantity, side}` — quantity is always in *shares*, correct? No notional/dollar orders ("buy $500 of AAPL")? The LLM may naturally want notional trades — should the schema support `{ticker, side, notional}` as an alternative, or is the LLM expected to convert using the price in its context? Also define: minimum quantity, rejection of zero/negative, and rounding precision for fractional shares. + +8. **Structured output support on the Cerebras path.** Confirm that `openrouter/openai/gpt-oss-120b` with the Cerebras provider actually enforces JSON-schema structured outputs through OpenRouter (some provider/model combinations only support `json_object` or ignore the schema). If enforcement isn't guaranteed, Section 9 should specify a parse-validate-retry-once strategy and a safe fallback (return the raw message with no actions). The `cerebras` skill should be consulted here. + +9. **Conversation history window.** Section 9 step 2 says "recent conversation history" — define the limit (last N messages or a token budget) to keep prompts bounded as `chat_messages` grows. + +10. **How does the frontend load prior chat history and initial state on refresh?** There's no `GET /api/chat/history` endpoint, so a page reload shows an empty conversation despite `chat_messages` being persisted. Either add that endpoint or fold chat history into a bootstrap response (see simplification #2). + +### B. Gaps & Risks + +11. **In-memory prices vs. persisted positions on restart.** Simulator prices reset to seed values on container restart, but `positions.avg_cost` and `cash_balance` persist in SQLite. After a restart, unrealized P&L will visibly jump. Options: persist last prices to the DB on shutdown / periodically, or document this as expected demo behavior. + +12. **`portfolio_snapshots` grows unbounded** — 2,880 rows/day at 30s cadence, forever, even when the app is idle. For a long-lived demo/deploy consider: a retention window, downsampling for the chart, or only snapshotting when portfolio value changed materially. `GET /api/portfolio/history` should also take `?from=&to=` or `?limit=` rather than returning the entire series. + +13. **Public deployment exposes an auth-free app that spends real money.** Section 11 offers cloud deployment as a stretch goal, but the no-auth design means anyone who finds the URL can drive unlimited LLM calls against your `OPENROUTER_API_KEY`. If cloud deploy is pursued, note the need for at least a shared secret / basic auth / rate limiting, and that the simulated portfolio is globally shared (single `user_id="default"`). + +14. **SSE keepalive / connection indicator in Massive mode.** With version-based change detection and 15s Polygon polling, the stream can be silent for long stretches (and overnight/weekends for real data). Confirm the server sends a periodic `: keepalive` comment so the client can distinguish "idle" from "disconnected" and keep the status dot accurate. + +15. **Trade execution atomicity.** Manual trades and LLM auto-executed trades both read cash/quantity, validate, then write. Specify that each trade runs in a single transaction (and, if chat can fire multiple trades, that they execute sequentially) so validation can't race. + +16. **Error response contract is undefined.** Section 8 lists endpoints but no error shape or status codes. Define a convention (e.g. `{ "error": "message" }` with 400 for validation, 404 for unknown ticker, 502 for LLM/upstream failures) so the frontend can render failures consistently — including the "trade failed validation" case surfaced through chat. + +17. **Frontend dev workflow / same-origin.** The single-origin design is clean for production, but `next dev` on :3000 calling :8000 needs a documented proxy (`next.config` rewrites) or the frontend team will hit CORS during development. + +18. **`output: 'export'` constraints.** Static export disables Next.js image optimization, route handlers, middleware, and dynamic routes. Worth a one-line note so the frontend agent designs within those limits from the start. + +19. **Timestamp format for range queries.** `portfolio_snapshots` / history queries rely on ordering TEXT timestamps — specify **UTC ISO-8601 with a `Z` suffix** everywhere so lexicographic sort == chronological sort. + +### C. Simplification Opportunities + +20. **Derive cash and positions from the `trades` log (event sourcing).** `cash_balance` is fully determined by `10000 − Σ(buys) + Σ(sells)`, and `positions` by replaying `trades`. Keeping `users_profile.cash_balance` and the `positions` table as separate mutable state introduces update-ordering bugs for zero benefit at this data scale. Consider computing both from `trades` on each request (single user, tiny table). If that feels too radical, at minimum treat `trades` as the source of truth and the others as a cache. This also gives realized P&L for free (#5). + +21. **One "bootstrap" endpoint for initial page load.** Instead of the frontend firing `/api/portfolio` + `/api/watchlist` + `/api/portfolio/history` + (missing) chat history on mount, offer `GET /api/bootstrap` returning everything needed for first paint in one round trip. Keeps the individual endpoints for later refreshes if wanted. + +22. **Pick one charting library.** Section 10 says "canvas-based preferred (Lightweight Charts **or** Recharts)" — but Recharts is SVG, and Lightweight Charts can't do the portfolio treemap. Choose one for all four visuals (sparkline, detail chart, P&L line, heatmap). Recharts covers line + treemap out of the box and is the simpler single choice unless canvas performance for the detail chart proves insufficient. + +23. **Make `docker-compose.yml` the single entrypoint.** Section 11 has both a hand-rolled `docker run` in the start scripts *and* an "optional" compose file — two sources of truth for ports/volumes/env that will drift. Let the scripts just wrap `docker compose up -d --build` / `docker compose down`; the "build image if needed" logic in the scripts disappears because compose handles it. + +24. **Drop the Playwright container / `docker-compose.test.yml`.** Running Playwright from the host against the running app container (`npx playwright test` in `test/`) is fewer moving parts and still keeps browser deps out of the production image (they're in `test/`, never in the Dockerfile). A dedicated test compose file is arguably overkill for one app service. + +25. **Trim the SSE payload.** `change` and `direction` are both derivable on the client from `price` vs `previous_price`. Sending just `{ticker, price, previous_price, timestamp}` removes server-side logic and shrinks the message. (Minor — skip if `direction` already exists and is convenient.) + +26. **Reconsider unused audit columns.** `created_at` on `users_profile` and `updated_at` on `positions` don't appear to drive any feature. Drop them unless a UI element needs them, or keep only if event-sourcing (#20) is rejected. + +27. **`npm ci` over `npm install` in the Dockerfile** (Section 11, Stage 1) for reproducible builds from the committed lockfile — a correctness fix as much as a simplification. + +### D. Minor / Editorial + +28. Enumerate the allowed values for `watchlist_changes[].action` in the Section 9 schema (the example only shows `"add"`; presumably `"remove"` too). +29. Define the SSE `direction` values precisely — `"up" | "down" | "flat"`? (`MARKET_DATA_SUMMARY.md` implies a third state exists.) +30. Specify what `GET /api/health` checks (process only, or also DB connectivity). +31. Section headers: the lone `## Project Specification` under the title is empty; `---` separators are used between sections 3→4 onward but not 1→2→3. Cosmetic consistency pass. +32. Section 9 references the skill as both "cerebras-inference skill" (prose) — the available skill is named `cerebras`. Align the name. +33. Consider adding a short "Agent Roles & Build Order" subsection (or a pointer to a separate doc) — the plan refers to "the Frontend Engineer agent" and "Backend/Market Data agents" and file-based coordination, but there's no index of which agent owns which deliverable or in what sequence.