docs: Claude Code UI docs + design-sync 21 -> 29, fully re-synced and verified in Claude Design - #136
Merged
Merged
Conversation
Documentation and design-sync build config only. No application source is modified and no UI behavior changes. Docs: CLAUDE.md (loaded every session) defers to AGENTS.md and routes to three on-demand skills — eddi-ui, eddi-screens, eddi-data. Verified the docs against the code rather than trusting them, and corrected: - shared/ has 13 components, not 14 (view-mode.ts is a helper), so the surface is 24, not 25; - the page skeleton documented channels.tsx, which is the outlier — the real convention is space-y-6 (29 of 40 pages, vs 2) with a text-3xl heading and an h-8 w-8 icon, so the reference is now agents.tsx; - eddi-data recommended tsc --noEmit, the exact no-op AGENTS.md warns about; pre-commit runs npm run typecheck. Design-sync surface 21 -> 29: adds the chrome (Sidebar, TopBar, PlatformStatus, PageLoader, MockDataBanner) plus three components that were always in the declared ui/+shared/ surface but never exported (DropdownMenu, ModeSwitcher, RefetchErrorNotice). AppLayout and ConfigEditorLayout stay excluded — both pull Monaco into the bundle. The sidebar-token bug described in the patch notes does not reproduce: :root carries the same 4 of 5 sidebar tokens with and without the layout @source line, and a control build scanning nothing emits the same 4, because Tailwind v4 emits this project's @theme into :root regardless of usage. The @source line is kept for the real reason — it emits the layout utilities (.fill-sidebar-accent, .border-s-2, ~8.7 KB; 45.4 -> 54.1 KB) without which a synced Sidebar/TopBar renders unstyled. --color-sidebar-accent-foreground is genuinely absent from :root in every configuration and ships only in .dark. Nothing uses that utility, so nothing renders wrong; fixing it means touching src/index.css, so it is recorded in .design-sync/NOTES.md instead. A design-system re-sync is required for the new components to appear, and the new viewport overrides need a full package-build, not a preview rebuild.
|
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:
📝 WalkthroughWalkthroughThe PR adds UI development guidance, expands Design Sync exports and previews, adds Design Sync typechecking, updates Design Sync tooling, and changes page error states, GDPR status handling, channel view persistence, localization, and page container spacing. ChangesUI guidance and Design Sync
Page error states and layout
Design Sync runtime and package validation
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…ement Two things, both found by reviewing the previous commit critically. 1. Double padding. AppLayout's <main> already applies p-6 inside `@container/main mx-auto max-w-screen-2xl`, but seven pages added their own, rendering at 48px: channels, channel-detail (all three branches), coordinator, orphans, schedules, secrets, variables. All render inside AppLayout (verified against src/app.tsx — landing, agent-studio and the workforce tree are the surfaces that legitimately own their frame, and are untouched). Verified in the running app: main 24px, page root 0px on every one. 2. The sidebar-token claim in the previous commit was wrong. It said the bug did not reproduce, on the strength of a control build that appeared to emit the tokens with nothing scanned. That control was invalid — its @import rewrite silently failed, so Tailwind fell back to scanning the whole project. Re-measured with an asserted harness. Tailwind v4 tree-shakes @theme tokens, emitting one only if a scanned file uses a utility that reads it: nothing scanned 0 of 5 ui + shared (pre-patch) 2 of 5 (-foreground, -accent) ui + shared + layout 4 of 5 (adds --color-sidebar, -border) So the original patch note was right that the scan was incomplete and the @source line is required — it just had the numbers wrong (2 -> 4, not 0 -> 5). The two that survive without it are held up by shared/mode-switcher.tsx alone; border-sidebar-border and fill-sidebar-accent live in layout/sidebar.tsx. --color-sidebar-accent-foreground reaches :root in no configuration, because nothing anywhere uses that utility (sidebar.tsx pairs bg-sidebar-accent with text-sidebar). It ships only via the plain-CSS .dark block. Also corrected in the docs: eddi-screens claimed every page renders inside AppLayout (three surfaces do not) and that text-3xl headings are the rule (it is 22 to 18 against text-2xl — genuinely mixed); CLAUDE.md conflated the 24-component ui+shared surface with the 29-component synced surface; eddi-ui omitted AlertDialog's `variant` and `children`.
ginccc
changed the title
…typecheck design-sync
Four improvements surfaced by the docs work.
1. Every page that loads data now has an error branch. The five that did not
each fell through to something misleading:
channels "No channels yet" -> ErrorState + retry
coordinator "the service may still be starting up" -> ErrorState + retry
group-wizard "No groups available" -> RefetchErrorNotice
agent-studio normal chrome, empty pipeline -> ErrorState + retry
gdpr green "Processing Active" badge -> "Status unavailable"
The gdpr one is the worst: a failed restriction check reported an unknown
legal-hold state as a known-safe one. Its restrict/unrestrict toggle is now
disabled too, because both the label and the action derive from the value
that failed to load. New key gdpr.statusUnknown, translated in all 11 locales.
Shape per surface: ErrorState where the container can be replaced,
RefetchErrorNotice for an inline control (with an explicit message — its
default wording says data is merely stale), a neutral "unknown" chip where
the value drives a decision.
coordinator.test.tsx was pinning one of these bugs:
it("shows empty state when coordinator status API fails") asserted the exact
wrong behaviour. Replaced with the corrected expectation rather than deleted.
New tests carry negative assertions so the fall-through cannot come back.
2. --color-sidebar-accent-foreground is no longer dead. The sidebar avatar
paired bg-sidebar-accent with text-sidebar; it now uses the token's actual
partner. Identical in light (#ffffff), one shade off in dark (#0c0a09 vs
#09090b) — both near-black on gold, so no visible change. That single call
site takes the design-system bundle from 4 of 5 sidebar tokens to 5 of 5,
and is the only thing keeping it there.
3. .design-sync/ is type-checked. New tsconfig.design-sync.json, referenced from
tsconfig.json, maps eddi-manager -> ds-entry.tsx and pulls in
src/vite-env.d.ts for import.meta.env / __APP_VERSION__. Verified rather than
assumed: passing collapsed="yes" to the Sidebar preview makes tsc -b fail
with TS2322. A wrong prop previously survived until someone ran a sync.
4. channels persists its view mode — the only one of the six ViewToggle pages
that reset on every visit.
Tests 4913 -> 4917.
ginccc
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
.claude/skills/eddi-ui/SKILL.md (1)
117-119: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the React 19 ref pattern in the primitive guidance.
Line [118] recommends
forwardReffor new primitives. This project targets React 19, where components can receiverefas a regular prop. Update the house style to use a typedrefprop, and retainforwardRefonly when a verified compatibility requirement exists.As per coding guidelines: TypeScript and TSX work targets React 19 and strict TypeScript conventions.
Proposed correction
- (`cva` + `cn()`, `forwardRef`, `displayName`) + (`cva` + `cn()`, the typed `ref` prop, and `displayName` when required)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/skills/eddi-ui/SKILL.md around lines 117 - 119, Update the primitive guidance in the house-style bullet to require a typed ref prop compatible with React 19 instead of `forwardRef`; retain `forwardRef` only for verified compatibility requirements, while preserving the existing `cva`, `cn()`, token, reusability, and syncability guidance.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/skills/eddi-screens/SKILL.md:
- Around line 32-34: Update every i18n example in the skill document, including
the calls around the visible page title/subtitle and the additional examples
around lines 87–95, to pass an English fallback as the second argument to t().
Ensure all user-visible strings follow the documented t("key", "English
fallback") pattern, consistent with the eddi-data skill guidance.
- Around line 124-128: Update the table example’s header cell styling to replace
the physical horizontal padding utility px-4 with the equivalent logical
inline-padding utility, while preserving the existing vertical padding and other
classes.
In `@HANDOFF.md`:
- Around line 6-18: Update the HANDOFF.md summary to reflect the final state:
remove the “no behavior or data changes” claim and change the test count from
4,913 to 4,917. Correct the design-sync surface description so DropdownMenu,
ModeSwitcher, and RefetchErrorNotice match their exports from ds-entry.tsx.
Reconcile the sidebar-token notes so --color-sidebar-accent-foreground
consistently states that it is now included and used, and remove superseded
pre-follow-up claims.
In `@src/pages/__tests__/channels.test.tsx`:
- Around line 302-347: Update ErrorState to expose stable error-state and
retry-control data-testid attributes, then replace translated-text assertions
with those IDs in src/pages/__tests__/channels.test.tsx lines 302-347 and the
error-state assertion in src/pages/__tests__/coordinator.test.tsx lines 441-456.
Keep the existing behavioral assertions unchanged.
In `@src/pages/agent-studio.tsx`:
- Around line 123-125: Update the error-state translations to include inline
fallbacks: in src/pages/agent-studio.tsx lines 123-125, src/pages/channels.tsx
lines 170-172, and src/pages/coordinator.tsx lines 223-225, pass “Something went
wrong” to t("common.error") and “Retry” to t("common.retry") while preserving
the existing ErrorState props and retry behavior.
- Around line 117-129: Update the agent-studio query flow around
getAgentDescriptors, getWorkflow, and the existing agentError branch to track
each prerequisite query’s loading and error state. Render the same ErrorState
when either descriptor or workflow loading fails, and have onRetry invoke only
the corresponding failed query’s refetch function; preserve the existing
agentError handling for the agent query.
In `@src/pages/group-wizard.tsx`:
- Around line 1947-1951: Update the RefetchErrorNotice invocation in the isError
branch to pass an inline fallback as the second argument to t when resolving
common.loadError, ensuring the notice remains understandable if the locale key
is unavailable.
---
Nitpick comments:
In @.claude/skills/eddi-ui/SKILL.md:
- Around line 117-119: Update the primitive guidance in the house-style bullet
to require a typed ref prop compatible with React 19 instead of `forwardRef`;
retain `forwardRef` only for verified compatibility requirements, while
preserving the existing `cva`, `cn()`, token, reusability, and syncability
guidance.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 23ada696-9878-4783-b861-ffd690fd5b55
📒 Files selected for processing (44)
.claude/skills/eddi-data/SKILL.md.claude/skills/eddi-screens/SKILL.md.claude/skills/eddi-ui/SKILL.md.design-sync/NOTES.md.design-sync/build-css.mjs.design-sync/config.json.design-sync/ds-entry.tsx.design-sync/previews/DropdownMenu.tsx.design-sync/previews/MockDataBanner.tsx.design-sync/previews/ModeSwitcher.tsx.design-sync/previews/PageLoader.tsx.design-sync/previews/PlatformStatus.tsx.design-sync/previews/RefetchErrorNotice.tsx.design-sync/previews/Sidebar.tsx.design-sync/previews/TopBar.tsxCLAUDE.mdHANDOFF.mdsrc/components/layout/sidebar.tsxsrc/i18n/locales/ar.jsonsrc/i18n/locales/de.jsonsrc/i18n/locales/en.jsonsrc/i18n/locales/es.jsonsrc/i18n/locales/fr.jsonsrc/i18n/locales/hi.jsonsrc/i18n/locales/ja.jsonsrc/i18n/locales/ko.jsonsrc/i18n/locales/pt.jsonsrc/i18n/locales/th.jsonsrc/i18n/locales/zh.jsonsrc/pages/__tests__/channels.test.tsxsrc/pages/__tests__/coordinator.test.tsxsrc/pages/__tests__/gdpr.test.tsxsrc/pages/agent-studio.tsxsrc/pages/channel-detail.tsxsrc/pages/channels.tsxsrc/pages/coordinator.tsxsrc/pages/gdpr.tsxsrc/pages/group-wizard.tsxsrc/pages/orphans.tsxsrc/pages/schedules.tsxsrc/pages/secrets.tsxsrc/pages/variables.tsxtsconfig.design-sync.jsontsconfig.json
Comment on lines
+124
to
+128
| Table: wrap in `rounded-xl border border-border/50 overflow-hidden`, header row | ||
| `border-b bg-muted/50` with `text-start px-4 py-3 font-medium` cells, body rows | ||
| `border-b border-border/30 hover:bg-muted/30 cursor-pointer transition-colors`, whole row | ||
| navigates, IDs in `font-mono text-xs text-muted-foreground`, numeric/version columns | ||
| `text-end`. Give each row `data-testid={\`thing-row-\${id}\`}`. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use logical padding in the table example.
Line [125] documents px-4, which uses physical horizontal padding. Replace it with logical utilities so the example follows the RTL rules.
Proposed correction
- header row `text-start px-4 py-3 font-medium`
+ header row `text-start ps-4 pe-4 py-3 font-medium`As per coding guidelines: Use logical properties and utilities for RTL support.
📝 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
| Table: wrap in `rounded-xl border border-border/50 overflow-hidden`, header row | |
| `border-b bg-muted/50` with `text-start px-4 py-3 font-medium` cells, body rows | |
| `border-b border-border/30 hover:bg-muted/30 cursor-pointer transition-colors`, whole row | |
| navigates, IDs in `font-mono text-xs text-muted-foreground`, numeric/version columns | |
| `text-end`. Give each row `data-testid={\`thing-row-\${id}\`}`. | |
| Table: wrap in `rounded-xl border border-border/50 overflow-hidden`, header row | |
| `border-b bg-muted/50` with `text-start ps-4 pe-4 py-3 font-medium` cells, body rows | |
| `border-b border-border/30 hover:bg-muted/30 cursor-pointer transition-colors`, whole row | |
| navigates, IDs in `font-mono text-xs text-muted-foreground`, numeric/version columns | |
| `text-end`. Give each row `data-testid={\`thing-row-\${id}\`}`. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/eddi-screens/SKILL.md around lines 124 - 128, Update the
table example’s header cell styling to replace the physical horizontal padding
utility px-4 with the equivalent logical inline-padding utility, while
preserving the existing vertical padding and other classes.
Source: Coding guidelines
One textual conflict: src/pages/group-wizard.tsx, adjacent import lines — main added useAvailableStyles/isStyleSupported, this branch added RefetchErrorNotice. Kept both. Checked what the merge changed underneath, since none of it conflicts textually: - AppLayout still applies p-6 inside @container/main mx-auto max-w-screen-2xl, so the double-padding fix still holds, and no page has reacquired its own p-6. - Sidebar and TopBar props are unchanged, so the design-sync previews still typecheck against them. - Every count the docs assert is unchanged: 11 ui / 13 shared / 7 layout, 40 pages, 29 space-y-6, 19 ErrorState, 9 EmptyState, 6/6 ViewToggle persisted, h1 sizes 22 text-3xl to 18 text-2xl. - All five --color-sidebar* tokens still reach :root. - All 11 locales are valid JSON and still carry gdpr.statusUnknown. - HANDOFF.md auto-merged without losing any of main's operator edits (WRITE_ENDPOINTS 22 -> 24, gate-guard, the EDDI 6.2.0+ floor). One regression the merge introduces, recorded in .design-sync/NOTES.md rather than silently absorbed: PR #137 moved OperatorDrawer out of AppLayout and into top-bar.tsx as a static import. TopBar is a synced component, so the design-system bundle grew 4.00 -> 4.17 MB (+179 KB) and now carries the operator tool-scope allow-list. Monaco is still excluded. NOTES.md documents the two ways out (lazy-load the drawer, or drop TopBar from the surface) and the esbuild command to measure either. Gates on the merged tree: 4977 tests passing (331 files), build green, tsc -b green.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.design-sync/NOTES.md (2)
47-49: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse one stylesheet-size measurement.
Line 47 documents a compiled stylesheet of approximately
53 KB. Line 107 reports54.3 KBfor the scoped sheet. State whether these values use different compression or measurement methods, or update one value so the notes do not conflict.Also applies to: 106-108
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.design-sync/NOTES.md around lines 47 - 49, Reconcile the stylesheet size measurements documented near the stable stylesheet note and the scoped-sheet note: either state the different compression or measurement methods used for approximately 53 KB and 54.3 KB, or update one value so both notes report a consistent measurement.
97-104: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAlign the sidebar token count with the utility list.
The table says that
uiplussharedpreserves 2 of 5 tokens. The next paragraph listsbg-sidebar,text-sidebar-foreground, andtext-sidebar-accent, which represent three distinct sidebar tokens. Re-run the measurement or revise the explanation so the count and utility list agree.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.design-sync/NOTES.md around lines 97 - 104, The sidebar token measurement and explanation in the notes disagree: the “2 of 5” count conflicts with the three utilities listed as surviving in shared/mode-switcher.tsx. Re-run the measurement for the ui + shared scope and update the table or explanatory utility list so both identify the same number of distinct tokens, while preserving the layout comparison.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.design-sync/NOTES.md:
- Around line 138-141: Update the documented third check after running build-css
to inspect .design-sync/.cache/compiled.css and assert that all five documented
--color-sidebar* tokens are present inside the :root block, failing when any
token is missing.
- Around line 122-133: Update the “Verifying the surface without a full sync”
checks in NOTES.md to validate cfg.componentSrcMap alongside ds-entry.tsx. Add a
check that every mapped path exists and that the map contains the expected
29-component set, or narrow the note’s claim to exclude componentSrcMap.
---
Outside diff comments:
In @.design-sync/NOTES.md:
- Around line 47-49: Reconcile the stylesheet size measurements documented near
the stable stylesheet note and the scoped-sheet note: either state the different
compression or measurement methods used for approximately 53 KB and 54.3 KB, or
update one value so both notes report a consistent measurement.
- Around line 97-104: The sidebar token measurement and explanation in the notes
disagree: the “2 of 5” count conflicts with the three utilities listed as
surviving in shared/mode-switcher.tsx. Re-run the measurement for the ui +
shared scope and update the table or explanatory utility list so both identify
the same number of distinct tokens, while preserving the layout comparison.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9cfe107b-0ae9-4b74-89b4-b89ccbc55f1b
📒 Files selected for processing (14)
.design-sync/NOTES.mdHANDOFF.mdsrc/i18n/locales/ar.jsonsrc/i18n/locales/de.jsonsrc/i18n/locales/en.jsonsrc/i18n/locales/es.jsonsrc/i18n/locales/fr.jsonsrc/i18n/locales/hi.jsonsrc/i18n/locales/ja.jsonsrc/i18n/locales/ko.jsonsrc/i18n/locales/pt.jsonsrc/i18n/locales/th.jsonsrc/i18n/locales/zh.jsonsrc/pages/group-wizard.tsx
🚧 Files skipped from review as they are similar to previous changes (13)
- src/i18n/locales/ko.json
- src/i18n/locales/pt.json
- src/i18n/locales/th.json
- src/i18n/locales/ja.json
- src/i18n/locales/hi.json
- src/i18n/locales/zh.json
- src/pages/group-wizard.tsx
- src/i18n/locales/fr.json
- src/i18n/locales/es.json
- HANDOFF.md
- src/i18n/locales/ar.json
- src/i18n/locales/de.json
- src/i18n/locales/en.json
…18n fallbacks
Five of the seven findings were valid.
Functional (the one that mattered): agent-studio only handled the agent query's
failure. Each studio query is gated on the previous succeeding
(enabled: !!agentDescriptor, enabled: !!workflowId), so a failed prerequisite
leaves the next query DISABLED rather than errored — it never reports isLoading
or isError. A failed descriptor or workflow fetch therefore fell straight
through to the studio chrome with an empty pipeline, indistinguishable from an
agent that has no workflow. Now all three are tracked and retried individually.
Both new tests were mutation-checked: reducing the guard back to `if (agentError)`
fails them.
ErrorState gained data-testid="error-state" and "error-state-retry"; the tests
that asserted on the translated copy ("Something went wrong", "Retry") now use
those, per the repo's own rule that anything a test asserts on carries a testid.
Inline t() fallbacks added where they were missing — agent-studio, channels,
coordinator, group-wizard — and in the eddi-screens examples, which were
teaching the pattern the doc's own rule forbids.
HANDOFF wording: "never exported" read as a present-tense claim contradicting
ds-entry.tsx; it means they were missing until this PR.
Not applied:
- `px-4` -> `ps-4 pe-4` in the doc's table example. px-* is symmetric and
RTL-safe; AGENTS.md bans pl-/pr-/ml-/mr-/left-/right-, not px-. The codebase
uses px-4 206 times.
- The HANDOFF inconsistencies about test counts and the sidebar token were
already fixed in the merge commit.
Tests 4977 -> 4979.
One textual conflict: HANDOFF.md, where both sides added a new entry at the top of "Completed Phases". Kept both. Checked what the merge changed underneath, since none of it conflicts textually: - Sidebar now imports useEddiVersion from the new update-check hook, and Sidebar is a synced component. The hook does NOT pull react-markdown/remark-gfm (the card does, and the card is not synced), so the design-system bundle stays free of them: 4.17 -> 4.36 MB, monaco/vscode/codicon still 0. - The avatar's text-sidebar-accent-foreground survived the auto-merge, and all five --color-sidebar* tokens still reach :root. - AppLayout still applies p-6, no page has reacquired its own, and the doc counts hold: 40 pages, 29 space-y-6, 19 ErrorState, 9 EmptyState, 6/6 ViewToggle. - All 11 locales valid JSON, all still carry gdpr.statusUnknown. main added two components to the declared surface, so the docs' inventories were updated to match: shared/update-check-card.tsx (shared 13 -> 14, ui+shared 24 -> 25) and layout/update-banner.tsx (layout 7 -> 8). Neither is synced — the synced surface stays at 29 — and NOTES.md records why syncing UpdateCheckCard is a real decision rather than a formality: it pulls react-markdown + remark-gfm and talks to api.github.com. Gates on the merged tree: 5043 tests passing, build green, tsc -b green.
TopBar is a synced component and, since PR #137 moved the operator launcher into both shells' headers, it statically imports OperatorDrawer — dragging the tool-scope allow-list (WRITE_ENDPOINTS), the activation flow and the operator chat into _ds_bundle.js. That is the exact creep the scoped entry exists to prevent. cfg.tsconfig now points at a new tsconfig.ds-bundle.json whose paths map @/components/operator/operator-drawer to .design-sync/stubs/operator-drawer.tsx. The stub renders the launcher button in its resting state and nothing else — the panel is behaviour, not design surface, needing an operator config, a chat transcript and an approval stream that no preview has. Measured through the converter's own resolver, not a stand-in: tool-scopes, WRITE_ENDPOINTS, operator-activation and useOperatorChat all go to 0, and the bundle drops 4.36 -> 4.26 MB (-99 KB). monaco/vscode stay 0. tsconfig.design-sync.json now EXTENDS the bundle tsconfig instead of repeating its paths, so the type-check and the bundle can never disagree about where an import resolves — which means tsc -b validates the stub against TopBar's real usage. Verified by giving the stub a required prop: top-bar.tsx:208 fails with TS2741. Lazy-loading was tested first and does not work. The converter emits format: 'iife', which esbuild cannot code-split, so React.lazy(() => import(...)) is inlined into the same file — a probe entry doing nothing but that still carried tool-scopes and WRITE_ENDPOINTS. OperatorDrawer also renders the launcher itself, so it is mounted on every page and would load immediately anyway. Landmine found and pinned along the way: tsconfigPathsPlugin strips comments with a regex and JSON.parses the result, returning null on any throw — which silently drops alias resolution for the WHOLE bundle rather than failing. The "@/*" key contains a block-comment opener, so one stray closer anywhere after it deletes the entire paths object. tsconfig.design-sync.json already tripped this via its include globs, which is why the converter reads a dedicated glob-free file. design-sync-tsconfig.test.ts pins the parse, the wildcard, the stub mapping, its ordering ahead of the wildcard, and the absence of a closing delimiter. Tests 5043 -> 5047.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.design-sync/NOTES.md (1)
117-126: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAlign the sidebar utility list with the 2-of-5 count.
Keep
2 of 5:bg-sidebar-accent,text-sidebar-accent, andtext-sidebar-foregroundmap to only two tokens. Replacebg-sidebarand removebg-sidebar-accentfrom the layout-only list. Regenerate.design-sync/.cache/compiled.cssbefore recording the:rootmeasurement.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.design-sync/NOTES.md around lines 117 - 126, The sidebar utility list is inconsistent with the documented 2-of-5 measurement. Update the relevant sidebar utility lists in NOTES.md so the 2-of-5 set contains bg-sidebar-accent, text-sidebar-accent, and text-sidebar-foreground, replace bg-sidebar in the layout-only list, and remove bg-sidebar-accent from that list; then regenerate .design-sync/.cache/compiled.css before recording the :root measurement.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.design-sync/NOTES.md:
- Around line 159-162: The dependency check in NOTES.md currently inspects a
plain esbuild bundle that bypasses tsconfigPathsPlugin and therefore does not
measure converter output. Update the documented command to build and inspect the
bundle through the Design Sync resolver from .ds-sync/lib/bundle.mjs, or
explicitly mark the plain bundle as diagnostic-only and make the check fail when
forbidden matches remain.
---
Outside diff comments:
In @.design-sync/NOTES.md:
- Around line 117-126: The sidebar utility list is inconsistent with the
documented 2-of-5 measurement. Update the relevant sidebar utility lists in
NOTES.md so the 2-of-5 set contains bg-sidebar-accent, text-sidebar-accent, and
text-sidebar-foreground, replace bg-sidebar in the layout-only list, and remove
bg-sidebar-accent from that list; then regenerate
.design-sync/.cache/compiled.css before recording the :root measurement.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1e3b5f2b-dba0-4868-b74d-89c5a01a9b51
📒 Files selected for processing (31)
.claude/skills/eddi-screens/SKILL.md.claude/skills/eddi-ui/SKILL.md.design-sync/NOTES.md.design-sync/build-css.mjs.design-sync/config.json.design-sync/stubs/operator-drawer.tsxCLAUDE.mdHANDOFF.mdsrc/__tests__/design-sync-tsconfig.test.tssrc/components/layout/sidebar.tsxsrc/components/shared/error-state.tsxsrc/i18n/locales/ar.jsonsrc/i18n/locales/de.jsonsrc/i18n/locales/en.jsonsrc/i18n/locales/es.jsonsrc/i18n/locales/fr.jsonsrc/i18n/locales/hi.jsonsrc/i18n/locales/ja.jsonsrc/i18n/locales/ko.jsonsrc/i18n/locales/pt.jsonsrc/i18n/locales/th.jsonsrc/i18n/locales/zh.jsonsrc/pages/__tests__/agent-studio.test.tsxsrc/pages/__tests__/channels.test.tsxsrc/pages/__tests__/coordinator.test.tsxsrc/pages/agent-studio.tsxsrc/pages/channels.tsxsrc/pages/coordinator.tsxsrc/pages/group-wizard.tsxtsconfig.design-sync.jsontsconfig.ds-bundle.json
🚧 Files skipped from review as they are similar to previous changes (21)
- src/i18n/locales/en.json
- src/i18n/locales/es.json
- src/i18n/locales/zh.json
- src/i18n/locales/de.json
- src/components/layout/sidebar.tsx
- src/pages/tests/channels.test.tsx
- src/i18n/locales/ja.json
- src/i18n/locales/ko.json
- src/pages/coordinator.tsx
- src/i18n/locales/fr.json
- CLAUDE.md
- src/pages/tests/coordinator.test.tsx
- src/pages/group-wizard.tsx
- .claude/skills/eddi-ui/SKILL.md
- src/i18n/locales/pt.json
- tsconfig.design-sync.json
- src/i18n/locales/ar.json
- src/i18n/locales/hi.json
- src/i18n/locales/th.json
- src/pages/channels.tsx
- .claude/skills/eddi-screens/SKILL.md
Comment on lines
+159
to
+162
| # NOTE: this plain esbuild call does NOT apply cfg.tsconfig's paths, so it will | ||
| # still show the operator subsystem. To measure what the converter actually | ||
| # produces, drive it through tsconfigPathsPlugin from .ds-sync/lib/bundle.mjs. | ||
| grep -oc 'monaco\|vscode\|codicon\|JsonEditor\|tool-scopes' /tmp/ds-bundle.js |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- target file ---'
sed -n '100,175p' .design-sync/NOTES.md
printf '%s\n' '--- related resolver and bundle references ---'
rg -n -C 3 'tsconfigPathsPlugin|cfg\.tsconfig|ds-bundle|esbuild|tool-scopes' .design-syncRepository: labsai/EDDI-Manager
Length of output: 11314
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- bundle module structure ---'
wc -l .ds-sync/lib/bundle.mjs
sed -n '1,260p' .ds-sync/lib/bundle.mjs
printf '%s\n' '--- config and tsconfig ---'
cat .design-sync/config.json
cat tsconfig.ds-bundle.json
printf '%s\n' '--- resolver usage ---'
rg -n -C 5 'tsconfigPathsPlugin|bundle\(|build\(|cfg\.tsconfig|config\.json' .ds-sync .design-sync --glob '*.mjs' --glob '*.js' --glob '*.json'Repository: labsai/EDDI-Manager
Length of output: 42067
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- Design Sync entry and operator imports ---'
cat -n .design-sync/ds-entry.tsx
rg -n -C 4 'operator-drawer|tool-scopes|monaco|vscode|codicon|JsonEditor' src .design-sync --glob '*.{ts,tsx,js,jsx,mjs}'
printf '%s\n' '--- package/build invocation around tsconfig ---'
sed -n '330,365p' .ds-sync/package-build.mjs
sed -n '250,285p' .ds-sync/lib/bundle.mjsRepository: labsai/EDDI-Manager
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import json, re
from pathlib import Path
root = Path('.').resolve()
entry = root / '.design-sync/ds-entry.tsx'
tsconfig = root / 'tsconfig.ds-bundle.json'
topbar = root / 'src/components/layout/top-bar.tsx'
stub = root / '.design-sync/stubs/operator-drawer.tsx'
operator = root / 'src/components/operator/operator-drawer.tsx'
# Mirror the resolver's comment stripping and first-match path behavior.
raw = tsconfig.read_text()
raw = re.sub(r'/\*[\s\S]*?\*/', '', raw)
raw = re.sub(r'(^|[^:])//.*$', r'\1', raw, flags=re.M)
paths = json.loads(raw)['compilerOptions']['paths']
import_line = next(
line for line in topbar.read_text().splitlines()
if 'operator-drawer' in line and 'from' in line
)
spec = re.search(r'from\s+["\']([^"\']+)["\']', import_line).group(1)
plain = root / 'src' / spec[2:] # --alias:@=./src
plugin = None
for key, targets in paths.items():
wild = key.endswith('*')
prefix = key[:-1] if wild else key
if (spec.startswith(prefix) if wild else spec == prefix):
tail = spec[len(prefix):] if wild else ''
candidate = root / targets[0].replace('*', '') / tail
plugin = candidate
break
print(f'import={spec}')
print(f'plain_alias={plain.relative_to(root)} exists={plain.exists()}')
print(f'tsconfig_first_match={plugin.relative_to(root) if plugin else None} exists={plugin.exists() if plugin else False}')
print(f'plain_is_real_operator={plain.resolve() == operator.resolve()}')
print(f'plugin_is_stub={plugin.resolve() == stub.resolve() if plugin else False}')
for label, path in [('plain', plain), ('plugin', plugin)]:
text = path.read_text() if path and path.exists() else ''
hits = [token for token in ('monaco', 'vscode', 'codicon', 'JsonEditor', 'tool-scopes') if token in text]
print(f'{label}_forbidden_tokens={hits}')
PY
printf '%s\n' '--- production bundle call ---'
sed -n '340,360p' .ds-sync/package-build.mjs
printf '%s\n' '--- actual converter option construction ---'
sed -n '155,185p' .ds-sync/lib/bundle.mjsRepository: labsai/EDDI-Manager
Length of output: 2682
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- operator drawer paths ---'
fd -i 'operator.*drawer|drawer.*operator' . --type f
printf '%s\n' '--- top-bar import and nearby source files ---'
sed -n '1,30p' src/components/layout/top-bar.tsx
find src/components -maxdepth 3 -type f | sort | grep -E 'operator|drawer|top-bar'
printf '%s\n' '--- path-map tests and their resolver model ---'
sed -n '1,130p' src/__tests__/design-sync-tsconfig.test.tsRepository: labsai/EDDI-Manager
Length of output: 5521
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import json, re
from pathlib import Path
root = Path('.').resolve()
tsconfig = root / 'tsconfig.ds-bundle.json'
topbar = root / 'src/components/layout/top-bar.tsx'
raw = tsconfig.read_text()
raw = re.sub(r'/\*[\s\S]*?\*/', '', raw)
raw = re.sub(r'(^|[^:])//.*$', r'\1', raw, flags=re.M)
paths = json.loads(raw)['compilerOptions']['paths']
spec = '`@/components/operator/operator-drawer`'
def resolve_with_ext(path):
for suffix in ('', '.ts', '.tsx', '.js', '.jsx', '.mjs',
'/index.ts', '/index.tsx', '/index.js', '/index.jsx'):
candidate = Path(str(path) + suffix)
if candidate.exists():
return candidate
return None
plain = resolve_with_ext(root / 'src' / spec[2:])
plugin = None
for key, targets in paths.items():
wild = key.endswith('*')
prefix = key[:-1] if wild else key
if wild and not spec.startswith(prefix):
continue
if not wild and spec != prefix:
continue
tail = spec[len(prefix):] if wild else ''
for target in targets if isinstance(targets, list) else [targets]:
plugin = resolve_with_ext(root / target.replace('*', '') / tail)
if plugin:
break
if plugin:
break
print(f'plain_alias={plain.relative_to(root) if plain else None}')
print(f'tsconfig_first_match={plugin.relative_to(root) if plugin else None}')
for label, path in [('plain', plain), ('plugin', plugin)]:
text = path.read_text() if path else ''
hits = [token for token in ('monaco', 'vscode', 'codicon', 'JsonEditor', 'tool-scopes')
if token in text]
print(f'{label}_forbidden_tokens={hits}')
PY
printf '%s\n' '--- relevant operator source references ---'
rg -n 'monaco|vscode|codicon|JsonEditor|tool-scopes|operator-chat|operator-activation' \
src/components/operator/operator-drawer.tsx src/components/operator/operator-chat.tsx \
src/components/operator/operator-activation.tsxRepository: labsai/EDDI-Manager
Length of output: 1230
Run the dependency check with the Design Sync resolver.
--alias:@=./src resolves the real operator drawer, while TSCONFIG_PATH maps it to .design-sync/stubs/operator-drawer.tsx. Therefore /tmp/ds-bundle.js does not represent converter output. Run the check against the resolver-backed bundle, or label it diagnostic-only and fail when forbidden matches remain.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.design-sync/NOTES.md around lines 159 - 162, The dependency check in
NOTES.md currently inspects a plain esbuild bundle that bypasses
tsconfigPathsPlugin and therefore does not measure converter output. Update the
documented command to build and inspect the bundle through the Design Sync
resolver from .ds-sync/lib/bundle.mjs, or explicitly mark the plain bundle as
diagnostic-only and make the check fail when forbidden matches remain.
Full re-sync to the Claude Design project (21 -> 29 components uploaded). Three
defects surfaced on the way; all three are fixed here.
1. EVERY component shipped an empty API contract. lib/dts.mjs builds its
ts-morph project from *.d.ts files only, and this repo is an app with no
declaration tree — so the extractor parsed exactly one file
(src/vite-env.d.ts) and all 29 fell back to `[key: string]: unknown`.
Validate exits 0 either way, so it was invisible. The design agent had no
idea Button takes variant/size or that Sidebar takes collapsed/onToggle.
cfg.dtsPropsFor now carries a hand-written body for all 29, transcribed
from each component's real props interface. The first sync shipped the 21
with this same empty contract, so this fixes them too.
2. Sidebar threw on every render. __APP_VERSION__ is a Vite `define` and
esbuild does not apply it; the converter has no define hook. In a preview
there is no backend, so the version footer ALWAYS takes the
`EDDI Demo ${__APP_VERSION__}` branch. ds-entry.tsx defines it at bundle
init, which covers designs built with the DS, not just preview cards.
3. The expanded Sidebar rendered a broken image. /logo_eddi.png was an
absolute public/ path, which the DS bundle cannot ship. Moved to
src/assets/ and imported by both sidebars: at 2 KB it is under Vite's 4 KB
assetsInlineLimit so the app inlines it (same pixels, one fewer request),
and esbuild's .png dataurl loader inlines it for the bundle.
Verification: render check 29/29 clean (0 bad, 0 thin, 0 identical, 0 page
errors); all 44 preview cells graded good against the absolute rubric from
freshly captured sheets; 29 carried forward / 0 cleared on the final capture,
which is the proof the next re-sync is cheap. conventions.md re-validated
against the fresh build — every class, token, prop and component it names
still resolves — and extended with the app-chrome section, since the surface
it described no longer matched what ships.
NOTES.md records the dtsPropsFor maintenance burden, the PlatformStatus
checking-state gotcha, and the two app-asset defects, plus re-sync risks for
each.
Tests unchanged at 5047; build green.
…mponent checklist, handoff Findings from a full re-review of the sync, each verified before fixing: - ResourceTypeBadge's contract comment listed only the 9 AGENTS.md resource slugs as if they were the accepted set. The component's own color map takes 17 keys (agent, workflow, behavior, httpcalls, property, ... — the ones the test generation actually used), unknown slugs fall back to a neutral chip, and a trailing ".json" is stripped. The comment now says all of that. The corrected .d.ts is already re-uploaded to the design project (delta upload: ResourceTypeBadge + bundle + anchor; driver verdict showed verification untouched — 29 unchanged, nothing regraded). - CLAUDE.md and NOTES.md both described adding a synced component as "ds-entry + componentSrcMap (+ preview)" — missing the dtsPropsFor entry that has been mandatory since the contracts became hand-written. Both now name all four steps. - HANDOFF.md still said the 8 new components "need a design-system re-sync to appear". The re-sync is done; the entry now records the completed upload, the end-to-end test generation, and the three defects it surfaced. Also cross-checked all 29 dtsPropsFor contracts against source mechanically (prop-name diff in both directions): no missing and no invented props — the one flag (Button variant/size) is the checker being blind to VariantProps, and those values were verified against the cva config directly. Remote _ds_sync.json compared byte-level with the local build: identical.
ginccc
changed the title
There was a problem hiding this comment.
Pull request overview
Expands EDDI Manager’s Claude Design surface, documents UI conventions, and improves page-state handling and layout consistency.
Changes:
- Expands design-sync from 21 to 29 components with typed previews and app chrome.
- Fixes duplicate page padding, error states, view persistence, and sidebar assets/tokens.
- Adds Claude Code guidance, localization, and regression tests.
Reviewed changes
Copilot reviewed 54 out of 56 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
tsconfig.json |
Adds design-sync type-checking. |
tsconfig.ds-bundle.json |
Configures bundle aliases and operator stub. |
tsconfig.design-sync.json |
Defines strict design-sync compilation. |
src/pages/variables.tsx |
Removes duplicate padding. |
src/pages/secrets.tsx |
Removes duplicate padding. |
src/pages/schedules.tsx |
Removes duplicate padding. |
src/pages/orphans.tsx |
Removes duplicate padding. |
src/pages/group-wizard.tsx |
Adds nested-group load errors. |
src/pages/gdpr.tsx |
Handles unknown restriction status. |
src/pages/coordinator.tsx |
Adds status-fetch error handling. |
src/pages/channels.tsx |
Adds errors, shared empty state, and persisted view. |
src/pages/channel-detail.tsx |
Removes duplicate padding. |
src/pages/agent-studio.tsx |
Adds prerequisite-query errors. |
src/pages/__tests__/gdpr.test.tsx |
Tests unknown restriction behavior. |
src/pages/__tests__/coordinator.test.tsx |
Corrects failed-fetch expectations. |
src/pages/__tests__/channels.test.tsx |
Tests channel error and retry states. |
src/pages/__tests__/agent-studio.test.tsx |
Tests studio query failures. |
src/i18n/locales/zh.json |
Adds translated unknown status. |
src/i18n/locales/th.json |
Adds translated unknown status. |
src/i18n/locales/pt.json |
Adds translated unknown status. |
src/i18n/locales/ko.json |
Adds translated unknown status. |
src/i18n/locales/ja.json |
Adds translated unknown status. |
src/i18n/locales/hi.json |
Adds translated unknown status. |
src/i18n/locales/fr.json |
Adds translated unknown status. |
src/i18n/locales/es.json |
Adds translated unknown status. |
src/i18n/locales/en.json |
Defines unknown-status copy. |
src/i18n/locales/de.json |
Adds translated unknown status. |
src/i18n/locales/ar.json |
Adds translated unknown status. |
src/components/workforce/workforce-sidebar.tsx |
Imports the bundled logo asset. |
src/components/shared/error-state.tsx |
Adds stable test identifiers. |
src/components/layout/sidebar.tsx |
Bundles logo and restores token usage. |
src/assets/logo_eddi.png |
Adds importable wordmark asset. |
src/__tests__/design-sync-tsconfig.test.ts |
Guards converter alias parsing. |
HANDOFF.md |
Documents completed work and verification. |
CLAUDE.md |
Adds always-loaded UI guidance. |
.ds-sync/storybook/SKILL.md |
Updates upload-size guidance. |
.ds-sync/package-validate.mjs |
Uses ts-morph for declaration checks. |
.ds-sync/package-build.mjs |
Updates the upload size limit. |
.ds-sync/lib/bundle.mjs |
Corrects JSX runtime child handling. |
.design-sync/stubs/operator-drawer.tsx |
Adds a lightweight operator launcher stub. |
.design-sync/previews/TopBar.tsx |
Adds TopBar preview. |
.design-sync/previews/Sidebar.tsx |
Adds sidebar previews. |
.design-sync/previews/RefetchErrorNotice.tsx |
Adds error-notice previews. |
.design-sync/previews/PlatformStatus.tsx |
Adds platform-status preview. |
.design-sync/previews/PageLoader.tsx |
Adds loader preview. |
.design-sync/previews/ModeSwitcher.tsx |
Adds mode-switcher previews. |
.design-sync/previews/MockDataBanner.tsx |
Adds mock-banner preview. |
.design-sync/previews/DropdownMenu.tsx |
Adds dropdown preview. |
.design-sync/NOTES.md |
Documents design-sync architecture and risks. |
.design-sync/ds-entry.tsx |
Exports the expanded component surface. |
.design-sync/conventions.md |
Documents app-chrome composition. |
.design-sync/config.json |
Registers components and prop contracts. |
.design-sync/build-css.mjs |
Includes layout and stub utilities. |
.claude/skills/eddi-ui/SKILL.md |
Documents UI primitives and tokens. |
.claude/skills/eddi-screens/SKILL.md |
Documents screen construction patterns. |
.claude/skills/eddi-data/SKILL.md |
Documents data and testing conventions. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| <div key={i} className="h-40 rounded-xl border border-border/50 bg-card animate-pulse" /> | ||
| ))} | ||
| </div> | ||
| ) : isError ? ( |
|
|
||
| return ( | ||
| <div className="flex flex-col gap-6 p-6"> | ||
| <div className="flex flex-col gap-6"> |
Comment on lines
+122
to
+127
| if (descriptorsError || agentError || workflowError) { | ||
| const retryFailed = descriptorsError | ||
| ? refetchDescriptors | ||
| : agentError | ||
| ? refetchAgent | ||
| : refetchWorkflow; |
Comment on lines
+100
to
+104
| "Button": "variant?: \"primary\" | \"secondary\" | \"destructive\" | \"outline\" | \"ghost\" | \"link\";\n size?: \"sm\" | \"md\" | \"lg\" | \"icon\";\n /** Render the child element instead of a <button> (Radix Slot) */\n asChild?: boolean;\n disabled?: boolean;\n type?: \"button\" | \"submit\" | \"reset\";\n onClick?: React.MouseEventHandler<HTMLButtonElement>;\n className?: string;\n children?: React.ReactNode;", | ||
| "Card": "className?: string;\n children?: React.ReactNode;", | ||
| "DropdownMenu": "/** Radix root. Compose with DropdownMenuTrigger / DropdownMenuContent /\n * DropdownMenuItem / DropdownMenuSeparator / DropdownMenuLabel. */\n open?: boolean;\n defaultOpen?: boolean;\n onOpenChange?: (open: boolean) => void;\n modal?: boolean;\n children?: React.ReactNode;", | ||
| "ErrorBoundary": "children: React.ReactNode;\n /** Custom fallback — receives the error and a reset callback */\n fallback?: (error: Error, reset: () => void) => React.ReactNode;\n /** Changing this value auto-resets any caught error */\n resetKey?: string;", | ||
| "Input": "type?: string;\n value?: string | number;\n defaultValue?: string | number;\n placeholder?: string;\n disabled?: boolean;\n readOnly?: boolean;\n onChange?: React.ChangeEventHandler<HTMLInputElement>;\n className?: string;", |
Comment on lines
+3
to
+4
| // No backend in preview, so the pill settles on its offline state after the | ||
| // first probe — that is the state worth documenting anyway. |
Comment on lines
+16
to
+18
| // So the converter resolves the module to this file instead, via an exact | ||
| // `paths` entry in `tsconfig.design-sync.json` (which `config.json` points | ||
| // `cfg.tsconfig` at). That same tsconfig type-checks `.design-sync/`, so `tsc -b` |
… the 8th padded page, contract accuracy Thirteen new review comments triaged; eleven acted on, each verified first. Functional (Copilot, all three confirmed reachable because the app leaves refetchOnWindowFocus at its default): - channels, agent-studio, group-wizard gated their new error branches on isError alone. TanStack Query keeps the last good data when a background refetch fails, so a focus blip during a backend hiccup replaced a usable page (or unmounted the studio's editor state) with a full error. All three now gate on the data being absent (isError && !data); coordinator already did this and gdpr's unknown-status chip is deliberately ungated — a stale legal-hold state should not present as known-good. - logs.tsx was an 8th double-padded page: its `flex h-full flex-col p-6` root escaped the original sweep's grep pattern. Fixed; HANDOFF's page list and the eddi-screens claim are corrected, and the skill now teaches the isError && !data gating rule. - New regression test: channels view mode survives an unmount/remount via localStorage (the persistence this branch added had no coverage). 5047 -> 5048 tests. Contract accuracy (Copilot): - dtsPropsFor bodies REPLACE the source props, so curated contracts narrowed native DOM APIs (Button lost onKeyDown/name/form, Input lost required/autoComplete/...). The override format cannot emit an extends clause, so the five HTML-passthrough contracts (Button, Input, Badge, Card, Skeleton) now carry an explicit doc line that unlisted native attributes spread through — the agent learns the truth without weakening the typing. - Three stale comments fixed: the PlatformStatus preview claimed the pill settles on offline (it captures in checking state — NOTES.md had it right); the operator stub named tsconfig.design-sync.json as the alias carrier (it is tsconfig.ds-bundle.json); tsconfig.ds-bundle.json referenced a test file by a name that never existed (design-sync-tsconfig.test.ts is the guard). - HANDOFF's test count was stale twice over (5043 -> 5048/335). NOTES.md verification block (CodeRabbit): check 1b now validates every componentSrcMap path exists and that componentSrcMap and dtsPropsFor cover the identical 29-component set; check 3 actually asserts the five sidebar tokens after the build instead of claiming to. Both commands were executed verbatim before being committed. A shell gotcha discovered while re-verifying is recorded: Git Bash can strip backslashes from inline node -e regexes, making a checker match nothing while printing success — regex checkers belong in .mjs files. The changed contracts are already re-uploaded to the design project (delta: 7 components + bundle + styling, sentinel-fenced, anchor last; driver verdict clean, PlatformStatus regraded good from a fresh identical sheet). Gates: 5048 tests passing, build green, tsc -b green.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/__tests__/channels.test.tsx (1)
350-353: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse a stable test ID for the empty state.
Lines [352] and [378] assert
"No channels yet"by visible text. Add or use a stable empty-statedata-testid, then query that ID in these tests. Keep text assertions only in tests that specifically verify localization copy.As per coding guidelines, “Add
data-testidto anything tests assert on, including rows, buttons, inputs, and states, using established names such aschannel-row-${id},create-channel-btn, andview-toggle-card.”Also applies to: 377-380
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/__tests__/channels.test.tsx` around lines 350 - 353, Replace the visible-text queries for the empty state in the affected channel tests with the established stable data-testid, adding that test ID to the empty-state component if it does not already exist. Update both assertions around the failed-fetch and empty-state cases, while retaining text assertions only where localization copy is being tested.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/skills/eddi-screens/SKILL.md:
- Around line 116-120: Update the canonical error-handling example near the
existing isError check to guard the error branch with both isError and absent
data, preserving cached content when background refetches fail.
---
Outside diff comments:
In `@src/pages/__tests__/channels.test.tsx`:
- Around line 350-353: Replace the visible-text queries for the empty state in
the affected channel tests with the established stable data-testid, adding that
test ID to the empty-state component if it does not already exist. Update both
assertions around the failed-fetch and empty-state cases, while retaining text
assertions only where localization copy is being tested.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 680f79af-a9a2-460e-97d2-414231625740
📒 Files selected for processing (13)
.claude/skills/eddi-screens/SKILL.md.design-sync/NOTES.md.design-sync/config.json.design-sync/previews/PlatformStatus.tsx.design-sync/stubs/operator-drawer.tsxCLAUDE.mdHANDOFF.mdsrc/pages/__tests__/channels.test.tsxsrc/pages/agent-studio.tsxsrc/pages/channels.tsxsrc/pages/group-wizard.tsxsrc/pages/logs.tsxtsconfig.ds-bundle.json
🚧 Files skipped from review as they are similar to previous changes (9)
- .design-sync/previews/PlatformStatus.tsx
- HANDOFF.md
- src/pages/channels.tsx
- CLAUDE.md
- src/pages/group-wizard.tsx
- .design-sync/stubs/operator-drawer.tsx
- src/pages/agent-studio.tsx
- .design-sync/config.json
- .design-sync/NOTES.md
…ating rule The previous commit added the "gate on isError && !data" rule to the prose but left the code example above it branching on bare isError — the doc taught the exact pattern it warns against, and the example is what gets copied. The example now hoists `const loadFailed = isError && !data`, uses it for the ErrorState branch, and threads it through the empty-state guard so the two cannot drift apart. That also matches the shape the fixed pages use (channels inline, agent-studio via per-query blocked consts). Caught by CodeRabbit.
ginccc
added a commit
that referenced
this pull request
Aug 12, 2026
…gate blind spot Merges main (12 commits, #136) and addresses all 13 review findings. Four were real defects, and two of those undermined fixes made earlier in this branch. RETRY CAP DEFEATED BY ITS OWN RESET. BearerEventSource refilled the retry budget as soon as response headers arrived. A server answering 200 and closing the body immediately therefore reset the counter, fell out of the read loop and retried at attempt zero — reinstating the unbounded 5s loop the cap exists to prevent, just wearing a success status. The budget now refills only once the stream has produced bytes. i18n GATE WAS BLIND TO A THIRD OF THE CODEBASE. collectUsedKeys scanned line by line, so it could not match `t(` against a key on the next line — and Prettier wraps long calls constantly. Measured: 2,955 keys seen versus 3,287 actually present. Scanning whole files found a genuinely missing key on the first run (Workforce.chat.toggleDetails), which turned out to be `t(key, ternary)` — one key for two strings, in two files. Both are now split. The collision detector also reads the object form `t(key, { defaultValue })`, which 111 call sites use. UNBOUNDED RESPONSE BUFFERING. The OpenAPI spec fetch called `res.text()` and checked the size afterwards, so the memory was already spent — and `.length` counts UTF-16 units, not bytes. It now streams, counting byteLength, and aborts at the cap. DEPLOY COULD DELETE THE LIVE BUNDLE. Stale-asset cleanup ran before the copy and before the HTML shells were repointed, so a failed copy left the shells referencing files already removed — a broken UI and no rollback. Cleanup now runs last; sandbox-verified. Also: TextDecoder is flushed at EOF in readGroupSSE (a multi-byte character split across the final chunk boundary lost its last byte); language switching serialises so an out-of-order completion cannot win; renovate blocks only MAJOR react-router bumps so security patches stay visible; the stale-exemption guard in i18n-quality asserts both sides exist rather than passing on undefined === undefined; the routing test navigates inside its own tree instead of mounting a second router; a Portuguese plural typo (variávelis → variáveis); the PowerShell script gets a UTF-8 BOM; AGENTS.md no longer claims every page is lazy when three are deliberately eager; and the secrets test asserts through its testid. 5,096 tests pass. Entry chunk 1.15 MB / 347 KB gzipped.
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
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.
Claude Code documentation, the design-sync surface extended to the app chrome, the UI-layer
bugs that work surfaced — and the completed re-sync: all 29 components are live in the
Claude Design project and verified end-to-end.
1. Claude Code docs (4 new files)
CLAUDE.md.claude/skills/eddi-ui/SKILL.mddescription.claude/skills/eddi-screens/SKILL.md.claude/skills/eddi-data/SKILL.mdCLAUDE.mddefers toAGENTS.mdfor workflow, gates, i18n and architecture. It adds the UIlayer and routes to the three skills, so the always-on context stays small.
Corrections made while verifying the docs against the code: component counts (
view-mode.tsis a helper, not a component); the page skeleton documented
channels.tsx, the outlier — theconvention is
space-y-6(29 of 40 pages), reference nowagents.tsx; "every page rendersinside
AppLayout" was false (landing, agent-studio, workforce render outside it); headingsize is genuinely mixed (22
text-3xl/ 18text-2xl);eddi-datarecommendedtsc --noEmit, the exact no-opAGENTS.md§2 warns about;AlertDialog'svariant/childrenwere missing from
eddi-ui.2. UI fixes the docs work surfaced
AppLayout's already-padded<main>and rendered at 48px. Verified live at 24px after the fix.
states on a failed fetch; worst was
gdprshowing the green "Processing Active" badge foran unknown legal-hold state (its toggle is disabled while unknown, new
gdpr.statusUnknownkey in all 11 locales). A test in
coordinator.test.tsxwas pinning the bug and now pinsthe fix.
--color-sidebar-accent-foregroundrevived — the sidebar avatar now uses the token'sactual partner; that single call site is what keeps it in the compiled CSS (5/5 sidebar
tokens ship).
.design-sync/is type-checked (tsconfig.design-sync.jsonintsc -b); proven bymutation (a wrong preview prop fails with TS2322).
channelspersists its view mode — the onlyViewTogglepage that reset per visit.3. Design-sync: 21 → 29, re-synced, and verified in Claude Design
Adds the chrome (
Sidebar,TopBar,PlatformStatus,PageLoader,MockDataBanner) plusthree never-exported members of the declared surface (
DropdownMenu,ModeSwitcher,RefetchErrorNotice).AppLayout/ConfigEditorLayoutstay excluded (Monaco).The operator drawer is stubbed out of the bundle. PR #137 made
TopBarstatically importOperatorDrawer, dragging the tool-scope allow-list into the design-system bundle (+99 KB).cfg.tsconfignow points at a glob-freetsconfig.ds-bundle.jsonmapping the drawer to alauncher-only stub;
tsconfig.design-sync.jsonextends it sotsc -bvalidates the stubagainst
TopBar's real usage. Lazy-loading was tested and does not work (the converter emitsan IIFE, which esbuild cannot code-split). A parser landmine in the converter's tsconfig
reader (block-comment stripping deletes the whole
pathsobject if any*/follows the"@/*"key) is pinned bysrc/__tests__/design-sync-tsconfig.test.ts.The re-sync ran and uploaded (161 files, no deletions). Verified: render check 29/29
clean; all 44 preview cells graded good from fresh screenshots; carried-forward proof on the
final capture (29/0); remote
_ds_sync.jsonbyte-identical to the local build. A live testgeneration in claude.ai/design then built the full Agents screen from the bundle — Sidebar +
TopBar shell, gold-token Buttons/Badges/Cards, EmptyState/ErrorState — confirming the agent
consumes the components, contracts and tokens. (The test project was deleted afterwards.)
Three defects the re-sync surfaced, all fixed here:
[key: string]: unknown). Theconverter extracts props from
.d.tstrees, and an app repo has none — invisible becausevalidate exits 0.
cfg.dtsPropsFornow hand-carries all 29 contracts, transcribed fromsource and mechanically cross-checked (no missing, no invented props). Adding a synced
component now requires a
dtsPropsForentry — CLAUDE.md and NOTES.md say so.Sidebarthrew__APP_VERSION__ is not definedin the bundle (Vitedefine; esbuilddoesn't apply it). Shimmed at bundle init in
ds-entry.tsx./logo_eddi.pngabsolute path).Moved to
src/assets/and imported — at 2 KB both Vite (≤4 KB inline limit) and esbuild(
dataurlloader) inline it, so the app loses one request and the bundle gains the image.4. Verification
npm run test5047 passing / 332 files,npm run buildgreen, pre-commit(
eslint --max-warnings 0+tsc -b) clean, CI green on the head.--color-sidebar*tokens in the compiled DS stylesheet; bundle free ofmonaco/vscode/codicon/tool-scopes (measured through the converter's own resolver).
package-lock.jsonuntouched throughout (npm cionly); the nested@tailwindcss/oxide-wasm32-wasiwasm deps intact per the AGENTS.md Windows warning.Post-merge
Nothing pending — the design system is uploaded, anchored, and verified. Future re-syncs are
one driver command and will skip everything unchanged via the uploaded anchor.
Summary by CodeRabbit
Bug Fixes
Enhancements
Documentation