paperclip/doc/design/DECISION-SHEET.md

72 lines
16 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# DECISION-SHEET — Run 1 human review
Every open question from TOKEN-AUDIT.md §8 + batch logs and COMPONENT-INVENTORY.md §6, each with a recommendation, blast radius, and where it lands. Statuses: **PENDING** → APPROVED / OVERRIDDEN (with note) / DEFERRED.
## A. Quick wins — low risk, do in this review phase
| # | Decision | Recommendation | Blast radius | Status |
|---|---|---|---|---|
| A1 | `agentStatusBadge` vs `brandChipBadge` byte-identical maps (status-colors.ts) | Collapse to `brandChipBadge`, re-point imports | code dedup, zero pixels | APPROVED — done 5ecc0f9e4: `agentStatusBadge` had ZERO importing call sites, so it was deleted outright (no re-pointing, no alias needed); `brandChipBadge` is the single map, `AgentBadgeColor` kept (subset of `BrandChipColor`) |
| A2 | Contrast-pair triplication (color-contrast.ts / worktree-branding.ts / ThemeContext.tsx) | One shared constant IF values are truly identical; verify per-pair first, keep semantically distinct ones separate | 3 files, zero pixels if identical | APPROVED — done 383460bb2: only `#f8fafc`/`#111827` is byte-identical across files → exported as `READABLE_TEXT_LIGHT`/`READABLE_TEXT_DARK` from color-contrast.ts, imported by worktree-branding.ts; ThemeContext's `#18181b`/`#ffffff` meta-theme-color pair, the DARK_BG/LIGHT_BG rgb-object compositing backgrounds, and worktree-branding's `#000000` parse fallback are semantically distinct → untouched |
| A3 | Project-color fallbacks `#6366f1` / `#64748b` (14 sites) | Two semantic tokens (`--project-seed`, `--project-none`) — file pattern shows two intents | rename-only, zero pixels | APPROVED — done 7e3e59db2: pure rename `--hex-6366f1`→`--project-seed` (7 sites) and `--hex-64748b`→`--project-none` (9 sites incl. ActivityCharts 'backlog') + index.css definitions, values unchanged |
| A4 | Test-file hardcoded hex (56 sites) | Leave alone; update lockstep only when asserted values actually change | none | APPROVED — policy adopted, no code change |
| A5 | `FileViewerSheet` half-migrated `var(--paperclip-code-highlight-*, fallback)` | Mint the two vars in index.css at the fallback values (identical pixels today; makes the intended token real) | 1 file | APPROVED — done 7929aabeb: both vars minted in the extracted-tokens :root block at exactly the former fallback values; chose to SIMPLIFY the Batch 4 `--code-highlight-*-resolved` wrappers to plain `var(--x)` (fallbacks now redundant; nothing sets the vars at runtime); FileViewerSheet.tsx call sites unchanged |
| A6 | "Liveness blue" chat bubble reusing `--status-task-in_progress` (semantic coincidence) | Decouple: mint `--liveness-blue` with same value so a future status-hue change doesn't drag the chat bubble along | 2 sites, zero pixels | APPROVED — done b76a7955a: `--liveness-blue: #2563eb` minted; IssueChatThread.tsx bubble class + IssueChatThread.test.tsx lockstep assertion re-pointed |
## B. Policy calls
| # | 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 | 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 | EXECUTED (pending contact-sheet) — scripts/codemod-type-ladder.mjs (idempotent, committed): 730 font-size sites -> --text-nano (10px, incl. 9->10) / --text-micro (11px, incl. 0.7rem) / text-xs (12px) / --text-compact (13px) / text-sm (14px, incl. 15->14); 202 tracking sites -> --tracking-label (0.08em) / --tracking-eyebrow (0.14em) / --tracking-caps (0.2em); all --fs-* and --ls-* definitions deleted. NOTE: text-xs/text-sm sites also pick up Tailwind scale line-height (contact-sheet reviewable). PRIOR-ART "sm 13" tier renamed --text-compact (name collides with Tailwind text-sm=14px) |
| 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 & CLOSED — user approved on before/after contact sheet (Jul 6); 2 chart snapshots re-baselined, suite 510/510 on new baseline. Known trade documented: To-Do amber vs priority-Medium amber adjacency, revisit at preset session |
## C. Component calls (from COMPONENT-INVENTORY.md §6)
| # | Decision | Recommendation | Status |
|---|---|---|---|
| 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 |
| C11 | Sidebar agents section: hand-rolled rows (own spacing/icon colors, palette-blue liveness dot) instead of `SidebarNavItem`; only section that is collapsible | Run 3 item: unify rows onto `SidebarNavItem`, settle collapsibility policy across sections, liveness dot → canonical status blue. Wholesale shadcn Sidebar adoption REJECTED for now (app already has equivalent machinery incl. icon-rail height trick; highest-regression chrome) — re-evaluate as a dedicated item after Run 3 only if its behaviors (kbd shortcut, persisted state, mobile sheet) are wanted | APPROVED — user-spotted during :3300 test drive (Jul 6), queued for Run 3 |
## Gallery feedback round 1 (preset-tune session, Jul 6) — executed
User rulings from the tune-session gallery review; all intentionally visible, snapshots NOT re-baselined (fresh before/after triplets regenerated in tests/storybook-visual/test-results/ against the old baseline):
1. **Dark destructive red reverted** — `.dark --destructive` back to master's original `oklch(0.637 0.237 25.331)` (preset's softer `oklch(0.704 0.191 22.216)` rejected); light mode untouched.
2. **Budget/quota BAR FILLS reuse status colors** — moving-fill elements only: healthy → `bg-(--status-task-done)`, warning → `bg-(--status-task-todo)`, exceeded/hard-stop → `bg-(--status-task-blocked)` in BudgetPolicyCard.tsx, QuotaBar.tsx (feeds ProviderQuotaCard/BillerSpendCard pages), Costs.tsx, CodexSubscriptionPanel.tsx (escalation tiers only). Inspected and deliberately LEFT: Org.tsx status dot (not a bar), BudgetPolicyCard chip washes/notice borders (not fills), CodexSubscriptionPanel healthy `bg-primary/70` + null `bg-zinc-700` (healthy tier uses brand primary by design — flagged as ambiguous, not emerald).
3. **RUNNING = status blue, not cyan/teal** — IssueChatThread running chip now composes `brandChipBadge.blue` (layout classes unchanged); RunTranscriptView running label uses new `runningLabelText` export (`text-[#1D4ED8] dark:text-[#2563EB]`, hexes kept in lib/status-colors.ts for gate cleanliness); `statusBadge.running` + `agentStatusDot.running` maps and AgentDetail `runStatusIcons.running` re-pointed cyan→blue. Deliberately LEFT + flagged: `externalObjectStatusIcon/Badge.running` (same-map collision — `open` is already blue there; documented UX-spec tone system), and the cyan "Live" branding family (LiveRunWidget theme, AgentDetail live-card border + Live pulse dots, DesignGuide Live sample) — "Live" is a distinct motif from RUNNING chips; note AgentDetail's mobile Live pill is already blue, so a dedicated Live-color decision is recommended.
## Gallery feedback round 2 (preset-tune session, Jul 6) — executed
1. **BudgetIncidentCard light-mode legibility (pre-existing bug)** — the hard-stop card's eyebrow/title/description/banner used dark-tuned red-50/100/200 text with no light variants over the light pink gradient. Light mode now uses red-600..950-tier text (matching the app's existing `text-red-700 dark:text-red-300` light-red-surface pattern); dark classes preserved verbatim behind `dark:`. Sibling fix: BudgetPolicyCard statusTone chips (hard_stop/warning/ok) + its red banner had the same dark-only text — same treatment. Gradient backgrounds (kept B1 tokens) untouched.
2. **Bar fills, remaining stragglers → status hues** — ClaudeSubscriptionPanel fillClass ("Current week Opus only" salmon red) and ProviderQuotaCard quota-window fills: red-400→`bg-(--status-task-blocked)`, amber/yellow-400→`bg-(--status-task-todo)`, green-400→`bg-(--status-task-done)`. Healthy `bg-primary/70` + null `bg-zinc-700` tiers unchanged (r1 ruling). Inspected, NOT a bar: BudgetSidebarMarker circular icon badge (left).
3. **Systematic cyan→status-blue liveness sweep** (~50 sites / 20 files; supersedes r1's "Live family left" note per user ruling): running-status tones (CommentThread, interrupt-handoff, runRetryState, AgentDetail run chip + status maps), live dots/pings (AgentDetail, OnboardingChat, ArtifactsPanel generating, ActiveAgentsPanel, IssueDetail Live pill, RunTranscriptView, IssueRunLedger live chip, DesignGuide sample), Live surfaces (LiveRunWidget theme, ActiveAgentsPanel Live-now box, AgentDetail live-card border), scheduled-retry family (IssueScheduledRetryCard, runRetryState, IssueRunLedger retry-pending), externalObjectStatus icon/badge `running` (now shares blue with `open`; liveness pulse differentiates — flagged), timeline "now" marker `#2dd4bf`→`#2563eb` (1.5px line vs `#5b9bf6` delegated bars, shape differentiates — flagged), and the liveness glow shadow tokens `--shadow-extract-1/11/14` value-edited `rgba(6,182,212,0.08)`→`rgba(37,99,235,0.08)` (kept-token VALUES changed, call sites unchanged).
Deliberately LEFT (non-liveness cyan, one-line reasons): xterm terminal cursor (CompanyEnvironments — terminal chrome); `on_demand` invocation-source chips (AgentDetail x2 + DesignGuide sample — source tag, not liveness); CompanySkills "Includes assets" chip (content-type tag); CompanyImport renamed-file mono text x2 (rename annotation); BlockedReasonChip `recovery_required` (blocked-reason category); IssueRunLedger "Advanced" outcome + "Silence snoozed" tones (outcome/pause semantics, not live); UxLab decorative gradients (B1 allowlisted decoration).
## Gallery feedback round 3 (preset-tune session, Jul 6) — executed
1+2. **Toggle unification** — hunt found exactly ONE second switch implementation: `ToggleField` in agent-config-primitives.tsx (hand-rolled h-5 w-9 pill, `bg-green-600` track — the "other green"). It now renders the canonical `ToggleSwitch` (3da1bbcc5 capsule, on = `var(--status-task-done)`), same props/behavior/testid. All other named suspects (AgentConfigForm, Instance*Settings, RoutineDetail, PipelineSettings, story fixtures) already used ToggleSwitch; remaining `bg-green-600` hits are buttons, not tracks. Every switch now renders the one capsule + one green.
3. **Agent-status chips → canonical colors** ("Org snippets and quick scan identity" = StatusBadge in control-plane-surfaces story): `statusBadge` agent keys now route through `brandChipBadge` families (bordered brand chips): running → blue, idle → GRAY (was yellow tint), paused → amber, **active → green (no canonical agent status exists — user-ruled mapping to the brand green/done family)**; error already rides the shared run-status red. brandChipBadge block moved above statusBadge in status-colors.ts (declaration order). Org.tsx's hand-rolled status-dot ternary now routes through `agentStatusDot` (same hues + gains the blue running dot). Left: Companies.tsx company-status chip (company entity, not agent), AgentConfigForm "current" model tag (not a status).
4. **Dark-text-on-light-wash sweep** — the flagged BudgetPolicyCard banner was already fixed in round 2 (screenshot predated it). Systematic sweep (bare `text-{red,amber,emerald,sky,green,yellow,cyan}-{50..300}` without dark: protection): **58 sites fixed across 14 files** (Dashboard budget alert, BudgetPolicyCard remaining-amount, AgentConfigForm banners x3, AgentDetail banners/chips x9, ProjectDetail x3, CompanyAccess, IssueDocumentsSection x6, RoutineHistoryTab x11, DocumentDiffModal x2, PipelineItemBodyDocument x2, RoutineSaveBar, DocumentFrameHeader, OutputFileTile x3, CompanySkills x10) — pattern: 50→950, 100→900, 200→800, 300→700 in light + original behind `dark:`, opacity suffixes preserved. Verified-safe leftovers: InviteLanding x4 (dark-styled standalone page), SidebarNavItem badge + DocumentAnnotationLayer tail + DevRestartBanner (solid dark/colored bg or dark:-protected).
## Verification status (this review)
- `pnpm check:token-gates` — re-run independently: 3/3 CLEAN (468 files, 31 allowlist entries).
- `pnpm typecheck` + full `pnpm test:storybook-visual` — re-running independently (in progress).
- Eyeball-pass note: the Phase 0 baseline was captured at the master fork point before any change, and the suite compares current rendering to it at `maxDiffPixels: 0` — pixel-equality with master-at-fork is machine-proven; side-by-side Storybook remains available on request (`pnpm storybook` here + `-p 6007` on master).
## Tune session — CLOSED (Jul 6, 2026)
User approved the complete new design language via gallery v4 + live test drive on the :3300 worktree instance ("ship it"). Merged origin/master (12 commits; one conflict — upstream deliberately removed the Wakes-on-confirm chip, deletion accepted). 296 snapshots re-baselined; gates 3/3 CLEAN; typecheck green; final suite verification run against the new baseline. Remaining roadmap: Run 3 (cards/pills/C11 sidebar + investigations + AgentDetail story), issue→task rename run, Run 4 (palette classes + toast), ESLint ratchet.