Add platform axis (web / ios / android / adaptive)#269
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4706da2. Configure here.
|
Welcome back, cool to see you picked this up after all! The plumbing is good, it's a clean mirror of register and the gating and defaults are right. Two conceptual things and two blockers. On register vs platform: you're right that register is a web concept, brand is the product's landing page (display fonts, go wild), product is the app itself. The question native raises: there are native apps that throw out the platform and do their own layout, UI and fonts, but that's mostly either a game (which the HIG/Material refs can't help with anyway) or just bad practice, so we shouldn't advocate for it. Which means native compresses the register axis: on native, platform conformance is the bar for structure, interaction and accessibility regardless of register, and brand only lives in the expressive layer the platform already gives you (theming, type scale, color, motion). That's why it felt like it should be one value, native really does squeeze register down to "how hard do you lean on the expressive layer," not "do you follow the platform." I'd still keep two fields (web genuinely needs register, and adaptive has to branch out to both rulebooks), but the native refs should say register's role is narrow here, and right now they contradict each other. ios.md says "native is the opposite of brand," android.md says "let brand express through Material's theming, not by discarding it." android's framing is the right one. ios.md reads as "no brand on native, ever," which is too far the other way (Calm, Duolingo, Spotify all have strong identity inside the platform). Unify both on: earned familiarity is the bar, brand within the rails, never by breaking them. Separately, ios.md and android.md carry full Accessibility sections and they load at design time (setup step 5). We keep a11y in audit.md on purpose so design-time output doesn't go timid, and this PR already adds native a11y to audit.md's Platform section, so I'd strip it from the design-time refs and let audit own it. Two blockers regardless of the above: The branch is 113 commits behind main, bumps to 3.6.0 when main is already at 3.7.1, and conflicts on ~49 files. It needs a rebase before I can really review the build (build:skills currently fails on the branch as-is). It also commits all the generated provider trees plus ./plugin. Drop those and keep it source-first: just skill/, scripts/, tests/, CLAUDE.md, NOTICE.md, and the changelog/version, and let the sync workflow regenerate the rest. Smaller: adaptive isn't threaded through everywhere (it was added late, so init Step 2, the Step 7 summary, the CLAUDE.md heading and the per-command divergence notes still say only web/ios/android). And there's no skill-behavior scenario for the platform flow, the extractPlatform unit tests are solid but don't check that the agent actually loads ios.md when platform is ios. |
Orthogonal to register: register decides whether design IS or SERVES the product; platform decides the delivery target and which native conventions apply. Set `## Platform` in PRODUCT.md; a missing field defaults to `web`, so legacy projects are unaffected. - extractPlatform() in skill/scripts/context.mjs (mirrors extractRegister); the CLI appends a NEXT STEP directive to read the native reference(s). `adaptive` (Flutter / RN / KMP shipping both iOS and Android) loads both ios.md and android.md. - New reference/ios.md (Apple HIG distilled) and reference/android.md (Material 3 distilled); reference/web.md is a thin pointer. The native refs frame register's role as narrow: platform conformance is the bar, brand lives in the expressive layer the platform gives you, never by breaking the rails. - Setup step 5 loads the native reference(s) when platform is native. Live mode and the detect CLI stay web-only, gated off ios/android/adaptive. - init asks platform right after register; adapt/audit/animate/layout carry short platform divergence notes; all secondary spots thread `adaptive`. - a11y stays in audit.md (loading it at design time makes output timid), so the native refs carry no Accessibility section; audit.md's Platform section owns native a11y. - Tests: extractPlatform unit coverage + skill-behavior scenario 10 (PRODUCT.md platform ios -> agent loads ios.md). Source-first: only skill/, scripts/, tests/, CLAUDE.md, NOTICE.md, the changelog and version are committed; the sync workflow regenerates the provider trees and ./plugin on merge. ios.md / android.md are distilled from the MIT-licensed ehmo/platform-design-skills; attribution in NOTICE.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
vinaypokharkar
force-pushed
the
feat/platform-axis
branch
from
4706da2 to
ecb8c03
Compare
Greptile SummaryIntroduces a platform axis (
Confidence Score: 4/5The platform axis is a purely additive, backward-compatible change — missing Platform fields default to web so no existing project is affected. The implementation is thorough: extractPlatform is logically sound, all edge cases are unit-tested, the CLI directive and signal exposure are consistent, and the SKILL routing for native platforms is correctly applied. The one gap — no integration test asserting the android NEXT STEP directive — is a test coverage omission rather than a functional defect. tests/context.test.mjs — the CLI integration block tests ios, adaptive, and web but omits an android case; worth adding for symmetry. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[PRODUCT.md loaded] --> B{Platform section?}
B -- absent --> C[null treated as web]
B -- present --> D{First non-empty line}
D -- web --> E[return web]
D -- ios --> F[return ios]
D -- android --> G[return android]
D -- adaptive --> H[return adaptive]
D -- ios and android --> H
D -- other --> I[return null, web default]
F --> J{CLI output}
G --> J
H --> J
J -- ios --> K[NEXT STEP: read reference/ios.md]
J -- android --> L[NEXT STEP: read reference/android.md]
J -- adaptive --> M[NEXT STEP: read both ios.md and android.md]
E --> N[no extra NEXT STEP]
C --> N
I --> N
J --> O[context-signals: setup.platform]
O --> P{SKILL.src.md routing}
P -- ios/android/adaptive --> Q[skip live and detect.mjs]
P -- web/null --> R[live and detect.mjs available]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[PRODUCT.md loaded] --> B{Platform section?}
B -- absent --> C[null treated as web]
B -- present --> D{First non-empty line}
D -- web --> E[return web]
D -- ios --> F[return ios]
D -- android --> G[return android]
D -- adaptive --> H[return adaptive]
D -- ios and android --> H
D -- other --> I[return null, web default]
F --> J{CLI output}
G --> J
H --> J
J -- ios --> K[NEXT STEP: read reference/ios.md]
J -- android --> L[NEXT STEP: read reference/android.md]
J -- adaptive --> M[NEXT STEP: read both ios.md and android.md]
E --> N[no extra NEXT STEP]
C --> N
I --> N
J --> O[context-signals: setup.platform]
O --> P{SKILL.src.md routing}
P -- ios/android/adaptive --> Q[skip live and detect.mjs]
P -- web/null --> R[live and detect.mjs available]
Reviews (1): Last reviewed commit: "Add a platform axis (web / ios / android..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| it('appends a native platform directive for an ios project', async () => { | ||
| write('PRODUCT.md', '# Acme\n\n## Register\n\nproduct\n\n## Platform\n\nios\n'); | ||
| const { spawnSync } = await import('node:child_process'); | ||
| const res = spawnSync(process.execPath, [SCRIPT_PATH], { cwd: scratch, encoding: 'utf8', env: { ...process.env, IMPECCABLE_NO_UPDATE_CHECK: '1' } }); | ||
| assert.equal(res.status, 0); | ||
| assert.match(res.stdout, /This project targets `ios`\./); | ||
| assert.match(res.stdout, /read `reference\/ios\.md`/); | ||
| }); | ||
|
|
||
| it('appends both native directives for an adaptive project', async () => { | ||
| write('PRODUCT.md', '# Acme\n\n## Register\n\nproduct\n\n## Platform\n\nadaptive\n'); | ||
| const { spawnSync } = await import('node:child_process'); | ||
| const res = spawnSync(process.execPath, [SCRIPT_PATH], { cwd: scratch, encoding: 'utf8', env: { ...process.env, IMPECCABLE_NO_UPDATE_CHECK: '1' } }); | ||
| assert.equal(res.status, 0); | ||
| assert.match(res.stdout, /targets `adaptive` \(both iOS and Android\)/); | ||
| assert.match(res.stdout, /reference\/ios\.md` and `reference\/android\.md`/); | ||
| }); | ||
|
|
||
| it('appends no native platform directive for a web project', async () => { | ||
| write('PRODUCT.md', '# Acme\n\n## Register\n\nproduct\n\n## Platform\n\nweb\n'); | ||
| const { spawnSync } = await import('node:child_process'); | ||
| const res = spawnSync(process.execPath, [SCRIPT_PATH], { cwd: scratch, encoding: 'utf8', env: { ...process.env, IMPECCABLE_NO_UPDATE_CHECK: '1' } }); | ||
| assert.equal(res.status, 0); | ||
| assert.equal(res.stdout.includes('This project targets'), false); | ||
| assert.equal(res.stdout.includes('reference/ios.md'), false); | ||
| }); |
There was a problem hiding this comment.
android platform
The CLI test suite covers ios, adaptive, and web platform directives, but there is no corresponding test for android. The code path for android inside the CLI (nativeRefs = [platform]) is identical to ios, so a regression there (e.g. a typo in the string 'android') would go undetected by integration tests. Adding a fourth case alongside the existing three would close the gap symmetrically.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
Thanks for the thorough review — all of it addressed. Pushed a rebased, source-first rewrite ( Blockers
Native compresses register
a11y ownership
Smaller
Build / tests
One unrelated note: there are two pre-existing Windows-only test failures in the |
|
Is this stop? May I help here? I wpuld love to use thsi in my native apps |
3 participants
Closes #99.
Adds a platform axis orthogonal to register, mirroring the existing brand/product pattern. PRODUCT.md gains a
## Platformfield; a missing field defaults toweb, so existing projects are unaffected.What
web(default),ios,android, andadaptivefor cross-platform apps that ship both from one codebase (Flutter, React Native, KMP). A line naming both targets (e.g.ios, android) is read asadaptive.extractPlatform()inskill/scripts/context.mjs(mirrorsextractRegister()); CLI emits a NEXT STEP directive to read the native reference(s) forios/android, and both foradaptive.reference/web.md(thin pointer),reference/ios.md(Apple HIG distilled),reference/android.md(Material Design 3 distilled), adapted from the MIT-licensed ehmo/platform-design-skills (attributed inNOTICE.md).init.md: platform hypothesis (Flutter/RN/SwiftUI/Compose detection), question after register, PRODUCT.md field, and guidance to choose by the design language the app renders, not the toolchain.## Platformsections inadapt,audit,animate,layoutwhere native guidance diverges.context-signals.mjsexposesplatform; SKILL routing marksliveand thedetectCLI as web-only.CLAUDE.mdPlatform-axis subsection.Scope
scripts/build.js: normalize path separators so the slop-page prose exclusion works on Windows.Tests
tests/context.test.mjs:extractPlatformunit + comma-list + adaptive/ios/web CLI coverage — 39/39 pass.bun run build:skillsclean (prose validator + counts green); harness output dirs regenerated.Note
Low Risk
Skill and documentation changes with backward-compatible defaults (web); no auth, payment, or runtime API surface changes beyond context parsing and agent guidance.
Overview
v3.9.0 adds a platform axis alongside register:
PRODUCT.mdgets## Platform(web/ios/android/adaptive). Missing platform still behaves as web.extractPlatform()incontext.mjsparses the field (includingios, android→adaptive), emits NEXT STEP directives to load native references, and exposesplatformincontext-signals.mjs. Setup step 5 and routing now load HIG (ios.md) and/or Material 3 (android.md) on top of brand/product;web.mdis a thin pointer. Content is adapted from ehmo’s MITplatform-design-skillswith attribution inNOTICE.md./impeccable initinfers platform from the repo, asks after register, writes the field, and skips live recommendations for native targets. adapt, audit, animate, and layout gain## Platformsections; audit/detect/liveare documented as web-only for native projects.Tests cover
extractPlatform, CLI directives, and skill-behavior scenario 10 (iOS loadsios.md). Minor Windows fix inscripts/build.jsprose scan path normalization.Reviewed by Cursor Bugbot for commit ecb8c03. Bugbot is set up for automated code reviews on this repo. Configure here.