From 08f105fb600b78483f5e29e63a424b4226fbc5a5 Mon Sep 17 00:00:00 2001 From: Khoi Nguyen Date: Thu, 18 Jun 2026 17:18:09 +0700 Subject: [PATCH 01/15] ALFMOB-264: Add design system token integration epic plan --- .../phase-1-token-pipeline.md | 86 ++++++++ .../phase-2-color.md | 49 +++++ .../phase-3-typography.md | 69 +++++++ .../phase-4-spacing-shape.md | 35 ++++ .../phase-5-component-refactors.md | 60 ++++++ .../phase-6-snapshot-baselines.md | 44 ++++ .../plan.md | 192 ++++++++++++++++++ .../red-team.md | 77 +++++++ .../scout.md | 59 ++++++ .../validation.md | 30 +++ 10 files changed, 701 insertions(+) create mode 100644 Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/phase-1-token-pipeline.md create mode 100644 Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/phase-2-color.md create mode 100644 Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/phase-3-typography.md create mode 100644 Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/phase-4-spacing-shape.md create mode 100644 Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/phase-5-component-refactors.md create mode 100644 Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/phase-6-snapshot-baselines.md create mode 100644 Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/plan.md create mode 100644 Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/red-team.md create mode 100644 Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/scout.md create mode 100644 Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/validation.md diff --git a/Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/phase-1-token-pipeline.md b/Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/phase-1-token-pipeline.md new file mode 100644 index 00000000..46592efa --- /dev/null +++ b/Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/phase-1-token-pipeline.md @@ -0,0 +1,86 @@ +## Phase 1: Token pipeline foundation (ALFMOB-272) + +### Goal +Stand up the DTCG → Swift codegen pipeline: pull JSON from the private token repo, parse + +resolve references in Swift, emit committed type-safe Swift that preserves the theme→primitive +reference graph. No theme types consume it yet (that's P2–P4). Gates the whole epic. + +### Prereqs +- Access to `Mindera/Alfie-Mobile-Design-Tokens` (Open Q5). +- ADR-293 (Done) for the contract/alias rules. + +### Steps + +1. **Pull + inspect the token export** (new: `Alfie/scripts/pull-design-tokens.sh`) + - Mirror `Alfie/scripts/run-apollo-codegen.sh` style: `set -ex`, clone/sparse-fetch the private + repo into a temp dir, copy the 14 DTCG files into `Alfie/AlfieKit/Sources/SharedUI/DesignTokens/`. + - **First run is manual/exploratory**: inspect the file set, token categories, the typography + ramp names, the `{reference}` alias depth, and which font families are referenced. Record + findings → resolves Open Q1/Q2. *Do not write the emitters until the JSON shape is known.* + - why: the ticket says copy-in; user wants pull-on-demand. This script is the reconciliation. + +2. **Build the generator** (new SwiftPM executable `Tools/DesignTokenGen/`) + - `Package.swift` (executable target, Foundation only — no third-party deps). + - `Sources/DesignTokenGen/`: + - `DTCG.swift` — Codable models for DTCG (`$value`, `$type`, group nesting, `{alias}` strings). + - `AliasResolver.swift` — resolve `{a.b.c}` references; handle chains ≤5 deep; **detect cycles** + and **error on missing refs** (don't silently emit empty). This is the unit-test core. + - `Emit+Color.swift`, `Emit+Typography.swift`, `Emit+Spacing.swift`, `Emit+Radius.swift` — + string-builder emitters per category. + - `main.swift` — read input dir, write `*+Generated.swift` to the output dir, stamp an + "AUTO-GENERATED — do not edit" header **and `// swiftlint:disable all`** on every file + (red-team M1: generated identifiers like `mono050`/`green050` + file length trip + `opt_in_rules: [all]`, which runs as a build phase). + - **Reference-graph requirement**: emit primitives as one namespace and semantic tokens as + references to them, e.g. + ```swift + enum Primitives { enum Color { static let neutrals800 = Color(red: …) } } + enum ThemeTokens { static let buttonPrimaryBackground = Primitives.Color.neutrals800 } // reference preserved + ``` + (Flatten only at the leaf hex; keep the alias as a symbol reference, per AC.) + - why: standalone Swift = no Node; testable; output committed so the package build never runs it. + +3. **Generation wrapper** (new: `Alfie/scripts/generate-design-tokens.sh`) + - `swift run --package-path Tools/DesignTokenGen DesignTokenGen \ + --input Alfie/AlfieKit/Sources/SharedUI/DesignTokens \ + --output Alfie/AlfieKit/Sources/SharedUI/GeneratedTokens` + - Document that devs run `pull` then `generate` then commit, only when tokens change. + +4. **Wire into the package** (`Alfie/AlfieKit/Package.swift:277` SharedUI target) + - Add `exclude: ["DesignTokens"]` so the committed JSON is **not** compiled/bundled at runtime + (only the generated Swift under `GeneratedTokens/` is a normal source — needs no resource entry). + - Do **not** add `Tools/DesignTokenGen` to the AlfieKit graph (separate package → app/CI builds + don't compile the generator). + - test: `swift build` of AlfieKit succeeds with the new `GeneratedTokens` sources + excluded JSON. + +5. **Lint exclude** (`Alfie/.swiftlint.yml`) — add `AlfieKit/Sources/SharedUI/GeneratedTokens` to + `excluded:` (alongside the existing `L10n+Generated.swift` / `BFFGraph` entries). Belt-and-braces + with the per-file `swiftlint:disable all` header (step 2). Without this, P1's own verify gate fails. + +6. **Drift detection — LOCAL this stage** (validation: no CI involvement yet). Document in + `Docs/DesignTokens.md` that the iOS dev runs `generate-design-tokens.sh` then `git diff` before + committing, so generated Swift can't silently diverge from `design-token.json` (red-team M2). + *Deferred:* promote to a CI `git diff --exit-code -- '**/GeneratedTokens/*'` gate (Swift toolchain + only, no private-repo access needed) when CI is in scope. + +7. **Mark generated paths** — add `Alfie/AlfieKit/Sources/SharedUI/GeneratedTokens/` to + `generated_paths` in `.claude/ios-profile.md` and to the NEVER-edit list in `CLAUDE.md`/`AGENTS.md`, + matching how `L10n+Generated.swift` / `BFFGraph/API` are treated. + +8. **Docs** (new: `Docs/DesignTokens.md`) — how to pull, generate, commit; the alias→Swift mapping; + the "never hand-edit generated" rule. + +### Verification +- Run `./Alfie/scripts/verify.sh` → must reach `✅ FULL VERIFICATION PASSED` (incl. SwiftLint build phase). +- Generator unit tests pass: `swift test --package-path Tools/DesignTokenGen` — alias chains, cycle, + missing-ref error, hex/number formatting, **AND an assertion that a semantic token emits as a + reference to a primitive symbol (e.g. `= Primitives.Color.neutrals800`), not a flat literal** + (red-team M4 — satisfies the reference-graph AC). +- Generated files compile in AlfieKit; nothing references them yet (no behaviour change → app + snapshots unchanged). +- Manual: `generate-design-tokens.sh` run twice produces a **byte-identical** diff (deterministic output). + +### Estimated Effort +**Spike first** (pull + inspect JSON + resolve Q1/Q2/Q5 auth) = S–M, **blocked on private-repo creds**. +Emitters + wiring + lint/drift + tests = L–XL once the JSON shape is known. Do NOT write emitters +against an assumed contract (red-team M3). diff --git a/Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/phase-2-color.md b/Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/phase-2-color.md new file mode 100644 index 00000000..72c53257 --- /dev/null +++ b/Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/phase-2-color.md @@ -0,0 +1,49 @@ +## Phase 2: Color integration (ALFMOB-274) + +### Goal +Back `PrimaryColorsProtocol` / `SecondaryColorsProtocol` with generated token values instead of +`Colors.xcassets`. Keep `Colors.primary.*` / `Colors.secondary.*` call sites unchanged. + +### Depends on +Phase 1 (generator emits `GeneratedColors`). + +### Steps + +1. **Emit colors** (P1 emitter → `SharedUI/GeneratedTokens/Colors+Generated.swift`) + - **Reference graph (red-team M4):** only *primitives* hold hex (`Primitives.Color.neutralsXXX = + Color(red:green:blue:opacity:)`). *Semantic* tokens are emitted as references to primitive + symbols (`static let buttonPrimaryBackground = Primitives.Color.neutrals800`) — never a re-flattened + literal. The protocol members below read the semantic/primitive symbols. + - Map token names → the exact protocol members in `PrimaryColors.swift:20` (mono900…mono050, + black, white) and `SecondaryColors.swift:52` (green/red/blue 050–900, yellow/orange 050–500). + - **Verify cardinality**: protocol expects 10 mono; green/red/blue 050–900 (10 each); yellow/orange + 050–500 (6 each). If tokens differ, reconcile the protocol — don't silently drop. + +2. **Re-point the structs** (`Theme/Color/PrimaryColors.swift:20`, `SecondaryColors.swift:52`) + - Replace each `Color("Mono900", bundle:)` with the generated primitive + (`GeneratedColors.mono900` / `Primitives.Color.…`). Protocol unchanged → zero call-site churn. + +3. **Retire the asset catalog** (Option B) + - **Dark mode = non-issue (red-team m1, CONFIRMED):** 53/54 colorsets are `idiom:universal`; the + one dark variant (`Blue050`) is RGB-identical to light (no-op); no `overrideUserInterfaceStyle`/ + `preferredColorScheme` anywhere. So code-based `Color` loses nothing real. Proceed with Option B. + - Grep repo-wide for direct asset access that bypasses the protocol: + `Grep 'Color("'`, `'UIColor(named:'`, `'Colors.xcassets'` across `source_roots`. + Any hit outside the two color files must be re-pointed first. + - Delete `Theme/Color/Colors.xcassets`; remove `.process("Theme/Color/Colors.xcassets")` from + `Package.swift:289`. + - keep `Color.ui` extension + `Colors` accessor (`Color.swift`) as-is — `Color.ui = UIColor(self)` + is a pure value conversion (`Utils/Extensions/Color+Extension.swift:4`), no asset lookup, so + `ThemeProvider.setupAppearance()`'s `.ui` calls survive the deletion. + +4. **`.ui` / UIColor path** — `ThemeProvider.setupAppearance()` (`ThemeProvider.swift:28,34,38,45,73`) + uses `Colors.primary.*.ui`; confirm `Color.ui` still resolves once colors are code-based (no asset). + +### Verification +- `./Alfie/scripts/verify.sh` → `✅ FULL VERIFICATION PASSED`. +- App snapshot suites in `Alfie/AlfieTests/Snapshots/` show **no diff** if token hex == current asset + hex; any diff is a flagged baseline decision (Open Q3), not an automatic pass. +- Manual: visual spot-check of key screens (dark-mode parity already confirmed a non-issue, m1). + +### Estimated Effort +M diff --git a/Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/phase-3-typography.md b/Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/phase-3-typography.md new file mode 100644 index 00000000..6d1cf9c7 --- /dev/null +++ b/Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/phase-3-typography.md @@ -0,0 +1,69 @@ +## Phase 3: Typography (ALFMOB-266) — split 3a / 3b (red-team C3/C4) + +The original single phase assumed a "thin" shim and bundled fonts. Reality: ~15 `UIFont` getters +and ~70 `AttributedString` builders across h1/h2/h3/paragraph/small/tiny (normal/bold/italic/ +underline/strike), and only `SF-Pro-Display-Medium.otf` is bundled — today the code renders **every** +level/variant in SF Pro despite doc-comments naming `freightBook`/`circularBold`/`circularMedium`. +So: do the nameable, shippable work in **3a**; isolate the externally-blocked font swap in **3b**. + +--- + +## Phase 3a: Figma-name typography + compat shim (shippable, keeps SF Pro) + +### Goal +Generate Figma-verbatim typography tokens (`Typography.display.large`, `heading.medium`, `body.small`, +…) and re-implement the legacy `header/paragraph/small/tiny` protocols to forward to them. Sizes/ +weights/line-heights/letter-spacing come from tokens; **font family stays SF Pro** until 3b. Every +call site (`ThemeProvider.shared.font.header.h1(...)`) keeps compiling. + +### Depends on +Phase 1 (generator) + the agreed legacy→Figma mapping table (Open Q2 — design sign-off, BLOCKS 3a). + +### Steps +1. **Mapping table** (design sign-off) — map all ~15 getters: `h1/h2/h3 → display/heading.*`, + `paragraph(normal/bold/italic) → body.*`, `small → body.small/label.*`, `tiny → label.small`, etc. + Underline/strike stay render-time flags (`.build(isUnderlined:strike:)`), NOT separate tokens. + Record in `Docs/DesignTokens.md`. *Do not guess — pull the real ramp first.* +2. **Emit typography** (`GeneratedTokens/Typography+Generated.swift`) — per token: family (SF Pro for + now), size, weight, line-height, letter-spacing; shaped to feed + `AttributedString.build(font:lineHeight:letterSpacing:…)` (`Helpers/Font+Extensions.swift:7`). +3. **Shim the concrete classes** (`Specifications/TypographyHeaderProtocol.swift:30`, + `TypographyParagraphProtocol.swift:44`, `TypographySmallProtocol.swift:40`, `TypographyTinyProtocol.swift:31`) + — replace hardcoded `FontNames.sfProMedium.withSize(36)` with token-derived metrics; preserve all + ~70 builder method bodies + `UIFont` getter signatures verbatim. + - Conscious decision on the pre-existing bug `TypographyHeader.h3(_ res:)` → calls `h2(...)` + (`:63–65`): fix or preserve? If preserved, the eventual P6 baseline locks it in — file as separate ticket (m3). +4. **TypographyProvider** (`TypographyProvider.swift:14`) — surface unchanged; now token-backed. + +### Verification +- `./Alfie/scripts/verify.sh` → `✅ FULL VERIFICATION PASSED`. +- Manual review: text geometry may shift if token sizes ≠ current (36/24/20/16/14/12) — accepted per + validation (tokens = source of truth). RTL still correct. Final baselines captured in P6. + +### Estimated Effort +M–L (the ~15→~70 mapping is the bulk; gated on the mapping table). + +--- + +## Phase 3b: Real font-family swap (EXTERNALLY BLOCKED — do not bundle into 3a) + +### Goal +Replace SF Pro with the actual token-referenced families (`freightBook`/`circular*`). + +### Blocker +The `.otf` files are **not in the repo** and are licensed assets the team must source. Until they +arrive, 3b cannot start; "Figma-verbatim typography" is cosmetic (names only). This is Open Q1. + +### Steps (when unblocked) +1. Obtain + license `.otf`s; add under `Theme/Typography/Resources/`; add to `Package.swift` resources + (`.copy`) + `Fonts.xcassets`. +2. Extend `FontNames` cases + `FontManager.registerAll()` (note: currently runs in preview mode only — + ensure runtime registration for app-wide use). +3. Re-point the generated typography family fields from SF Pro → the new families; regenerate. + +### Verification +- `./Alfie/scripts/verify.sh` → `✅ FULL VERIFICATION PASSED`; fonts render on device (not just preview). +- P6 baselines re-recorded intentionally (this is a deliberate visual change). + +### Estimated Effort +M once fonts exist; **indefinitely blocked** until they do. diff --git a/Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/phase-4-spacing-shape.md b/Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/phase-4-spacing-shape.md new file mode 100644 index 00000000..c55016ee --- /dev/null +++ b/Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/phase-4-spacing-shape.md @@ -0,0 +1,35 @@ +## Phase 4: Spacing & shape integration (ALFMOB-270) + +### Goal +Source `Spacing.*` and `CornerRadius.*` values (and shape border/shadow, if present) from generated +tokens. Keep the `Spacing.space200` / `CornerRadius.s` access patterns unchanged. + +### Depends on +Phase 1 (generator emits spacing/radius). + +### Steps + +1. **Emit spacing + radius** (`SharedUI/GeneratedTokens/Spacing+Generated.swift`, + `Radius+Generated.swift`) + - Map token semantic names → the existing constants in `Theme/Spacing/Spacing.swift:6` + (`space0`…`space1000`, 0–80pt) and `Theme/CornerRadius/CornerRadius.swift:7` + (`none/xxs/xs/s/m/l/xl/full`). + +2. **Forward the enums** — two viable shapes; pick one and apply consistently: + - (a) keep `enum Spacing` / `enum CornerRadius` but set each `static let` = the generated value + (`= GeneratedSpacing.space200`), or + - (b) generate the enum directly and delete the hand-written one. + - **Prefer (a)**: smallest diff, preserves the doc-comments + `swiftlint:disable` in + `CornerRadius.swift:3`, zero call-site churn. AC "no hardcoded numeric values remain in theme + files" is met because the literal now lives in the generated file. + +3. **Shape** (`Theme/Shape/DefaultShapeProvider.swift:3`, `ShapeProviderProtocol.swift:4`) + - Current protocol only exposes `unavailableCrossedOutShape()`. If tokens add border widths / + shadows, extend the provider to read them; otherwise no change (don't invent surface — YAGNI). + +### Verification +- `./Alfie/scripts/verify.sh` → `✅ FULL VERIFICATION PASSED`. +- App snapshots: spacing changes cascade across layouts — watch for diffs; baseline decisions per Open Q3. + +### Estimated Effort +S–M diff --git a/Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/phase-5-component-refactors.md b/Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/phase-5-component-refactors.md new file mode 100644 index 00000000..997335ef --- /dev/null +++ b/Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/phase-5-component-refactors.md @@ -0,0 +1,60 @@ +## Phase 5: Component refactors + snapshot harness (ALFMOB-271/268/273/269/267) + +### Goal +Refactor the 5 core components to consume token-generated values; keep every public API +backward-compatible; establish the SharedUI snapshot harness (none exists today) and baseline each +component before/after. + +### Depends on +Phases 2–4 (color/typography/spacing/shape are token-backed). After P2–P4 most refactors are +already partly done (components read `Colors.*`/`Spacing.*`/`CornerRadius.*` which are now token-backed); +remaining work = replace any **hardcoded literals** inside each component + add snapshot tests. + +### Verification approach (no component snapshot guard yet) +The SharedUI snapshot harness + baselines are built in **P6 (last)**, not here — per stakeholder, +baselining components mid-refactor has no value when tokens are the source of truth and the look is +expected to change. During P5, verify each component via `verify.sh` (build + unit tests) + manual +visual review of all states + the existing app-level snapshots (`Alfie/AlfieTests/Snapshots/`). The +final token-driven appearance is captured as the regression baseline in P6. + +### Per component + +1. **ThemedButton** (ALFMOB-271) — `Theme/Buttons/ButtonTheme.swift:23`, `ThemedButton.swift:5` + - `ButtonThemeSpec` literals (`Colors.primary.mono900/500/300/100`, `.white`, `Spacing.space100/200`, + `CornerRadius.s`, fonts) → token refs. Cover 4 styles × 3 sizes × {normal,disabled,pressed,loading}. + - Owner: dev-1. + +2. **ThemedInput** (ALFMOB-268) — `Theme/Inputs/ThemedInput.swift:5` + - Per-state border/bg/placeholder/error/success colors, `CornerRadius.xs`, `Spacing.*`, fonts → tokens. + - States: empty/info/success/error + disabled + icon + char-limit. Owner: dev-2. + +3. **Chip** (ALFMOB-273) — `Components/Chips/Chip.swift:44` + - Colors/`Spacing.*`/`CornerRadius.full`/fonts → tokens; replace hardcoded heights (36/44) + + border widths (1.0/2.0) with token sizing if tokens define them, else keep + comment. + - Selected/unselected. Owner: dev-2. + +4. **Badge** (ALFMOB-269) — `Components/Indicators/BadgeViewModifier.swift:9`, `BadgeTabViewModifier.swift:9` + - `Colors.secondary.red700`, `.white`, `CornerRadius.full`, `theme.font.tiny`, hardcoded sizes + (badgeHeight 16, paddings 4, indicator 12, border 1) → tokens where defined. + - `BadgeTabViewModifier` also sets `UITabBarItem.appearance()` — keep. Counts 1/9/99/999+. + `BadgeHelperTests` must stay green. Owner: dev-3. + +5. **Typography/Label** (ALFMOB-267) — `Theme/Typography/**` + - Largely satisfied by Phase 3. This story = **verification**: grep the render path for any + remaining hardcoded font size/weight/family; confirm `AttributedString`/`UIFont`/`Text.build` + all flow through tokens. Owner: dev-3. + +### Backward-compat rule +No initializer / enum-case signature changes (`ButtonType`, `ButtonTheme`, `InputStatus`, +`ChipConfiguration`, `badgeView`/`tabItemBadge`). If a token-driven value forces an API change, stop +and raise — the epic AC forbids call-site churn. + +### Verification +- `./Alfie/scripts/verify.sh` → `✅ FULL VERIFICATION PASSED` after each component. +- Manual visual review of every state per component (token-driven changes accepted per validation). +- App snapshots in `Alfie/AlfieTests/Snapshots/` — diffs reviewed/accepted, not auto-blocking. +- Component snapshot baselines are recorded later in **P6**. + +### Estimated Effort +M per component (L total). Parallelizable across dev-1/2/3 once P2–P4 land (generated files +owned by dev-1 to avoid merge churn). diff --git a/Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/phase-6-snapshot-baselines.md b/Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/phase-6-snapshot-baselines.md new file mode 100644 index 00000000..8296cfe2 --- /dev/null +++ b/Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/phase-6-snapshot-baselines.md @@ -0,0 +1,44 @@ +## Phase 6: Snapshot harness & baselines (FINAL — moved from first, per stakeholder) + +### Goal +Establish the SharedUI snapshot harness and capture the **final, token-driven** appearance of the 5 +components as the going-forward regression baseline. Also audit theme-consumer coverage. + +### Why this is LAST, not first +The original plan put this first to prove "no visual change." Validation decided **tokens are the +source of truth** — the look is *expected* to change across P2–P5 — so a pre-refactor baseline has no +assertion value and would just be re-recorded. Capture baselines once the components are stable. + +**Accepted trade-off:** during P2–P5 there is **no component-level snapshot guard**. Refactor +regressions are caught instead by `verify.sh` (build + unit tests) + manual visual review + the +existing app-level snapshots (`Alfie/AlfieTests/Snapshots/`). This phase closes that gap permanently +for future design-system work. (This consciously overrides red-team C1's "baseline first" stance, +which assumed a no-visual-change refactor.) + +### Depends on +P5 complete — all 5 components on final token values (and P3a typography settled). Do not record +baselines mid-refactor. + +### Steps + +1. **New snapshot target** (`Package.swift`) — add `SharedUISnapshotTests`, deps `["SharedUI", "TestUtils"]` + (TestUtils already vends `SnapshotTesting` 1.18.3, `Package.swift:301`). Write a SharedUI-local + snapshot helper using only public SharedUI API — do **not** reuse the app's + `View+Snapshots.swift` (`@testable import Alfie`, not portable). + +2. **Record final baselines** — ThemedButton (4 styles × 3 sizes × {normal,disabled,pressed,loading}), + ThemedInput (empty/info/success/error + disabled + icon + char-limit), Chip (selected/unselected, + small/large), Badge (1/9/99/999+, both modifiers), Label (h1–tiny incl. underline/strike/italic). + These become the regression baseline for all future token changes. + +3. **Coverage audit** — enumerate which app-wide theme consumers (`Colors.` 78 files / `Spacing.` 108 / + typography 79) and which app screens in `Alfie/AlfieTests/Snapshots/` do/don't have snapshot + coverage; record the gap so future work knows what's unguarded. + +### Verification +- `./Alfie/scripts/verify.sh` → `✅ FULL VERIFICATION PASSED` with the new target recording + a clean + no-op re-run. +- Baselines committed; re-running the suite passes (proves the harness is stable on final values). + +### Estimated Effort +M–L (new target + SharedUI-local helper + 5 component baselines + coverage audit). diff --git a/Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/plan.md b/Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/plan.md new file mode 100644 index 00000000..c3af69c7 --- /dev/null +++ b/Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/plan.md @@ -0,0 +1,192 @@ +--- +title: Design System — Token Integration & Component Refactoring +ticket: ALFMOB-264 +status: pending +complexity: HIGH +mode: hard +blockedBy: [] +blocks: [] +created: 2026-06-18 +--- + +## Overview + +Drive the iOS design system from the shared W3C **DTCG** design-token contract instead of +hardcoded Swift. A Swift codegen step reads the DTCG JSON (pulled from the private +`Mindera/Alfie-Mobile-Design-Tokens` repo) and emits type-safe Swift that preserves the token +reference graph (theme → primitives). Existing `SharedUI` theme types are re-pointed at the +generated values; 5 core components are refactored to consume them. All public APIs stay +backward-compatible. Epic = 9 stories across 2 phases (infra + integration, then component +refactors). + +## Decisions locked (this planning session) + +- **Generator = standalone Swift** (a `Tools/DesignTokenGen` SwiftPM executable run via a shell + script), **not** Style Dictionary/Node. Repo has zero Node today; mirrors the existing + `run-apollo-codegen.sh` + SwiftGen-plugin precedent. Cross-platform parity is guaranteed by + the shared DTCG JSON, not the generator. +- **Tokens pulled on demand**: `pull-design-tokens.sh` fetches DTCG JSON from the private repo, + commits it under `SharedUI/DesignTokens/`; codegen then runs offline for every other dev/CI + build. Satisfies the ticket's "stored in repo" AC while keeping private-repo access to refresh-time only. +- **Typography naming = Figma-verbatim + compat shim**: generated `Typography.display.large` / + `heading.medium` / `body.small` become the source of truth; legacy `header.h1` / `paragraph` / + `small` / `tiny` protocols are kept as thin forwarders so all call sites compile (ADR-293). +- **Color backing = Option B (code-based `Color` from tokens)**, per ticket recommendation — + retires `Colors.xcassets` as the source of truth (single source of truth, no asset-catalog drift). + +## Acceptance Criteria (epic-level, from ALFMOB-264) + +- [ ] DTCG JSON stored in repo (`SharedUI/DesignTokens/`) +- [ ] Build-time Swift codegen reads DTCG → generates Swift +- [ ] Generated Swift preserves token references (theme → primitives), not flat values +- [ ] Color, typography, spacing, shape tokens integrated into the existing theme system +- [ ] Typography uses Figma token names (`display.large`, `heading.medium`) — not `Header.h1` +- [ ] 5 core components use token-generated values +- [ ] Existing component APIs remain backward-compatible +- [ ] Snapshot tests validate visual consistency + +## Approach + +> **Scope reframe (red-team C2):** this is NOT a "5 component" change. P2–P4 re-point shared theme +> types consumed app-wide — `Colors.` in **78 files**, `Spacing.` in **108**, typography in **79**. +> Any generated value ≠ today's hardcoded value shifts the whole app, not just the 5 components. +> Per validation, **tokens are the source of truth** — those shifts are intended and accepted. + +> **Snapshot phase moved to LAST (stakeholder):** since P2–P5 intentionally change the components, +> baselining the current look first has no assertion value. The SharedUI snapshot harness + baselines +> are captured at the **end** (P6), recording the final token-driven appearance as the regression +> baseline. During P2–P5 the refactor is guarded by `verify.sh` + manual review + app-level snapshots. + +Sequential dependency chain (the ticket's stated `Depends On` links are internally +inconsistent — see Risks; this is the corrected order): + +``` +P1 pipeline ─▶ P2 274 color ─▶ P3a 266 rename+shim ─▶ P4 270 spacing+shape ─▶ P5 components ─▶ P6 snapshots +(272) 271/268/273/269/267 (baselines) + P3b font-swap = SEPARATE, externally blocked (licensed .otf) +``` + +Generated output is **committed** (no per-build codegen). Each theme type keeps its current +public surface and is re-implemented to read generated values, so component refactors (P5) are +mostly find/replace of `Colors.*` / `Spacing.*` / `CornerRadius.*` literals → generated refs. +Component snapshot baselines are recorded afterward in **P6**. + +## Phases + +1. **Token pipeline foundation** (ALFMOB-272) — pull script, DTCG parser + alias resolver, + Swift emitters (reference-graph contract), lint-exclude + drift-detection, committed output, docs. + *Front-loaded spike: pull + inspect the JSON before writing emitters.* Gates P2–P5. +2. **Color integration** (ALFMOB-274) — re-point `PrimaryColors`/`SecondaryColors` at generated + primitives; retire `Colors.xcassets` (dark-mode is a confirmed non-issue — see Risks). +3a. **Typography rename + shim** (ALFMOB-266) — generated Figma-named typography mapped 1:1 to the + ~15 legacy getters / ~70 builders; legacy protocols forward to it. **Keeps SF Pro rendering.** Shippable. +3b. **Typography font swap** (ALFMOB-266, split) — bundle/register the real `freightBook`/`circular*` + families. **Externally blocked** on licensed `.otf` delivery; do not bundle into the 3a estimate. +4. **Spacing & shape integration** (ALFMOB-270) — generated `Spacing`/`CornerRadius`/shape values. +5. **Component refactors** (ALFMOB-271/268/273/269/267) — Button, Input, Chip, Badge, Label; verified + by `verify.sh` + manual review + app-level snapshots (component baselines come in P6). +6. **Snapshot harness & baselines** (no ticket; runs LAST) — build the `SharedUISnapshotTests` target + (none exists; app helper is `@testable import Alfie`-bound, not portable) and record the **final** + token-driven component appearance as the going-forward regression baseline; audit coverage. + +## File Changes (Summary) + +| File / group | Module | Type | Change | Phase | Owner | +|---|---|---|---|---|---| +| `Alfie/.swiftlint.yml` | config | Edit | Add `GeneratedTokens` to `excluded` | P1 | dev-1 | +| `.github/workflows/alfie.yml` | CI | Edit | Drift check (regenerate + `git diff --exit-code`) — **DEFERRED** per validation; not this stage | later | dev-1 | +| `Tools/DesignTokenGen/**` (new SwiftPM exe) | tooling | New | DTCG models, alias resolver, reference-graph emitters; stamps `swiftlint:disable all` | P1 | dev-1 | +| `Alfie/scripts/pull-design-tokens.sh` | scripts | New | Fetch DTCG JSON from private repo | P1 | dev-1 | +| `Alfie/scripts/generate-design-tokens.sh` | scripts | New | `swift run` the generator | P1 | dev-1 | +| `SharedUI/DesignTokens/*.json` | SharedUI | New | Committed DTCG source (14 files) | P1 | dev-1 | +| `SharedUI/GeneratedTokens/*+Generated.swift` | SharedUI | New | Committed generated Swift | P1–P4 | dev-1 | +| `AlfieKit/Package.swift` | AlfieKit | Edit | Exclude DTCG JSON from target; drop `Colors.xcassets` resource | P1/P2 | dev-1 | +| `Docs/DesignTokens.md` | docs | New | Update/refresh process | P1 | dev-1 | +| `Theme/Color/PrimaryColors.swift`, `SecondaryColors.swift` | SharedUI | Edit | Back protocols with generated values | P2 | dev-1 | +| `Theme/Color/Colors.xcassets` | SharedUI | Delete | Retire (Option B) | P2 | dev-1 | +| `Theme/Typography/Specifications/*Protocol.swift` | SharedUI | Edit | Forward legacy names → generated Figma tokens | P3 | dev-1 | +| `Theme/Typography/Helpers/FontNames.swift`, `FontManager` | SharedUI | Edit | Register token-referenced font families | P3 | dev-1 | +| `Theme/Spacing/Spacing.swift`, `CornerRadius/CornerRadius.swift` | SharedUI | Edit | Forward to generated values | P4 | dev-1 | +| `Theme/Shape/DefaultShapeProvider.swift` | SharedUI | Edit | Token border/shadow (if present) | P4 | dev-1 | +| `Theme/Buttons/ButtonTheme.swift`, `ThemedButton.swift` | SharedUI | Edit | Token refs | P5 | dev-1 | +| `Theme/Inputs/ThemedInput.swift` | SharedUI | Edit | Token refs | P5 | dev-2 | +| `Components/Chips/Chip.swift` | SharedUI | Edit | Token refs | P5 | dev-2 | +| `Components/Indicators/Badge*ViewModifier.swift` | SharedUI | Edit | Token refs | P5 | dev-3 | +| `Theme/Typography/**` (Label render path) | SharedUI | Verify | Confirm all text uses tokens | P5 | dev-3 | +| `Tests/SharedUISnapshotTests/**` (new target) | tests | New | SharedUI-local helper + final component baselines (no `@testable import Alfie`) | P6 | dev-1 | +| `AlfieKit/Package.swift` | AlfieKit | Edit | Add `SharedUISnapshotTests` target (deps SharedUI+TestUtils) | P6 | dev-1 | + +## Feature Flag + +`n/a` for the codegen/theme swap (pure internal refactor, no behaviour change). **But** P2–P4 +change *rendered pixels* if any token value differs from today's hardcoded value. There is no +runtime flag to gate this; the safety net is snapshot tests + a deliberate baseline review +(see Open Questions / ALFMOB-274 AC). If the team wants a staged rollout, the only lever is +shipping P1 (infra, invisible) ahead of P2–P5. + +## Testing Strategy + +- **Snapshot harness is the FINAL phase (P6), not first** — new `SharedUISnapshotTests` target with a + SharedUI-local helper (the app's `View+Snapshots` is `@testable import Alfie`-bound, unusable here). + Baselines record the **final** token-driven appearance (tokens are source of truth → no value in a + pre-refactor baseline). +- **During P2–P5 (no component snapshot guard)**: regressions caught by `verify.sh` + manual visual + review + existing app-level snapshots in `Alfie/AlfieTests/Snapshots/` (partial coverage — gap + audited in P6). Token-driven diffs are accepted per validation. +- **Unit**: DTCG alias resolver (P1) — table-driven tests incl. 5-level chains, cycles, missing refs, + AND an assertion that semantic tokens emit as references to primitive symbols (not flat hex). + Existing `BadgeHelperTests` stays green. +- **Drift gate**: this stage = **local** — the iOS dev runs `generate-design-tokens.sh` + `git diff` + before committing (per validation: no CI involvement yet). Promote to a CI `git diff --exit-code` + gate later when CI is in scope. +- **Verify gate**: `./Alfie/scripts/verify.sh` must end in `✅ FULL VERIFICATION PASSED` per phase. + +## Risks & Mitigations + +| Risk | Likelihood | Mitigation | +|---|---|---| +| **App-wide reskin under-scoped** — P2–P4 shift 78/108/79 files, not 5 (red-team C2) | High | Tokens = source of truth (validation): shifts intended/accepted; P6 coverage audit + design/PM awareness | +| **No component-level snapshot guard during P2–P5** (snapshots moved to P6 per stakeholder; tokens = source of truth) | Medium | Guard refactor via `verify.sh` + manual review + app-level snapshots; capture final SharedUI baselines in P6. Conscious trade-off | +| **Typography shim is large, not "thin"** — ~15 getters / ~70 builders, no 1:1 Figma ramp (red-team C3) | High | Pull real ramp first; agree mapping table; P3a maps explicitly | +| **Intended fonts not bundled; code fakes all families with SF Pro** (red-team C4) | High | Split P3b as externally-blocked; P3a renames-only, keeps SF Pro — shippable | +| Token JSON structure unknown until private repo pulled | High | P1 spike = pull + inspect before writing emitters; resolves Open Q1/Q2 | +| Pixel drift when token values ≠ current hardcoded values | High | Accepted per validation (tokens = source of truth); manual review during P2–P5, final baselines in P6 | +| **Generated files fail SwiftLint (`opt_in_rules: all`, build-phase)** (red-team M1) | High | P1: add `GeneratedTokens` to `.swiftlint.yml excluded` + emit `swiftlint:disable all` header | +| **Stale generated Swift drifts from JSON — no detection** (red-team M2) | Medium | This stage: local regen + `git diff` before commit (no CI yet, per validation). CI gate deferred | +| Reference-graph AC vs flat-hex emitter contradiction (red-team M4) | Medium | Lock P1 contract: semantic `static let = Primitives.Color.x`; unit-test the emitted RHS is a symbol | +| Retiring `Colors.xcassets` breaks direct `Color("Mono900", bundle:)` users outside the color files | Medium | Grep all `Color("`/`UIColor(named:` repo-wide before deleting (P2 step 3) | +| ~~Dark mode breaks under code-based Color~~ **CLOSED** (red-team m1) | — | 53/54 colorsets universal; the 1 dark variant (Blue050) is a no-op; no dark mode wired app-wide. Not a blocker | +| Ticket `Depends On` graph is circular/inconsistent | Medium | Use corrected order in this plan; update tickets | +| Committed generated files cause noisy diffs / merge conflicts during P5 parallel work | Low | One owner for generated files (dev-1); regenerate-and-commit only on token refresh | + +## Out of Scope + +- Refactoring the other ~35 SharedUI components (only the 5 named). +- Dark-mode / multi-theme support (unless tokens already encode it — verify, don't build). +- Android/Flutter token consumption. +- Automating token pulls in CI (manual `pull` + commit is the agreed model). + +## Open Questions + +1. **Font families**: does the typography token set reference fonts beyond SF Pro Display Medium + (doc-comments mention `freightBook`/`circularBold`/`circularMedium` — and today the code renders + ALL of them as SF Pro)? Who provides + licenses the `.otf` files? **Blocks P3b (font swap); P3a + ships without it.** +2. **Legacy→Figma type map**: exact mapping of `h1/h2/h3/paragraph/small/tiny` (~15 getters / ~70 + builders) → `display/heading/body/label.*`. Needs design confirmation. **Blocks P3a shim.** + _Dark mode: CLOSED — confirmed non-issue (see Risks)._ +3. ~~**Baseline policy**~~ **RESOLVED (validation)**: tokens are the source of truth — token-driven + pixel changes are re-baselined + accepted after review. Not a no-visual-change refactor. +4. **DTCG JSON commit vs gitignore**: plan commits it (ticket AC). Generation is a **local dev step** + (validation) — no CI pull. Commit both JSON + generated Swift. +5. ~~**Private repo / CI auth**~~ **RESOLVED (validation)**: no CI this stage; an iOS dev runs the + generate script locally against `design-token.json`. (CI drift gate deferred.) + +> **Execution scope this round (validation):** the **P1 spike** only (pull/inspect `design-token.json` +> + scaffold the generator). P2+ gated on the legacy→Figma mapping (Open Q2, design) and a usable +> `design-token.json`. P6 snapshots run last, after the components are final. Solo start. + +## Per-phase detail + +See `phase-1-token-pipeline.md` → `phase-6-snapshot-baselines.md`. Red-team findings + dispositions +in `red-team.md`; stakeholder decisions in `validation.md` (incl. snapshot phase moved to last). diff --git a/Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/red-team.md b/Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/red-team.md new file mode 100644 index 00000000..2bd9e2bb --- /dev/null +++ b/Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/red-team.md @@ -0,0 +1,77 @@ +# Red Team Review: ALFMOB-264 Design System Token Plan + +_Adversarial review, 2026-06-18. Findings grounded in the repo. Plan updated to absorb these — see "Disposition" tags._ + +> **Superseded note:** C1/C2 dispositions below say "Phase 0 added (first)." A later stakeholder +> decision (see `validation.md`) **moved the snapshot harness + baselines to LAST (P6)** — tokens are +> the source of truth, so pre-refactor baselines have no value. The harness/SharedUI-local-helper +> substance of C1 still stands; only its position changed. + +## Verdict +Not shippable as written. Technically literate but rests on two load-bearing falsehoods: (1) snapshot baselines can be recorded "before refactor" — no SharedUI snapshot infra exists and the existing helper is welded to the app target; (2) this is a "5 component" change — 108 files consume `Spacing.`, 78 consume `Colors.`, 79 consume typography, so P2–P4 silently re-skin the whole app. Biggest single risk: P2–P4 are un-flagged, un-revertable big-bang theme swaps validated by a snapshot net that doesn't exist yet and can't be built where the plan says. + +## Critical (block — must fix before execute) + +### C1. Snapshot "baseline before refactor" is a fiction — no SharedUI snapshot infra; helper is app-bound +- **Evidence**: `Tests/SharedUITests/` has only `BadgeHelperTests`, `LocalizationTests`, and an empty `StyleGuideTests.testExample()`. `SharedUITests` target (`Package.swift:418`) depends only on `"SharedUI"` — no `TestUtils`/`SnapshotTesting`. App helper `Alfie/AlfieTests/Helpers/View+Snapshots.swift` does `@testable import Alfie`; not portable to a package target. +- **Why it breaks**: P5 "Step 0 — record reference images on main" is impossible (no harness on main). P2–P4 change pixels *before* P5 builds the harness → "baseline" is post-change → validates nothing. +- **Fix**: New **Phase 0** — build a real `SharedUISnapshotTests` target (add `TestUtils` dep), write a SharedUI-local snapshot helper (no `@testable import Alfie`), baseline on untouched `main` before P2. Budget as its own M/L task. **Disposition: ACCEPTED → Phase 0 added.** + +### C2. Scope mis-sold as "5 components"; P2–P4 re-skin 100+ files with no coverage +- **Evidence**: repo-wide grep — `Colors.` in 78 files, `Spacing.` in 108, `font.{header|paragraph|small|tiny}` in 79. +- **Why it breaks**: P2/P3/P4 re-point shared theme types; any generated value ≠ current literal shifts every consumer app-wide, not just the 5 components. No inventory of which app screens are snapshot-guarded → unguarded screens drift to prod. +- **Fix**: Phase 0 also audits `Alfie/AlfieTests/Snapshots/` coverage and enumerates unguarded consumers; explicit design/PM sign-off on the gap. Reframe epic as app-wide reskin. **Disposition: ACCEPTED → Phase 0 + scope reframed.** + +### C3. Typography cannot map 1:1; legacy variants vastly outnumber a Figma ramp +- **Evidence**: ~15 `UIFont` getters and ~70 `AttributedString` entry points across h1/h2/h3/paragraph/small/tiny, each with normal/bold/italic/underline/strike permutations. Doc-comments demand distinct families per level (freightBook/circularBold/circularMedium/circularBook…). +- **Why it breaks**: A Figma display/heading/body/label ramp won't carry tokens for every underline/strike combo — those are render-time attributes (`.build(isUnderlined:strike:)`), not font tokens. The "thin forwarder" shim is actually ~15 getter maps + ~70 preserved method bodies. The phase-3 example mapping is guesswork gated on design sign-off. +- **Fix**: Treat the legacy→Figma map + shim as the dominant cost of P3; pull the real ramp first; don't estimate P3 until then. **Disposition: ACCEPTED → P3 re-scoped + split (see C4).** + +### C4. Intended fonts NOT bundled — only SF Pro; current code fakes every family with it +- **Evidence**: only `SF-Pro-Display-Medium.otf` on disk; `FontNames` has one case. `TypographyHeader.h1` (doc: freightBook) returns `sfProMedium.withSize(36)`; all paragraph/small/tiny variants and the *italic* getters also return SF Pro. +- **Why it breaks**: App currently renders everything in SF Pro regardless of intent. "Figma-verbatim typography" references freightBook/circular* families that don't exist in-repo and are licensed fonts the plan can't produce. P3 is effectively blocked, yet estimated "L (M if no new fonts)." +- **Fix**: Split P3 → **P3a** (rename/alias to Figma token names, keep SF Pro rendering — shippable now) + **P3b** (actual font swap — separate, externally-blocked story). **Disposition: ACCEPTED → P3 split.** + +## Major (significant rework / wrong estimate) + +### M1. Generated files WILL be linted and trip multiple rules — not excluded +- **Evidence**: `Alfie/.swiftlint.yml` has `opt_in_rules: [all]`; `excluded:` names `L10n+Generated.swift`/`BFFGraph` but not `GeneratedTokens/`. SwiftLint runs as a build phase (`project.pbxproj:350`), so `verify.sh` lints it. Hand-written `CornerRadius.swift:3` already needs `swiftlint:disable discouraged_none_name identifier_name`. +- **Why it breaks**: `mono050`/`green050` (leading-zero identifiers), `type_body_length`/`file_length` caps, etc. fail under `all`. P1's own verify gate fails. +- **Fix**: P1 adds `GeneratedTokens` to `.swiftlint.yml excluded` AND emitter stamps `// swiftlint:disable all` per file. **Disposition: ACCEPTED → P1 step added.** + +### M2. No drift detection — stale generated Swift can diverge from JSON +- **Evidence**: CI (`.github/workflows/alfie.yml`) runs only `fastlane ios test`; no codegen/drift step (Apollo output is also committed+trusted). `generate-design-tokens.sh` is a manual dev step. +- **Why it breaks**: Edit JSON, forget to regenerate, commit → CI green, ships stale values. P1's "run twice → identical" proves determinism, not freshness. +- **Fix**: CI step runs `generate-design-tokens.sh` then `git diff --exit-code GeneratedTokens/`. Needs Swift toolchain only (not the private repo). **Disposition: ACCEPTED → P1 step added.** + +### M3. P1 "L" optimistic — gated on an unpulled private repo of unknown shape +- **Evidence**: P1 step 1 itself says "don't write emitters until JSON shape is known"; Open Q1/Q2/Q5 unresolved. +- **Fix**: Make "pull + inspect + resolve Q1/Q2 + auth" a separate **spike** with its own estimate; re-estimate P1 emitters after JSON is in hand. **Disposition: ACCEPTED → P1 spike split, estimate L→XL.** + +### M4. Reference-graph AC satisfiable, but phase-2 contradicts the risk-table sketch +- **Evidence**: AC wants theme→primitive references; risk-table emits `static let buttonPrimaryBackground = Primitives.Color.neutrals800` (good), but phase-2 says emit primitives as `Color(red:…)` and "flatten aliases at gen time." SwiftUI `Color` is a value type — only *symbol-level* indirection satisfies the AC. +- **Fix**: Lock emitter contract in P1: semantic tokens emitted as references to primitive symbols; primitives hold hex. Add unit test asserting semantic RHS is a primitive symbol, not a literal. **Disposition: ACCEPTED → P1 contract locked, phase-2 reworded.** + +## Minor + +### m1. Dark mode is a NON-issue — plan flagged it but never checked +- **Evidence**: 54 colorsets inspected; 53 are `idiom:universal` only. Sole exception `Blue050.colorset` has a `luminosity:dark` whose RGB == light (no-op). No `overrideUserInterfaceStyle`/`preferredColorScheme` anywhere. `Color.ui = UIColor(self)` (value conversion, no asset lookup). +- **Fix**: Option B loses *automatic* dark adaptivity in principle, but there are no real dark values to lose. **Downgrade risk to CLOSED.** **Disposition: ACCEPTED → risk closed in plan.** + +### m2. `exclude: ["DesignTokens"]` is correct (verified — not `.copy`). No change. +### m3. Pre-existing bug `TypographyHeader.h3(_ res:)` calls `h2(...)` (`:63–65`) — file a separate ticket; conscious decision before a baseline locks it in. +### m4. Spacing `space0…space1000` cardinality matches plan. Fine. + +## Unverifiable assumptions (unconfirmed — do not build against) +- DTCG JSON structure, file count ("14"), typography ramp names, alias depth (≤5) — private repo not pulled. +- Whether tokens carry freightBook/circular* families and who licenses/provides the `.otf`s (P3b blocker). +- Whether token hex == current asset/literal values (if not, C2's app-wide drift fires). +- Private-repo CI auth (Q5): committed-output model DOES remove CI's build-time need for the repo (CI runs only `fastlane test`); but the *generator* is still needed for drift (M2), and a human/runner still needs creds for periodic `pull`. Partly resolved. + +## What the plan got RIGHT (don't regress) +- Standalone Swift generator over Style Dictionary/Node — correct for this repo (zero Node; mirrors committed `run-apollo-codegen.sh`). +- Committed-output genuinely removes CI's private-repo dependency at build time — confirmed. +- `Color.ui = UIColor(self)` survives xcassets deletion (`Utils/Extensions/Color+Extension.swift:4`) — `setupAppearance()` `.ui` calls keep working. +- Corrected the circular ticket `Depends On` graph into a linear order. +- Forwarding-enum for Spacing/CornerRadius (keep enum, `static let = Generated.x`) — minimal diff, preserves the `swiftlint:disable` header. +- Flagging the `h3→h2` bug with "don't silently improve" discipline. diff --git a/Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/scout.md b/Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/scout.md new file mode 100644 index 00000000..e0e1db50 --- /dev/null +++ b/Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/scout.md @@ -0,0 +1,59 @@ +# Scout Report: ALFMOB-264 — Design System Token Integration & Component Refactoring + +_Scouting pass that preceded the plan. Branch: `claude/exciting-leavitt-64caf7`. All paths relative to repo root._ + +## Epic → story map (9 sub-tickets) + +| Plan phase | Story | Title | Primary target | +|---|---|---|---| +| P1 | ALFMOB-272 | Set up token JSON parser & codegen | new `SharedUI/DesignTokens/` + `GeneratedTokens/` | +| P2 | ALFMOB-274 | Integrate color tokens | `Theme/Color/` | +| P3a | ALFMOB-266 | Integrate typography tokens | `Theme/Typography/` | +| P4 | ALFMOB-270 | Integrate spacing & shape tokens | `Theme/Spacing/`, `CornerRadius/`, `Shape/` | +| P5 | ALFMOB-267 | Refactor Typography/Label | `Theme/Typography/` | +| P5 | ALFMOB-271 | Refactor ThemedButton | `Theme/Buttons/` | +| P5 | ALFMOB-268 | Refactor ThemedInput | `Theme/Inputs/` | +| P5 | ALFMOB-273 | Refactor Chip | `Components/Chips/` | +| P5 | ALFMOB-269 | Refactor Badge | `Components/Indicators/` | + +## Relevant files + +### Token foundations +- `Alfie/AlfieKit/Sources/SharedUI/Theme/Color/Color.swift:3` — `enum Colors` accessor (`primary`, `secondary`) +- `Theme/Color/PrimaryColors.swift:4,20` — `PrimaryColorsProtocol` (mono900–050, black, white); struct loads from `Colors.xcassets` via `Bundle.module` +- `Theme/Color/SecondaryColors.swift:3,52` — `SecondaryColorsProtocol` (green/red/blue 050–900, yellow/orange 050–500) +- `Theme/Color/Colors.xcassets/` — 54 colorsets (53 universal; the one dark variant, Blue050, is a no-op) +- `Theme/Typography/TypographyProvider.swift:5,14` — `TypographyProviderProtocol` (header/paragraph/small/tiny) +- `Theme/Typography/Specifications/TypographyHeaderProtocol.swift` (h1 36 / h2 24 / h3 20pt), `…ParagraphProtocol` (16), `…SmallProtocol` (14), `…TinyProtocol` (12) — each with normal/bold/italic/underline/strike builder variants (~15 UIFont getters, ~70 AttributedString builders total) +- `Theme/Typography/Helpers/FontNames.swift:8` — `FontNames.sfProMedium` + `FontManager.registerAll()` +- `Theme/Typography/Helpers/Font+Extensions.swift:7` — `AttributedString.build(font:lineHeight:letterSpacing:…)`, HTML + `Text.build` +- `Theme/Spacing/Spacing.swift:6` — `enum Spacing` space0→space1000 (0–80pt) +- `Theme/CornerRadius/CornerRadius.swift:7` — `enum CornerRadius` none/xxs/xs/s/m/l/xl/full (+ `swiftlint:disable` header) +- `Theme/Shape/ShapeProviderProtocol.swift:4` + `DefaultShapeProvider.swift:3` +- `Theme/ThemeProvider.swift:14` — `ThemeProvider.shared`; exposes `font` + `shape`, `setupAppearance()` (uses `Colors.primary.*.ui`) + +### Components (P5) +- Button — `Theme/Buttons/ThemedButton.swift:5`, `ButtonTheme.swift:23` (`ButtonThemeSpec`; refs `Colors.primary.mono*`, `Spacing.space100/200`, `CornerRadius.s`) +- Input — `Theme/Inputs/ThemedInput.swift:5` (`InputStatus`; refs mono*, secondary green800/red800, `CornerRadius.xs`) +- Typography/Label — `Theme/Typography/**` + `Helpers/Font+Extensions.swift:162` (`Text.build`) +- Chip — `Components/Chips/Chip.swift:44` (`ChipConfiguration`; `CornerRadius.full`; hardcoded heights 36/44) +- Badge — `Components/Indicators/BadgeViewModifier.swift:9`, `BadgeTabViewModifier.swift:9`, `BadgeHelper.swift` (`Colors.secondary.red700`, hardcoded sizes) + +### Build & tests +- `Alfie/AlfieKit/Package.swift` — SharedUI target `277–295` (resources `286–294`); product `71–74`; SwiftGenPlugin `:100`; swift-snapshot-testing `:94`; TestUtils target `297–303` (vends SnapshotTesting) +- `Alfie/AlfieKit/Sources/SharedUI/swiftgen.yml` + `Resources/Templates/codegen-strings-structured.stencil` — codegen precedent to mirror +- `Alfie/scripts/run-apollo-codegen.sh`, `verify.sh` — script conventions +- `Alfie/.swiftlint.yml` — `opt_in_rules: [all]`, runs as Xcode build phase; `excluded` lists `L10n+Generated.swift`/`BFFGraph` +- Tests: `Alfie/AlfieKit/Tests/SharedUITests/BadgeHelperTests.swift`, `StyleGuideTests.swift` (empty stub). App snapshots in `Alfie/AlfieTests/Snapshots/` + `Helpers/` (pointfree 1.18.3, `@testable import Alfie`) + +## Patterns observed +- Components access theme via `var theme: ThemeProviderProtocol { ThemeProvider.shared }` then `theme.font.*`. Colors/Spacing/CornerRadius are accessed **statically** (`Colors.primary.mono900`), not via the provider. +- Codegen precedent: SwiftGen (SPM plugin + build phase) + Apollo (shell script). Generated output committed. **No DTCG / Style Dictionary / Node tooling exists** — Phase 1 is greenfield. +- Colors are asset-backed (`Colors.xcassets`); typography currently renders **everything** in SF Pro Display Medium despite doc-comments naming freightBook/circular*. + +## Key unknowns (carried into the plan as open questions) +- External token repo `Mindera/Alfie-Mobile-Design-Tokens` (DTCG JSON) not yet in this repo. +- Licensed fonts (freightBook/circular*) not bundled — only SF Pro. +- Legacy→Figma typography mapping needs design sign-off. + +See `plan.md` for the full implementation plan; `red-team.md` + `validation.md` for review/decisions. diff --git a/Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/validation.md b/Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/validation.md new file mode 100644 index 00000000..a5de65bb --- /dev/null +++ b/Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/validation.md @@ -0,0 +1,30 @@ +# Validation Interview: ALFMOB-264 Design System Token Plan + +_Interview 2026-06-18. Decisions below are authoritative; plan + phase files updated to match._ + +## Decisions + +| # | Question | Decision | Plan impact | +|---|---|---|---| +| 1 | Private token-repo access / CI | **No CI involvement this stage.** An iOS dev runs the generate script locally against `design-token.json` and commits the Swift output. | Drop the CI drift-gate from P1 scope (deferred, not cancelled). Generation = local manual step. Private-repo pull is a manual/local concern, not a CI secret. | +| 2 | Licensed fonts (freightBook/circular*) | **Design is sourcing them.** | P3a (Figma rename + shim, SF Pro retained) proceeds now. P3b (real font swap) stays queued, no date — externally blocked as planned. | +| 3 | Snapshot baseline policy (ALFMOB-274 AC) | **Tokens are the source of truth.** Pixel changes caused by token values differing from today's hardcoded values are re-baselined and accepted after review; design owns the look. | Open Q3 CLOSED. Snapshot diffs in P2–P5 are reviewed-then-approved, not auto-blockers. This is NOT a pure no-visual-change refactor. | +| 4 | Execution mode | **P1 spike only, this round.** Hold P2+ until the legacy→Figma mapping (design) and a usable `design-token.json` are in hand. | Solo start. Team/parallel deferred until P1 lands. | +| 5 | Snapshot phase ordering (post-validation note) | **Snapshot harness + baselines moved to LAST (P6).** Baselining before the token refactor has no value when tokens are the source of truth (decision 3) — the look is expected to change. | Former "Phase 0" → **P6**, after components are final. During P2–P5: no component snapshot guard; rely on `verify.sh` + manual review + app-level snapshots. | + +## Readiness check + +- **ACs mapped to phases**: yes — epic ACs ↔ P1 (codegen/reference graph), P2 (color), P3a (Figma names/back-compat), P4 (spacing/shape), P5 (5 components + snapshots). Font-rendering AC is explicitly split to P3b. +- **Feature flag**: n/a (no runtime flag; not a `high_rigor_domain`). Rollback = git revert of committed generated files + theme edits; baseline policy (decision 3) makes intended diffs explicit. +- **Design sign-off**: typography mapping table is the one **still-open** design dependency → blocks P3a start (not the P1 spike). Tracked as Open Q2. +- **Localization / accessibility ids**: no new L10n keys or accessibility ids — this is an internal theming refactor; existing strings/ids untouched. +- **QA / snapshot bandwidth**: P6 establishes the SharedUI snapshot harness (last); during P2–P5, reviewing token-driven visual diffs needs design/QA review cycles (decision 3). Flag to QA before P2. + +## Remaining open items (post-validation) +- **Open Q2** — legacy→Figma typography mapping table (design). Blocks P3a. +- **`design-token.json` availability** — P1 emitters need the real file in hand (decision 1 makes this a local dev artefact, not CI). +- **Fonts (P3b)** — awaiting design delivery (decision 2). + +## Status +Plan validated and ready to execute the **P1 spike**. P2+ gated on the items above; snapshot harness + +baselines run **last (P6)** once components are final. From 187f39e5186ef138a516507a41ad3df1e3de053f Mon Sep 17 00:00:00 2001 From: Khoi Nguyen Date: Fri, 19 Jun 2026 18:52:48 +0700 Subject: [PATCH 02/15] ALFMOB-272: Add Phase 1 token pipeline red-team, validation & spike findings --- .../phase-1-token-pipeline.md | 64 +++++++ .../red-team-272-pipeline.md | 176 ++++++++++++++++++ .../spike-findings-272.md | 103 ++++++++++ .../validation-272.md | 76 ++++++++ 4 files changed, 419 insertions(+) create mode 100644 Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/red-team-272-pipeline.md create mode 100644 Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/spike-findings-272.md create mode 100644 Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/validation-272.md diff --git a/Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/phase-1-token-pipeline.md b/Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/phase-1-token-pipeline.md index 46592efa..4f025210 100644 --- a/Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/phase-1-token-pipeline.md +++ b/Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/phase-1-token-pipeline.md @@ -84,3 +84,67 @@ reference graph. No theme types consume it yet (that's P2–P4). Gates the whole **Spike first** (pull + inspect JSON + resolve Q1/Q2/Q5 auth) = S–M, **blocked on private-repo creds**. Emitters + wiring + lint/drift + tests = L–XL once the JSON shape is known. Do NOT write emitters against an assumed contract (red-team M3). + +--- + +### Red-team additions — pipeline hardening (2026-06-19) +_Source: `red-team-272-pipeline.md`. These are accepted and MUST be folded into the steps above._ + +**Spike: ✅ DONE (2026-06-19) — see `spike-findings-272.md`.** The token repo ships a full contract +(`DESIGN_TOKENS_FORMAT.md` + `PLAN.md`); the unknowns are resolved. Net contract facts that change P1: +- **color** = sRGB float `components:[r,g,b(,a)]` (NOT hex; no alpha in current export) → `Color(.sRGB, + red:green:blue:opacity:)`, default opacity 1 (corrects J4). +- **dimension** = `{value, unit:"px"}` **px-only** → `CGFloat(value)`, error on non-px (J5 downgraded). +- **typography** = composite `{fontFamily, fontWeight, fontSize, lineHeight, letterSpacing}`, subfields + are refs; `fontWeight` inlined as `"Regular"`/`"Medium"` → 400/500 (confirms J2). +- **manifest-driven, mode-selected load** (resolves J1): read `manifest.json`; build ONE name→token map + from the **iOS load list** — `.primitives`, `theme`, `sizing`, `typography.alfie-theme`, + `typography.styles`, `system.ios`, `screen-size.small-(s)`; **skip** `system.{android,web}`, + `screen-size.{m,l,xl}`, `.documentation`, and any `~~doc-*` token. Mode-selection de-collides names. +- **NEW — allowlist handling (not in the original plan):** the resolver must honor + `.cycle-allowlist.json` (7 known `(file,token)` cycle edges → resolve to primitive + warn; fail on + any other cycle) and `.broken-ref-allowlist.json` (2 FONT_STYLE font-weight targets). Both are + **exhaustive** — stale entries must fail. Replaces the plan's blunt "error on cycles/missing refs." +- **Fonts NOT blocked** (overturns epic): brand = Libre Baskerville (free OFL), primary-ios = SF Pro + (system). The "licensed freightBook/circular" blocker was wrong → P3b font-swap is effectively + unblocked (just bundle Libre Baskerville `.otf`). + +Still to verify against real data when implementing: exact `toSwiftIdentifier()` collisions across the +loaded set (J3), and that `screen-size.small` actually supplies the responsive sub-tokens the +composites reference. + +**Generator contract (extend step 2):** +- `DTCG.swift` models `$value` as a **tagged union on `$type`** (color/dimension/fontWeight/typography/ + shadow…), not a single scalar — a single composite token otherwise fails decode → no output (J2). +- **Merge all input files into ONE path-keyed graph BEFORE resolving aliases** (cross-file refs are the + norm); detect duplicate-path collisions, decide last-write-wins vs error, test it (J1). +- Add `toSwiftIdentifier()`: leading-digit prefix (Figma `050`/`2xl`), separator→camelCase + (`display/large`), Swift-keyword backtick-escape (`default`/`case`/`static`/`repeat`/`operator`); + plus a **collision check** (two distinct paths must not map to one identifier). The team already + hand-hacks this (`Spacing.space025`, `CornerRadius` swiftlint header) — bake it in (J3). +- **Deterministic output**: sort every emitted collection by token path; **no timestamp/version/`Date()` + in the generated header** (C3). +- **Clean before emit**: `rm` all `*+Generated.swift` (or `git clean -fdx GeneratedTokens/`) before + writing, so a removed token category can't leave an orphaned stale file (C2). + +**Wiring & gating:** +- `generate-design-tokens.sh` runs `swift test --package-path Tools/DesignTokenGen` **fail-fast before + emitting** — gated here (the dev's "tokens/generator changed" task), **NOT in `verify.sh` and NOT in + CI** (validation-272 decision 3: regeneration is rare; CI out of scope this stage). `verify.sh`/CI + only `xcodebuild` the Alfie scheme and never touch the generator package, so document in + `Docs/DesignTokens.md` that the generator's tests are the developer's responsibility on token change (C1). +- `pull-design-tokens.sh`: use SSH or `gh` auth (no token in the command line); wrap any auth-bearing + line in `set +x`; copy **only** `*.json` by explicit glob (never `cp -R` the clone); `rm -rf` temp + clone in a `trap` (n2). +- Path-exact `exclude:`; assert `swift build` of SharedUI emits **zero** "unhandled files" warnings (n1). + +**Verification additions (extend the Verification section):** +- Generator tests also assert: **byte-identical output on two runs** (C3), a token category removed → + its generated file is deleted (C2), cross-file alias resolves (J1), composite token decodes (J2), + `toSwiftIdentifier` handles leading-digit/keyword/separator + collision (J3), 8-digit hex preserves + alpha (J4), `rem`/`px` dimension converts to the right point value (J5). + +**Open process item (J6 — NOT a code change):** ALFMOB-272's own ticket ACs (conform to existing +protocols, replace `Colors.xcassets`, Figma names verbatim) are met by **P2–P4**, not P1-as-scoped. +Reconcile in Jira before anyone marks ALFMOB-272 Done — re-scope the ticket to "pipeline only" or move +those ACs to ALFMOB-274/266. Confirm with PM. diff --git a/Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/red-team-272-pipeline.md b/Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/red-team-272-pipeline.md new file mode 100644 index 00000000..d61bca3b --- /dev/null +++ b/Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/red-team-272-pipeline.md @@ -0,0 +1,176 @@ +# Red Team Review: ALFMOB-272 — Phase 1 Token Pipeline (DTCG parser + Swift codegen) + +_Adversarial review, 2026-06-19. Scope: **ALFMOB-272 / Phase 1 only** — the standalone Swift +DTCG→Swift generator. Findings grounded in the worktree. Complements the epic-level +`red-team.md` (M1–M4); everything here is **new** and specific to pipeline mechanics. +Plan updated to absorb the Accepted items — see "Disposition" tags + `phase-1-token-pipeline.md`._ + +## Verdict +The Phase-1 plan is directionally right (standalone Swift generator, committed output, reference +graph) but **under-specifies the DTCG contract and has no real verification of its own core +deliverable**. The generator's tests never run in any mandatory gate; "byte-identical determinism" +and "error on missing refs" are asserted with no implementing mechanism; and the spike step isn't +told to probe the contract features most likely to break it (composite tokens, color/alpha formats, +px/rem units, cross-file aliases). Several of these bake **silently-wrong values** into committed +Swift that nothing in P1 can surface — they detonate in P2–P4. + +--- + +## Critical (block — fix before execute) + +### C1. The generator's unit tests — the story's core deliverable — never run in any mandatory gate +- **Evidence**: `verify.sh` → `build-for-verification.sh` + `test-for-verification.sh` run **only** + `xcodebuild -scheme Alfie` against `Alfie/Alfie.xcodeproj`. CI runs only `fastlane ios test`. + The plan deliberately keeps `Tools/DesignTokenGen` **out of the AlfieKit graph** + (`phase-1-token-pipeline.md:52-53`). So `swift test --package-path Tools/DesignTokenGen` (the + alias-resolver/cycle/reference-graph tests — `:75-78`) is invoked by **nothing** automatic. +- **Why it breaks P1**: A dev can land a broken resolver, run `verify.sh`, get + `✅ FULL VERIFICATION PASSED` (it only recompiles the *already-committed* generated Swift), and ship. + The project's one completion signal is satisfiable with a completely broken generator. +- **Fix**: `generate-design-tokens.sh` runs `swift test --package-path Tools/DesignTokenGen` + (fail-fast) **before** emitting — i.e. gated as part of the developer's "tokens/generator changed" + task; document that `verify.sh` does NOT cover the generator. **Disposition: ACCEPTED (validation-272 + decision 3) → gate in the generate script ONLY. NOT in `verify.sh`, NOT in CI — regeneration is rare + and CI is out of scope this stage (prior no-CI decision).** + +### C2. No output cleaning on token removal — orphaned `*+Generated.swift` survives the drift gate +- **Evidence**: `main.swift` is "write `*+Generated.swift` to the output dir" (`:30`) — overwrite + only, one file per category. The drift gate (step 6) is `git diff`, which flags content changes to + **tracked** files, not files that should no longer exist. Output is normal compiled SharedUI source. +- **Why it breaks P1** (distinct from epic M2 = content drift): drop a category from the export (e.g. + `radius.json`) → generator simply doesn't emit `Radius+Generated.swift`, but the committed one stays + on disk, stays compiled, and `git diff` shows nothing. Committed Swift now references primitives the + JSON no longer defines → silent stale values that pass the local drift gate. +- **Fix**: `main.swift` (or `generate-design-tokens.sh`) cleans the output dir before emitting + (`rm` all `*+Generated.swift` / `git clean -fdx GeneratedTokens/`), so `git diff --exit-code` + catches deletions too. **Disposition: ACCEPTED → P1 step added.** + +### C3. "Run twice → byte-identical" has no mechanism — Dictionary/JSON key order is randomized +- **Evidence**: Verification AC `:81`. DTCG groups are open-ended name→token maps decoding into Swift + `Dictionary`, whose iteration order is non-deterministic across runs (randomized hash seed). No sort + step is specified in `main.swift` or any `Emit+*.swift` (`:28-30`). +- **Why it breaks P1**: the **local `git diff` drift gate is P1's only freshness signal** (epic M2). + Non-deterministic ordering → regenerate produces reordered-but-equivalent files → `git diff` is + permanently noisy → devs learn to ignore it → M2's mitigation collapses and the byte-identical AC + silently fails. +- **Fix**: sort every emitted collection by token path before string-building; add a generator test + that emits twice on the same input and asserts byte-equality. **No timestamp/version/`Date()` in the + generated header.** **Disposition: ACCEPTED → P1 contract + test added.** + +--- + +## Major (significant rework / silent-wrong-value risk) + +### J1. Cross-FILE alias resolution asserted, but no "merge 14 files into one graph" step +- **Evidence**: 14 separate JSON files (`:16`); resolver resolves `{a.b.c}`, errors on missing refs + (`:27-28`). No stage says all files merge into one path-keyed graph **before** resolution. Semantic + files routinely alias into primitive files. +- **Why it breaks P1**: per-file resolution → every cross-file `{primitive.color…}` is "missing ref" + → generator errors and emits nothing (or emits broken refs). The reference-graph AC (M4) is + unreachable if the graph was never unified. +- **Fix**: explicit "load all files → merge into one path-keyed dict → detect duplicate-path + collisions (themes/modes: last-write-wins vs error — decide + test) → resolve" stage; test a + reference that crosses two files. **Disposition: ACCEPTED → resolver contract expanded.** + +### J2. Composite tokens (typography/shadow/border) are OBJECTS, not scalars +- **Evidence**: DTCG `$type: "typography"` has an **object** `$value` + (`{fontFamily, fontSize, fontWeight, lineHeight, letterSpacing}`); shadow/border likewise. Existing + typography is exactly this composite (`TypographySmallProtocol.swift`: family+size+weight+decoration + variants). The emitter spec only describes the **color/leaf-hex** case (`:34-40`); `Emit+Typography` + is listed with no composite-decode contract. DTCG `fontWeight` is string-or-number; `fontSize`/ + `lineHeight` are dimension objects. +- **Why it breaks P1**: if `DTCG.swift` types `$value` as a scalar, the first composite token fails to + decode → no output. "Flatten at leaf hex" is meaningless for a typography object (no hex leaf). +- **Fix**: model `$value` as a tagged union on `$type`; decide the Swift representation of composites + **before** writing emitters; decode-test each `$type`. The spike must explicitly enumerate composite + types as a known unknown. **Disposition: ACCEPTED → spike checklist + DTCG model contract expanded.** + +### J3. Swift identifier safety — leading-digit / reserved-word / dotted-path names won't compile +- **Evidence**: the team already hand-hacked around this: `CornerRadius.swift:9` ships + `// swiftlint:disable discouraged_none_name identifier_name` for `none`/`xxs`; `Spacing.space025` + carries a `space` prefix **only** to dodge the leading-digit identifier (`025`). Figma uses + `display/large`, `2xl`, `050`. The emitter injects names verbatim into `static let `. +- **Why it breaks P1**: a token named `2xl`/`050`/`default`/`repeat`/`case`/`static`/`operator` emits + non-compiling Swift → `build-for-verification.sh` fails → P1's own gate red. +- **Fix**: define + unit-test a deterministic `toSwiftIdentifier()` (leading-digit prefix, + separator→camelCase, keyword backtick-escape) **and** a collision check (two paths must not collapse + to one identifier). **Disposition: ACCEPTED → P1 step + test added.** + +### J4. Color-format coverage undefined; no existing hex→Color helper to lean on +- **Evidence**: zero hex parser in SharedUI (`grep "init(hex"`, `Color(red:` → nothing); colors come + from `Colors.xcassets`. DTCG color `$value` is legally `#RGB`/`#RRGGBB`/`#RRGGBBAA`/`rgba()`/DTCG + color object `{colorSpace, components, alpha}`. The emit example hand-writes `Color(red:…)` with no + conversion routine and **no `opacity:`**. +- **Why it breaks P1**: generator invents its own parser (Foundation-only). Handle only `#RRGGBB` and + the export uses 8-digit hex / objects → parse failure (no output) or silently dropped alpha + (wrong committed color, undetectable until P2). +- **Fix**: enumerate accepted formats in the spike; implement + test each (3/6/8-digit, `rgba()`, DTCG + object); emit `Color(red:green:blue:opacity:)` preserving alpha; round-trip test for alpha. + **Disposition: ACCEPTED → spike checklist + emitter contract.** + +### J5. Dimension units — DTCG `{value, unit}` may be px/rem; iOS uses points +- **Evidence**: existing tokens are raw points (`Spacing.space050 = 4`, `CornerRadius.xs = 4.0`). DTCG + dimensions are `{value, unit:"px"|"rem"}`. Emitters (`:29`) state no unit policy; the spike note + (`:18`) doesn't list px/rem. +- **Why it breaks P1**: `rem` (common when web leads the token export) needs ×base — `1.5rem`→`24pt`, + not `1.5`. Naive emit bakes a 10×-wrong value into committed Swift that nothing in P1 surfaces; it + detonates in P4. +- **Fix**: spike records `unit` for every dimension token; implement explicit unit→point conversion + (px→pt 1:1 by iOS convention, rem→pt × base, **error on unknown unit**); unit-test it. + **Disposition: ACCEPTED → spike checklist + emitter contract.** + +### J6. AC/scope mismatch — ALFMOB-272's own ticket ACs are silently deferred to P2–P4 +- **Evidence**: the ALFMOB-272 ticket ACs require: generated code **conforms to existing theme + protocols** (`PrimaryColorsProtocol`, `TypographyHeaderProtocol`…), **Figma names verbatim**, and + **replace `Colors.xcassets`/hardcoded values**. The plan scopes P1 as "pipeline only — nothing + consumes it yet (that's P2–P4)" (`:5-6`); protocol re-pointing → P2, xcassets deletion → P2, + typography forwarders → P3 (`plan.md:103-110`). So P1-as-scoped does **not** meet ALFMOB-272's own + ticket ACs. +- **Why it breaks P1**: a reviewer either wrongly closes ALFMOB-272 with unmet ACs, or is blocked + because P1's deliverable can't satisfy its own ticket. The plan re-mapped the ticket's ACs onto + P2–P4 **without confirming the Jira ticket was re-scoped** (validation decision 4 says "P1 spike + only this round" but doesn't reconcile the ticket ACs). +- **Fix**: reconcile in Jira before execution — either re-scope ALFMOB-272 to "pipeline only, no + consumers" (move the protocol/xcassets/Figma-name ACs to ALFMOB-274/266), or pull minimal + protocol-conformance into P1. Flag to PM. **Disposition: ACCEPTED (process) → tracked as Open Q; + needs PM/Jira action, not a code change.** + +--- + +## Minor + +### n1. `exclude: ["DesignTokens"]` + co-located `GeneratedTokens/` is path-fragile +- SharedUI target has no `exclude:` today (`Package.swift:278-294`). Generated Swift lands in a sibling + dir under the same `Sources/SharedUI/` root as the excluded JSON. A slightly-wrong path either makes + SPM try to compile/bundle the 14 JSON files (unhandled-resource build failure) or over-excludes the + generated Swift (symbols vanish, P2 fails). **Fix**: unambiguous separate subdirs; assert + `swift build` emits zero "unhandled files" warnings for SharedUI. **Disposition: ACCEPTED → P1 build + check added.** + +### n2. `pull-design-tokens.sh` clones a private repo — credential / secret-leak exposure +- Mirroring `run-apollo-codegen.sh` means `set -ex`, which **echoes every command** — a tokenized + HTTPS clone URL would print creds into CI/Bitrise logs. A naive `cp -R` of the clone could drag + `.git/config` (credential) or stray files into `Sources/SharedUI/DesignTokens/` and commit them. + Design tokens are pulled **outside** the repo's `git-secret` mechanism. **Fix**: SSH or `gh` auth + that never appears on the command line; `set +x` around any auth-bearing line; copy **only** `*.json` + by explicit glob (never `cp -R`); `rm -rf` the temp clone in a `trap`. **Disposition: ACCEPTED → P1 + script hardening note.** + +--- + +## What the plan already got right (don't regress) +- Standalone Swift generator (no Node), committed output, reference-graph contract (epic "got right"). +- Spike-first discipline ("don't write emitters against an assumed contract") — C-tier findings make + that spike's **checklist** explicit rather than relying on the dev to remember composites/units/alpha. + +## Net new asks for the Phase-1 plan +1. Run generator tests in `generate-design-tokens.sh` (C1). +2. Clean output dir before emit (C2). +3. Sort emitted output + no header timestamp + byte-identical test (C3). +4. Merge-all-files-into-one-graph stage + collision policy (J1). +5. DTCG model as tagged union on `$type`; define composite Swift representation (J2). +6. `toSwiftIdentifier()` + collision check, unit-tested (J3). +7. Color format matrix incl. alpha → `opacity:` (J4). +8. Dimension unit→point conversion, error on unknown (J5). +9. Reconcile ALFMOB-272 ticket ACs with the "pipeline only" scope in Jira (J6 — PM action). +10. Path-exact exclude + zero-unhandled-files build check (n1); pull-script credential hardening (n2). diff --git a/Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/spike-findings-272.md b/Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/spike-findings-272.md new file mode 100644 index 00000000..7aa9085c --- /dev/null +++ b/Docs/Plans/260618-1552-ALFMOB-264-design-system-tokens/spike-findings-272.md @@ -0,0 +1,103 @@ +# Spike Findings: ALFMOB-272 — DTCG contract (resolved against the real export) + +_2026-06-19. Source: local clone `Mindera/Alfie-Mobile-Design-Tokens` @ working copy. This is the +"pull + inspect before writing emitters" spike the plan demanded — it resolves the contract unknowns +the red-team flagged (and corrects two epic assumptions). The repo ships its own contract spec +(`DESIGN_TOKENS_FORMAT.md`) + `PLAN.md` — read those; this summarizes the iOS-relevant facts._ + +## File set (the "14 files") +`design-tokens/` = `manifest.json` + 13 `*.tokens.json`. **Read `manifest.json` first** (don't scan +the dir). Collections & modes: + +| Collection | Modes | iOS uses | +|---|---|---| +| `.primitives` | `alfie-theme` | ✅ all (72 tokens: 40 dimension, 27 color, 4 fontFamily, 1 fontWeight) | +| `theme` | `alfie-theme` | ✅ 31 semantic color tokens (surface/content/border/button/link), all references | +| `sizing` | `alfie-theme` | ✅ radii / icon sizes / interactive padding (all reference `spacing-*`) | +| `typography` | `alfie-theme` | ✅ needed to resolve composite sub-refs (NOT emitted as its own surface) | +| `typography.styles` | (styles) | ✅ **the canonical consumption path** — 15 real composite styles (+ `~~doc-*` to skip) | +| `system` | `ios` / `android` / `web` | ✅ **`ios` only** — font families + screen routing | +| `screen-size` | `small(s)` / `m` / `l` / `xl` | ✅ **`small-(s)` only** (PLAN Q3: mobile codegen uses Small at codegen-time) | +| `.documentation` | `mode-1` | ❌ **skip** entirely | + +**Mode selection is the key to avoiding name collisions** (red-team J1): references resolve by **name, +not path**, into one flat `name→token` map — but you load only the **active mode** per multi-mode +collection (`system.ios`, `screen-size.small`), plus the single-mode files. That de-collides the +duplicate names across the other platform/breakpoint files. Also skip `.documentation` + any +`~~doc-`-prefixed token. + +## Value shapes (confirmed) — corrects red-team J4 / J5 + +- **color**: `{ "colorSpace": "srgb", "components": [r,g,b] | [r,g,b,a] }`, floats `0.0–1.0`. + **NOT hex.** Current export: all 27 are 3-component (no alpha present today). → emit + `Color(.sRGB, red:, green:, blue:, opacity:)`, default opacity `1` when only 3 components; still + read an optional 4th defensively. **No hex parser needed** (J4 was wrong about the format). +- **dimension**: `{ "value": , "unit": "px" }`. **Only `px`** in the whole export; contract + says iOS points = px @1×. → emit `CGFloat(value)`; **error on any non-`px` unit** defensively + (J5's rem concern does not occur today, keep the guard). +- **fontFamily**: `string` (e.g. `"Libre Baskerville"`, `"SF Pro"`). iOS resolves brand→primary via + `system.ios`. +- **fontWeight**: primitive scale is CSS `100–900` (number); in composites it is **inlined as a + string** `"Regular"`/`"Medium"` → map to `400`/`500` (per `.broken-ref-allowlist.json` fix_strategy). +- **typography** (composite — confirms red-team J2): `{ fontFamily, fontWeight, fontSize, lineHeight, + letterSpacing }`; every subfield except the inlined `fontWeight` is a `{reference}`. Example: + ```json + "display-large": { "$type":"typography", "$value": { + "fontFamily":"{display-large-font-family}", "fontWeight":"Regular", + "fontSize":"{display-large-font-size}", "lineHeight":"{display-large-line-height}", + "letterSpacing":"{display-large-kerning}" } } + ``` + +## References, cycles, broken refs — supersedes the plan's "error on missing/cycle" + +- Refs are `"{token-name}"`, by name, chain depth **≤5**. Resolution algo is specified in + `DESIGN_TOKENS_FORMAT.md` §Resolution. +- **`.cycle-allowlist.json` (7 edges)** — known plugin-artifact cycles around + `typography-font-family-brand`/`-system-brand`. Resolver MUST: detect cycles → match the offending + `(file, token)` edge against the allowlist → if listed, resolve to the primitive concrete value + (`Libre Baskerville`) + warn; if not listed, **fail**. Allowlist is **exhaustive**: stale entries + (allow-listed but no longer real) must fail the build. +- **`.broken-ref-allowlist.json` (2 targets)** — `typography-font-weight-regular`/`-medium` are + FONT_STYLE-scoped and filtered out by the exporter. They only dangle in `typography.alfie-theme` + (which iOS doesn't emit); composites inline the weight, so **iOS is unaffected** — but the resolver + must load this allowlist and not hard-fail on those targets. +- ⇒ The plan's blunt "detect cycles & error on missing refs" must become **"honor both allowlists + (exhaustively)"**. This is new work the plan didn't budget. + +## Naming → Swift (confirms red-team J3; target shape given by PLAN.md) +Names are hyphenated, verbatim from Figma: `colours-neutrals-800`, `spacing-spacing-0`, +`display-large`, `surface-background-primary`. PLAN.md fixes the emitted Swift shape: +- primitives: `Primitives.Color.neutrals800`, semantic: `static let buttonPrimaryBackground = + Primitives.Color.neutrals800` (**reference graph preserved — confirms M4 / ticket AC**, PLAN §252). +- typography: `Typography.display.large` (PLAN §275) — Figma names verbatim, **no `Header.h1` mapping** + (PLAN Q1 decision: no platform-native renaming). +- ⇒ still need a deterministic `toSwiftIdentifier()` + collision check: strip the redundant + `spacing-spacing-`/`colours-` group prefixes into nested enums, handle leading-digit leaves + (`-800`, `-0`, `-12`), map `-` segments to nested types / camelCase. J3 stands. + +## Two epic assumptions OVERTURNED +1. **Fonts are NOT licensed/blocked.** brand = **Libre Baskerville** (free, OFL — Google Fonts), + primary-ios = **SF Pro** (system font). The epic's "P3b externally blocked on licensed + freightBook/circular `.otf`" is **wrong** — the doc-comment family names in the current Swift were + never the real design intent. Only work for the font swap: bundle Libre Baskerville `.otf` + (freely available); SF Pro needs no bundling. **P3b is effectively unblocked.** +2. **Spike is unblocked and essentially complete** — the contract is fully documented in-repo; no + private-repo credential dance needed (the user has a local clone). Epic M3's "blocked on creds" + no longer applies. + +## iOS load list (concrete) for the generator +Load into one name→token map: `.primitives.alfie-theme`, `theme.alfie-theme`, `sizing.alfie-theme`, +`typography.alfie-theme`, `typography.styles`, `system.ios`, `screen-size.small-(s)`. Skip +`system.{android,web}`, `screen-size.{m,l,xl}`, `.documentation`, and `~~doc-*` tokens. Honor both +allowlists. Emit: `Primitives.{Color,Spacing,FontSize,LineHeight,…}`, semantic `Theme.*` as references +to primitives, `Typography..