docs: add post-redesign systematic audit (52 items across 7 buckets)
Catalogues every gap, bug, and miss found in the web redesign work: - A: critical broken integrations (wrong endpoint, wrong payload, missing auth) - B: functional gaps from the original plan (chart indicators, watchlist buttons, company name placeholder, etc.) - C: security & correctness (sandboxing, FMP cache, query-param leakage) - D: UX/UI polish (skeletons, dark-tab contrast, responsive breakpoints) - E: build/infra (1 MB bundle, lockfile drift, Monaco workers, README) - F: test coverage (zero tests for marketApi, builders, screener, endpoints) - G: mobile parity (none of the new features exist on mobile) Each row has a severity tag, status box, and a slot for the fix-commit hash. Subsequent commits will reference items by ID (e.g. fix(A1): ...). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
938ed86044
commit
4a09d4ba26
111
docs/AUDIT_REDESIGN.md
Normal file
111
docs/AUDIT_REDESIGN.md
Normal file
@ -0,0 +1,111 @@
|
||||
# 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. | 🔴 | ⬜ | |
|
||||
| A2 | `CodeStrategyEditor` payload sends `{symbol, strategyCode, mode}`. Backend `/api/backtest/run` requires `{symbols[], strategyConfig}`. | 🔴 | ⬜ | |
|
||||
| A3 | `VisualRuleBuilder` save → `/api/profiles` body uses `{strategyType, visualRules, description}`. `saveTradeProfileForUser` expects `strategy_config` shape. Result: 400 or silently-discarded fields. | 🔴 | ⬜ | |
|
||||
| A4 | `RightPanel.NewsFeed` calls `fetch()` with no `Authorization` header. `/api/news` is `requireAuth`. Result: 401 every render. | 🔴 | ⬜ | |
|
||||
| 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. | 🟠 | ⬜ | |
|
||||
| A6 | Backend `/api/chart/bars` previously crashed on crypto symbols (`BTC/USD`) because `/v2/stocks` rejects them. (Already partially fixed in 938ed86 — verify the encode path doesn't double-encode `/`.) | 🟠 | ⬜ | |
|
||||
|
||||
## 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. | 🟠 | ⬜ | |
|
||||
| B2 | `TickerHeader` "★ Watchlist" and 🔔 buttons have no `onClick` — purely cosmetic. | 🟠 | ⬜ | |
|
||||
| B3 | `TickerHeader` company name is the symbol (placeholder). Should pull from `fetchResearchProfile`. | 🟠 | ⬜ | |
|
||||
| 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. | 🟠 | ⬜ | |
|
||||
| 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. | 🔴 | ⬜ | |
|
||||
| C2 | No FMP response cache. Free tier = 250 req/day. Every Home view load = 3 req. 80 page loads/day → quota burnt by lunch. | 🟠 | ⬜ | |
|
||||
| C3 | `/api/screener` passes `sector` query through to FMP without an allow-list. Low-impact injection, but should validate. | 🟡 | ⬜ | |
|
||||
| C4 | `/api/news` passes `symbols` through to Alpaca without validation. | 🟡 | ⬜ | |
|
||||
| C5 | Header `fetchMarketIndices` polls every 60 s even when the tab is hidden. Should pause via `document.visibilityState`. | 🟡 | ⬜ | |
|
||||
| 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. | 🟡 | ⬜ | |
|
||||
| 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). | 🟡 | ⬜ | |
|
||||
|
||||
## 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. | 🟡 | ⬜ | |
|
||||
| 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. | 🟠 | ⬜ | |
|
||||
| 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. | 🔴 | ⬜ | |
|
||||
| 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`). | 🟠 | ⬜ | |
|
||||
| F7 | Pre-existing: 3 `useTabFeatureFlags.dom.test.tsx` failures from module-level cache leaking between tests. | 🟡 | ⬜ | |
|
||||
| F8 | Pre-existing: 1 `EntryForm.dom.test.tsx` failure ("alerts error when trade execution fails"). | 🟡 | ⬜ | |
|
||||
|
||||
## 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.
|
||||
Loading…
Reference in New Issue
Block a user