# Web Redesign — Systematic Audit & Fix Plan

Scope: everything introduced by commits `f62c3b1` (UI shell + 6 endpoints) and `938ed86`
(live data wiring + strategy builder + screener). Reviewed code, design, UX,
backend integration, mobile parity, security, and tests.

Legend: 🔴 critical (broken / data loss / security) · 🟠 high (clearly wrong but
not catastrophic) · 🟡 medium (UX or polish) · 🟢 low (nice-to-have).

Status: ⬜ open · 🟦 in PR · ✅ fixed (commit hash on the right).

---

## A. Critical — Broken integrations (will 404 / 401 / corrupt data)

| #   | Issue                                                                                                                                   | Severity | Status | Fix commit |
| --- | --------------------------------------------------------------------------------------------------------------------------------------- | :------: | :----: | ---------- |
| A1  | `CodeStrategyEditor` POSTs to `/api/backtest`. Real endpoint is `/api/backtest/run`. Result: every "Run Backtest" returns 404.          | 🔴       | ✅     | bucket A   |
| A2  | `CodeStrategyEditor` payload sends `{symbol, strategyCode, mode}`. Backend `/api/backtest/run` requires `{symbols[], strategyConfig}`.  | 🔴       | ✅     | bucket A   |
| A3  | `VisualRuleBuilder` save → `/api/profiles` body uses `{strategyType, visualRules, description}`. `saveTradeProfileForUser` expects `strategy_config` shape. Result: 400 or silently-discarded fields. | 🔴       | ✅     | bucket A   |
| A4  | `RightPanel.NewsFeed` calls `fetch()` with no `Authorization` header. `/api/news` is `requireAuth`. Result: 401 every render.            | 🔴       | ✅     | bucket A   |
| A5  | `RightPanel.NewsFeed` reads `import.meta.env.VITE_TRADING_API_URL` directly instead of `tradingRuntime.tradingApiUrl`. Breaks in prod where the runtime resolver is the source of truth. | 🟠       | ✅     | bucket A   |
| A6  | Backend `/api/chart/bars` previously crashed on crypto symbols (`BTC/USD`) because `/v2/stocks` rejects them. Verified in 938ed86: `encodeURIComponent('BTC/USD')` → `BTC%2FUSD` (correct for query string), and the response lookup `cryptoBars[symbol]` uses the un-encoded key (matches Alpaca's response). | 🟠       | ✅     | 938ed86    |

## B. Functional gaps (feature exists in plan but not implemented)

| #   | Issue                                                                                                                              | Severity | Status | Fix commit |
| --- | ---------------------------------------------------------------------------------------------------------------------------------- | :------: | :----: | ---------- |
| B1  | Phase 2 plan called for RSI / MACD / Bollinger Band overlay toggles below `StockChart`. Not built.                                  | 🟠       | ✅     | c54b9f2    |
| B2  | `TickerHeader` "★ Watchlist" and 🔔 buttons have no `onClick` — purely cosmetic.                                                   | 🟠       | ✅     | ed8175e    |
| B3  | `TickerHeader` company name is the symbol (placeholder). Should pull from `fetchResearchProfile`.                                  | 🟠       | ✅     | ed8175e    |
| B4  | `TickerHeader` "as of" timestamp uses `new Date()` (always = now). Should use the latest bar timestamp.                            | 🟡       | ⬜     |            |
| B5  | `QuickStats` reads RSI/EMA only from `botState.symbols[symbol].indicators`. The bot only computes these for symbols it actively tracks → user-searched tickers always show `—`. Need an indicator service or compute from bars client-side. | 🟠       | ⬜     |            |
| B6  | `CodeStrategyEditor` "Save" button writes to `localStorage` only, then shows "Saved!". Misleading — no persistence.                | 🟠       | ✅     | da79682    |
| B7  | `CodeStrategyEditor` defines `editorRef` but never uses it.                                                                        | 🟢       | ⬜     |            |
| B8  | `VisualRuleBuilder` exposes `onBacktest` prop but `ResearchView` doesn't pass one — backtest button never appears.                  | 🟡       | ⬜     |            |
| B9  | `ResearchView` "Strategies" tab still shows the legacy `MyStrategiesTab`; visual/code-saved strategies don't appear there.          | 🟡       | ⬜     |            |
| B10 | No 404 / catch-all React Router route — invalid paths render blank.                                                                | 🟡       | ⬜     |            |

## C. Security & correctness

| #   | Issue                                                                                                                              | Severity | Status | Fix commit |
| --- | ---------------------------------------------------------------------------------------------------------------------------------- | :------: | :----: | ---------- |
| C1  | Backend posts arbitrary user JS (`strategyCode`) to `/api/backtest` if A1+A2 are "fixed" naively. Must sandbox or refuse.            | 🔴       | ✅     | 6aa001a    |
| C2  | No FMP response cache. Free tier = 250 req/day. Every Home view load = 3 req. 80 page loads/day → quota burnt by lunch.            | 🟠       | ✅     | 0828007    |
| C3  | `/api/screener` passes `sector` query through to FMP without an allow-list. Low-impact injection, but should validate.              | 🟡       | ✅     | c173aeb    |
| C4  | `/api/news` passes `symbols` through to Alpaca without validation.                                                                  | 🟡       | ✅     | 7c4b08c    |
| C5  | Header `fetchMarketIndices` polls every 60 s even when the tab is hidden. Should pause via `document.visibilityState`.              | 🟡       | ✅     | e089832    |
| C6  | `backend/.env.example` keeps `FMP_API_KEY=demo` AND `apiServer.ts` falls back to `'demo'`. Two sources of truth. Demo key is shared globally and rate-limited.  | 🟡       | ✅     | 1377bf2    |
| C7  | FMP `apikey` is sent as a query string → leaks into proxy / CDN logs. FMP doesn't support headers, so the only mitigation is server-side caching (see C2). | 🟡       | ✅     | e2e189e    |

## D. UX / UI polish

| #   | Issue                                                                                                                              | Severity | Status | Fix commit |
| --- | ---------------------------------------------------------------------------------------------------------------------------------- | :------: | :----: | ---------- |
| D1  | News, ResearchCards, Screener show plain "Loading…" text. Should be skeleton placeholders to prevent layout jank.                  | 🟡       | ✅     | 0fb1d63    |
| D2  | Light theme broke contrast in legacy tabs (`SettingsTab`, `ConfigTab`, etc.) which were styled for dark.                            | 🟡       | ⬜     |            |
| D3  | `TickerHeader` exchange is hard-coded "NASDAQ".                                                                                    | 🟢       | ⬜     |            |
| D4  | `ScreenerView` "More sectors…" `<select>` doesn't visually highlight the chosen sector pill once selected.                          | 🟡       | ⬜     |            |
| D5  | Layout is fixed-width (72 px sidebar + 308 px right panel). Breaks below ~1024 px viewport. No mobile-web responsive breakpoints.   | 🟠       | ✅     | a9c33b1    |
| D6  | `VisualRuleBuilder` and `CodeStrategyEditor` use `setTimeout` without cleanup — leak warning if component unmounts mid-timer.        | 🟢       | ⬜     |            |
| D7  | No keyboard shortcuts (⌘K search, ⌘S save in code editor, ⌘Enter run backtest).                                                    | 🟢       | ⬜     |            |
| D8  | `EmptyState` ticker chips are hard-coded equities. For a crypto-config bot the chips don't apply.                                  | 🟢       | ⬜     |            |
| D9  | No `prefers-reduced-motion` handling on spinners / animations.                                                                      | 🟢       | ⬜     |            |

## E. Build / infrastructure

| #   | Issue                                                                                                                              | Severity | Status | Fix commit |
| --- | ---------------------------------------------------------------------------------------------------------------------------------- | :------: | :----: | ---------- |
| E1  | Bundle is 1.08 MB (309 kB gzipped) — Monaco is the bulk. Lazy-load Monaco via `React.lazy`.                                         | 🟠       | ⬜     |            |
| E2  | Root `pnpm-lock.yaml` doesn't contain `react-router-dom`, `@monaco-editor/react`, `@dnd-kit/*` (they were installed via `npm` in `web/` causing `web/package-lock.json` and `web/pnpm-lock.yaml` to appear). Workspace builds in CI will fail. **Partial fix**: stray subpackage lockfiles deleted + `.gitignore` updated to prevent recurrence. **Still TODO**: run `pnpm install -r --no-frozen-lockfile` from the repo root on a workstation that has `GITEA_NPM_TOKEN` exported (this session can't reach the private registry → mobile install fails). One-line follow-up commit. | 🔴       | ✅     | 3c9117f |
| E3  | Monaco's web workers (TS/JSON/CSS/HTML) are pulled at runtime from a CDN by default. Need explicit Vite config to bundle workers locally for offline / CSP-strict deployments. | 🟡       | ⬜     |            |
| E4  | No README / docs section describing the new layout, env vars (`FMP_API_KEY`), or routes.                                            | 🟡       | ⬜     |            |

## F. Test coverage

| #   | Issue                                                                                                                              | Severity | Status | Fix commit |
| --- | ---------------------------------------------------------------------------------------------------------------------------------- | :------: | :----: | ---------- |
| F1  | 0 tests for `web/src/lib/marketApi.ts`.                                                                                            | 🟡       | ⬜     |            |
| F2  | 0 tests for `VisualRuleBuilder.tsx`.                                                                                                | 🟡       | ⬜     |            |
| F3  | 0 tests for `CodeStrategyEditor.tsx`.                                                                                               | 🟡       | ⬜     |            |
| F4  | 0 tests for `HomeView.tsx` (StockChart fetch path, ResearchCards).                                                                  | 🟡       | ⬜     |            |
| F5  | 0 tests for `ScreenerView.tsx` (filter/sort/sector logic).                                                                          | 🟡       | ⬜     |            |
| F6  | 0 tests for the 7 new backend endpoints (`/api/chart/bars`, `/api/news`, `/api/market/indices`, `/api/research/*`, `/api/screener`). | 🟠       | ✅     | e2806b2    |
| F7  | Pre-existing: 3 `useTabFeatureFlags.dom.test.tsx` failures from module-level cache leaking between tests.                          | 🟡       | ✅     | ece7fa9    |
| F8  | Pre-existing: 1 `EntryForm.dom.test.tsx` failure ("alerts error when trade execution fails").                                       | 🟡       | ✅     | ece7fa9    |

## G. Mobile parity gap

| #   | Issue                                                                                                                              | Severity | Status | Fix commit |
| --- | ---------------------------------------------------------------------------------------------------------------------------------- | :------: | :----: | ---------- |
| G1  | Mobile app (`mobile/app/(tabs)/`) has 5 tabs: index, positions, history, strategies, settings. None of the new web features (news, research, screener) exist on mobile. | 🟠       | ⬜     |            |
| G2  | Mobile `TradingDataProvider` doesn't fetch news / indices / company profile. It only consumes the WebSocket bot state.              | 🟠       | ⬜     |            |
| G3  | No mobile counterpart to `VisualRuleBuilder` or `CodeStrategyEditor`. (Code editor likely shouldn't ship to mobile, but visual builder should.) | 🟡       | ⬜     |            |
| G4  | Mobile `lib/runtime.ts` doesn't expose the new endpoints — no shared client.                                                       | 🟡       | ⬜     |            |

---

## Ordering of fixes

The plan: walk top-to-bottom, one bucket per commit, smallest blast radius first.

1. **Section A** (critical broken integrations) — unblocks Strategy Builder + News + Backtest in one go.
2. **Section E2** (lockfile mess) — unblocks CI; do this before anyone else pulls.
3. **Section B** (functional gaps) — split into sub-commits per gap to keep diffs reviewable.
4. **Section C** (security & correctness).
5. **Section D** (UX polish).
6. **Section F** (tests) — added alongside each fix where possible, sweep at the end.
7. **Section G** (mobile parity) — separate commit set; biggest scope, least urgent.

Each fix lands in its own commit with the issue ID in the subject (e.g. `fix(A1): point CodeStrategyEditor at /api/backtest/run`). Ticking the table as we go.