Sprint 0+1: Security, UX, Architecture & Infrastructure - #3
Merged
Conversation
…on awareness, progressive loading P0 (coherence fixes): - Wire all 7 AI review categories to UI (was 4 — 40% of Claude tokens wasted) - Add labels for visualQuality + microcopy lint categories in IssuesList - Expand Design Health Score to 7 weighted categories (25/18/10/25/7/8/7) - Forward token analysis summary to backend for AI context - Fix hardcoded hasAutoLayout/childCount — now reads real values from Figma node - Add selection change detection with stale results banner + Re-analyze button P1 (UX improvements): - Expose radius fixes in Fix All + walkthrough (was spacing-only) - Progressive 4-phase loading indicator (rules → screenshot → AI → Refero) - Persistent ignore state via figma.root.setPluginData (survives restarts) - 3-step onboarding flow replacing blank empty state - "AI-generated" badge on AiReviewCard to distinguish facts from opinions Audit reports: - AUDIT-REPORT.md: 16-agent consolidated audit (~200 findings) - PIPELINE-EVALUATION.md: 3-agent pipeline coherence/completeness evaluation - backend/TEST_PLAN.md: 107 test cases across 7 endpoints Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Security: - Add security headers (HSTS, X-Frame-Options, nosniff) to Caddyfile - Add 25MB body size limit to prevent OOM from large screenshots - Redact error internals from API responses (no stack traces to client) - Redact API key from plugin console logs - Fix CORS to return empty string for disallowed origins - Validate ANTHROPIC_API_KEY at startup with warning UX & Accessibility: - Auto-rescan after batch fixes to show updated score - Fix stale closure in handleRescan (read prev.score inside setState) - Add cleanup for Refero polling interval on unmount - Add aria-hidden to all decorative SVG icons - Add aria-label to settings close button and API key input - Replace PluginEvent catch-all with explicit screenshot-error type - Add text-10 fontSize to Tailwind config Backend Architecture: - Consolidate Anthropic client into shared singleton (getAnthropicClient) - Export MODEL constant from claude.ts, remove duplicates - Fix appendConversation race condition with immediate transaction - Add SQLite busy_timeout=5000 for concurrent access - Remove duplicate conversation history from system prompt - Add tokenSummary to AnalyzeRequest type Infrastructure: - Pin Docker images (node:22.15-alpine3.21, caddy:2.9-alpine) - Add Docker HEALTHCHECK hitting /api/health - Add backend/.dockerignore - Improve backend/.env.example with documentation - Add data/, *.db, .env to .gitignore - Delete dead ui-enhanced.html (284KB x2) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughЭтот PR добавляет документацию и тест-план, обновляет DevOps (Docker/Caddy/.gitignore), централизует Anthropic-клиент, вводит tokenSummary и события selection-changed, усиливает SQLite транзакции и таймауты MCP/Refero, унифицирует обработку ошибок и расширяет логику линта и UI типы/интерфейсы. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as "Plugin UI (ui/)"
participant Plugin as "Figma Plugin\n(code.ts / message-handler)"
participant Backend as "Backend API"
participant Anthropic as "Anthropic (getAnthropicClient)"
participant DB as "SQLite DB"
UI->>Plugin: Пользователь → Analyze / меняет selection
Plugin->>UI: отправляет `selection-changed`
Plugin->>Backend: POST /analyze (screenshot + lint + tokenSummary + metadata)
Backend->>Anthropic: вызывает ML (getAnthropicClient / MODEL)
Anthropic-->>Backend: возвращает AI-ответ (ai-review, scoreBreakdown)
Backend->>DB: appendConversation (immediate transaction)
DB-->>Backend: подтверждение записи
Backend-->>Plugin: ответ анализа (ai-review, scoreBreakdown)
Plugin->>UI: обновить UI (issues, score, пометить stale)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan Review Summary by QodoSprint 0+1: Security hardening, UX improvements, architecture consolidation, and infrastructure automation
WalkthroughsDescription**Core Coherence Fixes (Sprint 0)** • Expanded score model from 5 to 7 categories by wiring visualQuality and microcopy lint categories into UI display and score computation with adjusted weights (25/18/10/25/7/8/7) • Wired 3 hidden AI review categories (visualBalance, microcopyQuality, cognitiveLoad) into AiReviewCard with conditional rendering • Connected orphaned token analysis data to backend AI context via tokenSummary field in requests • Included radius errors in batch fix operations and fixable count filtering • Wired real hasAutoLayout and childCount metadata into backend analysis requests **UX Improvements (Sprint 1)** • Added selection change detection with stale results banner and re-analyze button when selection changes to different node • Implemented 4-phase analysis progress indicator (lint → screenshot → ai-review → refero) with AnalysisPhaseIndicator component • Persistent ignored state via figma.root.setPluginData() for document-level storage • Enhanced onboarding empty state with 3-step numbered guide (Select, Analyze, Fix) • Added "AI-generated" badge on AI Design Review card • Auto-rescan after batch fixes with 500ms delay to display updated score **Security Hardening** • Added security headers to Caddyfile (HSTS, X-Frame-Options, X-Content-Type-Options, Referrer-Policy, XSS-Protection) • Implemented 25MB request body size limit to prevent OOM attacks • Redacted error messages across all API routes to prevent internal detail leakage • Redacted API keys in plugin console logs • Fixed CORS origin handling to return empty string instead of 'null' for disallowed origins **Backend Architecture** • Consolidated 3 duplicate Anthropic client instances into shared getAnthropicClient() singleton from claude.ts • Consolidated duplicate MODEL constant exports for shared use across services • Fixed appendConversation race condition with atomic transaction and immediate locking • Added SQLite busy_timeout = 5000 pragma for concurrent access handling • Removed duplicate conversation history from system prompt (now passed via messages array only) **Infrastructure & DevOps** • Pinned Docker images: node:22.15-alpine3.21 and caddy:2.9-alpine for reproducible builds • Added Docker HEALTHCHECK on /api/health endpoint with 30s interval, 5s timeout, 3 retries • Created .dockerignore to optimize build context • Created .env.example with documented environment variables • Updated .gitignore for data/, *.db, .env files • Deleted legacy ui-enhanced.html (568KB) in favor of modular source structure **Accessibility Improvements** • Added aria-hidden="true" to all decorative SVG icons (settings gear, checkmark, send button) • Added aria-label attributes to settings controls (close button, API key input) • Replaced catch-all PluginEvent type with explicit event types (selection-changed, screenshot-error) **Documentation** • Added comprehensive 753-line backend API test plan with 107 test cases, infrastructure recommendations, and 5 example tests • Added consolidated 573-line security and quality audit from 16 agents covering security, architecture, frontend, accessibility, UX, UI design, and DevOps • Added 390-line pipeline evaluation report analyzing 3-layer architecture, coherence gaps, competitive landscape, and strategic roadmap with value estimation ($76k annual value) • Updated README to reference modular ui/ source structure instead of legacy artifact **Bug Fixes** • Fixed stale closure bug in handleRescan by moving state access inside setState callback • Fixed Refero polling cleanup on component unmount to prevent memory leaks • Added startup validation warning for missing ANTHROPIC_API_KEY Diagramflowchart LR
A["Score Model<br/>5→7 Categories"] -->|"visualQuality<br/>microcopy"| B["ScoreCard<br/>Display"]
C["AI Review<br/>Hidden Categories"] -->|"visualBalance<br/>microcopyQuality<br/>cognitiveLoad"| D["AiReviewCard<br/>Render"]
E["Token Analysis<br/>Orphaned Data"] -->|"tokenSummary"| F["Backend AI<br/>Context"]
G["Selection<br/>Change Event"] -->|"stale detection"| H["Stale Banner<br/>+ Re-analyze"]
I["Batch Fixes<br/>spacing + radius"] -->|"auto-rescan"| J["Score Update<br/>Display"]
K["3 Anthropic<br/>Clients"] -->|"consolidate"| L["Shared<br/>getAnthropicClient"]
M["Error Messages<br/>Internal Details"] -->|"redact"| N["Generic<br/>Error Response"]
O["Security Headers<br/>+ Body Limits"] -->|"harden"| P["Caddyfile<br/>+ Middleware"]
Q["Docker Images<br/>Generic Versions"] -->|"pin"| R["Reproducible<br/>Builds"]
File Changes1. ui/src/hooks/useChat.ts
|
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ui/src/components/chat/StickyHeader.tsx (1)
45-50:⚠️ Potential issue | 🟡 MinorДобавьте явный
aria-labelна кнопку с иконкой.Сейчас у кнопки есть
title, но для стабильной доступности лучше задать доступное имя черезaria-label.💡 Предлагаемое изменение
<button onClick={onOpenSettings} className="shrink-0 w-7 h-7 flex items-center justify-center text-fg-tertiary hover:text-fg rounded-md hover:bg-bg-hover transition-colors" title="Settings" + aria-label="Open settings" > <svg width="14" height="14" viewBox="0 0 24 24" fill="none" stroke="currentColor" strokeWidth="2" aria-hidden="true">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/chat/StickyHeader.tsx` around lines 45 - 50, В компоненте StickyHeader добавьте явный доступный ярлык для кнопки настроек: в JSX кнопки, где используется onClick={onOpenSettings} и есть title="Settings", добавьте атрибут aria-label="Settings" (или локализованную строку) чтобы обеспечить доступное имя для вспомогательных технологий; сохраните существующий title и остальные атрибуты и убедитесь, что aria-label отражает ту же семантику, что и title.
🧹 Nitpick comments (3)
backend/src/db/queries.ts (1)
75-99: Транзакционный подход корректен, но есть незначительная неэффективность.Исправление race condition через immediate-транзакцию — правильное решение. Однако
newMessageсериализуется в JSON на строке 77, а затем парсится обратно на строке 94, что создаёт лишний цикл сериализации.♻️ Предлагаемый рефакторинг для устранения двойной сериализации
export function appendConversation(id: string, role: string, content: string): void { const db = getDb(); - const newMessage = JSON.stringify({ role, content, timestamp: Date.now() }); + const newMessage = { role, content, timestamp: Date.now() }; // Use a transaction with immediate locking to prevent lost updates // under concurrent requests. The read + modify + write is atomic. const append = db.transaction(() => { const session = db.prepare('SELECT conversation FROM sessions WHERE id = ?').get(id) as | { conversation: string } | undefined; if (!session) return; let conversation: Array<{ role: string; content: string; timestamp: number }> = []; try { const parsed = JSON.parse(session.conversation || '[]'); conversation = Array.isArray(parsed) ? parsed : []; } catch { conversation = []; } - conversation.push(JSON.parse(newMessage)); + conversation.push(newMessage); db.prepare('UPDATE sessions SET conversation = ?, updated_at = CURRENT_TIMESTAMP WHERE id = ?') .run(JSON.stringify(conversation), id); }); append.immediate(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/db/queries.ts` around lines 75 - 99, The code currently creates newMessage as a JSON string then immediately JSON.parses it inside appendConversation, causing unnecessary double serialization; change newMessage to be an object (e.g., const newMessage = { role, content, timestamp: Date.now() }) and push that object into conversation instead of JSON.parse(newMessage), keeping the final JSON.stringify(conversation) only once when running the UPDATE; update references in the append transaction (db.prepare SELECT/UPDATE, conversation parsing) accordingly so appendConversation performs a single serialization on write.backend/src/services/analyzer.ts (1)
157-166: Рассмотрите логирование ошибок в фоновой задаче Refero.В режиме
quickошибка Refero полностью подавляется пустым.catch(). Для диагностики рекомендуется хотя бы логировать ошибку.📝 Предлагаемое улучшение
runReferoComparison(pageType, componentInfo, req.screenshot, getAnthropicClient()) .then(result => { if (result) { saveReferoResult(sessionId, result); } }) - .catch(() => { /* Refero failure is non-critical */ }); + .catch((err) => { + console.warn('Background Refero comparison failed:', err); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/services/analyzer.ts` around lines 157 - 166, The background Refero call swallows all errors in an empty .catch(), so update the promise chain around runReferoComparison(pageType, componentInfo, req.screenshot, getAnthropicClient()) to log failures instead of ignoring them; in the .catch(...) call record the error (including context like sessionId and pageType) using the project's logger (or console.error if no logger exists) and still avoid throwing so quick-mode remains non-blocking, and keep the existing saveReferoResult(sessionId, result) behavior in the .then branch.ui/src/lib/messages.ts (1)
204-204: ДобавьтеtokenSummaryв общий типscreenshot-result.
src/ui/message-handler.tsуже кладетtokenSummaryв payload, аui/src/App.tsxпоэтому читает его черезas any. Пока поле не описано вPluginEvent, контракт между main thread и UI остается незащищенным и легко разъедется при следующем рефакторинге.Предлагаемое изменение
- | { type: 'screenshot-result'; data: { nodeId: string; nodeName: string; screenshot: string; width: number; height: number; hasAutoLayout?: boolean; childCount?: number } } + | { type: 'screenshot-result'; data: { nodeId: string; nodeName: string; screenshot: string; width: number; height: number; hasAutoLayout?: boolean; childCount?: number; tokenSummary?: { totalTokens: number; boundToVariables: number; boundToStyles: number; hardCoded: number } } }Лучше вынести этот payload в отдельный интерфейс и переиспользовать его в
ui/src/App.tsx, чтобы убратьas any.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/lib/messages.ts` at line 204, Update the PluginEvent union to include tokenSummary on the 'screenshot-result' payload: add tokenSummary?: <appropriate type> to the screenshot-result variant (or better, define a new interface like ScreenshotResultPayload with nodeId, nodeName, screenshot, width, height, hasAutoLayout?, childCount?, tokenSummary?) and replace the inline payload union member with that interface so ui/src/App.tsx can import/reuse ScreenshotResultPayload instead of casting to any; ensure the same type name is used where App.tsx reads the payload to remove the as any cast.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/.dockerignore`:
- Line 7: В .dockerignore есть запись "dist/" которая исключает артефакты, а
Dockerfile выполняет "COPY dist/ ..." — из-за этого COPY падает; исправьте либо
удалив/закомментировав строку "dist/" в .dockerignore, либо явно разрешив её
копирование добавлением исключения (например, восстанавливая dist в контекст
через исключение), либо изменив Dockerfile чтобы копировать нужные файлы из
текущего контекста после сборки; найдите запись "dist/" в файле .dockerignore и
соответствующий "COPY dist/ ..." в Dockerfile и приведите их в согласие.
In `@backend/src/index.ts`:
- Around line 42-43: The CORS behavior differs between tests and production
because the CORS origin fallback returns an empty string; change the logic in
the CORS handler (the function containing "return allowed.includes(origin) ?
origin : ''") to return the literal string 'null' for disallowed origins so it
matches TEST_PLAN.md and test X-05 expectations; update the same branch used by
buildApp()/your app initialization so both test and prod CORS configurations
produce "Access-Control-Allow-Origin: null" for invalid origins.
In `@backend/src/routes/flow.ts`:
- Around line 24-26: Replace the raw error logging in the catch block that
currently calls console.error('Flow analysis error:', error) so you don't emit
full error objects to prod logs; either pass the error through your redaction
helper (e.g., console.error('Flow analysis error:', redactError(error))) or log
a generic message (e.g., console.error('Flow analysis error: <redacted>')) while
keeping the existing generic response returned via c.json({ error: 'Flow
analysis failed. Please try again.' }, 500).
In `@backend/src/services/flow-analyzer.ts`:
- Around line 102-104: Move the API key existence check before creating the
Anthropic client: instead of calling getAnthropicClient() first, verify
process.env.ANTHROPIC_API_KEY and return null if missing, then call
getAnthropicClient(); update the code around the client initialization in
flow-analyzer.ts (the getAnthropicClient() call and the
process.env.ANTHROPIC_API_KEY check) so the fail-fast check runs first.
In `@backend/TEST_PLAN.md`:
- Around line 18-21: The table-of-contents link fragments for entries like "POST
/api/stream/:sessionId", "GET /api/session/:id", "GET /api/session/:id/refero",
and "POST /api/analyze-flow" are likely not matching the generated markdown
header IDs; update the anchors in TEST_PLAN.md so they exactly match the
automatic slug rules (lowercase, spaces → hyphens, remove/encode punctuation and
colons) or replace them with explicit HTML anchors immediately above the
corresponding headings; verify each link points to the exact header text (or add
explicit <a id="..."> anchors) and adjust the TOC entries to use those IDs.
- Line 745: The "No request body size limit" entry in TEST_PLAN.md is outdated
because the server now registers bodyLimit({ maxSize: 25 * 1024 * 1024 }) in the
app initialization; update TEST_PLAN.md to mark that risk as mitigated (or
remove/annotate the entry) and reference the actual mitigation (the bodyLimit({
maxSize: 25 * 1024 * 1024 }) middleware added to the server startup code) so the
test plan accurately reflects the current codebase.
In `@ui/src/App.tsx`:
- Around line 179-186: The selection-changed handler only sets selectionStale to
true when the new selection differs from analyzedNodeId.current, but never
clears it when the selection returns to the analyzed node; update the case
'selection-changed' logic (where selData is derived and setCurrentNodeName is
called) to also setSelectionStale(false) when selData.nodeId equals
analyzedNodeId.current (and chat.lintResult exists), so the stale banner is
cleared once the selection matches the previously analyzed node.
- Around line 143-149: The case 'batch-fix-v2-result' handler currently calls
chat.handleBatchFixResult and then schedules a setTimeout that adds an
"Re-scanning..." message and calls post('rescan-lint'), causing a duplicate
rescan because src/ui/message-handler.ts already triggers a lint after
batch-fix; remove the setTimeout block (or at minimum remove the
post('rescan-lint') call inside it) so only chat.handleBatchFixResult is relied
on to trigger the rescan and avoid duplicate score-update/issues-list and extra
selection passes.
- Around line 194-202: The refero polling interval is only cleared on unmount
(useEffect cleanup) which allows an old interval to continue when a new analysis
is started; modify the function that starts analysis (e.g., the handler that
triggers Refero/analysis start — locate names like startAnalysis, handleAnalyze,
onAnalyze or startReferoPolling) to proactively stop any existing interval by
checking referoPollingRef.current, calling
clearInterval(referoPollingRef.current) and setting referoPollingRef.current =
null before initiating a new run; also reset any refero-related transient state
(e.g., refero-gallery/suggestions buffers) so results from the old sessionId
cannot leak into the new chat.
In `@ui/src/components/chat/MessageList.tsx`:
- Around line 144-156: The SVG used as a decorative status icon in the
AnalysisPhaseIndicator output (the <svg> with the checkmark/polyline) is missing
aria-hidden="true"; update AnalysisPhaseIndicator to add aria-hidden="true" to
that SVG (and any other purely decorative SVGs in the same component) so screen
readers ignore them, matching the pattern used elsewhere in MessageList (lines
with similar decorative icons).
In `@ui/src/components/messages/ScoreCard.tsx`:
- Around line 61-67: The UI weights in ScoreCard do not match the weights used
to compute designHealthScore in analyzer.ts; update the CategoryBar props in
ScoreCard so their weight values match the backend: set Spacing weight="15%",
Visual Quality weight="10%", Microcopy weight="10%", and Naming weight="5%"
(leave Tokens at 25%, Accessibility 25%, Layout 10% unchanged) so the displayed
weights align with designHealthScore.
---
Outside diff comments:
In `@ui/src/components/chat/StickyHeader.tsx`:
- Around line 45-50: В компоненте StickyHeader добавьте явный доступный ярлык
для кнопки настроек: в JSX кнопки, где используется onClick={onOpenSettings} и
есть title="Settings", добавьте атрибут aria-label="Settings" (или
локализованную строку) чтобы обеспечить доступное имя для вспомогательных
технологий; сохраните существующий title и остальные атрибуты и убедитесь, что
aria-label отражает ту же семантику, что и title.
---
Nitpick comments:
In `@backend/src/db/queries.ts`:
- Around line 75-99: The code currently creates newMessage as a JSON string then
immediately JSON.parses it inside appendConversation, causing unnecessary double
serialization; change newMessage to be an object (e.g., const newMessage = {
role, content, timestamp: Date.now() }) and push that object into conversation
instead of JSON.parse(newMessage), keeping the final
JSON.stringify(conversation) only once when running the UPDATE; update
references in the append transaction (db.prepare SELECT/UPDATE, conversation
parsing) accordingly so appendConversation performs a single serialization on
write.
In `@backend/src/services/analyzer.ts`:
- Around line 157-166: The background Refero call swallows all errors in an
empty .catch(), so update the promise chain around runReferoComparison(pageType,
componentInfo, req.screenshot, getAnthropicClient()) to log failures instead of
ignoring them; in the .catch(...) call record the error (including context like
sessionId and pageType) using the project's logger (or console.error if no
logger exists) and still avoid throwing so quick-mode remains non-blocking, and
keep the existing saveReferoResult(sessionId, result) behavior in the .then
branch.
In `@ui/src/lib/messages.ts`:
- Line 204: Update the PluginEvent union to include tokenSummary on the
'screenshot-result' payload: add tokenSummary?: <appropriate type> to the
screenshot-result variant (or better, define a new interface like
ScreenshotResultPayload with nodeId, nodeName, screenshot, width, height,
hasAutoLayout?, childCount?, tokenSummary?) and replace the inline payload union
member with that interface so ui/src/App.tsx can import/reuse
ScreenshotResultPayload instead of casting to any; ensure the same type name is
used where App.tsx reads the payload to remove the as any cast.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b5135ba6-a808-49e2-ad3e-46effc2dfe6a
⛔ Files ignored due to path filters (3)
dist/code.jsis excluded by!**/dist/**dist/ui-enhanced.htmlis excluded by!**/dist/**dist/ui.htmlis excluded by!**/dist/**
📒 Files selected for processing (38)
.gitignoreAUDIT-REPORT.mdPIPELINE-EVALUATION.mdREADME.mdbackend/.dockerignorebackend/.env.examplebackend/Caddyfilebackend/Dockerfilebackend/TEST_PLAN.mdbackend/docker-compose.ymlbackend/src/db/queries.tsbackend/src/index.tsbackend/src/prompts/chat-followup.tsbackend/src/routes/analyze.tsbackend/src/routes/chat.tsbackend/src/routes/flow.tsbackend/src/routes/stream.tsbackend/src/services/analyzer.tsbackend/src/services/claude.tsbackend/src/services/flow-analyzer.tsbackend/src/services/refero.tssrc/code.tssrc/ui/message-handler.tsui-enhanced.htmlui/src/App.tsxui/src/components/chat/ChatContainer.tsxui/src/components/chat/InputBar.tsxui/src/components/chat/MessageList.tsxui/src/components/chat/StickyHeader.tsxui/src/components/messages/AiReviewCard.tsxui/src/components/messages/FixResult.tsxui/src/components/messages/IssuesList.tsxui/src/components/messages/ScoreCard.tsxui/src/components/shared/SettingsPanel.tsxui/src/hooks/useChat.tsui/src/lib/api.tsui/src/lib/messages.tsui/tailwind.config.js
💤 Files with no reviewable changes (1)
- backend/src/prompts/chat-followup.ts
Comment on lines
+42
to
+43
| // Return the origin if allowed; empty string tells Hono to omit the header | ||
| return allowed.includes(origin) ? origin : ''; |
There was a problem hiding this comment.
Несоответствие CORS-логики между тестами и продакшен-кодом.
В TEST_PLAN.md тестовый buildApp() возвращает 'null' для недопустимых origin (строки 458-459), а продакшен-код возвращает пустую строку. Тест X-05 ожидает Access-Control-Allow-Origin: null, но продакшен опустит заголовок целиком.
Рекомендуется синхронизировать CORS-конфигурацию в тестах с продакшен-кодом.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/index.ts` around lines 42 - 43, The CORS behavior differs between
tests and production because the CORS origin fallback returns an empty string;
change the logic in the CORS handler (the function containing "return
allowed.includes(origin) ? origin : ''") to return the literal string 'null' for
disallowed origins so it matches TEST_PLAN.md and test X-05 expectations; update
the same branch used by buildApp()/your app initialization so both test and prod
CORS configurations produce "Access-Control-Allow-Origin: null" for invalid
origins.
Comment on lines
+18
to
+21
| - [POST /api/stream/:sessionId](#34-post-apistreamssessionid-sse) | ||
| - [GET /api/session/:id](#35-get-apisessionid) | ||
| - [GET /api/session/:id/refero](#36-get-apisessionidrefero) | ||
| - [POST /api/analyze-flow](#37-post-apianalyze-flow) |
There was a problem hiding this comment.
Статический анализ: возможно неверные фрагменты ссылок.
markdownlint предупреждает о потенциально недействительных link fragments в оглавлении (строки 18-21). Рекомендуется проверить, что якоря соответствуют автоматически сгенерированным ID заголовков.
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 18-18: Link fragments should be valid
(MD051, link-fragments)
[warning] 19-19: Link fragments should be valid
(MD051, link-fragments)
[warning] 20-20: Link fragments should be valid
(MD051, link-fragments)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/TEST_PLAN.md` around lines 18 - 21, The table-of-contents link
fragments for entries like "POST /api/stream/:sessionId", "GET
/api/session/:id", "GET /api/session/:id/refero", and "POST /api/analyze-flow"
are likely not matching the generated markdown header IDs; update the anchors in
TEST_PLAN.md so they exactly match the automatic slug rules (lowercase, spaces →
hyphens, remove/encode punctuation and colons) or replace them with explicit
HTML anchors immediately above the corresponding headings; verify each link
points to the exact header text (or add explicit <a id="..."> anchors) and
adjust the TOC entries to use those IDs.
Comment on lines
+61
to
+67
| <CategoryBar label="Tokens" weight="25%" score={data.tokens.score} failed={data.tokens.failed} /> | ||
| <CategoryBar label="Spacing" weight="18%" score={data.spacing.score} failed={data.spacing.failed} /> | ||
| <CategoryBar label="Layout" weight="10%" score={data.layout.score} failed={data.layout.failed} /> | ||
| <CategoryBar label="Accessibility" weight="30%" score={data.accessibility.score} failed={data.accessibility.failed} /> | ||
| <CategoryBar label="Naming" weight="10%" score={data.naming.score} failed={data.naming.failed} /> | ||
| <CategoryBar label="Accessibility" weight="25%" score={data.accessibility.score} failed={data.accessibility.failed} /> | ||
| <CategoryBar label="Naming" weight="7%" score={data.naming.score} failed={data.naming.failed} /> | ||
| <CategoryBar label="Visual Quality" weight="8%" score={data.visualQuality.score} failed={data.visualQuality.failed} /> | ||
| <CategoryBar label="Microcopy" weight="7%" score={data.microcopy.score} failed={data.microcopy.failed} /> |
There was a problem hiding this comment.
Несоответствие весов категорий между UI и backend.
Веса, отображаемые в UI, не соответствуют весам, используемым при расчёте designHealthScore в backend/src/services/analyzer.ts (строки 169, 196-202):
| Категория | UI (ScoreCard) | Backend (analyzer.ts) |
|---|---|---|
| Spacing | 18% | 15% |
| Visual Quality | 8% | 10% |
| Microcopy | 7% | 10% |
| Naming | 7% | 5% |
Это приводит к тому, что пользователь видит одни веса, а score рассчитывается по другим.
🔧 Предлагаемое исправление
- <CategoryBar label="Tokens" weight="25%" score={data.tokens.score} failed={data.tokens.failed} />
- <CategoryBar label="Spacing" weight="18%" score={data.spacing.score} failed={data.spacing.failed} />
+ <CategoryBar label="Tokens" weight="25%" score={data.tokens.score} failed={data.tokens.failed} />
+ <CategoryBar label="Spacing" weight="15%" score={data.spacing.score} failed={data.spacing.failed} />
<CategoryBar label="Layout" weight="10%" score={data.layout.score} failed={data.layout.failed} />
- <CategoryBar label="Accessibility" weight="25%" score={data.accessibility.score} failed={data.accessibility.failed} />
- <CategoryBar label="Naming" weight="7%" score={data.naming.score} failed={data.naming.failed} />
- <CategoryBar label="Visual Quality" weight="8%" score={data.visualQuality.score} failed={data.visualQuality.failed} />
- <CategoryBar label="Microcopy" weight="7%" score={data.microcopy.score} failed={data.microcopy.failed} />
+ <CategoryBar label="Accessibility" weight="25%" score={data.accessibility.score} failed={data.accessibility.failed} />
+ <CategoryBar label="Naming" weight="5%" score={data.naming.score} failed={data.naming.failed} />
+ <CategoryBar label="Visual Quality" weight="10%" score={data.visualQuality.score} failed={data.visualQuality.failed} />
+ <CategoryBar label="Microcopy" weight="10%" score={data.microcopy.score} failed={data.microcopy.failed} />📝 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.
Suggested change
| <CategoryBar label="Tokens" weight="25%" score={data.tokens.score} failed={data.tokens.failed} /> | |
| <CategoryBar label="Spacing" weight="18%" score={data.spacing.score} failed={data.spacing.failed} /> | |
| <CategoryBar label="Layout" weight="10%" score={data.layout.score} failed={data.layout.failed} /> | |
| <CategoryBar label="Accessibility" weight="30%" score={data.accessibility.score} failed={data.accessibility.failed} /> | |
| <CategoryBar label="Naming" weight="10%" score={data.naming.score} failed={data.naming.failed} /> | |
| <CategoryBar label="Accessibility" weight="25%" score={data.accessibility.score} failed={data.accessibility.failed} /> | |
| <CategoryBar label="Naming" weight="7%" score={data.naming.score} failed={data.naming.failed} /> | |
| <CategoryBar label="Visual Quality" weight="8%" score={data.visualQuality.score} failed={data.visualQuality.failed} /> | |
| <CategoryBar label="Microcopy" weight="7%" score={data.microcopy.score} failed={data.microcopy.failed} /> | |
| <CategoryBar label="Tokens" weight="25%" score={data.tokens.score} failed={data.tokens.failed} /> | |
| <CategoryBar label="Spacing" weight="15%" score={data.spacing.score} failed={data.spacing.failed} /> | |
| <CategoryBar label="Layout" weight="10%" score={data.layout.score} failed={data.layout.failed} /> | |
| <CategoryBar label="Accessibility" weight="25%" score={data.accessibility.score} failed={data.accessibility.failed} /> | |
| <CategoryBar label="Naming" weight="5%" score={data.naming.score} failed={data.naming.failed} /> | |
| <CategoryBar label="Visual Quality" weight="10%" score={data.visualQuality.score} failed={data.visualQuality.failed} /> | |
| <CategoryBar label="Microcopy" weight="10%" score={data.microcopy.score} failed={data.microcopy.failed} /> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/components/messages/ScoreCard.tsx` around lines 61 - 67, The UI
weights in ScoreCard do not match the weights used to compute designHealthScore
in analyzer.ts; update the CategoryBar props in ScoreCard so their weight values
match the backend: set Spacing weight="15%", Visual Quality weight="10%",
Microcopy weight="10%", and Naming weight="5%" (leave Tokens at 25%,
Accessibility 25%, Layout 10% unchanged) so the displayed weights align with
designHealthScore.
Critical: - Add missing `history` to fallback chat payload (prevents crash) Bugs: - Exclude BACK-only frames from dead-end detection in flow graph - Return success:false when analyzeFlow() returns null (no API key) Major: - Store lint scope snapshot for consistent re-scans after ignore/jump - Reset to DEFAULT_LINT_SETTINGS before applying team config - Propagate real nodeType from plugin instead of hardcoded 'FRAME' Minor: - Add issue-detail renderer to MessageList switch - Copy schema.sql to dist/ in backend build script Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
ui/src/App.tsx (3)
179-186:⚠️ Potential issue | 🟡 Minor
selectionStaleне сбрасывается при возврате к проанализированному узлу.Флаг
selectionStaleтолько устанавливается вtrue, но не сбрасывается, когда пользователь возвращается кanalyzedNodeId.current. Баннер будет висеть, даже если результаты снова актуальны.🐛 Предлагаемое исправление
case 'selection-changed': { const selData = event.data as { hasSelection: boolean; nodeId: string | null; nodeName: string | null }; setCurrentNodeName(selData.nodeName); - // Mark results as stale if we have results and selection changed to a different node - if (chat.lintResult && analyzedNodeId.current && selData.nodeId !== analyzedNodeId.current) { - setSelectionStale(true); - } + setSelectionStale( + Boolean(chat.lintResult && analyzedNodeId.current && selData.nodeId !== analyzedNodeId.current) + ); break; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/App.tsx` around lines 179 - 186, The selection-stale flag is only set to true in the 'selection-changed' handler and never cleared; update the 'selection-changed' case (look for the switch handling event.type 'selection-changed' in App.tsx) to explicitly clear selectionStale when the new selection matches analyzedNodeId.current — e.g., check selData.nodeId === analyzedNodeId.current and call setSelectionStale(false) (use the existing setSelectionStale setter) while keeping the existing logic that sets it true when chat.lintResult exists and the node differs.
194-202:⚠️ Potential issue | 🟠 MajorPolling Refero не останавливается при запуске нового анализа.
Очистка интервала происходит только при unmount. Если пользователь запустит новый анализ, пока предыдущий polling ещё активен, старые результаты
refero-galleryмогут попасть в новый чат.🐛 Предлагаемое исправление
Извлеките логику очистки в отдельную функцию и вызывайте её в
handleAnalyze:+ const clearReferoPolling = useCallback(() => { + if (referoPollingRef.current) { + clearInterval(referoPollingRef.current); + referoPollingRef.current = null; + } + }, []); + // Clean up Refero polling interval on unmount useEffect(() => { - return () => { - if (referoPollingRef.current) { - clearInterval(referoPollingRef.current); - referoPollingRef.current = null; - } - }; - }, []); + return clearReferoPolling; + }, [clearReferoPolling]); // ... const handleAnalyze = useCallback(() => { + clearReferoPolling(); chat.startAnalysis(); walkthroughIndex.current = 0; setSelectionStale(false); post('run-design-lint'); - }, [chat, post]); + }, [chat, post, clearReferoPolling]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/App.tsx` around lines 194 - 202, The refero polling interval is only cleared on unmount via the useEffect cleanup, so starting a new analysis can leave the old interval running; extract the cleanup into a new helper (e.g., clearReferoPolling) that calls clearInterval(referoPollingRef.current) and sets referoPollingRef.current = null, then replace the inline cleanup in the useEffect with a call to clearReferoPolling and also call clearReferoPolling at the start of handleAnalyze to ensure any previous polling is stopped before starting a new one; reference useEffect, referoPollingRef, clearInterval, and handleAnalyze when making the change.
145-149:⚠️ Potential issue | 🟡 MinorДвойной rescan после batch-fix.
setTimeoutсpost('rescan-lint')вызывает повторное сканирование, еслиsrc/ui/message-handler.tsуже отправляет lint послеbatch-fix-v2-result. Это приведёт к дублирующимся сообщениямscore-update/issues-list.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/App.tsx` around lines 145 - 149, The auto-rescan setTimeout block with post('rescan-lint') in App.tsx is causing duplicate rescans because src/ui/message-handler.ts already triggers a lint rescan on the batch-fix-v2-result event; remove the unconditional setTimeout call (or replace it with a guard/debounce) so only one rescan is triggered: either delete the setTimeout/post('rescan-lint') block in App.tsx or add a boolean flag or short debounce keyed to batch-fix actions (check symbols setTimeout and post('rescan-lint') in App.tsx and batch-fix-v2-result handling in message-handler.ts) to ensure a single rescan/score-update is emitted.
🧹 Nitpick comments (5)
backend/package.json (1)
8-8: Рассмотрите выделение inline-скрипта в отдельный файл для улучшения читаемости.Inline Node.js код в JSON-строке сложно читать и поддерживать. Можно вынести логику копирования в отдельный скрипт.
♻️ Предложенный рефакторинг
Создайте файл
scripts/copy-schema.mjs:import { mkdirSync, copyFileSync } from 'fs'; import { join } from 'path'; mkdirSync(join('dist', 'db'), { recursive: true }); copyFileSync(join('src', 'db', 'schema.sql'), join('dist', 'db', 'schema.sql'));Затем обновите
package.json:- "build": "tsc && node -e \"const fs=require('fs');const p=require('path');fs.mkdirSync(p.join('dist','db'),{recursive:true});fs.copyFileSync(p.join('src','db','schema.sql'),p.join('dist','db','schema.sql'))\"", + "build": "tsc && node scripts/copy-schema.mjs",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/package.json` at line 8, The inline Node.js command in the package.json "build" script is hard to read and maintain; extract that logic into a separate script (e.g., create scripts/copy-schema.mjs containing the mkdir/copy logic) and then update the "build" script to invoke that new module (node scripts/copy-schema.mjs) so the build entry in package.json is concise and the file-copy logic is isolated in scripts/copy-schema.mjs.ui/src/components/chat/MessageList.tsx (1)
137-178: Рассмотрите вынесениеAnalysisPhaseIndicatorв отдельный файл.Компонент
AnalysisPhaseIndicatorвместе с константамиPHASE_LABELSиPHASE_ORDERопределён после экспорта по умолчанию. Для лучшей организации кода и возможности повторного использования рекомендуется:
- Переместить компонент и константы в отдельный файл (например,
ui/src/components/messages/AnalysisPhaseIndicator.tsx)- Или разместить их перед основным компонентом
MessageList🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/chat/MessageList.tsx` around lines 137 - 178, The AnalysisPhaseIndicator component and its related constants PHASE_LABELS and PHASE_ORDER are defined after the default export in MessageList which harms organization and reusability; move them into their own module (e.g., create ui/src/components/messages/AnalysisPhaseIndicator.tsx) exporting AnalysisPhaseIndicator, PHASE_LABELS, and PHASE_ORDER (or at least the component) and then import and use AnalysisPhaseIndicator from MessageList, or alternatively relocate the declarations above the MessageList component so they are defined before the default export; ensure the prop type AnalysisPhase is imported/available and update any imports/exports accordingly.ui/src/App.tsx (2)
460-473: Рассмотрите добавлениеaria-liveдля объявления баннера скринридерам.Баннер об устаревших результатах полезен, но динамически появляющийся контент лучше аннонсировать через
aria-live.♿ Предлагаемое улучшение accessibility
{/* Stale selection banner */} {selectionStale && ( - <div className="flex items-center gap-2 px-3 py-1.5 bg-bg-warning text-fg-warning text-11 border-b border-border"> + <div className="flex items-center gap-2 px-3 py-1.5 bg-bg-warning text-fg-warning text-11 border-b border-border" role="alert" aria-live="polite"> <span className="flex-1">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/App.tsx` around lines 460 - 473, Add an ARIA live region to the stale-selection banner so screen readers announce it when it appears: update the container rendered when selectionStale is true (the div that references selectionStale and currentNodeName and contains the Re-analyze button which calls handleAnalyze) to include appropriate attributes such as aria-live="polite" and role="status" (and aria-atomic="true" if you want the entire message spoken) so the dynamic message about currentNodeName is announced to assistive technologies.
42-48: Уточните тип параметраscreenshotвместо множественныхas any.Параметр
screenshotвtryBackendAnalysis(строка 28) не включает поляnodeType,hasAutoLayout,childCount,tokenSummary, но они используются черезas any. Это снижает типобезопасность и скрывает потенциальные ошибки.♻️ Предлагаемое исправление
Обновите тип параметра в строке 28:
const tryBackendAnalysis = useCallback(async ( lintResult: LintResult, - screenshot: { screenshot: string; nodeId: string; nodeName: string; width: number; height: number } + screenshot: { screenshot: string; nodeId: string; nodeName: string; width: number; height: number; nodeType?: string; hasAutoLayout?: boolean; childCount?: number; tokenSummary?: unknown } ) => {Затем уберите приведения
as any:metadata: { nodeId: screenshot.nodeId, - nodeType: (screenshot as any).nodeType ?? 'FRAME', + nodeType: screenshot.nodeType ?? 'FRAME', width: screenshot.width, height: screenshot.height, - hasAutoLayout: (screenshot as any).hasAutoLayout ?? false, - childCount: (screenshot as any).childCount ?? 0, + hasAutoLayout: screenshot.hasAutoLayout ?? false, + childCount: screenshot.childCount ?? 0, }, - tokenSummary: (screenshot as any).tokenSummary, + tokenSummary: screenshot.tokenSummary,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/App.tsx` around lines 42 - 48, Change the loose casting to a proper typed parameter by updating the type of the screenshot argument in tryBackendAnalysis to include nodeType, width, height, hasAutoLayout, childCount and tokenSummary (or create an interface like ScreenshotWithMeta and use it for the function signature), then remove the multiple "(screenshot as any)" casts in the object construction where nodeType, hasAutoLayout, childCount and tokenSummary are used so the fields are accessed with real typings; keep the existing nullish defaults (?? 'FRAME', ?? false, ?? 0) where needed to preserve behavior.src/ui/message-handler.ts (1)
1013-1035: Повторяющийся post-action код в ignore-handler'ах лучше вынести в helper.Сейчас один и тот же шаблон (
persistIgnoredState(); handleRunDesignLint();) повторяется в нескольких местах. Лучше централизовать, чтобы не словить расхождения при следующем изменении.♻️ Вариант упрощения
+function rerunLintAfterIgnoreMutation(mutator: () => void): void { + mutator(); + persistIgnoredState(); + handleRunDesignLint(); +} + function handleLintIgnoreNode(data: { nodeId: string }): void { - ignoreNode(data.nodeId); - persistIgnoredState(); - handleRunDesignLint(); + rerunLintAfterIgnoreMutation(() => ignoreNode(data.nodeId)); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ui/message-handler.ts` around lines 1013 - 1035, Multiple handlers (handleLintIgnoreNode, handleLintIgnoreError, handleLintIgnoreAllOfType, handleLintClearIgnored) repeat the same post-action calls (persistIgnoredState(); handleRunDesignLint()); extract these into a single helper (e.g., persistIgnoredAndRerun or persistAndRunDesignLint) and call it from each handler instead of duplicating the two lines; ensure the helper preserves the exact call order and is used in handleLintIgnoreNode, handleLintIgnoreError, handleLintIgnoreAllOfType, and handleLintClearIgnored so behavior remains identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/src/routes/flow.ts`:
- Around line 23-25: The handler currently returns a 200 with an internal
message when flow analysis is unavailable; change the if (!result) branch to
return an appropriate error status (prefer 503 Service Unavailable) and replace
the detailed text with a generic message like "Flow analysis service is
currently unavailable". Update the response from c.json(...) to use
c.status(503).json(...) (or c.status(501) if you prefer Not Implemented) and
remove any mention of configuration or API keys to avoid leaking internals;
locate the if (!result) check in the flow route handler in flow.ts to apply this
change.
In `@src/flow/graph-builder.ts`:
- Around line 136-147: The dead-end detection only excludes frames with BACK
edges by checking destinationFrameId === '__BACK__'; update the loop over
validEdges (and the hasBackEdge set) to also treat CLOSE navigation as a
non-dead-end: check edge.navigation === 'CLOSE' (and/or destinationFrameId ===
'__CLOSE__' if that form exists) and add edge.sourceFrameId to the set when
either condition matches; this keeps analyzeFlowGraph's behavior consistent
(which already checks e.navigation === 'BACK' || e.navigation === 'CLOSE') and
prevents CLOSE-linked frames from being classified as dead ends.
In `@src/ui/message-handler.ts`:
- Around line 89-90: The current console.log in message-handler.ts exposes user
payloads via the logData variable (const logData = ...) and
console.log('Received message:', type, logData); — stop logging full payloads
for sensitive event types (e.g., 'chat-message') and only log non-sensitive
metadata (e.g., the event type, timestamps, or a count) or a masked summary;
keep the existing masking for 'save-api-key' but explicitly branch on type ===
'chat-message' (and any other sensitive types) to either omit payload logging or
replace it with a sanitized placeholder before calling console.log or the app
logger.
- Around line 986-1001: handleRescanLint bypasses the preserved lint scope by
calling lintSelection(currentLintSettings) directly, causing different results
after batch-fix; update handleRescanLint to reuse the same scope logic in
handleRunDesignLint (or delegate to it) so lintScopeNodeIds is respected — e.g.,
have handleRescanLint call handleRunDesignLint({ settings: currentLintSettings,
resetScope: false }) or change lintSelection invocation in handleRescanLint to
honor lintScopeNodeIds when present (use the same selection-restoration logic as
handleRunDesignLint) so the fixed scope is applied consistently.
---
Duplicate comments:
In `@ui/src/App.tsx`:
- Around line 179-186: The selection-stale flag is only set to true in the
'selection-changed' handler and never cleared; update the 'selection-changed'
case (look for the switch handling event.type 'selection-changed' in App.tsx) to
explicitly clear selectionStale when the new selection matches
analyzedNodeId.current — e.g., check selData.nodeId === analyzedNodeId.current
and call setSelectionStale(false) (use the existing setSelectionStale setter)
while keeping the existing logic that sets it true when chat.lintResult exists
and the node differs.
- Around line 194-202: The refero polling interval is only cleared on unmount
via the useEffect cleanup, so starting a new analysis can leave the old interval
running; extract the cleanup into a new helper (e.g., clearReferoPolling) that
calls clearInterval(referoPollingRef.current) and sets referoPollingRef.current
= null, then replace the inline cleanup in the useEffect with a call to
clearReferoPolling and also call clearReferoPolling at the start of
handleAnalyze to ensure any previous polling is stopped before starting a new
one; reference useEffect, referoPollingRef, clearInterval, and handleAnalyze
when making the change.
- Around line 145-149: The auto-rescan setTimeout block with post('rescan-lint')
in App.tsx is causing duplicate rescans because src/ui/message-handler.ts
already triggers a lint rescan on the batch-fix-v2-result event; remove the
unconditional setTimeout call (or replace it with a guard/debounce) so only one
rescan is triggered: either delete the setTimeout/post('rescan-lint') block in
App.tsx or add a boolean flag or short debounce keyed to batch-fix actions
(check symbols setTimeout and post('rescan-lint') in App.tsx and
batch-fix-v2-result handling in message-handler.ts) to ensure a single
rescan/score-update is emitted.
---
Nitpick comments:
In `@backend/package.json`:
- Line 8: The inline Node.js command in the package.json "build" script is hard
to read and maintain; extract that logic into a separate script (e.g., create
scripts/copy-schema.mjs containing the mkdir/copy logic) and then update the
"build" script to invoke that new module (node scripts/copy-schema.mjs) so the
build entry in package.json is concise and the file-copy logic is isolated in
scripts/copy-schema.mjs.
In `@src/ui/message-handler.ts`:
- Around line 1013-1035: Multiple handlers (handleLintIgnoreNode,
handleLintIgnoreError, handleLintIgnoreAllOfType, handleLintClearIgnored) repeat
the same post-action calls (persistIgnoredState(); handleRunDesignLint());
extract these into a single helper (e.g., persistIgnoredAndRerun or
persistAndRunDesignLint) and call it from each handler instead of duplicating
the two lines; ensure the helper preserves the exact call order and is used in
handleLintIgnoreNode, handleLintIgnoreError, handleLintIgnoreAllOfType, and
handleLintClearIgnored so behavior remains identical.
In `@ui/src/App.tsx`:
- Around line 460-473: Add an ARIA live region to the stale-selection banner so
screen readers announce it when it appears: update the container rendered when
selectionStale is true (the div that references selectionStale and
currentNodeName and contains the Re-analyze button which calls handleAnalyze) to
include appropriate attributes such as aria-live="polite" and role="status" (and
aria-atomic="true" if you want the entire message spoken) so the dynamic message
about currentNodeName is announced to assistive technologies.
- Around line 42-48: Change the loose casting to a proper typed parameter by
updating the type of the screenshot argument in tryBackendAnalysis to include
nodeType, width, height, hasAutoLayout, childCount and tokenSummary (or create
an interface like ScreenshotWithMeta and use it for the function signature),
then remove the multiple "(screenshot as any)" casts in the object construction
where nodeType, hasAutoLayout, childCount and tokenSummary are used so the
fields are accessed with real typings; keep the existing nullish defaults (??
'FRAME', ?? false, ?? 0) where needed to preserve behavior.
In `@ui/src/components/chat/MessageList.tsx`:
- Around line 137-178: The AnalysisPhaseIndicator component and its related
constants PHASE_LABELS and PHASE_ORDER are defined after the default export in
MessageList which harms organization and reusability; move them into their own
module (e.g., create ui/src/components/messages/AnalysisPhaseIndicator.tsx)
exporting AnalysisPhaseIndicator, PHASE_LABELS, and PHASE_ORDER (or at least the
component) and then import and use AnalysisPhaseIndicator from MessageList, or
alternatively relocate the declarations above the MessageList component so they
are defined before the default export; ensure the prop type AnalysisPhase is
imported/available and update any imports/exports accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 99f4b4ac-3a57-4d09-8677-19640b243f77
⛔ Files ignored due to path filters (2)
dist/code.jsis excluded by!**/dist/**dist/ui.htmlis excluded by!**/dist/**
📒 Files selected for processing (6)
backend/package.jsonbackend/src/routes/flow.tssrc/flow/graph-builder.tssrc/ui/message-handler.tsui/src/App.tsxui/src/components/chat/MessageList.tsx
Comment on lines
+136
to
147
| // Frames that have BACK/CLOSE edges are not truly dead ends | ||
| const hasBackEdge = new Set<string>(); | ||
| for (const edge of validEdges) { | ||
| if (edge.destinationFrameId === '__BACK__') { | ||
| hasBackEdge.add(edge.sourceFrameId); | ||
| } | ||
| } | ||
|
|
||
| // Dead ends: frames with no outgoing connections AND no BACK navigation | ||
| const deadEnds = frames | ||
| .filter(f => (outgoing.get(f.id)?.size || 0) === 0) | ||
| .filter(f => (outgoing.get(f.id)?.size || 0) === 0 && !hasBackEdge.has(f.id)) | ||
| .map(f => f.id); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Проверка текущей обработки BACK/CLOSE в графе
rg -n -C3 --type=ts "collectReactions|action\\.type === 'BACK'|action\\.type === 'CLOSE'|destinationFrameId: '__BACK__'|navigation:\\s*'CLOSE'" src/flow/graph-builder.ts
# Проверка, где вообще создаются FlowEdge с CLOSE
rg -n -C2 --type=ts "navigation:\\s*'CLOSE'|type:\\s*'CLOSE'|destinationFrameId:\\s*'__BACK__'" srcRepository: lemone112/figmalint
Length of output: 1522
🏁 Script executed:
# Check the FlowEdge interface definition
rg -n -B2 -A5 "interface FlowEdge|type FlowEdge" src/flow/graph-builder.ts
# Check the analyzeFlowGraph function (around line 334-336)
sed -n '330,350p' src/flow/graph-builder.ts
# Search entire codebase for CLOSE references (not just in graph-builder)
rg -n "CLOSE|'CLOSE'|\"CLOSE\"" src --type=ts
# Check if there are any Figma API type definitions
fd -e ts -e d.ts | xargs grep -l "type.*CLOSE\|action.*CLOSE" 2>/dev/null | head -5Repository: lemone112/figmalint
Length of output: 1172
🏁 Script executed:
# Check if CLOSE edges are created anywhere (not just from BACK actions)
rg -n "navigation.*CLOSE|navigation:\s*'CLOSE'" src --type=ts
# Look at entire collectReactions function to see all edge creation
sed -n '27,71p' src/flow/graph-builder.ts
# Check if there are other places where FlowEdge is created besides BACK
rg -n "edges\.push" src/flow/graph-builder.ts -B5 -A3
# Search for any mention of Figma close/CLOSE action in documentation or comments
rg -n "close|CLOSE" src/flow/graph-builder.tsRepository: lemone112/figmalint
Length of output: 2951
CLOSE-навигация не исключается из dead-end фреймов.
На строке 136–147 проверяется только destinationFrameId === '__BACK__', но фреймы с исходящей CLOSE-навигацией (из NODE-действий) не исключаются из dead-ends, хотя функция analyzeFlowGraph (строка 335) уже учитывает обе: (e.navigation === 'BACK' || e.navigation === 'CLOSE').
Предлагаемая правка
// Frames that have BACK/CLOSE edges are not truly dead ends
- const hasBackEdge = new Set<string>();
+ const hasBackOrCloseEdge = new Set<string>();
for (const edge of validEdges) {
- if (edge.destinationFrameId === '__BACK__') {
- hasBackEdge.add(edge.sourceFrameId);
+ if (edge.destinationFrameId === '__BACK__' || edge.navigation === 'CLOSE') {
+ hasBackOrCloseEdge.add(edge.sourceFrameId);
}
}
// Dead ends: frames with no outgoing connections AND no BACK navigation
const deadEnds = frames
- .filter(f => (outgoing.get(f.id)?.size || 0) === 0 && !hasBackEdge.has(f.id))
+ .filter(f => (outgoing.get(f.id)?.size || 0) === 0 && !hasBackOrCloseEdge.has(f.id))
.map(f => f.id);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/flow/graph-builder.ts` around lines 136 - 147, The dead-end detection
only excludes frames with BACK edges by checking destinationFrameId ===
'__BACK__'; update the loop over validEdges (and the hasBackEdge set) to also
treat CLOSE navigation as a non-dead-end: check edge.navigation === 'CLOSE'
(and/or destinationFrameId === '__CLOSE__' if that form exists) and add
edge.sourceFrameId to the set when either condition matches; this keeps
analyzeFlowGraph's behavior consistent (which already checks e.navigation ===
'BACK' || e.navigation === 'CLOSE') and prevents CLOSE-linked frames from being
classified as dead ends.
Bugs: - Remove duplicate rescan after batch fix (plugin already rescans) - Clean up analysis-phase indicator on AI failure (prevents stuck state) - Reset selectionStale when user returns to analyzed node Major: - Clear Refero polling on new analysis start (prevents stale injection) - Remove dist/ from .dockerignore (was breaking Docker build) - Move API key check before getAnthropicClient() in flow-analyzer - Align backend score weights with UI (25/18/10/25/7/8/7) Minor: - Add aria-hidden to analysis phase checkmark SVG Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Resolves merge conflicts between our security/UX branch and main's flow analysis features. Adopts 9-category design health scoring (+conversion, +cognitive) across backend, UI hooks, and ScoreCard. Adds flow analysis event types to PluginEvent union. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- flow.ts: return 503 (not 200) when API key missing, redact error logs - message-handler: strip payload from console logs to prevent data leaks - message-handler: handleRescanLint now restores lintScopeNodeIds for consistent re-scan results after batch fixes - StickyHeader: add aria-label="Open settings" on icon button - Rebuild dist artifacts Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
backend/src/services/analyzer.ts (1)
169-207:⚠️ Potential issue | 🟡 MinorОбновить комментарий с весами в соответствии с реализацией
Комментарий содержит устаревшие данные о 7 категориях. Фактический код использует 9 категорий с правильными весами, которые уже синхронизированы с UI-частью проекта.
Рекомендуемое исправление
- // Weights aligned with UI: Tokens 25%, A11y 25%, Spacing 18%, Layout 10%, Naming 7%, Visual 8%, Microcopy 7% + // Weights: Tokens 20%, A11y 20%, Spacing 12%, Visual 10%, Conversion 10%, Microcopy 8%, Cognitive 8%, Layout 8%, Naming 4%🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/services/analyzer.ts` around lines 169 - 207, Update the outdated top comment that lists seven weights to reflect the nine categories and their actual weights used in the computation (as implemented in the designHealthScore expression): tokens 20%, accessibility 20%, spacing 12%, visualQuality 10%, conversion 10%, microcopy 8%, cognitive 8%, layout 8%, naming 4%; locate the comment near SEVERITY_WEIGHT and the severityScore/designHealthScore logic and replace the old sentence with a concise line describing these nine categories and their percentages so the comment matches the code.ui/src/App.tsx (3)
510-518:⚠️ Potential issue | 🟡 MinorКнопке настроек всё ещё не хватает доступного имени.
titleнедостаточен для icon-only controls. Добавьтеaria-labelна сам<button>, иначе screen reader получит безымянную кнопку.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/App.tsx` around lines 510 - 518, The settings button (the button that calls setShowSettings(true) and renders the gear SVG) lacks an accessible name: add an aria-label attribute to the same <button> (e.g., aria-label="Settings" or a localized equivalent) so screen readers receive a clear name for this icon-only control; keep the existing title if desired but ensure aria-label is present on that button element in App.tsx.
268-287:⚠️ Potential issue | 🟠 MajorНе отправляйте
chat.messagesв plugin как history без преобразования.
chat.messages— это UI-структуры изui/src/lib/messages.ts({ id, timestamp, message }), а plugin-sidehandleChatMessage()ожидает элементы сrole/contentи сразу читаетmsg.roleиmsg.contentвcreateChatPromptWithContext(). В fallback-режиме история сейчас уходит в неправильном формате, и контекст чата деградирует.Минимальный вариант преобразования
- post('chat-message', { message: text, history: chat.messages }); + const history = chat.messages.flatMap((m) => { + if (m.message.kind === 'user-text') { + return [{ + id: m.id, + role: 'user' as const, + content: m.message.content, + timestamp: m.timestamp, + }]; + } + if (m.message.kind === 'ai-text' && !m.message.streaming) { + return [{ + id: m.id, + role: 'assistant' as const, + content: m.message.content, + timestamp: m.timestamp, + }]; + } + return []; + }); + post('chat-message', { message: text, history });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/App.tsx` around lines 268 - 287, The fallback branch in handleSendMessage is sending UI message objects (chat.messages) directly to post('chat-message'), but the plugin's handleChatMessage/createChatPromptWithContext expect messages with {role, content}; convert chat.messages to that shape before posting: map UI kinds (e.g., 'user-text' -> role: 'user', 'ai-text' -> role: 'assistant', any system/metadata -> 'system' or appropriate role) and set content from the UI message text field, then call post('chat-message', { message: text, history: transformedHistory }); ensure this transformation happens in the else branch where post('chat-message') is called so plugin receives role/content pairs.
153-160:⚠️ Potential issue | 🟠 MajorСвяжите
screenshot-resultс конкретным запуском анализа.Сейчас любой пришедший screenshot сразу склеивается с текущим
pendingLintResult.current. Если поздний ответ от предыдущегоexport-screenshotдоедет после нового Analyze, backend получит mixed payload из двух разных selection.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/App.tsx` around lines 153 - 160, The screenshot-result must be associated with the specific analysis run instead of blindly pairing any pendingScreenshot.current with pendingLintResult.current; modify the event handling around pendingScreenshot.current / pendingLintResult.current and tryBackendAnalysis so each pending item carries a runId (or unique request id) and you only call tryBackendAnalysis when the runId on pendingScreenshot.current matches the runId on pendingLintResult.current; drop or ignore screenshots whose runId does not match the current pending lint result (or vice versa) to prevent mixing payloads from different exports.
♻️ Duplicate comments (2)
src/ui/message-handler.ts (2)
989-999:⚠️ Potential issue | 🟠 MajorВынесите восстановление
lintScopeNodeIdsв общий helper.Сейчас fixed scope живёт только в этом пути.
handleRescanLint()иhandleLintIgnoreAllOfType()всё ещё идут вlintSelection(...)напрямую, поэтому после batch-fix/ignore-all результаты снова расходятся с исходным scope.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ui/message-handler.ts` around lines 989 - 999, The current code restores the selection/lint scope inline using lintScopeNodeIds, causing other flows (handleRescanLint, handleLintIgnoreAllOfType) that call lintSelection(...) to miss that restoration; extract the restore logic into a shared helper (e.g., restoreLintScope or applyLintScope) that takes lintScopeNodeIds and sets figma.currentPage.selection after mapping/filtering with figma.getNodeById and SceneNode/type checks, then replace the inline block in message-handler.ts with a call to that helper and update handleRescanLint and handleLintIgnoreAllOfType to call the same helper before or after invoking lintSelection(...) so all paths consistently restore the fixed scope.
90-91:⚠️ Potential issue | 🟠 MajorНе логируйте весь payload входящих UI-сообщений.
Здесь по-прежнему утекают
chat-message(текст + history), screenshot payload-ы и прочие пользовательские данные. Оставьте в логах толькоtypeи безопасный allowlist метаданных.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ui/message-handler.ts` around lines 90 - 91, The current console.log in message handler exposes full incoming payload via logData (const logData = ... and console.log('Received message:', type, logData)); change it to only log the message type and a safe allowlist of metadata fields (e.g., messageId, userId, timestamp) and explicitly omit or redact sensitive payloads like chat-message text, chat history, screenshot/image blobs and any `payload`/`data` contents; implement a small helper (e.g., buildSafeLogMeta(type, data)) used in place of logData so only allowlisted keys are output and everything else is removed or replaced with a fixed placeholder like "[REDACTED]".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ui/src/App.tsx`:
- Line 32: В App.tsx добавляется только одно сообщение chat.addMessage({ kind:
'analysis-phase', phase: 'ai-review' }), поэтому индикатор проходит лишь фазу
ai-review; нужно эмитить все четыре фазы (lint, screenshot, refero, ai-review) в
правиль порядке перед запуском обработки и/или изменить логику в
useChat.handleAiReview так, чтобы он не удалял все messages с
kind==='analysis-phase' целиком до завершения всех фаз; обновите места вызова
chat.addMessage (в App.tsx) чтобы создать по одному сообщению для каждой фазы
(phase: 'lint'|'screenshot'|'refero'|'ai-review') или измените handleAiReview
чтобы помечать фазы как завершённые по-отдельности, чтобы AnalysisPhaseIndicator
видел полный end-to-end прогресс.
In `@ui/src/components/chat/MessageList.tsx`:
- Around line 156-158: The isComplete logic marks future phases complete when
done is true; change it so only previous phases (i < currentIdx) and the current
phase (i === currentIdx) become complete when done is true. Update the
isComplete computation used in MessageList (referencing variables i, currentIdx,
done and the computed isActive/isPending) to: isComplete should be true when i <
currentIdx OR when i === currentIdx && done, leaving isActive as i ===
currentIdx && !done and isPending as i > currentIdx && !done.
In `@ui/src/hooks/useChat.ts`:
- Around line 453-454: computeScoreBreakdown() already counts conversion and
cognitive issues but the UI string builds only visualQuality and microcopy, so
extend the parts assembly in useChat (where parts is populated) to also push
entries for byType.conversion and byType.cognitive when their counts are > 0;
update the same block that currently checks byType.visualQuality and
byType.microcopy to include checks for byType.conversion and byType.cognitive
and format them like the others (e.g., `${byType.conversion} conversion issues`,
`${byType.cognitive} cognitive issues`) so the "Found N issues ..." string
includes those categories.
In `@ui/src/lib/messages.ts`:
- Line 207: Расширьте контракт union-посылки для варианта 'screenshot-result' в
ui/src/lib/messages.ts: добавьте в объект payload явные поля nodeType: string и
tokenSummary: Record<string, any> (или более конкретную структуру, если
известна), чтобы соответствовать тому, что отправляет src/ui/message-handler.ts
и что читает ui/src/App.tsx; обновите типизацию там, где используется этот
вариант, чтобы больше не требовался приведение через any.
---
Outside diff comments:
In `@backend/src/services/analyzer.ts`:
- Around line 169-207: Update the outdated top comment that lists seven weights
to reflect the nine categories and their actual weights used in the computation
(as implemented in the designHealthScore expression): tokens 20%, accessibility
20%, spacing 12%, visualQuality 10%, conversion 10%, microcopy 8%, cognitive 8%,
layout 8%, naming 4%; locate the comment near SEVERITY_WEIGHT and the
severityScore/designHealthScore logic and replace the old sentence with a
concise line describing these nine categories and their percentages so the
comment matches the code.
In `@ui/src/App.tsx`:
- Around line 510-518: The settings button (the button that calls
setShowSettings(true) and renders the gear SVG) lacks an accessible name: add an
aria-label attribute to the same <button> (e.g., aria-label="Settings" or a
localized equivalent) so screen readers receive a clear name for this icon-only
control; keep the existing title if desired but ensure aria-label is present on
that button element in App.tsx.
- Around line 268-287: The fallback branch in handleSendMessage is sending UI
message objects (chat.messages) directly to post('chat-message'), but the
plugin's handleChatMessage/createChatPromptWithContext expect messages with
{role, content}; convert chat.messages to that shape before posting: map UI
kinds (e.g., 'user-text' -> role: 'user', 'ai-text' -> role: 'assistant', any
system/metadata -> 'system' or appropriate role) and set content from the UI
message text field, then call post('chat-message', { message: text, history:
transformedHistory }); ensure this transformation happens in the else branch
where post('chat-message') is called so plugin receives role/content pairs.
- Around line 153-160: The screenshot-result must be associated with the
specific analysis run instead of blindly pairing any pendingScreenshot.current
with pendingLintResult.current; modify the event handling around
pendingScreenshot.current / pendingLintResult.current and tryBackendAnalysis so
each pending item carries a runId (or unique request id) and you only call
tryBackendAnalysis when the runId on pendingScreenshot.current matches the runId
on pendingLintResult.current; drop or ignore screenshots whose runId does not
match the current pending lint result (or vice versa) to prevent mixing payloads
from different exports.
---
Duplicate comments:
In `@src/ui/message-handler.ts`:
- Around line 989-999: The current code restores the selection/lint scope inline
using lintScopeNodeIds, causing other flows (handleRescanLint,
handleLintIgnoreAllOfType) that call lintSelection(...) to miss that
restoration; extract the restore logic into a shared helper (e.g.,
restoreLintScope or applyLintScope) that takes lintScopeNodeIds and sets
figma.currentPage.selection after mapping/filtering with figma.getNodeById and
SceneNode/type checks, then replace the inline block in message-handler.ts with
a call to that helper and update handleRescanLint and handleLintIgnoreAllOfType
to call the same helper before or after invoking lintSelection(...) so all paths
consistently restore the fixed scope.
- Around line 90-91: The current console.log in message handler exposes full
incoming payload via logData (const logData = ... and console.log('Received
message:', type, logData)); change it to only log the message type and a safe
allowlist of metadata fields (e.g., messageId, userId, timestamp) and explicitly
omit or redact sensitive payloads like chat-message text, chat history,
screenshot/image blobs and any `payload`/`data` contents; implement a small
helper (e.g., buildSafeLogMeta(type, data)) used in place of logData so only
allowlisted keys are output and everything else is removed or replaced with a
fixed placeholder like "[REDACTED]".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9fc10db6-dc8b-480e-bd85-69714ab81861
⛔ Files ignored due to path filters (2)
dist/code.jsis excluded by!**/dist/**dist/ui.htmlis excluded by!**/dist/**
📒 Files selected for processing (10)
backend/.dockerignorebackend/src/services/analyzer.tsbackend/src/services/flow-analyzer.tssrc/ui/message-handler.tsui/src/App.tsxui/src/components/chat/MessageList.tsxui/src/components/messages/ScoreCard.tsxui/src/hooks/useChat.tsui/src/lib/api.tsui/src/lib/messages.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- ui/src/components/messages/ScoreCard.tsx
- backend/.dockerignore
- ui/src/lib/api.ts
Comment on lines
+453
to
+454
| if (byType.visualQuality > 0) parts.push(`${byType.visualQuality} visual quality issues`); | ||
| if (byType.microcopy > 0) parts.push(`${byType.microcopy} microcopy issues`); |
There was a problem hiding this comment.
Сводка lint всё ещё теряет conversion и cognitive.
computeScoreBreakdown() уже учитывает эти категории, но здесь parts пополняется только visualQuality и microcopy. Если ошибки будут только в новых bucket-ах, пользователь увидит пустое Found N issues ... : ..
Минимальная правка
if (byType.visualQuality > 0) parts.push(`${byType.visualQuality} visual quality issues`);
if (byType.microcopy > 0) parts.push(`${byType.microcopy} microcopy issues`);
+ if (byType.conversion > 0) parts.push(`${byType.conversion} conversion issues`);
+ if (byType.cognitive > 0) parts.push(`${byType.cognitive} cognitive load issues`);📝 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.
Suggested change
| if (byType.visualQuality > 0) parts.push(`${byType.visualQuality} visual quality issues`); | |
| if (byType.microcopy > 0) parts.push(`${byType.microcopy} microcopy issues`); | |
| if (byType.visualQuality > 0) parts.push(`${byType.visualQuality} visual quality issues`); | |
| if (byType.microcopy > 0) parts.push(`${byType.microcopy} microcopy issues`); | |
| if (byType.conversion > 0) parts.push(`${byType.conversion} conversion issues`); | |
| if (byType.cognitive > 0) parts.push(`${byType.cognitive} cognitive load issues`); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/hooks/useChat.ts` around lines 453 - 454, computeScoreBreakdown()
already counts conversion and cognitive issues but the UI string builds only
visualQuality and microcopy, so extend the parts assembly in useChat (where
parts is populated) to also push entries for byType.conversion and
byType.cognitive when their counts are > 0; update the same block that currently
checks byType.visualQuality and byType.microcopy to include checks for
byType.conversion and byType.cognitive and format them like the others (e.g.,
`${byType.conversion} conversion issues`, `${byType.cognitive} cognitive
issues`) so the "Found N issues ..." string includes those categories.
| | { type: 'api-key-saved'; data: { success: boolean } } | ||
| | { type: 'screenshot-result'; data: { nodeId: string; nodeName: string; screenshot: string; width: number; height: number } } | ||
| | { type: string; data: unknown }; | ||
| | { type: 'screenshot-result'; data: { nodeId: string; nodeName: string; screenshot: string; width: number; height: number; hasAutoLayout?: boolean; childCount?: number } } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Дополните контракт screenshot-result реальными полями payload.
src/ui/message-handler.ts уже отправляет сюда nodeType и tokenSummary, а ui/src/App.tsx из-за этого читает payload через as any. Пока union их не описывает, граница Plugin ↔ UI теряет type-safety, и следующая регрессия здесь пройдёт мимо компилятора.
Минимальный вариант правки
- | { type: 'screenshot-result'; data: { nodeId: string; nodeName: string; screenshot: string; width: number; height: number; hasAutoLayout?: boolean; childCount?: number } }
+ | {
+ type: 'screenshot-result';
+ data: {
+ nodeId: string;
+ nodeName: string;
+ nodeType: string;
+ screenshot: string;
+ width: number;
+ height: number;
+ hasAutoLayout?: boolean;
+ childCount?: number;
+ tokenSummary?: {
+ totalTokens: number;
+ boundToVariables: number;
+ boundToStyles: number;
+ hardCoded: number;
+ };
+ };
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/lib/messages.ts` at line 207, Расширьте контракт union-посылки для
варианта 'screenshot-result' в ui/src/lib/messages.ts: добавьте в объект payload
явные поля nodeType: string и tokenSummary: Record<string, any> (или более
конкретную структуру, если известна), чтобы соответствовать тому, что отправляет
src/ui/message-handler.ts и что читает ui/src/App.tsx; обновите типизацию там,
где используется этот вариант, чтобы больше не требовался приведение через any.
Beyond the original Sprint 0-2 plan (fully completed), this adds: Security: - Bearer auth middleware (BACKEND_AUTH_TOKEN env var) - In-memory rate limiter per IP (60 req/min default) - AbortController on streaming chat (cancel previous on new request) - MCP client timeout wrappers (15s Promise.race) Architecture: - Merge src/fixes/ into src/fix/ (single directory) - Include lint settings in consistency engine cache hash - Settings panel → proper dialog (role, aria-modal, focus trap, Escape) - aria-live="polite" + role="log" on message list Infrastructure: - GitHub Actions CI/CD (typecheck + build + Docker) - esbuild --minify (405KB → 212KB, -48%) - DB indexes on node_id, created_at, updated_at - Session TTL cleanup (7d, runs on startup + every 6h) - .env.example updated with auth + rate limit docs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
ui/src/components/chat/MessageList.tsx (1)
156-158:⚠️ Potential issue | 🟡 MinorЛогика
isCompleteпомечает будущие фазы как завершённые.Когда
done=true, выражениеi < currentIdx || doneбудет истинно для всех фаз, включая те, которые ещё не выполнялись. Например, еслиphase='lint'иdone=true, фазыscreenshot,ai-reviewиreferoтакже отобразятся как завершённые.🐛 Предлагаемое исправление
const isActive = i === currentIdx && !done; - const isComplete = i < currentIdx || done; - const isPending = i > currentIdx && !done; + const isComplete = i < currentIdx || (done && i === currentIdx); + const isPending = i > currentIdx;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/chat/MessageList.tsx` around lines 156 - 158, The isComplete expression currently uses "i < currentIdx || done" which marks all phases complete when done is true; change it to "i < currentIdx || (done && i <= currentIdx)" so only phases before currentIdx or the current phase when done is true are treated as complete; update the variable in MessageList.tsx where isComplete is defined (referencing isActive, isComplete, isPending, currentIdx, done, and loop index i).
🧹 Nitpick comments (5)
backend/src/mcp/design-systems-client.ts (1)
15-43: Опционально: рассмотреть создание фабричной функции для MCP-клиентов.Структура
getDesignSystemsClientиgetReferoClient(вclient.ts) практически идентична. При добавлении новых MCP-серверов имеет смысл выделить общую логику в фабрикуcreateMcpClientFactory(url, name).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/mcp/design-systems-client.ts` around lines 15 - 43, Extract the duplicated connection logic from getDesignSystemsClient and getReferoClient into a factory function createMcpClientFactory(url, name) that returns an async getter; the factory should create a Client with provided name/version, use StreamableHTTPClientTransport(new URL(url)), call withTimeout(client.connect(transport), 15_000, '... connect') and manage shared state (cached client, connectionFailed flag, and retry setTimeout) exactly as getDesignSystemsClient does; replace getDesignSystemsClient/getReferoClient bodies to call the factory-produced getter and pass the corresponding DESIGN_SYSTEMS_MCP_URL and the service name so both functions delegate to the single implementation.backend/src/mcp/parse-tool-result.ts (1)
5-16: Утечка таймера:setTimeoutне очищается при успешном завершении промиса.Когда исходный промис разрешается раньше таймаута,
setTimeoutпродолжает висеть в очереди событий до истеченияms. При частых вызовах это приводит к накоплению "мёртвых" таймеров. Рекомендуется очищать таймер при завершении промиса.♻️ Предлагаемое исправление с очисткой таймера
export function withTimeout<T>( promise: Promise<T>, ms = 15_000, label = 'MCP call', ): Promise<T> { + let timeoutId: ReturnType<typeof setTimeout>; + const timeoutPromise = new Promise<never>((_, reject) => { + timeoutId = setTimeout(() => reject(new Error(`${label} timed out after ${ms}ms`)), ms); + }); + return Promise.race([ - promise, - new Promise<never>((_, reject) => - setTimeout(() => reject(new Error(`${label} timed out after ${ms}ms`)), ms), - ), - ]); + promise.finally(() => clearTimeout(timeoutId)), + timeoutPromise, + ]) as Promise<T>; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/mcp/parse-tool-result.ts` around lines 5 - 16, The withTimeout function creates a timer with setTimeout that is never cleared when the input promise settles; modify withTimeout to store the timer id (from setTimeout) and ensure clearTimeout(timer) is invoked once either the original promise resolves or rejects so the timer is not left pending. Specifically, change Promise.race to create the timeout via a let timer = setTimeout(...), then attach handlers (e.g., wrap the original promise with .then/.catch or use finally) to clearTimeout(timer) before resolving/rejecting so the timer is always cleared; references: withTimeout, the setTimeout-created timer and the Promise.race wrapper.backend/src/mcp/design-systems-tools.ts (1)
157-164: Опционально:extractTextдублирует часть логикиparseToolResult.Функция
extractTextизвлекает текст из MCP-ответа аналогично началуparseToolResult. Можно переиспользовать общую логику, вынеся извлечениеtextBlockв отдельный хелпер или параметризовавparseToolResult.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/mcp/design-systems-tools.ts` around lines 157 - 164, extractText duplicates the initial extraction logic in parseToolResult; refactor to remove duplication by extracting the shared logic into a small helper (e.g., getTextBlock or getContentArray) or by making parseToolResult accept a flag/utility to return the textBlock, then have extractText call that helper; update references to use the helper and remove the duplicated find(c => c.type === 'text') logic so extractText simply delegates to the new helper (referencing extractText, parseToolResult, and the textBlock lookup)..github/workflows/ci.yml (1)
59-59: Сделайте docker-job зависимым от всех проверок, а не только от backend.Сейчас
dockerна Line 59 стартует послеbackend, даже еслиpluginилиuiуже упали. Рекомендуется дождаться всех quality gates.Предлагаемое изменение
- needs: [backend] + needs: [plugin, ui, backend]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml at line 59, В job "docker" в .github/workflows/ci.yml текущая зависимость указана только как needs: [backend]; обновите ключ needs для job'а docker так, чтобы он перечислял все quality-gate job'ы (например needs: [backend, plugin, ui]) — найдите блок с именем/идентификатором docker и измените массив в поле needs, чтобы дождаться успешного завершения всех перечисленных задач перед запуском docker.backend/src/middleware/rate-limit.ts (1)
29-31: Рассмотрите использование встроенных механизмов Hono для определения IP вместо прямого чтения заголовков.Текущая реализация опирается на X-Forwarded-For/X-Real-IP, что работает с Caddy, но создаёт скрытые зависимости от конфигурации reverse proxy. Лучше использовать
ConnInfoиз@hono/node-server/conninfoдля получения реального IP клиента или встроенноеipRestrictionmiddleware Hono. Это сделает код более явным и защитит от случайного деплоя без reverse proxy, когда все запросы попадут в bucketunknown.Текущая архитектура (Caddy → backend) защищена от подделки IP, поскольку Caddy по умолчанию генерирует X-Forwarded-For на основе фактического подключения и игнорирует значения, отправленные клиентом.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/middleware/rate-limit.ts` around lines 29 - 31, Replace the manual header parsing that creates the "const ip = ..." value with Hono's supported client-IP mechanisms: import and use ConnInfo from "@hono/node-server/conninfo" to derive the real client address (replace the "const ip = ..." assignment in the rate-limit middleware) or alternatively switch to Hono's built-in ipRestriction middleware; update the rate-limit middleware function (the code that computes/uses the ip variable and any bucket lookup keys) to obtain the client IP via ConnInfo (or ipRestriction) instead of reading x-forwarded-for/x-real-ip and remove the fallback to 'unknown'.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 14-16: В файле .github/workflows/ci.yml замените все плавающие
теги в ключах uses (например actions/checkout@v4, actions/setup-node@v4 и
остальные occurrences с `@v3/`@v6) на фиксированные полные commit SHA для каждого
action; откройте каждую строку с uses, найдите соответствующий action reference
(например actions/checkout, actions/setup-node и т.д.), заменить '@vX' на
'@<full-commit-sha>' и при желании добавить комментарий с соответствующей
семантической версией (например "# v4") для ясности.
In `@backend/src/db/queries.ts`:
- Around line 106-109: The cleanupExpiredSessions function is deleting rows by
comparing updated_at (stored as SQLite CURRENT_TIMESTAMP "YYYY-MM-DD HH:MM:SS")
against a cutoff produced with toISOString("YYYY-MM-DDTHH:MM:SS.sssZ"), causing
incorrect lexical comparisons; fix cleanupExpiredSessions by producing a cutoff
string in the same format as SQLite CURRENT_TIMESTAMP (YYYY-MM-DD HH:MM:SS) —
e.g., create the cutoff Date, convert it to ISO, replace the 'T' with a space
and strip milliseconds and trailing 'Z' (or format via Date methods) and then
pass that normalized cutoff to db.prepare('DELETE FROM sessions WHERE updated_at
< ?').run(cutoff) so comparisons are correct.
In `@backend/src/index.ts`:
- Around line 35-56: The middleware order causes CORS preflight to be rejected
and unauthenticated floods to bypass rate limiting; reorder the middleware so
CORS runs first, then rateLimit(), then bearerAuth(), and keep bodyLimit() after
auth/body parsing where appropriate. Concretely, move the cors(...) app.use('*',
cors(...)) above the '/api/*' middlewares, ensure app.use('/api/*', rateLimit())
is registered before app.use('/api/*', bearerAuth()), and place
app.use('/api/*', bodyLimit(...)) after authentication/limits (or at least after
cors) so preflight OPTIONS isn’t blocked by bearerAuth() and rate limiting
applies to unauthenticated requests.
In `@backend/src/middleware/auth.ts`:
- Around line 10-12: The middleware currently fail-opens when
process.env.BACKEND_AUTH_TOKEN is empty (the token variable in
backend/src/middleware/auth.ts), which makes /api/* public if the env is
forgotten; change this to fail-closed by validating configuration at startup and
only allowing auth to be disabled via an explicit dev flag. Concretely: add a
startup config check that reads BACKEND_AUTH_TOKEN and a clear opt-out (e.g.
NODE_ENV === "development" or a BACKEND_AUTH_DISABLED=true flag), throw an error
and abort start if BACKEND_AUTH_TOKEN is missing in non-dev environments, and
update the auth middleware (the code path that currently does if (!token) return
next()) to treat a missing token as unauthorized (return 401/forbidden) rather
than bypassing auth unless the explicit dev flag was set and validated at
startup.
In `@ui/src/App.tsx`:
- Line 33: Нужно эмитить все четыре фазы анализа вместо только 'ai-review':
добавьте вызовы chat.addMessage({ kind: 'analysis-phase', phase: 'lint' }) в
начало функции handleRunDesignLint, chat.addMessage({ kind: 'analysis-phase',
phase: 'screenshot' }) в том месте, где инициируется запрос скриншота (тот же
обработчик/функция, которая вызывает получение скриншота), оставьте существующий
chat.addMessage({ kind: 'analysis-phase', phase: 'ai-review' }) при отправке на
бэкенд, и добавьте chat.addMessage({ kind: 'analysis-phase', phase: 'refero' })
при получении Refero-данных или при завершении polling; это обеспечит, что
компонент AnalysisPhaseIndicator увидит последовательность фаз lint → screenshot
→ ai-review → refero.
---
Duplicate comments:
In `@ui/src/components/chat/MessageList.tsx`:
- Around line 156-158: The isComplete expression currently uses "i < currentIdx
|| done" which marks all phases complete when done is true; change it to "i <
currentIdx || (done && i <= currentIdx)" so only phases before currentIdx or the
current phase when done is true are treated as complete; update the variable in
MessageList.tsx where isComplete is defined (referencing isActive, isComplete,
isPending, currentIdx, done, and loop index i).
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Line 59: В job "docker" в .github/workflows/ci.yml текущая зависимость указана
только как needs: [backend]; обновите ключ needs для job'а docker так, чтобы он
перечислял все quality-gate job'ы (например needs: [backend, plugin, ui]) —
найдите блок с именем/идентификатором docker и измените массив в поле needs,
чтобы дождаться успешного завершения всех перечисленных задач перед запуском
docker.
In `@backend/src/mcp/design-systems-client.ts`:
- Around line 15-43: Extract the duplicated connection logic from
getDesignSystemsClient and getReferoClient into a factory function
createMcpClientFactory(url, name) that returns an async getter; the factory
should create a Client with provided name/version, use
StreamableHTTPClientTransport(new URL(url)), call
withTimeout(client.connect(transport), 15_000, '... connect') and manage shared
state (cached client, connectionFailed flag, and retry setTimeout) exactly as
getDesignSystemsClient does; replace getDesignSystemsClient/getReferoClient
bodies to call the factory-produced getter and pass the corresponding
DESIGN_SYSTEMS_MCP_URL and the service name so both functions delegate to the
single implementation.
In `@backend/src/mcp/design-systems-tools.ts`:
- Around line 157-164: extractText duplicates the initial extraction logic in
parseToolResult; refactor to remove duplication by extracting the shared logic
into a small helper (e.g., getTextBlock or getContentArray) or by making
parseToolResult accept a flag/utility to return the textBlock, then have
extractText call that helper; update references to use the helper and remove the
duplicated find(c => c.type === 'text') logic so extractText simply delegates to
the new helper (referencing extractText, parseToolResult, and the textBlock
lookup).
In `@backend/src/mcp/parse-tool-result.ts`:
- Around line 5-16: The withTimeout function creates a timer with setTimeout
that is never cleared when the input promise settles; modify withTimeout to
store the timer id (from setTimeout) and ensure clearTimeout(timer) is invoked
once either the original promise resolves or rejects so the timer is not left
pending. Specifically, change Promise.race to create the timeout via a let timer
= setTimeout(...), then attach handlers (e.g., wrap the original promise with
.then/.catch or use finally) to clearTimeout(timer) before resolving/rejecting
so the timer is always cleared; references: withTimeout, the setTimeout-created
timer and the Promise.race wrapper.
In `@backend/src/middleware/rate-limit.ts`:
- Around line 29-31: Replace the manual header parsing that creates the "const
ip = ..." value with Hono's supported client-IP mechanisms: import and use
ConnInfo from "@hono/node-server/conninfo" to derive the real client address
(replace the "const ip = ..." assignment in the rate-limit middleware) or
alternatively switch to Hono's built-in ipRestriction middleware; update the
rate-limit middleware function (the code that computes/uses the ip variable and
any bucket lookup keys) to obtain the client IP via ConnInfo (or ipRestriction)
instead of reading x-forwarded-for/x-real-ip and remove the fallback to
'unknown'.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 55414141-4984-417b-ada0-70d9c814698f
⛔ Files ignored due to path filters (2)
dist/code.jsis excluded by!**/dist/**dist/ui.htmlis excluded by!**/dist/**
📒 Files selected for processing (23)
.github/workflows/ci.ymlbackend/.env.examplebackend/src/db/queries.tsbackend/src/db/schema.sqlbackend/src/index.tsbackend/src/mcp/client.tsbackend/src/mcp/design-systems-client.tsbackend/src/mcp/design-systems-tools.tsbackend/src/mcp/parse-tool-result.tsbackend/src/mcp/refero-tools.tsbackend/src/middleware/auth.tsbackend/src/middleware/rate-limit.tspackage.jsonsrc/core/component-analyzer.tssrc/core/consistency-engine.tssrc/fix/color-distance.tssrc/fix/naming-fixer.tssrc/fix/token-fixer.tssrc/ui/message-handler.tsui/src/App.tsxui/src/components/chat/MessageList.tsxui/src/components/shared/SettingsPanel.tsxui/src/lib/api.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/.env.example
- ui/src/lib/api.ts
Comment on lines
+10
to
+12
| const token = process.env.BACKEND_AUTH_TOKEN; | ||
| // If no token configured, auth is disabled (development mode) | ||
| if (!token) return next(); |
There was a problem hiding this comment.
Аутентификация сейчас fail-open при пустом BACKEND_AUTH_TOKEN.
Если переменную забыли задать в проде, весь /api/* становится публичным. Лучше валидировать конфиг на старте и разрешать отключение auth только через явный dev-флаг, а не через отсутствие секрета.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/middleware/auth.ts` around lines 10 - 12, The middleware
currently fail-opens when process.env.BACKEND_AUTH_TOKEN is empty (the token
variable in backend/src/middleware/auth.ts), which makes /api/* public if the
env is forgotten; change this to fail-closed by validating configuration at
startup and only allowing auth to be disabled via an explicit dev flag.
Concretely: add a startup config check that reads BACKEND_AUTH_TOKEN and a clear
opt-out (e.g. NODE_ENV === "development" or a BACKEND_AUTH_DISABLED=true flag),
throw an error and abort start if BACKEND_AUTH_TOKEN is missing in non-dev
environments, and update the auth middleware (the code path that currently does
if (!token) return next()) to treat a missing token as unauthorized (return
401/forbidden) rather than bypassing auth unless the explicit dev flag was set
and validated at startup.
Sprint 5 (Baseline & Diff): - Baseline snapshot storage with LZ-string compression + chunked pluginData - Diff engine comparing current scan vs saved baseline - DiffCard UI with score delta, per-category was→now, new/fixed/remaining - Trend indicator (arrow + delta) in StickyHeader - Save Baseline / Diff buttons in QuickActions bar - Export reports enhanced with baseline comparison section CodeRabbit review round 3 fixes: - Pin CI actions to full commit SHAs - Fix SQLite date format in cleanupExpiredSessions - Reorder middleware: CORS → bodyLimit → rateLimit → bearerAuth - Auth fail-closed in production (bypass only in dev) - Emit all 4 analysis phases (lint → screenshot → ai-review → refero) - Fix AnalysisPhaseIndicator isComplete logic for future phases - Fix withTimeout timer leak on promise settlement - Docker job depends on all quality gates Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove unused `setIntersection` in cross-screen-checks.ts - Remove unused `LintSeverity` import in visual-quality.ts Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- docker/build-push-action SHA was non-existent, fixed to v6 tag SHA - Updated checkout and setup-buildx-action to current tag SHAs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The Docker build was failing because it expected a pre-built dist/ directory. Switch to multi-stage build: build stage compiles TypeScript, production stage copies only the compiled output. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This was referenced Mar 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Consolidates findings from the 16-agent audit and 3-agent pipeline evaluation into two implementation sprints:
Sprint 0 — P0 Coherence Fixes (6 items)
Sprint 1 — P1 UX Improvements (6 items)
Sprint 2 — Security, Backend, UX Polish, Infrastructure (5 agent tracks)
Security hardening:
UX & Accessibility:
Backend Architecture:
Infrastructure:
Verification
npm run bundle) — 374KBtsc --noEmit) — 0 errorsnpm run build) — 253KBtsc --noEmit) — 0 errorsTest plan
curl -I🤖 Generated with Claude Code
Summary by CodeRabbit
Заметки о выпуске
Новые функции
Улучшения
Документация
Хаускипинг