From 41c0b80941a8d7069472605fc5452de850b1f535 Mon Sep 17 00:00:00 2001 From: scotttong Date: Mon, 6 Jul 2026 16:42:01 -0700 Subject: [PATCH] docs: record B1/B4/B5 + C1-C10 rulings; name palette classes in DESIGN.md principle 2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DESIGN.md principle 2 now states Tailwind palette classes are hardcoded values in spirit — in-scope debt for a dedicated Run 4, not currently gated — and notes the B1 first-party-decoration allowlist exception. COMPONENT-INVENTORY.md section 6 items get their decided dispositions (C1 split deliberate, C2/C3 Run 3 prep investigations, C4 keep five, C5 Run 3 swap list, C6 dedicated review excluded from Run 3, C7/C8 deliberate custom/intentional exception, C9 deferred to Run 4 with sonner-facade alternative, agentStatusBadge item marked resolved by A1). DECISION-SHEET.md: B1 APPROVED (middle path, executed), B4 DEFERRED to preset session, B5 APPROVED pending contact-sheet review, C1-C10 set. Co-Authored-By: Claude Fable 5 --- COMPONENT-INVENTORY.md | 20 ++++++++++---------- DECISION-SHEET.md | 26 +++++++++++++------------- DESIGN.md | 2 +- 3 files changed, 24 insertions(+), 24 deletions(-) diff --git a/COMPONENT-INVENTORY.md b/COMPONENT-INVENTORY.md index 40e437fffe..3554292745 100644 --- a/COMPONENT-INVENTORY.md +++ b/COMPONENT-INVENTORY.md @@ -365,13 +365,13 @@ All of the below are **leads for human review**, not verdicts, per KNOWN-DUPLICA ## 6. Needs human decision (required section) -1. **`ChatComposer` vs `MarkdownEditor`-based composer** — KNOWN-DUPLICATES.md already flags this as deliberately unmerged (PAP-101). This audit confirms the split is real (not accidental) given the large capability gap. Human call: keep permanently split, or revisit now. -2. **`FileTree.tsx` vs `WorkspaceFileBrowser.tsx` independent tree models** — is `WorkspaceFileBrowser`'s separate `WorkspaceFileTreeNode` model justified by different data needs, or is it drift that should be refactored onto `FileTree`'s exported `buildFileTree`/`FileTreeNode`? Needs someone who knows both call sites' actual data shapes. -3. **`agentStatusBadge` vs `brandChipBadge`** (`lib/status-colors.ts`) — these are provably byte-identical for their 4 shared keys. Recommend collapsing, but doing so touches every call site importing `agentStatusBadge`, so it's a human-approved Run 2/3 item, not automatic. -4. **Entity-picker family** (`ReportsToPicker`, `ExecutionParticipantPicker`, `InlineEntitySelector`, `SearchableSelect`) — plausible consolidation candidate on prop-surface similarity alone; needs a closer prop-by-prop and behavior diff (not done in this pass) before any merge recommendation can be made with confidence. -5. **Finance card family** (5 components, section 5.4) — needs a domain-knowledgeable human to confirm whether all 5 are truly distinct dashboard needs or 2-3 could share a base `FinanceCard`. -6. **Hand-rolled card-shaped containers vs. `Card` primitive** (~26 files) and **hand-rolled pill spans vs. `Badge`** (~34 files) — both are large, low-risk-looking consolidation opportunities, but "low risk" was assessed at a glance only; a real swap pass needs per-site visual verification (this is exactly what Storybook snapshots from Phase 0 would catch if these were touched). -7. **`plugins/launchers.tsx` custom multiplexed overlay** — the most structurally custom modal-like component in the app; recommend a dedicated closer look before deciding whether/how to route it through `Dialog`/`Sheet`/`Popover`, since it currently does something none of the three do alone (switch shell type per plugin action). -8. **`radio-card.tsx` / `toggle-switch.tsx` non-standard "shadcn" primitives** — confirm whether these were deliberately custom-built (and should stay documented as such) or are stale/incidental deviations from `radio-group`/`switch` that should be swapped in a later run. -9. **`StatusBadge`/`AgentStatusBadge` not wrapping the installed `Badge` primitive** — worth a human call on whether unifying the base markup (while keeping the custom `.status-chip` color-mix mechanic) is worth the churn, or whether the current bespoke `` approach should just be documented as an intentional, permanent exception (similar to the StatusIcon/ExternalObject/mention-chip three-way split already documented). -10. **Toast system has no installed shadcn primitive to compare against** — `ToastViewport`/`ToastContext` is fully custom because no `sonner`/toast component is installed at all. Human call: install one and migrate, or formally document the custom toast as the system's permanent choice (it already has a working tone/variant system). +1. **`ChatComposer` vs `MarkdownEditor`-based composer** — KNOWN-DUPLICATES.md already flags this as deliberately unmerged (PAP-101). This audit confirms the split is real (not accidental) given the large capability gap. Human call: keep permanently split, or revisit now. **DECIDED (Run 2 review, DECISION-SHEET.md C1): keep the split, documented as deliberate — user re-confirmed after visual review.** +2. **`FileTree.tsx` vs `WorkspaceFileBrowser.tsx` independent tree models** — is `WorkspaceFileBrowser`'s separate `WorkspaceFileTreeNode` model justified by different data needs, or is it drift that should be refactored onto `FileTree`'s exported `buildFileTree`/`FileTreeNode`? Needs someone who knows both call sites' actual data shapes. **DECIDED (Run 2 review, DECISION-SHEET.md C2): investigate data-shape needs as Run 3 prep; refactor onto FileTree only if shapes align.** +3. **`agentStatusBadge` vs `brandChipBadge`** (`lib/status-colors.ts`) — these are provably byte-identical for their 4 shared keys. Recommend collapsing, but doing so touches every call site importing `agentStatusBadge`, so it's a human-approved Run 2/3 item, not automatic. **RESOLVED (Run 2 review, DECISION-SHEET.md A1): collapsed — `agentStatusBadge` had zero importing call sites and was deleted; `brandChipBadge` is the single map.** +4. **Entity-picker family** (`ReportsToPicker`, `ExecutionParticipantPicker`, `InlineEntitySelector`, `SearchableSelect`) — plausible consolidation candidate on prop-surface similarity alone; needs a closer prop-by-prop and behavior diff (not done in this pass) before any merge recommendation can be made with confidence. **DECIDED (Run 2 review, DECISION-SHEET.md C3): prop-by-prop diff queued as Run 3 prep; no merge without it.** +5. **Finance card family** (5 components, section 5.4) — needs a domain-knowledgeable human to confirm whether all 5 are truly distinct dashboard needs or 2-3 could share a base `FinanceCard`. **DECIDED (Run 2 review, DECISION-SHEET.md C4): keep all five; revisit only if a sixth appears.** +6. **Hand-rolled card-shaped containers vs. `Card` primitive** (~26 files) and **hand-rolled pill spans vs. `Badge`** (~34 files) — both are large, low-risk-looking consolidation opportunities, but "low risk" was assessed at a glance only; a real swap pass needs per-site visual verification (this is exactly what Storybook snapshots from Phase 0 would catch if these were touched). **DECIDED (Run 2 review, DECISION-SHEET.md C5): queued for Run 3 as the shadcn-swap list, with per-site snapshot verification.** +7. **`plugins/launchers.tsx` custom multiplexed overlay** — the most structurally custom modal-like component in the app; recommend a dedicated closer look before deciding whether/how to route it through `Dialog`/`Sheet`/`Popover`, since it currently does something none of the three do alone (switch shell type per plugin action). **DECIDED (Run 2 review, DECISION-SHEET.md C6): dedicated review task; excluded from Run 3.** +8. **`radio-card.tsx` / `toggle-switch.tsx` non-standard "shadcn" primitives** — confirm whether these were deliberately custom-built (and should stay documented as such) or are stale/incidental deviations from `radio-group`/`switch` that should be swapped in a later run. **DECIDED (Run 2 review, DECISION-SHEET.md C7): deliberately custom — documented as such; no swap.** +9. **`StatusBadge`/`AgentStatusBadge` not wrapping the installed `Badge` primitive** — worth a human call on whether unifying the base markup (while keeping the custom `.status-chip` color-mix mechanic) is worth the churn, or whether the current bespoke `` approach should just be documented as an intentional, permanent exception (similar to the StatusIcon/ExternalObject/mention-chip three-way split already documented). **DECIDED (Run 2 review, DECISION-SHEET.md C8): documented as an intentional exception — the WCAG-tuned `.status-chip` mechanic stays bespoke.** +10. **Toast system has no installed shadcn primitive to compare against** — `ToastViewport`/`ToastContext` is fully custom because no `sonner`/toast component is installed at all. Human call: install one and migrate, or formally document the custom toast as the system's permanent choice (it already has a working tone/variant system). **DECIDED (Run 2 review, DECISION-SHEET.md C9): decision deferred to Run 4, when the toast's palette colors get retokenized; sonner-behind-a-pushToast-facade is the alternative to evaluate then.** diff --git a/DECISION-SHEET.md b/DECISION-SHEET.md index 745fb422d3..b8334799e8 100644 --- a/DECISION-SHEET.md +++ b/DECISION-SHEET.md @@ -17,26 +17,26 @@ Every open question from TOKEN-AUDIT.md §8 + batch logs and COMPONENT-INVENTORY | # | Decision | Recommendation | Status | |---|---|---|---| -| B1 | One-off decorative gradients/shadows (5 production + UxLab; 38 arbitrary shadows, no `--shadow-*` tokens existed) | Allowlist as documented "intentional one-off decoration" (extend allowlist criteria beyond third-party); DELETE the ~20 never-reused singleton `--gradient-extract-*` tokens back to inline, keep only reused ones. Spirit over letter of principle 2 | PENDING | +| B1 | One-off decorative gradients/shadows (5 production + UxLab; 38 arbitrary shadows, no `--shadow-*` tokens existed) | Allowlist as documented "intentional one-off decoration" (extend allowlist criteria beyond third-party); DELETE the ~20 never-reused singleton `--gradient-extract-*` tokens back to inline, keep only reused ones. Spirit over letter of principle 2 | APPROVED — middle path, executed: 27 demo-only tokens reverted inline (19 gradients: --gradient-extract-5,6,8-24; 8 shadows: --shadow-extract-15,16,17,19-23 — all consumed solely by *UxLab.tsx pages), 35 call sites restored to original bracket literals, definitions deleted, 4 UxLab pages allowlisted + criteria doc-comment extended to first-party intentional decoration. KEPT 22 production tokens (gradients 1,2,3,4,7,25,26 + 15 shadows) — NOTE: --shadow-extract-4/5 kept contrary to the audit's first cut because ChatComposer.tsx and IssueChatThread.tsx (production) consume them, not just ChatComposer.test.tsx | | B2 | Tailwind palette classes (`bg-red-500` etc.) — 3,115 sites / 145 files | Own future run (Run 4): cluster-by-cluster mapping to semantic tokens, starting with status-adjacent colors; NOT wholesale now, NOT permanent exemption. Update DESIGN.md principle 2 to name palette classes explicitly | APPROVED — own Run 4 later | | B3 | Micro type cluster 9–15px (730 sites) + letter-spacing 9 values (202 sites) | Adopt PRIOR-ART named ladder (map 9→10 nano, keep 10/11/12/13/14; 15→14; tracking → 3 steps) via contact-sheet review — executed in the preset-tune session, decided now | APPROVED — ladder locked, lands with preset session | -| B4 | Radius conflict (`--radius-lg/xl` = 0px vs stock 2xl/3xl) | Defer to preset session — it's a brand question. Candidate: PRIOR-ART monotonic 6/8/10/14/16 | PENDING | -| B5 | Chart palette vs canonical status hues (ActivityCharts in_progress = violet, elsewhere blue) | Re-point charts at `--status-task-*` (operator learns one vocabulary — DESIGN.md P5). Visible change → contact sheet | PENDING | +| B4 | Radius conflict (`--radius-lg/xl` = 0px vs stock 2xl/3xl) | Defer to preset session — it's a brand question. Candidate: PRIOR-ART monotonic 6/8/10/14/16 | DEFERRED to preset session | +| B5 | Chart palette vs canonical status hues (ActivityCharts in_progress = violet, elsewhere blue) | Re-point charts at `--status-task-*` (operator learns one vocabulary — DESIGN.md P5). Visible change → contact sheet | APPROVED pending contact-sheet review — code staged on its own commit, visual diffs exported for user review, snapshots NOT re-baselined | ## C. Component calls (from COMPONENT-INVENTORY.md §6) | # | Decision | Recommendation | Status | |---|---|---|---| -| C1 | ChatComposer vs MarkdownEditor split | Keep split; document as deliberate in COMPONENT-INVENTORY | PENDING | -| C2 | FileTree vs WorkspaceFileBrowser parallel tree models | Investigate data-shape needs in Run 3; refactor onto FileTree only if shapes align | PENDING | -| C3 | Entity-picker family (4 components) | Prop-by-prop diff as Run 3 prep task; no merge without it | PENDING | -| C4 | Finance card family (5 components) | Keep; revisit only if a 6th appears | PENDING | -| C5 | Hand-rolled cards (~26 files) → `Card`; pills (~34 files) → `Badge` | Run 3 shadcn-swap list, per-site snapshot verification | PENDING | -| C6 | `plugins/launchers.tsx` overlay | Dedicated review task; exclude from Run 3 | PENDING | -| C7 | radio-card / toggle-switch custom primitives | Document as deliberate custom; skip swaps | PENDING | -| C8 | StatusBadge not wrapping Badge primitive | Document as intentional exception (WCAG-tuned .status-chip mechanic) | PENDING | -| C9 | Toast system (no shadcn primitive installed) | Keep custom toast; document as permanent choice (working tone/variant system; sonner migration = churn without user-visible gain) | PENDING | -| C10 | FeatureGate wrapper pattern (3 near-identical gates) | Nice-to-have shared primitive; backlog, not a run | PENDING | +| C1 | ChatComposer vs MarkdownEditor split | Keep split; document as deliberate in COMPONENT-INVENTORY | APPROVED — keep split, documented deliberate (user re-confirmed after visual review) | +| C2 | FileTree vs WorkspaceFileBrowser parallel tree models | Investigate data-shape needs in Run 3; refactor onto FileTree only if shapes align | APPROVED — investigate data-shape needs in Run 3 prep | +| C3 | Entity-picker family (4 components) | Prop-by-prop diff as Run 3 prep task; no merge without it | APPROVED — prop-by-prop diff as Run 3 prep task; no merge without it | +| C4 | Finance card family (5 components) | Keep; revisit only if a 6th appears | APPROVED — keep all five; revisit only if a 6th appears | +| C5 | Hand-rolled cards (~26 files) → `Card`; pills (~34 files) → `Badge` | Run 3 shadcn-swap list, per-site snapshot verification | APPROVED — queued Run 3 shadcn-swap list, per-site snapshot verification | +| C6 | `plugins/launchers.tsx` overlay | Dedicated review task; exclude from Run 3 | APPROVED — dedicated review task, excluded from Run 3 | +| C7 | radio-card / toggle-switch custom primitives | Document as deliberate custom; skip swaps | APPROVED — documented as deliberate custom; skip swaps | +| C8 | StatusBadge not wrapping Badge primitive | Document as intentional exception (WCAG-tuned .status-chip mechanic) | APPROVED — documented as intentional exception (WCAG-tuned .status-chip mechanic) | +| C9 | Toast system (no shadcn primitive installed) | Keep custom toast; document as permanent choice (working tone/variant system; sonner migration = churn without user-visible gain) | DEFERRED to Run 4 — decide when toast palette colors get retokenized; sonner-behind-a-pushToast-facade is the alternative to evaluate | +| C10 | FeatureGate wrapper pattern (3 near-identical gates) | Nice-to-have shared primitive; backlog, not a run | APPROVED — backlog nice-to-have, not a run | ## Verification status (this review) diff --git a/DESIGN.md b/DESIGN.md index e8cdffee9b..4e5609fc90 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -27,7 +27,7 @@ Existing tiers already in index.css (~80+ tokens) — extraction maps to these o ## Principles 1. **One way to say each thing.** One component per job. One Button, one Card, one Badge, one Table, one EmptyState. Variants are props, not new components. Before creating a component, prove no existing one covers the job. -2. **Tokens are the only source of visual values.** All color, spacing, radius, type size/weight, shadow, and motion values come from the token layer. No hex, no raw px, no ad-hoc Tailwind arbitrary values (`p-[13px]`) in components. If a needed value doesn't exist, add a token — don't inline it. +2. **Tokens are the only source of visual values.** All color, spacing, radius, type size/weight, shadow, and motion values come from the token layer. No hex, no raw px, no ad-hoc Tailwind arbitrary values (`p-[13px]`) in components. If a needed value doesn't exist, add a token — don't inline it. Tailwind palette classes (`bg-red-500`, `text-zinc-400`, etc.) ARE hardcoded values in spirit: they name a literal color, not a semantic role. They are in-scope debt scheduled for a dedicated future run (Run 4, cluster-by-cluster mapping to semantic tokens per DECISION-SHEET.md B2) and are not currently gated by check-token-gates. Exception (DECISION-SHEET.md B1 user ruling): first-party intentional one-off decoration on demo/UX-lab surfaces stays inline and allowlisted rather than minted as singleton tokens. 3. **Spacing routes through tokens; the scale comes later.** During simplification, extract every spacing and radius value verbatim into tokens — do not normalize, round, or invent a scale. The final scale is a design decision made by a human after reviewing the token audit. Structural rules apply now: vertical rhythm within a container uses one gap value, not per-element margins, and siblings never carry both margin and gap. 4. **Hierarchy through structure, not decoration.** Prefer position, size, and weight over borders, backgrounds, and dividers. Every border, divider, and background fill must justify itself; when in doubt, remove it. A screen should survive the removal of one visual layer. 5. **Status is systematic.** States like running / paused / blocked / awaiting-approval / over-budget map to a single semantic status token set used identically everywhere (badge, row, chart, log). An operator learns the vocabulary once.