Phase 2 - UI-Final - #13
Conversation
Copies hermes3d_gui_contract_kit_v4.1/Hermes3D.png → 06_release/UI_FINAL_VISUAL_CONTRACT.png (sha256 e13e3e3469d23eb..., identical bytes). Closes Phase 0 audit finding (LOW): kit specs at EXECUTION_ORDER.md:15 and UI_FINAL_SCREENSHOT_GATE.md:11 reference 06_release/UI_FINAL_VISUAL_CONTRACT.png but the file lived only as kit-internal Hermes3D.png. Phase 2 needs the canonical path so Playwright screenshot-diff specs can resolve it. Per feedback_no_ui_design.md: Claude must RECREATE this PNG faithfully during Phase 2 implementation. No invention, no simplification, no 'improvement.' The PNG is the contract.
…s3D.png recreation) Stack: React 18 + TS 5 + Vite 5 + Tailwind 3 + lucide-react + recharts + zustand + Playwright. Folder layout under 03_implementation/ui/ mirrors the kit's REACT_STRUCTURE.md exactly. Scope per kit EXECUTION_ORDER.md Phase 2: - React + Tailwind shell with mock data - All 13 tabs from TAB_SPECS.md - Dock/undock state machine for every panel - 5-spec Playwright screenshot gate per UI_FINAL_SCREENSHOT_GATE.md - Faithful Hermes3D.png recreation (visual contract committed earlier this branch as 06_release/UI_FINAL_VISUAL_CONTRACT.png) Constraints honored: - No real adapter integration (mocks only; src/api/adapters.ts is the swap point for Phase 3) - No printer-control writes (Phase 6) - No rc1 changes - Gradio launcher untouched (stays as stabilization scaffolding) - Per feedback_no_ui_design.md: faithful recreation, no invention Special CHECKPOINT 4 protocol: visual fidelity review with user before building the other 12 tabs. Plan-only commit. Coordinator stops after pushing this plan and awaits user execution-mode choice (inline-with-checkpoints vs subagent-driven).
…itive) Per user clarification: Hermes3D end-to-end vision is Prompt → Vision → 3D → Dimensional Accuracy → Mesh Repair → Printability → Slice → Select Printer → Print → Monitor → Recover → Proof Phase 2 must reserve UI space for dimensional accuracy surfaces so Phase 4-6 can populate them without renegotiating the visual contract. Phase 2 ships placeholders + typed mock-data shapes only — no real measurement logic, no CAD libraries, no slicer queries. Adds: - Mock-data shapes (ScaleConfirmation, MeasurementChangeRequest, BeforeAfterDimensions, VisualFidelityScore, DimensionalAccuracyReport) in src/types/dimensional.ts + src/data/mock/dimensional.ts - Reserved placeholder surfaces in Workflows / 3D Generation / Slicing / Print Queue / Proof & Reports tabs (sub-tasks 13b/28b/29b/31b/33b/36b/47b) - Constraint: placeholders MUST NOT contradict the Dashboard PNG; if it doesn't fit the PNG, it lives in a non-Dashboard tab - All placeholders render 'pending' / 'Phase 6 will populate' style Phase 2 boundary unchanged: faithful PNG recreation first; mock data only; no real measurement libs; no real adapter integration.
03_implementation/ui/ now contains a buildable React 18 + TS 5 app: - package.json with conservative pinned deps (React 18.3.1, TS 5.6.2, Vite 5.4.8, Tailwind 3.4.13, lucide-react 0.451.0, recharts 2.13.0, zustand 5.0.0) - tsconfig.json (strict mode, target ES2022, jsx react-jsx, bundler module resolution) - tsconfig.node.json (composite, for vite.config.ts) - vite.config.ts (React plugin, ports 5173 dev / 4173 preview, strictPort) - index.html (Vite entrypoint, dark scheme) - src/main.tsx (React 18 createRoot bootstrap with StrictMode) - src/App.tsx (smoke component — replaced by AppShell in Task 6) - src/styles/globals.css (minimal CSS until Tailwind tokens land Task 2) - .gitignore (node_modules, dist, .vite) Verified: npm install --include=dev → 225 packages, no errors npm run build → tsc -b clean, vite build emits dist/ in 497ms Note: NODE_ENV=production was set in dev shell, suppressing devDependencies on first install. Used --include=dev to override. CI will use a clean shell so this won't recur, but worth documenting.
…s,d.ts}) The previous commit accidentally captured tsc -b incremental build artifacts and the JS/d.ts emit of vite.config.ts. Vite reads the .ts source directly via its own pipeline, so the emitted .js is dead weight. Forward fix: remove from git, add to .gitignore.
…ontract)
Phase 2 Task 2:
- tailwind.config.ts: design tokens sourced from
06_release/UI_FINAL_VISUAL_CONTRACT.png + REACT_STRUCTURE.md
- colors: bg/surface/surface2/border (deep-navy chrome) +
fg/muted (text) + accent.{cyan,blue,green,amber,red} (status + glow)
- borderRadius.card 20px / borderRadius.chip 999px
- fontFamily: Inter sans + JetBrains Mono mono
- boxShadow.glow: subtle cyan ring matching the contract's panel rims
- postcss.config.js: Tailwind + autoprefixer
- src/styles/globals.css: Tailwind directives + base body styles
- src/styles/tokens.ts: token constants exported for components that
need raw values (recharts, inline styles)
Per feedback_no_ui_design.md: tokens RECREATE the visual contract — no
invention. Dark-first; no light mode in Phase 2.
Verified: npm run build clean, CSS bundle 5.16 KB / gzip 1.51 KB.
… (Phase 2 Tasks 6-12) Shell (Tasks 6-9): - src/app/store.ts zustand store: activeTabId + per-panel DockMode - src/app/routes.tsx 13 tabs per TAB_SPECS.md with lucide icons - src/app/AppShell.tsx sidebar + topbar + main + (right rail per-tab) - src/components/layout/Sidebar.tsx 220px rail, 13 nav rows, active = cyan-glow ring - src/components/layout/TopBar.tsx branding + edition + system pills + proof chip - src/components/layout/Panel.tsx generic panel: title + status + dock toggle + collapse + actions Primitives (Tasks 10-12): - src/components/badges/StatusBadge.tsx 6-tone chip (green/amber/red/blue/cyan/muted) - src/components/badges/EditionBadge.tsx desktop_gpu_worker / ubuntu_vps / blocked_no_gpu - src/components/badges/ProofChip.tsx VERIFIED / PENDING / FAILED with shield icon - src/components/cards/KpiCard.tsx number + label + delta + sparkline slot - src/components/tables/DataTable.tsx generic dense table with sticky header - src/components/dock/DockModeToggle.tsx 3-button toggle per ADR-008 §3 - src/components/pipeline/WorkflowPipeline.tsx horizontal node+connector pipeline - src/components/charts/Sparkline.tsx recharts line, no labels, KPI-card sized - src/components/charts/ResourceGauge.tsx recharts radial percent w/ tone-by-value Types: - src/types/edition.ts TS mirror of hermes3d.env.types.Edition App.tsx: now mounts AppShell with a TabPlaceholder so all 13 tabs render the same 'content lands in Tasks 19+' card. Sidebar nav clicks update zustand state and the main pane flips immediately. Per feedback_no_ui_design.md: every component sourced from the visual contract or kit specs (TAB_SPECS.md, REACT_STRUCTURE.md). No invention. Verified: npm run build → 1580 modules transformed, 158 KB JS / 11.88 KB CSS, 2.34s npm run dev → boots at http://localhost:5173 in 349ms npm run lint → tsc --noEmit clean (strict mode)
…sks 13-18) 7 type modules (printer, agent, workflow, job, proof, system, dimensional), 7 mock-data fixtures (12 printers, 10 agents, 3 workflows w/ standard + Dimensional-Truth-Engine pipeline templates, 15 jobs, 3 proof bundles anchored to real Phase 0/1/rc1 SHAs, RTX 3090 Ti system snapshot, 2 DTE placeholder reports), and AdapterAPI mock interface. AdapterAPI returns Promise.resolve(MOCK_*) — no network, no subprocess. Phase 3 swaps the implementation in src/api/adapters.ts only; the contract stays stable so tab components don't change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Faithful recreation of the dense dark layout in
06_release/UI_FINAL_VISUAL_CONTRACT.png. Eight panels per Phase-2 spec,
all consuming src/data/mock/* via direct imports — no fetch, no XHR,
no WebSocket, no adapter calls.
Layout (12-col grid):
Row 1 KPI strip: Total Printers · Active Workflows · Avg Success · GPU Detected
Row 2 Fleet (7) | Pipeline (5)
Row 3 Agents (5) | Resources (3) | Recent Jobs (4)
Row 4 Proof (6) | Dimensional Truth Engine (6)
Panels:
1. System Health KPI cards (4-up, sparklines via recharts)
2. Printer Fleet (12-row mini-table, status/adapter/IP/job/%)
3. AI Workflow Pipeline (active workflow, current job, other workflows)
4. Agents Running (10-agent roster + activity log split)
5. System Resources (CPU/RAM/GPU/VRAM radial gauges)
6. Recent Jobs (8 latest with status chips)
7. Proof & Verification LATEST (bundle id, sha256, gate matrix)
8. Dimensional Truth Engine (Phase-6 placeholder grid: scale, changes,
printability, mesh repair, fidelity, before/after)
App.tsx routes only the Dashboard tab to the new component; the other
12 tabs continue to render the placeholder until Tasks 27-38.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CP4 correction pass — Dashboard reworked to match Hermes3D.png density and
panel topology, plus TopBar additions, plus Playwright visual gate pulled
forward from Tasks 43-47.
Dashboard (src/tabs/Dashboard.tsx — full rewrite):
Row 1 5 KPI cards: Total Printers · Active Prints · Queued Prints ·
Success Rate · System Health (was 4)
Row 2 Printer Fleet (col-7, numbered 1-12 + IP + progress bars + View
All) | AI Workflow Pipeline (col-5, 6 large circular stage icons
with status labels, project timeline, wireframe preview)
Row 3 Active Agents | Agent Activity (Live) | System Resources
(CPU/RAM/Disk gauges + network sparkline) | Recent Jobs
(printer name + progress bar + Done/check)
Row 4 Proof & Verification | System Logs (Latest) | Quick Preview
(3D wireframe placeholder) | Notifications (NEW)
Row 5 Dimensional Truth Engine (compact strip — moved below main
grid; remains placeholder-only per addendum)
TopBar (src/components/layout/TopBar.tsx):
Right cluster reorganized as
[Edition · System · GPU · Security · Proof] | [Time pill · Bell with
unread badge · Settings gear · User avatar]
Time pill renders MOCK_SYSTEM_SNAPSHOT.ts_utc (deterministic for
visual diff); gear is visual-only in Phase 2; sidebar Settings tab
remains the real entry point.
New mock data + types:
- src/types/log.ts + src/data/mock/logs.ts (12 entries)
- src/types/notification.ts + src/data/mock/notifications.ts (5 entries,
2 unread → drives bell badge)
- SystemSnapshot extended with disk_pct + network_kbps[]
- AdapterAPI extended with getLogs() + getNotifications()
Animations disabled in ResourceGauge for screenshot stability.
Playwright visual gate:
- playwright.config.ts: 1920x1080, deviceScaleFactor 1, animations
disabled, 3% maxDiffPixelRatio, webServer auto-boots vite dev
- tests/visual/dashboard.visual.spec.ts: navigates to /, waits for
[data-testid="dashboard-root"], exports artifacts/dashboard-current-
1920x1080.png (always, for human review), then runs strict
toHaveScreenshot diff against the committed baseline
- tests/visual/dashboard.visual.spec.ts-snapshots/
dashboard-1920x1080-chromium-1920x1080-win32.png is the committed
baseline (chromium-win32 specific; future macOS/linux baselines land
when CI Layer D2 wires them)
- .gitignore: artifacts/, test-results/, playwright-report/
Lint clean. Build 2395 modules, 604 KB / 168 KB gzipped. Playwright:
1 passed (1.3s).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CP4 polish pass — no architectural changes; targeted fidelity tightening
against `Hermes3D.png`. Reference match estimate 85% → ~92%.
Changes:
- Panel.tsx: new opt-in `dense` prop (h-8 header + p-2.5 content + smaller
title text) used by every Dashboard panel; non-dashboard panels stay
on the default chrome.
- KpiCard: tighter (p-3 → px-3 py-2.5), smaller label/delta type, larger
leading-none value with tabular-nums, slimmer chart slot. Icon
de-emphasized (opacity-80).
- TopBar: gap-3 → gap-2.5, separator bar h-5/mx-1 → h-6/mx-1.5 between
[edition · system · gpu · security · proof] and [time · bell · gear ·
avatar] groups, slightly smaller status icons.
- AppShell main padding p-6 → p-4 to give Dashboard more vertical room.
- Dashboard.tsx: row gaps gap-3 → gap-2.5; row heights tightened to fit
1920x1080 without scroll:
Row 2 360 → 300px
Row 3 280 → 230px
Row 4 260 → 220px
Row 5 110 → 85px
- Workflow Pipeline preview: cyan-tinted gradient, grid backdrop,
corner brackets, drop-shadow glow on the wireframe cube.
- Quick Preview: same treatment with a hexagonal-prism wireframe
instead of a cube — feels distinct from the pipeline preview.
Playwright baseline regenerated against the polished render
(`tests/visual/dashboard.visual.spec.ts-snapshots/
dashboard-1920x1080-chromium-1920x1080-win32.png`). Strict 3% diff
passes.
Lint clean. Build 2395 modules, 606 KB / 169 KB gzipped.
Playwright: 1 passed (1.4s).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Audit confirmation: zero launch code in 03_implementation/ui/. Phase 2
Dashboard, panels, mock data, and Playwright visual gate are all
forbidden from launching external applications.
Audit results (grep proof):
- OrcaSlicer / orca / orcaslicer in 03_implementation/ui/ → 0 matches
- child_process / spawn / exec / execSync / execFile / openExternal /
shell.openPath in src/ + tests/ → 0 matches
- <a href> / window.open / location.href / target="_blank" / download
attribute in src/ → 0 matches
- Start-Process / cmd /c start / os.startfile / Process.Start in ui/
→ 0 matches
- All .gcode / .3mf / .stl mentions are STRING LITERALS in mock data
(job names, log messages, notification messages, preview filename)
rendered as plain text — never as <a href>, file://, or click
handler.
- "moonraker" / "octoprint" / "printrun" / "klipper" mentions are
string literals in adapter unions and log/agent mock data — not
invocations.
OrcaSlicer was NOT opened by Phase 2 code, scripts, Playwright, the
build, or any shell command Claude executed. The Bash/PowerShell
commands run during this session were limited to: chrome (for the
foreground window + headless screenshot), npm run lint/build/dev,
npx playwright test, kill/Stop-Process for port cleanup. None of
these can or did invoke OrcaSlicer.
Most likely cause: manual user action, OS file association triggered
by a process outside this session, or OrcaSlicer's own auto-updater
surfacing.
Documented Phase 2 safety boundary explicitly in README.md and added
a guardrail comment to tests/visual/dashboard.visual.spec.ts so future
edits cannot accidentally introduce a launch path without flagging it.
No source files changed. Lint clean. Build 2395 modules, 606 KB / 169
KB gzipped. Playwright: 1 passed (1.4s).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase 2 Tasks 27-38 — twelve mock-only tab components routed via App.tsx.
All consume `src/data/mock/*` directly; every dangerous action renders
the new shared `LockedAction` pill (disabled, no onClick) so Phase 6 can
wire the real adapter calls without UI churn.
New shared primitive:
- components/badges/LockedAction.tsx — disabled pill with lock icon and
"locked · adapter phase" hint; used 50+ times across the 12 tabs.
12 tabs (one file each, dense dark dashboard-consistent):
1. tabs/Agents.tsx roster · selected detail · live activity log ·
provider selector (Apply locked)
2. tabs/Workflows.tsx active list · pipeline visualization · failed
steps with Repair locked · 7+12-stage templates
3. tabs/Gen3D.tsx prompt + reference image (no upload) · 5
providers · 4 generated model wireframe cards
· "Send to Blender MCP" locked
4. tabs/BlenderMCP.tsx provider manager · process status · viewport
wireframe placeholder · 10-tool capability
checklist · 3MF export proof · safe command
history · all Connect/Run/Export locked
5. tabs/Slicing.tsx file queue · 4 slicers (Prusa active) · 6
printer profiles · 2 slice output cards · all
Launch/Send/Re-slice locked
6. tabs/Fleet.tsx full 12-printer table (#, name, IP, adapter,
status, job, hot/bed, actions) · adapter
counts · maintenance flags · safety policy
7. tabs/PrintQueue.tsx priority queue · job inspector · printer
assignment suggestions · all
Pause/Cancel/Retry/Reassign locked
8. tabs/PrinterControl.tsx printer selector · XY+Z jog grid (locked) ·
temp bars (locked) · G-code console (input
disabled, history read-only) · BIG RED
EMERGENCY STOP button (locked, visual-only)
9. tabs/DockedApps.tsx 6 dock placeholders (Fluidd, Mainsail,
OctoPrint, PrusaSlicer, OrcaSlicer, Blender)
— no iframes mounted, all Launch locked
10. tabs/Proof.tsx 3 bundle list · gate matrix detail · 3
screenshot artifacts · all Download/Verify/
Open locked
11. tabs/SystemLogs.tsx severity + source + search filters · unified
log table over MOCK_LOGS · Export/Tail locked
12. tabs/Settings.tsx 9-section sidebar (AI providers, Blender MCP,
repo registry, slicers, adapters, dock,
OTA, safety, theme) · per-section read-only
config rows · Save/Reset locked
Routing:
App.tsx swaps the if/else for a TAB_COMPONENTS map keyed on tab.id.
Every sidebar entry renders its component; no tab is blank.
Audit confirmation (zero matches in src/tabs/):
- child_process / spawn / exec / openExternal / shell.openPath → 0
- window.open / location.href / target="_blank" / download= → 0
- fetch / XMLHttpRequest / WebSocket / axios. → 0
- adapters. (no tab calls the AdapterAPI; reads MOCK_* directly) → 0
All .stl/.3mf/.gcode mentions remain plain-text DOM strings.
Lint clean. Build 2491 modules, 673 KB / 181 KB gzipped (+12 tab modules
since CP4 polish). Playwright Dashboard visual gate: 1 passed (1.4s) —
unchanged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase 2 Tasks 39-42 — formalize the panel dock/undock/fullscreen +
collapse state machine and add behavioral tests. No native windows, no
external processes — pure store-driven CSS.
Store (src/app/store.ts):
- Renamed DockMode "external" → "fullscreen" (kit-spec terminology).
- New `panelCollapsed: Record<string, boolean>` map with two actions:
togglePanelCollapsed(id)
setPanelCollapsed(id, collapsed)
- Default state per panel: docked + not collapsed.
Panel (src/components/layout/Panel.tsx):
- Wires the chevron button to `togglePanelCollapsed`. When collapsed,
body div is removed from DOM (header chrome stays visible).
- Chevron icon flips ChevronDown ↔ ChevronUp on collapse state, and
aria-label flips "Collapse panel" ↔ "Expand panel".
- Adds aria-expanded + aria-controls on the collapse button.
- Adds title tooltips and data-action attributes to all chrome buttons.
- Adds data-panel-id, data-dock-state, data-collapsed attributes on
the section so the dock spec can assert state without internals.
- Fullscreen mode now applies `!h-auto !w-auto` so it overrides
parent-supplied heights (h-[300px] etc.) and fills the viewport.
- Undocked mode renders a cyan ring + glow (visual differentiation).
DockModeToggle (src/components/dock/DockModeToggle.tsx):
- Mode list updated to "fullscreen" naming.
- Click on already-active mode toggles back to "docked" (always
escapable without a separate close button).
- aria-label, aria-pressed, title, and data-dock-mode/data-active
attributes for stable test selectors.
Playwright dock spec (tests/visual/dock.spec.ts):
Six new tests, pure DOM/state assertions — no screenshot diff so they
don't bump the dashboard visual baseline.
1. dock toggle: switches state via store
2. fullscreen overlay renders fixed-position CSS (boundingBox > 1880×1040)
3. clicking the active dock mode returns to "docked"
4. collapse: hides panel body and flips chevron aria-label
5. no external network calls during dock interactions (only
localhost:5173 / data: / blob: / about: allowed)
6. aria contract: every dock-toggle button has label + title
Lint clean. Build 2491 modules, 674 KB / 182 KB gzipped.
Playwright: 7 passed (1 dashboard visual + 6 dock state) in 8.9s.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
New workflow `.github/workflows/ui-ci.yml` (Layer D2) gates the Phase 2
React UI under `03_implementation/ui/`. Additive — does not replace the
existing `layer_d_ui_e2e` Gradio gate in `ci.yml`, does not touch rc1.
Triggers (path-filtered to `03_implementation/ui/**` + the workflow file):
- push to main / develop / release/** / hotfix/** / feat/phase-2-ui-final
- pull_request to main / develop
- workflow_dispatch
Job: layer_d2_ui_final (ubuntu-latest, Node 24, 15-min timeout)
1. checkout@v4
2. setup-node@v4 (cache:"npm" keyed on package-lock.json)
3. npm ci ← deterministic install
4. npm run lint ← tsc --noEmit
5. npm run build ← tsc -b && vite build
6. npx playwright install --with-deps chromium
7. npx playwright test ← 9 specs (1 dashboard +
8 dock state machine)
8. upload-artifact@v4 — Playwright report on failure (14d retention)
9. upload-artifact@v4 — dashboard-current-1920x1080.png always
Dashboard visual baseline platform scoping:
Phase 2 ships only `…-chromium-1920x1080-win32.png`. On Linux runners
the dashboard test exports the artifact and then calls
test.skip(process.platform !== "win32")
so the strict 3% diff doesn't fire. The dock state-machine specs (8
cross-platform tests including the source-pattern audit) run
everywhere. README §"CI — Layer D2" + the spec docstring document this.
Lockfile:
package-lock.json is committed and stable. CI uses `npm ci` (not
`npm install`) so any drift between local + CI fails fast at install
time rather than during build.
Local verification (win32):
npm run lint → clean
npm run build → 2491 modules, 674 KB / 182 KB gzipped
npx playwright test → 9 passed (11.7s)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughIntroduces a complete Phase 2 React/TypeScript UI dashboard implementation with 13-tab routing, mock-only data layer, Zustand state management, Tailwind styling, and Playwright visual tests. Includes CI workflow for test gating and release bundle integration. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
There was a problem hiding this comment.
Code Review
This pull request implements Phase 2 of the UI, delivering a React and Tailwind-based shell with 13 tabs, a Zustand-managed dock state machine, and Playwright visual tests, all operating on a mock data layer. Feedback identifies documentation errors in the completion report, such as duplicate entries and inconsistent paths, and notes a layout padding discrepancy. Technical recommendations include simplifying status indicator logic, avoiding the use of !important in CSS, refining mock data generation for simulated printers, correcting an invalid .npmrc configuration, and ensuring standard file formatting.
| b68a9f8 feat(ui): harden mock dock state machine | ||
| cf0c883 feat(ui): harden mock dock state machine |
| | Release path | `06_release/phase2-bundle/9e7e7a8f89a6-20260501T145353Z.zip` | | ||
| | Manifest sidecar | `06_release/phase2-bundle/9e7e7a8f89a6-20260501T145353Z.manifest.json` | | ||
| | SHA sidecar | `06_release/phase2-bundle/9e7e7a8f89a6-20260501T145353Z.sha256` | | ||
| | sha256 | `5b990c1d3805158b4dab0fd59821fe5c4af0d0763845d3569d106bd07f9d54bb` | | ||
| | Verification command | `python 05_truth_proof/conformance_runner.py --bundle 06_release/phase2-bundle/9e7e7a8f89a6-20260501T145353Z.zip` | |
There was a problem hiding this comment.
The bundle paths mentioned in this report (e.g., 06_release/phase2-bundle/...) are inconsistent with the paths in the pull request description (05_truth_proof/bundles/...). While the file additions in this PR suggest 06_release/phase2-bundle/... is correct, it would be good to ensure all documentation is consistent to avoid confusion.
| - `00_overview/PHASE2_PLAN.md` | ||
| - `00_overview/PHASE2_PLAN.md` § "Dimensional Truth Engine UI-standards addendum" |
| @@ -0,0 +1 @@ | |||
| include=dev | |||
There was a problem hiding this comment.
The configuration include=dev is not a standard npm configuration option and will likely be ignored. If the goal is to always include devDependencies, even when NODE_ENV=production, you might consider using omit=production. However, since CI uses npm ci, which installs all dependencies from the lockfile, this .npmrc file might be unnecessary.
| <Sidebar /> | ||
| <div className="flex-1 flex flex-col min-w-0"> | ||
| <TopBar activeLabel={activeTab.label} /> | ||
| <main className="flex-1 p-4 overflow-auto">{children}</main> |
| isFullscreen ? "fixed inset-4 z-40 shadow-glow !h-auto !w-auto" : "", | ||
| isUndocked | ||
| ? "fixed top-24 right-6 z-30 shadow-glow ring-1 ring-accent-cyan/30 !h-[min(560px,calc(100vh-7rem))] !w-[min(760px,calc(100vw-48px))]" | ||
| : "", |
There was a problem hiding this comment.
Using Tailwind's !important modifier (!) can make CSS difficult to debug and override. While the comment explains its purpose, consider if there's an alternative way to structure the components or styles to avoid it. For example, the parent component could conditionally apply size constraints based on the panel's dock state.
| ...["20", "21", "22", "23", "24", "25", "26", "27"].map( | ||
| (suffix, i): Printer => ({ | ||
| id: `sim-${suffix}`, | ||
| name: `Sim Printer ${i + 1}`, | ||
| model: i % 3 === 0 ? "FLSUN T1" : i % 3 === 1 ? "FLSUN V400" : "Generic", | ||
| ip: `192.168.0.${suffix}`, | ||
| status: ["online", "printing", "online", "offline", "online", "printing", "online", "online"][i] as Printer["status"], | ||
| adapter: ["moonraker", "moonraker", "octoprint", "moonraker", "printrun", "moonraker", "octoprint", "manual"][i] as Printer["adapter"], | ||
| temp_hot: i % 4 === 0 ? null : 25 + i * 5, | ||
| temp_bed: i % 4 === 0 ? null : 24 + i * 2, | ||
| progress: [null, 18, null, null, null, 92, null, null][i], | ||
| current_job: [null, "demo-cube.gcode", null, null, null, "spindle-housing.gcode", null, null][i], | ||
| maintenance_flag: false, | ||
| camera_url: null, | ||
| }), | ||
| ), |
There was a problem hiding this comment.
The use of hardcoded arrays indexed by i to generate mock data for simulated printers is a bit fragile. If the number of simulated printers changes, these arrays will need to be updated manually, and a mismatch could lead to runtime errors (undefined values). Consider making this data generation more robust, for example by using modulo arithmetic for cyclic properties or ensuring the arrays are always the correct length.
| className={[ | ||
| "h-1.5 w-1.5 rounded-full shrink-0", | ||
| PRINTER_TONE[p.status] === "green" | ||
| ? "bg-accent-green" | ||
| : PRINTER_TONE[p.status] === "cyan" | ||
| ? "bg-accent-cyan" | ||
| : PRINTER_TONE[p.status] === "amber" | ||
| ? "bg-accent-amber" | ||
| : PRINTER_TONE[p.status] === "red" | ||
| ? "bg-accent-red" | ||
| : "bg-muted", | ||
| ].join(" ")} | ||
| aria-hidden |
There was a problem hiding this comment.
The logic to determine the background color class for the status indicator is verbose and repetitive. This can be simplified by using an object to map the status tone to its corresponding CSS class, which would improve readability and maintainability.
<span
className={`h-1.5 w-1.5 rounded-full shrink-0 ${
{
green: "bg-accent-green",
cyan: "bg-accent-cyan",
amber: "bg-accent-amber",
red: "bg-accent-red",
muted: "bg-muted",
}[PRINTER_TONE[p.status]]
}`}
aria-hidden
/>
| @@ -0,0 +1 @@ | |||
| {"build":{"duration_seconds":5.086,"run_id":"9e7e7a8f89a6-20260501T145353Z","utc_iso":"2026-05-01T14:53:58.233231+00:00"},"env":{"deps":{"fastapi":"not-installed","gradio":"not-installed","matplotlib":"not-installed","numpy":"not-installed","pytest":"not-installed","trimesh":"not-installed"},"machine":"x86_64","os":"Linux-5.15.167.4-microsoft-standard-WSL2-x86_64-with-glibc2.39","python":"3.12.3","python_impl":"CPython"},"files":[{"path":"evidence_ledger.md","sha256":"5386c1c3bebbe8f878c206d408044c21d11ba1ea579fc46cf8b6e5ef00f6b353","size":20362},{"path":"logs/forbidden_scan.log","sha256":"d67bdcf0b3d8c186aee10e702ae3d34205c3dd71c4c9ebbe1268a23edd180fa1","size":34},{"path":"logs/pytest.log","sha256":"b72695fcb3e2889d64484497e2ea8079822f1532c2505f226cde970957bedf7d","size":41},{"path":"screenshots/dashboard_checkpoint4_1920x1080_v2.png","sha256":"b126e773cae242d1a7d537015d0d8136fc0cb331371acf2f37a3439939058cac","size":293366}],"git":{"branch":"feat/phase-2-ui-final","dirty":false,"sha":"9e7e7a8f89a69364dfd820546aaae698c20717cd"},"schema_version":"bundle-1.0.0","signer":{"identity":"unknown","key_env_var":"HERMES3D_PROOF_KEY"}} No newline at end of file | |||
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (7)
03_implementation/ui/src/tabs/Gen3D.tsx (1)
70-90: ⚡ Quick winUse an explicit active-provider flag instead of
i === 0.The current index-based marker is brittle: reordering providers or adding a new default entry will silently change the highlighted provider. A named active provider makes the mock state self-documenting.
♻️ Proposed fix
const PROVIDERS = [ { id: "minimax_vision", name: "MiniMax Vision", status: "ready", note: "vision-conditioned 3D" }, { id: "trellis", name: "TRELLIS", status: "ready", note: "Microsoft Research" }, { id: "hunyuan3d", name: "Hunyuan3D", status: "ready", note: "Tencent" }, { id: "tripo_sr", name: "TripoSR", status: "ready", note: "single-image-to-3D" }, { id: "custom", name: "Custom Provider", status: "idle", note: "configure under Settings" }, ]; +const ACTIVE_PROVIDER_ID = "minimax_vision"; @@ - {PROVIDERS.map((p, i) => ( + {PROVIDERS.map((p) => { + const isActive = p.id === ACTIVE_PROVIDER_ID; + return ( <li key={p.id} className={[ "flex items-center gap-2 px-2 py-1.5 rounded border", - i === 0 ? "bg-surface2 border-accent-cyan/40" : "bg-surface2/30 border-border", + isActive ? "bg-surface2 border-accent-cyan/40" : "bg-surface2/30 border-border", ].join(" ")} > @@ - {i === 0 && <span className="text-accent-cyan text-[10px] uppercase shrink-0">active</span>} + {isActive && <span className="text-accent-cyan text-[10px] uppercase shrink-0">active</span>} </li> - ))} + ); + })}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@03_implementation/ui/src/tabs/Gen3D.tsx` around lines 70 - 90, The list highlighting currently uses the brittle index check (i === 0); update rendering to use an explicit active-provider flag instead: modify the mock PROVIDERS data to include an "active" boolean (or provide an activeProviderId variable) and replace occurrences of i === 0 with either p.active or p.id === activeProviderId in Gen3D.tsx (affecting the className conditional for the li and the conditional that renders the "active" badge) so the highlighted provider is driven by an explicit identifier rather than array order.03_implementation/ui/src/data/mock/proof.ts (1)
63-63: ⚡ Quick winDerive the latest bundle instead of relying on array order.
MOCK_PROOF_BUNDLES[0]works only as long as the list stays manually sorted newest-first. If someone prepends another fixture later, the UI’s “LATEST” card will point at the wrong bundle.♻️ Suggested fix
-export const LATEST_BUNDLE = MOCK_PROOF_BUNDLES[0]; +export const LATEST_BUNDLE = [...MOCK_PROOF_BUNDLES].sort( + (a, b) => b.ts_utc.localeCompare(a.ts_utc), +)[0];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@03_implementation/ui/src/data/mock/proof.ts` at line 63, Replace the hardcoded LATEST_BUNDLE assignment that uses MOCK_PROOF_BUNDLES[0] with logic that derives the newest bundle from MOCK_PROOF_BUNDLES (e.g., use Array.reduce or Array.sort to pick the bundle with the greatest timestamp/createdAt/updatedAt property) so the latest card always reflects the most recent fixture; update the LATEST_BUNDLE export to compute and return that bundle.03_implementation/ui/src/tabs/Agents.tsx (2)
49-57: ⚡ Quick winExpose selected state to assistive tech on roster buttons.
These buttons act as single-select options; adding pressed/selected semantics improves accessibility without changing behavior.
Proposed fix
<button type="button" onClick={() => setSelectedId(a.id)} + aria-pressed={a.id === selectedId} className={[🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@03_implementation/ui/src/tabs/Agents.tsx` around lines 49 - 57, The roster buttons currently setSelectedId on click but lack accessibility semantics; update the button element (the one using onClick={() => setSelectedId(a.id)} and comparing a.id === selectedId) to expose selection state to assistive tech by adding an ARIA attribute such as aria-pressed={a.id === selectedId} (or aria-selected if you also set role="option" on the button) so the active item conveys pressed/selected state to screen readers while preserving existing behavior and styling.
30-31: ⚡ Quick winGuard selected-agent initialization against empty data.
This currently assumes at least one agent and can crash render if the list is empty.
Proposed fix
- const [selectedId, setSelectedId] = useState<string>(MOCK_AGENTS[0].id); - const selected = MOCK_AGENTS.find((a) => a.id === selectedId) ?? MOCK_AGENTS[0]; + const [selectedId, setSelectedId] = useState<string | null>(MOCK_AGENTS[0]?.id ?? null); + const selected = MOCK_AGENTS.find((a) => a.id === selectedId) ?? null; + if (!selected) { + return <div className="text-muted text-sm">No agents available.</div>; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@03_implementation/ui/src/tabs/Agents.tsx` around lines 30 - 31, The code assumes MOCK_AGENTS has at least one item; initialize selectedId and compute selected defensively: set selectedId using the first agent's id only if MOCK_AGENTS.length > 0 (otherwise use an empty string or undefined) and compute selected via MOCK_AGENTS.find(...) ?? undefined (or a safe fallback object), so that components using selected won't crash when MOCK_AGENTS is empty; update the useState call (selectedId, setSelectedId) and the selected variable assignment to include these guards and ensure downstream code handles a potentially undefined selected.03_implementation/ui/src/api/adapters.ts (1)
49-58: ⚡ Quick winReturn cloned mock payloads to avoid shared mutable state leaks.
Returning raw mock references makes accidental mutation globally visible across tabs/tests.
Proposed fix
export const adapters: AdapterAPI = { - getPrinters: async () => MOCK_PRINTERS, + getPrinters: async () => structuredClone(MOCK_PRINTERS), - getAgents: async () => MOCK_AGENTS, + getAgents: async () => structuredClone(MOCK_AGENTS), - getActiveWorkflows: async () => MOCK_WORKFLOWS, + getActiveWorkflows: async () => structuredClone(MOCK_WORKFLOWS.filter((w) => w.status === "active")), - getRecentJobs: async () => MOCK_JOBS, + getRecentJobs: async () => structuredClone(MOCK_JOBS), - getProofBundles: async () => MOCK_PROOF_BUNDLES, + getProofBundles: async () => structuredClone(MOCK_PROOF_BUNDLES), - getLatestProofBundle: async () => LATEST_BUNDLE, + getLatestProofBundle: async () => structuredClone(LATEST_BUNDLE), - getSystemSnapshot: async () => MOCK_SYSTEM_SNAPSHOT, + getSystemSnapshot: async () => structuredClone(MOCK_SYSTEM_SNAPSHOT), - getDimensionalReports: async () => MOCK_DIMENSIONAL_REPORTS, + getDimensionalReports: async () => structuredClone(MOCK_DIMENSIONAL_REPORTS), - getLogs: async () => MOCK_LOGS, + getLogs: async () => structuredClone(MOCK_LOGS), - getNotifications: async () => MOCK_NOTIFICATIONS, + getNotifications: async () => structuredClone(MOCK_NOTIFICATIONS), };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@03_implementation/ui/src/api/adapters.ts` around lines 49 - 58, The mock adapter currently returns direct references to shared mock constants (e.g., getPrinters, getAgents, getActiveWorkflows, getRecentJobs, getProofBundles, getLatestProofBundle, getSystemSnapshot, getDimensionalReports, getLogs, getNotifications), which allows callers to mutate global state; change each async getter to return a deep clone of the corresponding mock payload instead of the raw reference (use structuredClone if available or a safe deep-clone utility like lodash.cloneDeep / JSON parse/stringify as appropriate) so consumers get an isolated copy and mutations won’t leak across tabs/tests.03_implementation/ui/src/tabs/Dashboard.tsx (1)
120-121: ⚡ Quick winAdd empty-data guards for adapter-swap resilience.
These reads assume non-empty arrays and can hard-fail render when data is absent (
activeWorkflow,network_kbps[-1],MOCK_DIMENSIONAL_REPORTS[0]). A tiny fallback keeps the dashboard stable during Phase 3 data wiring.Proposed defensive pattern
- const activeWorkflow = MOCK_WORKFLOWS.find((w) => w.status === "active") ?? MOCK_WORKFLOWS[0]; + const activeWorkflow = MOCK_WORKFLOWS.find((w) => w.status === "active") ?? null; ... - <span className="text-muted text-xs truncate max-w-[180px]">{activeWorkflow.name}</span> + <span className="text-muted text-xs truncate max-w-[180px]">{activeWorkflow?.name ?? "No active workflow"}</span>- {sys.network_kbps[sys.network_kbps.length - 1]} kbps + {(sys.network_kbps.at(-1) ?? 0)} kbps ... - <Sparkline data={sys.network_kbps} color={tokens.chartColors.cyan} /> + <Sparkline data={sys.network_kbps.length ? sys.network_kbps : [0]} color={tokens.chartColors.cyan} />function DimensionalStrip() { - const r = MOCK_DIMENSIONAL_REPORTS[0]; + const r = MOCK_DIMENSIONAL_REPORTS[0]; + if (!r) { + return <div className="text-muted text-xs">No dimensional reports.</div>; + }Also applies to: 611-615, 835-843
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@03_implementation/ui/src/tabs/Dashboard.tsx` around lines 120 - 121, The code assumes arrays are non-empty and will crash if data is missing; update usages like the activeWorkflow assignment (current: const activeWorkflow = MOCK_WORKFLOWS.find((w) => w.status === "active") ?? MOCK_WORKFLOWS[0]), any access of network_kbps[-1] or index accesses to MOCK_DIMENSIONAL_REPORTS[0], and the sections around the other spots (lines referenced 611-615 and 835-843) to defensively check for empty or undefined arrays before indexing or calling .find; replace direct index reads with safe fallbacks (e.g., default object or null-conditional logic) and guard render paths to handle undefined values so the Dashboard component can render a sensible empty state instead of throwing.03_implementation/ui/src/tabs/Workflows.tsx (1)
28-30: ⚡ Quick winGuard selected-workflow initialization for empty datasets.
Current initialization will throw if workflows are empty. A null-safe selection path avoids a hard crash and enables an explicit empty state.
Proposed fix
- const [selectedId, setSelectedId] = useState(MOCK_WORKFLOWS[0].id); - const selected = MOCK_WORKFLOWS.find((w) => w.id === selectedId) ?? MOCK_WORKFLOWS[0]; + const [selectedId, setSelectedId] = useState<string | null>(MOCK_WORKFLOWS[0]?.id ?? null); + const selected = MOCK_WORKFLOWS.find((w) => w.id === selectedId) ?? null; + if (!selected) { + return <div className="text-muted text-sm">No workflows available.</div>; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@03_implementation/ui/src/tabs/Workflows.tsx` around lines 28 - 30, The code assumes MOCK_WORKFLOWS is non-empty and will throw when it's empty; change the selectedId state initialization to a null-safe value (e.g., use MOCK_WORKFLOWS[0]?.id ?? null) and update its type to allow null/undefined, then compute selected with a null-safe lookup (e.g., MOCK_WORKFLOWS.find(w => w.id === selectedId) ?? undefined) and ensure UI checks for a missing selected workflow and renders an explicit empty state; update any usages of selectedId/setSelectedId and components expecting selected to handle the nullable case.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@00_overview/PHASE2_PLAN.md`:
- Line 802: The plan's DockMode enum is out of sync: it lists
("docked"|"undocked"|"external") but the code/tests use "fullscreen"; update the
plan or the code so they match. Modify the documented enum in PHASE2_PLAN.md to
("docked"|"undocked"|"fullscreen") to reflect the implemented value used across
DockMode in store.ts, Panel, DockModeToggle and the Playwright dock-undock spec,
or alternatively rename the implementation/value from "fullscreen" to "external"
everywhere (store.ts, Panel component, DockModeToggle, and the spec) if you
prefer the plan; keep the same single identifier across DockMode, Panel,
DockModeToggle and the Playwright test to restore consistency.
- Around line 51-54: Several fenced code blocks in PHASE2_PLAN.md are unlabeled
(e.g., the single-line process flow and the file-tree / .gitignore examples);
add a language label to each opening fence (use "text" for plain text blocks or
an appropriate language for snippets) so MarkdownLint MD040 is satisfied. Locate
the unlabeled fences around the process flow, the file-tree examples, and the
.gitignore example and change ``` to ```text (or ```gitignore for .gitignore
content) consistently across those blocks.
In `@03_implementation/ui/src/api/adapters.ts`:
- Around line 34-35: The getActiveWorkflows() implementation returns all
workflows but should only return active ones; update the implementation of
getActiveWorkflows() in adapters.ts to filter the returned workflows by their
active indicator (e.g., status === 'active' or isActive === true) before
resolving the Promise, and ensure any related helper function used there (same
file) applies the same filter; likewise review the similar method referenced at
the other occurrence and apply the same active-only filtering.
In `@03_implementation/ui/src/App.tsx`:
- Around line 39-43: The code currently masks invalid activeTabId by falling
back to TABS[0]; change the logic so an unknown tab id does not default to
Dashboard: stop using "?? TABS[0]" when resolving tab (look at activeTabId and
TABS.find), let tab be undefined if not found, and then derive Component from
TAB_COMPONENTS[tab?.id] ?? FallbackPlaceholder (or directly via
TAB_COMPONENTS[activeTabId] ?? FallbackPlaceholder) so that invalid ids render
FallbackPlaceholder instead of Dashboard; update the Component resolution to
reference TAB_COMPONENTS and FallbackPlaceholder accordingly.
In `@03_implementation/ui/src/components/charts/ResourceGauge.tsx`:
- Around line 13-21: ResourceGauge accepts a numeric value claimed to be 0–100
but still uses the raw value in pickTone(value), chart data, and the visible
percentage; clamp the incoming value to the [0,100] range first (e.g., const
clampedValue = Math.max(0, Math.min(100, value)) or a small clamp util) and then
use clampedValue everywhere: pass clampedValue into pickTone, build the chart
dataset from clampedValue, and render the displayed percentage from clampedValue
so out-of-range inputs cannot produce invalid gauge states.
In `@03_implementation/ui/src/components/tables/DataTable.tsx`:
- Around line 42-55: In DataTable.tsx where the header <th> elements are
rendered inside the columns.map in the DataTable component, add scope="col" to
each <th> so assistive tech can identify them as column headers; update the JSX
for the mapped <th> (the element using key={c.id} and className built from ALIGN
and c.width) to include the scope="col" attribute.
In `@03_implementation/ui/src/tabs/BlenderMCP.tsx`:
- Around line 54-79: The trailing action div currently sits inside the providers
<ul>, making invalid HTML; move the action controls out of the list by either
wrapping them in their own <li> or placing them in a sibling container after the
</ul>. Update the JSX around PROVIDERS.map rendering in BlenderMCP.tsx so the
LockedAction components (labels "Connect" and "Rollback") are no longer direct
children of the <ul> — for example, close the </ul> after the mapped <li>s and
render a separate div (or an extra <li> if you prefer all controls inside the
list) containing the LockedAction buttons; ensure StatusBadge and PROVIDERS
mapping remain unchanged.
In `@03_implementation/ui/src/tabs/PrintQueue.tsx`:
- Around line 24-30: The initial selection logic in PrintQueueTab uses
MOCK_JOBS[0].id which will throw if MOCK_JOBS is empty; update the
selectedId/state and selected resolution to safely handle empty fixtures by
using optional chaining and a safe fallback (e.g., allActive[0]?.id ??
MOCK_JOBS[0]?.id ?? undefined) and make sure selected is computed with the same
guarded lookup (e.g., find by selectedId and fallback to undefined or null),
then ensure the UI handles a missing selected job gracefully; update the
references in PrintQueueTab (selectedId, setSelectedId, selected, allActive)
accordingly.
In `@03_implementation/ui/src/tabs/Proof.tsx`:
- Around line 28-30: ProofTab currently assumes MOCK_PROOF_BUNDLES has at least
one element when initializing selectedId and computing selected; if the fixture
is empty the component will throw on mount. Change the initialization so
selectedId is safely derived (e.g., use MOCK_PROOF_BUNDLES[0]?.id or undefined)
and ensure selected falls back to undefined or an empty object when no bundle
exists; update any downstream usage of selected/selectedId in ProofTab to handle
a null/undefined selected (render an empty state or disabled controls) so the
component no longer crashes when MOCK_PROOF_BUNDLES is empty.
In `@03_implementation/ui/src/tabs/Slicing.tsx`:
- Around line 60-78: The footer control <div> inside the <ul> (rendering QUEUE
items) makes the DOM invalid; move those controls out of the unordered list or
wrap them in an <li> so the list only contains <li> children. Locate the block
that maps QUEUE (the <ul> rendering FileBox, StatusBadge, and the file name/size
span) and either replace the trailing <div className="mt-1..."> with an <li>
containing the LockedAction buttons, or close the </ul> before that footer and
render the footer <div> after the list; apply the same change for the other two
panels (the similar blocks around lines ~89-114 and ~125-143) to keep markup
semantic and accessible.
In `@03_implementation/ui/src/tabs/SystemLogs.tsx`:
- Around line 27-109: The source chips are static and not applied to the
filtered list; add state to track selected sources (e.g., selectedSources:
Set<string> with setter setSelectedSources), update each source chip render (the
sources map and its span) to toggle that source in selectedSources and visually
reflect selection, and include the source constraint in the filtered computation
(modify filtered to also require selectedSources.size === 0 ||
selectedSources.has(l.source)); keep existing levelFilter and search logic
intact and reference symbols: sources, filtered, levelFilter, setLevelFilter.
In `@03_implementation/ui/tests/visual/dock.spec.ts`:
- Around line 191-198: The request filtering in the requests.filter predicate
(creating external) is too narrow — it only allows "http://localhost" and
"ws://localhost" and misses variants like https, wss, IP loopbacks (127.0.0.1,
::1) or other hostnames that resolve locally; replace the string-prefix checks
with a URL-parse based check: for each request in requests, try new URL(req) and
treat it as local if url.protocol is "http:"/"https:" or "ws:"/"wss:" and
url.hostname is "localhost" or "127.0.0.1" or "::1" (or otherwise consider local
hosts you need), and always treat protocols starting with "data:", "blob:", or
"about:" as local/ignored; keep the rest as external so external =
requests.filter(u => { try parse and return !isLocal; catch return true }).
---
Nitpick comments:
In `@03_implementation/ui/src/api/adapters.ts`:
- Around line 49-58: The mock adapter currently returns direct references to
shared mock constants (e.g., getPrinters, getAgents, getActiveWorkflows,
getRecentJobs, getProofBundles, getLatestProofBundle, getSystemSnapshot,
getDimensionalReports, getLogs, getNotifications), which allows callers to
mutate global state; change each async getter to return a deep clone of the
corresponding mock payload instead of the raw reference (use structuredClone if
available or a safe deep-clone utility like lodash.cloneDeep / JSON
parse/stringify as appropriate) so consumers get an isolated copy and mutations
won’t leak across tabs/tests.
In `@03_implementation/ui/src/data/mock/proof.ts`:
- Line 63: Replace the hardcoded LATEST_BUNDLE assignment that uses
MOCK_PROOF_BUNDLES[0] with logic that derives the newest bundle from
MOCK_PROOF_BUNDLES (e.g., use Array.reduce or Array.sort to pick the bundle with
the greatest timestamp/createdAt/updatedAt property) so the latest card always
reflects the most recent fixture; update the LATEST_BUNDLE export to compute and
return that bundle.
In `@03_implementation/ui/src/tabs/Agents.tsx`:
- Around line 49-57: The roster buttons currently setSelectedId on click but
lack accessibility semantics; update the button element (the one using
onClick={() => setSelectedId(a.id)} and comparing a.id === selectedId) to expose
selection state to assistive tech by adding an ARIA attribute such as
aria-pressed={a.id === selectedId} (or aria-selected if you also set
role="option" on the button) so the active item conveys pressed/selected state
to screen readers while preserving existing behavior and styling.
- Around line 30-31: The code assumes MOCK_AGENTS has at least one item;
initialize selectedId and compute selected defensively: set selectedId using the
first agent's id only if MOCK_AGENTS.length > 0 (otherwise use an empty string
or undefined) and compute selected via MOCK_AGENTS.find(...) ?? undefined (or a
safe fallback object), so that components using selected won't crash when
MOCK_AGENTS is empty; update the useState call (selectedId, setSelectedId) and
the selected variable assignment to include these guards and ensure downstream
code handles a potentially undefined selected.
In `@03_implementation/ui/src/tabs/Dashboard.tsx`:
- Around line 120-121: The code assumes arrays are non-empty and will crash if
data is missing; update usages like the activeWorkflow assignment (current:
const activeWorkflow = MOCK_WORKFLOWS.find((w) => w.status === "active") ??
MOCK_WORKFLOWS[0]), any access of network_kbps[-1] or index accesses to
MOCK_DIMENSIONAL_REPORTS[0], and the sections around the other spots (lines
referenced 611-615 and 835-843) to defensively check for empty or undefined
arrays before indexing or calling .find; replace direct index reads with safe
fallbacks (e.g., default object or null-conditional logic) and guard render
paths to handle undefined values so the Dashboard component can render a
sensible empty state instead of throwing.
In `@03_implementation/ui/src/tabs/Gen3D.tsx`:
- Around line 70-90: The list highlighting currently uses the brittle index
check (i === 0); update rendering to use an explicit active-provider flag
instead: modify the mock PROVIDERS data to include an "active" boolean (or
provide an activeProviderId variable) and replace occurrences of i === 0 with
either p.active or p.id === activeProviderId in Gen3D.tsx (affecting the
className conditional for the li and the conditional that renders the "active"
badge) so the highlighted provider is driven by an explicit identifier rather
than array order.
In `@03_implementation/ui/src/tabs/Workflows.tsx`:
- Around line 28-30: The code assumes MOCK_WORKFLOWS is non-empty and will throw
when it's empty; change the selectedId state initialization to a null-safe value
(e.g., use MOCK_WORKFLOWS[0]?.id ?? null) and update its type to allow
null/undefined, then compute selected with a null-safe lookup (e.g.,
MOCK_WORKFLOWS.find(w => w.id === selectedId) ?? undefined) and ensure UI checks
for a missing selected workflow and renders an explicit empty state; update any
usages of selectedId/setSelectedId and components expecting selected to handle
the nullable case.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 747da9b0-4f29-43c9-8dff-5d3a29c6b9d9
⛔ Files ignored due to path filters (6)
03_implementation/ui/package-lock.jsonis excluded by!**/package-lock.json03_implementation/ui/tests/visual/dashboard.visual.spec.ts-snapshots/dashboard-1920x1080-chromium-1920x1080-win32.pngis excluded by!**/*.png06_release/UI_FINAL_VISUAL_CONTRACT.pngis excluded by!**/*.png06_release/dashboard_checkpoint4_1920x1080.pngis excluded by!**/*.png06_release/dashboard_checkpoint4_1920x1080_v2.pngis excluded by!**/*.png06_release/phase2-bundle/9e7e7a8f89a6-20260501T145353Z.zipis excluded by!**/*.zip
📒 Files selected for processing (72)
.github/workflows/ui-ci.yml00_overview/PHASE2_COMPLETION_REPORT.md00_overview/PHASE2_PLAN.md03_implementation/ui/.gitignore03_implementation/ui/.npmrc03_implementation/ui/README.md03_implementation/ui/index.html03_implementation/ui/package.json03_implementation/ui/playwright.config.ts03_implementation/ui/postcss.config.js03_implementation/ui/src/App.tsx03_implementation/ui/src/api/adapters.ts03_implementation/ui/src/app/AppShell.tsx03_implementation/ui/src/app/routes.tsx03_implementation/ui/src/app/store.ts03_implementation/ui/src/components/badges/EditionBadge.tsx03_implementation/ui/src/components/badges/LockedAction.tsx03_implementation/ui/src/components/badges/ProofChip.tsx03_implementation/ui/src/components/badges/StatusBadge.tsx03_implementation/ui/src/components/cards/KpiCard.tsx03_implementation/ui/src/components/charts/ResourceGauge.tsx03_implementation/ui/src/components/charts/Sparkline.tsx03_implementation/ui/src/components/dock/DockModeToggle.tsx03_implementation/ui/src/components/layout/Panel.tsx03_implementation/ui/src/components/layout/Sidebar.tsx03_implementation/ui/src/components/layout/TopBar.tsx03_implementation/ui/src/components/pipeline/WorkflowPipeline.tsx03_implementation/ui/src/components/tables/DataTable.tsx03_implementation/ui/src/data/mock/agents.ts03_implementation/ui/src/data/mock/dimensional.ts03_implementation/ui/src/data/mock/jobs.ts03_implementation/ui/src/data/mock/logs.ts03_implementation/ui/src/data/mock/notifications.ts03_implementation/ui/src/data/mock/printers.ts03_implementation/ui/src/data/mock/proof.ts03_implementation/ui/src/data/mock/system.ts03_implementation/ui/src/data/mock/workflows.ts03_implementation/ui/src/main.tsx03_implementation/ui/src/styles/globals.css03_implementation/ui/src/styles/tokens.ts03_implementation/ui/src/tabs/Agents.tsx03_implementation/ui/src/tabs/BlenderMCP.tsx03_implementation/ui/src/tabs/Dashboard.tsx03_implementation/ui/src/tabs/DockedApps.tsx03_implementation/ui/src/tabs/Fleet.tsx03_implementation/ui/src/tabs/Gen3D.tsx03_implementation/ui/src/tabs/PrintQueue.tsx03_implementation/ui/src/tabs/PrinterControl.tsx03_implementation/ui/src/tabs/Proof.tsx03_implementation/ui/src/tabs/Settings.tsx03_implementation/ui/src/tabs/Slicing.tsx03_implementation/ui/src/tabs/SystemLogs.tsx03_implementation/ui/src/tabs/Workflows.tsx03_implementation/ui/src/types/agent.ts03_implementation/ui/src/types/dimensional.ts03_implementation/ui/src/types/edition.ts03_implementation/ui/src/types/job.ts03_implementation/ui/src/types/log.ts03_implementation/ui/src/types/notification.ts03_implementation/ui/src/types/printer.ts03_implementation/ui/src/types/proof.ts03_implementation/ui/src/types/system.ts03_implementation/ui/src/types/workflow.ts03_implementation/ui/tailwind.config.ts03_implementation/ui/tests/visual/dashboard.visual.spec.ts03_implementation/ui/tests/visual/dock.spec.ts03_implementation/ui/tsconfig.json03_implementation/ui/tsconfig.node.json03_implementation/ui/vite.config.ts06_release/phase2-bundle/9e7e7a8f89a6-20260501T145353Z.manifest.json06_release/phase2-bundle/9e7e7a8f89a6-20260501T145353Z.sha256scripts/_build_bundle.py
| ``` | ||
| Prompt → Vision → 3D → Dimensional Accuracy → Mesh Repair → | ||
| Printability → Slice → Select Printer → Print → Monitor → Recover → Proof | ||
| ``` |
There was a problem hiding this comment.
Add fence languages to satisfy markdownlint MD040.
Several code fences are unlabeled; this keeps warning noise in CI/docs checks.
Example fix pattern
-```
+```text
Prompt → Vision → 3D → Dimensional Accuracy → Mesh Repair →
Printability → Slice → Select Printer → Print → Monitor → Recover → Proof</details>
Apply similarly to the file-tree and `.gitignore` fences.
Also applies to: 137-222, 383-388
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.22.1)</summary>
[warning] 51-51: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @00_overview/PHASE2_PLAN.md around lines 51 - 54, Several fenced code blocks
in PHASE2_PLAN.md are unlabeled (e.g., the single-line process flow and the
file-tree / .gitignore examples); add a language label to each opening fence
(use "text" for plain text blocks or an appropriate language for snippets) so
MarkdownLint MD040 is satisfied. Locate the unlabeled fences around the process
flow, the file-tree examples, and the .gitignore example and change totext (or ```gitignore for .gitignore content) consistently across those
blocks.
</details>
<!-- fingerprinting:phantom:medusa:grasshopper:8c2735d2-33ef-4c84-80b7-d2984b913f4d -->
<!-- d98c2f50 -->
<!-- This is an auto-generated comment by CodeRabbit -->
|
|
||
| **Placeholder scan:** None — every step has actual code or a concrete shell command. | ||
|
|
||
| **Type consistency:** `DockMode` ("docked"|"undocked"|"external") used identically across `store.ts` (Task 6), `Panel` (Task 9), `DockModeToggle` (Task 12), Playwright dock-undock spec (Task 46). `TabDef` shape consistent in `routes.tsx` (Task 6) and `Sidebar` (Task 7). |
There was a problem hiding this comment.
Dock mode enum in plan is inconsistent with current implementation.
This line documents ("docked"|"undocked"|"external"), but the implemented/store-tested mode is fullscreen, not external.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@00_overview/PHASE2_PLAN.md` at line 802, The plan's DockMode enum is out of
sync: it lists ("docked"|"undocked"|"external") but the code/tests use
"fullscreen"; update the plan or the code so they match. Modify the documented
enum in PHASE2_PLAN.md to ("docked"|"undocked"|"fullscreen") to reflect the
implemented value used across DockMode in store.ts, Panel, DockModeToggle and
the Playwright dock-undock spec, or alternatively rename the
implementation/value from "fullscreen" to "external" everywhere (store.ts, Panel
component, DockModeToggle, and the spec) if you prefer the plan; keep the same
single identifier across DockMode, Panel, DockModeToggle and the Playwright test
to restore consistency.
| getActiveWorkflows(): Promise<Workflow[]>; | ||
| getRecentJobs(): Promise<Job[]>; |
There was a problem hiding this comment.
getActiveWorkflows() currently violates its own contract.
The method name and interface imply active workflows, but the implementation returns all workflows.
Proposed fix
- getActiveWorkflows: async () => MOCK_WORKFLOWS,
+ getActiveWorkflows: async () => MOCK_WORKFLOWS.filter((w) => w.status === "active"),Also applies to: 51-51
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@03_implementation/ui/src/api/adapters.ts` around lines 34 - 35, The
getActiveWorkflows() implementation returns all workflows but should only return
active ones; update the implementation of getActiveWorkflows() in adapters.ts to
filter the returned workflows by their active indicator (e.g., status ===
'active' or isActive === true) before resolving the Promise, and ensure any
related helper function used there (same file) applies the same filter; likewise
review the similar method referenced at the other occurrence and apply the same
active-only filtering.
| export default function App() { | ||
| const activeTabId = useStore((s) => s.activeTabId); | ||
| const tab = TABS.find((t) => t.id === activeTabId) ?? TABS[0]; | ||
| const Component = TAB_COMPONENTS[tab.id] ?? FallbackPlaceholder; | ||
|
|
There was a problem hiding this comment.
Don't default unknown tab ids to Dashboard.
?? TABS[0] makes every invalid activeTabId render the Dashboard instead of FallbackPlaceholder, so bad state is hidden and the “Tab not found” path is effectively unreachable.
🔧 Proposed fix
export default function App() {
const activeTabId = useStore((s) => s.activeTabId);
- const tab = TABS.find((t) => t.id === activeTabId) ?? TABS[0];
- const Component = TAB_COMPONENTS[tab.id] ?? FallbackPlaceholder;
+ const tab = TABS.find((t) => t.id === activeTabId);
+ const Component = tab ? TAB_COMPONENTS[tab.id] ?? FallbackPlaceholder : FallbackPlaceholder;
return (
<AppShell>
<Component />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@03_implementation/ui/src/App.tsx` around lines 39 - 43, The code currently
masks invalid activeTabId by falling back to TABS[0]; change the logic so an
unknown tab id does not default to Dashboard: stop using "?? TABS[0]" when
resolving tab (look at activeTabId and TABS.find), let tab be undefined if not
found, and then derive Component from TAB_COMPONENTS[tab?.id] ??
FallbackPlaceholder (or directly via TAB_COMPONENTS[activeTabId] ??
FallbackPlaceholder) so that invalid ids render FallbackPlaceholder instead of
Dashboard; update the Component resolution to reference TAB_COMPONENTS and
FallbackPlaceholder accordingly.
| export function ResourceGauge({ | ||
| value, | ||
| label, | ||
| }: { | ||
| /** 0-100. */ | ||
| value: number; | ||
| label: string; | ||
| }) { | ||
| const tone = pickTone(value); |
There was a problem hiding this comment.
Clamp out-of-range gauge values before rendering.
The component documents a 0–100 contract, but it still feeds the raw value into the tone picker, chart data, and visible percentage. If upstream data ever drifts negative or above 100, the gauge will render an invalid state.
🛠️ Proposed fix
export function ResourceGauge({
value,
label,
}: {
/** 0-100. */
value: number;
label: string;
}) {
- const tone = pickTone(value);
+ const safeValue = Math.min(100, Math.max(0, value));
+ const tone = pickTone(safeValue);
return (
@@
- data={[{ value, fill: tone }]}
+ data={[{ value: safeValue, fill: tone }]}
@@
- {Math.round(value)}%
+ {Math.round(safeValue)}%Also applies to: 32-46
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@03_implementation/ui/src/components/charts/ResourceGauge.tsx` around lines 13
- 21, ResourceGauge accepts a numeric value claimed to be 0–100 but still uses
the raw value in pickTone(value), chart data, and the visible percentage; clamp
the incoming value to the [0,100] range first (e.g., const clampedValue =
Math.max(0, Math.min(100, value)) or a small clamp util) and then use
clampedValue everywhere: pass clampedValue into pickTone, build the chart
dataset from clampedValue, and render the displayed percentage from clampedValue
so out-of-range inputs cannot produce invalid gauge states.
| export function PrintQueueTab() { | ||
| const queue = MOCK_JOBS.filter((j) => j.status === "queued"); | ||
| const printing = MOCK_JOBS.filter((j) => j.status === "printing"); | ||
| const allActive = [...printing, ...queue]; | ||
| const [selectedId, setSelectedId] = useState(allActive[0]?.id ?? MOCK_JOBS[0].id); | ||
| const selected = MOCK_JOBS.find((j) => j.id === selectedId) ?? MOCK_JOBS[0]; | ||
| const printerNameById = new Map(MOCK_PRINTERS.map((p) => [p.id, p.name])); |
There was a problem hiding this comment.
Guard the initial selection against an empty job fixture.
MOCK_JOBS[0].id will throw if the mock list is ever empty, which turns this tab into a crash instead of an empty state. A small guard here avoids that failure mode.
♻️ Proposed fix
export function PrintQueueTab() {
const queue = MOCK_JOBS.filter((j) => j.status === "queued");
const printing = MOCK_JOBS.filter((j) => j.status === "printing");
const allActive = [...printing, ...queue];
- const [selectedId, setSelectedId] = useState(allActive[0]?.id ?? MOCK_JOBS[0].id);
- const selected = MOCK_JOBS.find((j) => j.id === selectedId) ?? MOCK_JOBS[0];
+ const fallbackJob = MOCK_JOBS[0];
+ const [selectedId, setSelectedId] = useState(allActive[0]?.id ?? fallbackJob?.id ?? "");
+ const selected = MOCK_JOBS.find((j) => j.id === selectedId) ?? fallbackJob;
+
+ if (!selected) {
+ return <div className="rounded border border-border bg-surface p-4 text-sm text-muted">No jobs available.</div>;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function PrintQueueTab() { | |
| const queue = MOCK_JOBS.filter((j) => j.status === "queued"); | |
| const printing = MOCK_JOBS.filter((j) => j.status === "printing"); | |
| const allActive = [...printing, ...queue]; | |
| const [selectedId, setSelectedId] = useState(allActive[0]?.id ?? MOCK_JOBS[0].id); | |
| const selected = MOCK_JOBS.find((j) => j.id === selectedId) ?? MOCK_JOBS[0]; | |
| const printerNameById = new Map(MOCK_PRINTERS.map((p) => [p.id, p.name])); | |
| export function PrintQueueTab() { | |
| const queue = MOCK_JOBS.filter((j) => j.status === "queued"); | |
| const printing = MOCK_JOBS.filter((j) => j.status === "printing"); | |
| const allActive = [...printing, ...queue]; | |
| const fallbackJob = MOCK_JOBS[0]; | |
| const [selectedId, setSelectedId] = useState(allActive[0]?.id ?? fallbackJob?.id ?? ""); | |
| const selected = MOCK_JOBS.find((j) => j.id === selectedId) ?? fallbackJob; | |
| if (!selected) { | |
| return <div className="rounded border border-border bg-surface p-4 text-sm text-muted">No jobs available.</div>; | |
| } | |
| const printerNameById = new Map(MOCK_PRINTERS.map((p) => [p.id, p.name])); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@03_implementation/ui/src/tabs/PrintQueue.tsx` around lines 24 - 30, The
initial selection logic in PrintQueueTab uses MOCK_JOBS[0].id which will throw
if MOCK_JOBS is empty; update the selectedId/state and selected resolution to
safely handle empty fixtures by using optional chaining and a safe fallback
(e.g., allActive[0]?.id ?? MOCK_JOBS[0]?.id ?? undefined) and make sure selected
is computed with the same guarded lookup (e.g., find by selectedId and fallback
to undefined or null), then ensure the UI handles a missing selected job
gracefully; update the references in PrintQueueTab (selectedId, setSelectedId,
selected, allActive) accordingly.
| export function ProofTab() { | ||
| const [selectedId, setSelectedId] = useState(MOCK_PROOF_BUNDLES[0].id); | ||
| const selected = MOCK_PROOF_BUNDLES.find((b) => b.id === selectedId) ?? MOCK_PROOF_BUNDLES[0]; |
There was a problem hiding this comment.
Guard the initial bundle selection.
Both the initial state and fallback assume MOCK_PROOF_BUNDLES always has at least one entry. If that fixture is ever emptied, the tab will throw on mount before it can render an empty state.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@03_implementation/ui/src/tabs/Proof.tsx` around lines 28 - 30, ProofTab
currently assumes MOCK_PROOF_BUNDLES has at least one element when initializing
selectedId and computing selected; if the fixture is empty the component will
throw on mount. Change the initialization so selectedId is safely derived (e.g.,
use MOCK_PROOF_BUNDLES[0]?.id or undefined) and ensure selected falls back to
undefined or an empty object when no bundle exists; update any downstream usage
of selected/selectedId in ProofTab to handle a null/undefined selected (render
an empty state or disabled controls) so the component no longer crashes when
MOCK_PROOF_BUNDLES is empty.
| <ul className="flex flex-col gap-1 h-full overflow-auto text-xs"> | ||
| {QUEUE.map((f) => ( | ||
| <li | ||
| key={f.id} | ||
| className="flex items-center gap-2 px-2 py-1.5 rounded bg-surface2/40 border border-border" | ||
| > | ||
| <FileBox size={13} className="text-muted shrink-0" /> | ||
| <span className="text-fg font-mono truncate flex-1">{f.name}</span> | ||
| <span className="text-muted text-[10px] font-mono shrink-0"> | ||
| {f.size_kb} KB | ||
| </span> | ||
| <StatusBadge tone={f.status === "sliced" ? "green" : "muted"} label={f.status} /> | ||
| </li> | ||
| ))} | ||
| <div className="mt-1 flex justify-end gap-1.5"> | ||
| <LockedAction label="Add file" /> | ||
| <LockedAction label="Dry-run slice" /> | ||
| </div> | ||
| </ul> |
There was a problem hiding this comment.
Move the footer controls out of the <ul>s.
Each list currently contains a bare <div> footer, which makes the DOM invalid in all three panels. Wrapping those controls in a <li> or placing them after the list keeps the markup semantic and avoids browser/a11y quirks.
Suggested fix
- <ul className="flex flex-col gap-1 h-full overflow-auto text-xs">
+ <ul className="flex flex-col gap-1 h-full overflow-auto text-xs">
{QUEUE.map((f) => (
<li key={f.id} ...>
...
</li>
))}
- <div className="mt-1 flex justify-end gap-1.5">
- <LockedAction label="Add file" />
- <LockedAction label="Dry-run slice" />
- </div>
+ <li className="mt-1 flex justify-end gap-1.5">
+ <LockedAction label="Add file" />
+ <LockedAction label="Dry-run slice" />
+ </li>
</ul>Also applies to: 89-114, 125-143
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@03_implementation/ui/src/tabs/Slicing.tsx` around lines 60 - 78, The footer
control <div> inside the <ul> (rendering QUEUE items) makes the DOM invalid;
move those controls out of the unordered list or wrap them in an <li> so the
list only contains <li> children. Locate the block that maps QUEUE (the <ul>
rendering FileBox, StatusBadge, and the file name/size span) and either replace
the trailing <div className="mt-1..."> with an <li> containing the LockedAction
buttons, or close the </ul> before that footer and render the footer <div> after
the list; apply the same change for the other two panels (the similar blocks
around lines ~89-114 and ~125-143) to keep markup semantic and accessible.
| const sources = useMemo( | ||
| () => Array.from(new Set(MOCK_LOGS.map((l) => l.source))).sort(), | ||
| [], | ||
| ); | ||
| const filtered = MOCK_LOGS.filter( | ||
| (l) => | ||
| levelFilter.has(l.level) && | ||
| (search === "" || | ||
| l.message.toLowerCase().includes(search.toLowerCase()) || | ||
| l.source.toLowerCase().includes(search.toLowerCase())), | ||
| ); | ||
|
|
||
| const counts = LEVELS.reduce( | ||
| (acc, lvl) => { | ||
| acc[lvl] = MOCK_LOGS.filter((l) => l.level === lvl).length; | ||
| return acc; | ||
| }, | ||
| {} as Record<LogLevel, number>, | ||
| ); | ||
|
|
||
| return ( | ||
| <div className="grid grid-cols-12 gap-2.5 auto-rows-min" data-testid="logs-root"> | ||
| <div className="col-span-12 lg:col-span-3"> | ||
| <Panel | ||
| id="logs.filters" | ||
| title="FILTERS" | ||
| dense | ||
| status={{ tone: "cyan", label: `${filtered.length}/${MOCK_LOGS.length}` }} | ||
| className="h-[480px]" | ||
| > | ||
| <div className="flex flex-col gap-3 h-full text-xs"> | ||
| <div className="flex flex-col gap-1.5"> | ||
| <div className="text-muted text-[10px] uppercase tracking-wide">Search</div> | ||
| <div className="flex items-center gap-1 px-2 py-1 rounded bg-surface2/40 border border-border"> | ||
| <Search size={11} className="text-muted shrink-0" /> | ||
| <input | ||
| type="text" | ||
| value={search} | ||
| onChange={(e) => setSearch(e.target.value)} | ||
| placeholder="message or source…" | ||
| className="bg-transparent text-fg text-[11px] outline-none flex-1 min-w-0" | ||
| /> | ||
| </div> | ||
| </div> | ||
| <div className="flex flex-col gap-1.5"> | ||
| <div className="text-muted text-[10px] uppercase tracking-wide">Severity</div> | ||
| {LEVELS.map((lvl) => ( | ||
| <label | ||
| key={lvl} | ||
| className="flex items-center gap-2 cursor-pointer px-1 py-0.5 rounded hover:bg-surface2/40" | ||
| > | ||
| <input | ||
| type="checkbox" | ||
| checked={levelFilter.has(lvl)} | ||
| onChange={() => { | ||
| const next = new Set(levelFilter); | ||
| if (next.has(lvl)) { | ||
| next.delete(lvl); | ||
| } else { | ||
| next.add(lvl); | ||
| } | ||
| setLevelFilter(next); | ||
| }} | ||
| className="accent-accent-cyan" | ||
| /> | ||
| <StatusBadge tone={LEVEL_TONE[lvl]} label={lvl} /> | ||
| <span className="text-muted text-[10px] font-mono ml-auto">{counts[lvl]}</span> | ||
| </label> | ||
| ))} | ||
| </div> | ||
| <div className="flex flex-col gap-1"> | ||
| <div className="text-muted text-[10px] uppercase tracking-wide">Sources</div> | ||
| <div className="flex flex-wrap gap-1"> | ||
| {sources.map((s) => ( | ||
| <span | ||
| key={s} | ||
| className="px-1.5 py-0.5 rounded bg-surface2/40 border border-border text-fg font-mono text-[10px]" | ||
| > | ||
| {s} | ||
| </span> | ||
| ))} | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
Wire the source chips into actual filtering.
The panel advertises level/source/search filtering, but sources is only displayed as static labels. filtered never constrains by a selected source, so the source facet is non-functional.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@03_implementation/ui/src/tabs/SystemLogs.tsx` around lines 27 - 109, The
source chips are static and not applied to the filtered list; add state to track
selected sources (e.g., selectedSources: Set<string> with setter
setSelectedSources), update each source chip render (the sources map and its
span) to toggle that source in selectedSources and visually reflect selection,
and include the source constraint in the filtered computation (modify filtered
to also require selectedSources.size === 0 || selectedSources.has(l.source));
keep existing levelFilter and search logic intact and reference symbols:
sources, filtered, levelFilter, setLevelFilter.
| const external = requests.filter( | ||
| (u) => | ||
| !u.startsWith("http://localhost:") && | ||
| !u.startsWith("ws://localhost:") && | ||
| !u.startsWith("data:") && | ||
| !u.startsWith("blob:") && | ||
| !u.startsWith("about:"), | ||
| ); |
There was a problem hiding this comment.
Broaden allowlist for local host/protocol variants in request filtering.
The current filter can false-fail when baseURL/protocol differs (127.0.0.1, https, wss) while still being local test traffic.
Proposed fix
const external = requests.filter(
(u) =>
- !u.startsWith("http://localhost:") &&
- !u.startsWith("ws://localhost:") &&
+ !u.startsWith("http://localhost:") &&
+ !u.startsWith("https://localhost:") &&
+ !u.startsWith("ws://localhost:") &&
+ !u.startsWith("wss://localhost:") &&
+ !u.startsWith("http://127.0.0.1:") &&
+ !u.startsWith("https://127.0.0.1:") &&
+ !u.startsWith("ws://127.0.0.1:") &&
+ !u.startsWith("wss://127.0.0.1:") &&
!u.startsWith("data:") &&
!u.startsWith("blob:") &&
!u.startsWith("about:"),
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const external = requests.filter( | |
| (u) => | |
| !u.startsWith("http://localhost:") && | |
| !u.startsWith("ws://localhost:") && | |
| !u.startsWith("data:") && | |
| !u.startsWith("blob:") && | |
| !u.startsWith("about:"), | |
| ); | |
| const external = requests.filter( | |
| (u) => | |
| !u.startsWith("http://localhost:") && | |
| !u.startsWith("https://localhost:") && | |
| !u.startsWith("ws://localhost:") && | |
| !u.startsWith("wss://localhost:") && | |
| !u.startsWith("http://127.0.0.1:") && | |
| !u.startsWith("https://127.0.0.1:") && | |
| !u.startsWith("ws://127.0.0.1:") && | |
| !u.startsWith("wss://127.0.0.1:") && | |
| !u.startsWith("data:") && | |
| !u.startsWith("blob:") && | |
| !u.startsWith("about:"), | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@03_implementation/ui/tests/visual/dock.spec.ts` around lines 191 - 198, The
request filtering in the requests.filter predicate (creating external) is too
narrow — it only allows "http://localhost" and "ws://localhost" and misses
variants like https, wss, IP loopbacks (127.0.0.1, ::1) or other hostnames that
resolve locally; replace the string-prefix checks with a URL-parse based check:
for each request in requests, try new URL(req) and treat it as local if
url.protocol is "http:"/"https:" or "ws:"/"wss:" and url.hostname is "localhost"
or "127.0.0.1" or "::1" (or otherwise consider local hosts you need), and always
treat protocols starting with "data:", "blob:", or "about:" as local/ignored;
keep the rest as external so external = requests.filter(u => { try parse and
return !isLocal; catch return true }).
Phase 2 - UI-Final closeout
Top-line numbers
feat/phase-2-ui-final9e7e7a8f89a69364dfd820546aaae698c20717cdorigin/develop: 67Final gates
From
03_implementation/ui:npm ci npm run lint npm run build npx playwright testResults:
npm ci: passednpm run lint: passednpm run build: passed, with Vite large-chunk warningnpx playwright test: passed, 9/9Proof bundle
05_truth_proof/bundles/9e7e7a8f89a6-20260501T145353Z.zip5b990c1d3805158b4dab0fd59821fe5c4af0d0763845d3569d106bd07f9d54bbpython 05_truth_proof/conformance_runner.py --bundle 05_truth_proof/bundles/9e7e7a8f89a6-20260501T145353Z.zipOK - signature + file hashes + cross-refs verifieddirty=Falsescreenshots/dashboard_checkpoint4_1920x1080_v2.pngConstraints honored
dist/is not tracked.What is next
Phase 3 should only begin after explicit approval. It may add read-only adapter plumbing behind the existing mock UI swap points without promoting dangerous actions.
Summary by CodeRabbit
New Features
Tests
Documentation
Chores