Saravana Achu Mac
3c9117f229
Regenerate the root pnpm lockfile from the web workspace so CI can resolve the redesigned dashboard dependencies without relying on stray subpackage locks. The full recursive install path is still blocked locally by the mobile private-registry tarball route, so this keeps the E2 scope focused on the web importer entries and their package snapshots. Refs: docs/AUDIT_REDESIGN.md item E2. Co-Authored-By: GPT-5 Codex <noreply@openai.com>
112 lines
13 KiB
Markdown
112 lines
13 KiB
Markdown
# 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. | 🟠 | ⬜ | |
|
|
| 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. **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. | 🔴 | ✅ | 9196459 |
|
|
| 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.
|