learning_ai_invt_trdg/docs/AUDIT_REDESIGN.md
Saravana Achu Mac 9e22f6460c docs(C3): mark screener sector validation complete
Record the implementation commit that allow-lists /api/screener sector filters.

Refs: docs/AUDIT_REDESIGN.md item C3.

Co-Authored-By: GPT-5 Codex <noreply@openai.com>
2026-05-04 16:44:45 -07:00

13 KiB

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. 🟡
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. 🟡 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.