feat(gate): measure the design-first claim, and stop pointing Copilot at Claude paths - #5
Merged
Merged
Conversation
… at Claude paths Mirrored from the uptizm fork, where both were measured. This repository is the boilerplate, so tooling that catches a class of mistake belongs here before it belongs in any fork of it. `bin/design-tokens` scans lib/**/*.dart for raw `Color(0x` and `Colors.*` and exits non-zero with the offending file:line. Nothing measured this: `design:lint` validates DESIGN.md's own YAML and never reads the Dart that is supposed to obey it, and `flutter analyze` has no opinion about a hardcoded colour. A component could ignore every semantic token and the gate would stay green. Two details in it were paid for downstream rather than guessed at here, and the script's header says so. Comments are stripped first, because a component's own docstring routinely says "no raw hex, `Color(0xFF...)`, or `Colors.*` anywhere" and a naive grep flags the sentence claiming compliance; three components in the fork did exactly that. And `Colors.` needs a left word boundary or it matches the tail of any identifier ending in "Colors", so an ordinary `kBrandColors.first` reads as a violation; that single false positive put a file on the fork's allowlist that had no real violation, which is a hole rather than an exemption. The allowlist has one entry, `lib/config/wind_theme.g.dart`, which is generated. That is the whole surface here, measured. It prefers an exact file to a directory on purpose: a directory entry also exempts every file added to it later, under a reason that may not apply to them. `bin/sync-instructions` now rewrites Claude-only references as it stages a rule body. `design.md` sent Copilot to `CLAUDE.md` and to a `.claude/agents/` definition, neither of which it loads. The source rule keeps those paths, because they are correct for the one tool that reads them, and the generator's own provenance banner keeps the real source path too: its whole job is to say which file to edit instead of the generated one. Evidence: `bin/check` 5/5 green, `--fast` 3/3, `bin/sync-instructions --check` clean and stable across runs, the source rule unchanged by the rewrite, and the new job proven able to fail by planting `Color(0xFF123456)` in lib/ui/components/callout/callout.dart and watching it report `callout.dart:64`, then reverting. `Design tokens` is a new CI job with a new name, so it reports without blocking until someone adds it to branch protection. That is the right order for a gate that has never been green on main.
There was a problem hiding this comment.
Pull request overview
Introduces an automated “design tokens” gate to enforce the repo’s design-first constraint (no raw Color(0x...) / Colors.* in lib/**), and improves generated Copilot instruction mirrors by rewriting Claude-only path references in staged rule bodies.
Changes:
- Added
bin/design-tokensto scanlib/**/*.dartfor raw color literals (with comment stripping + allowlist support). - Wired the new gate into
bin/check(including--fast) and added a standalone “Design tokens” CI job. - Updated
bin/sync-instructionsto rewrite Claude-only references inside mirrored rule bodies (without mutating the source.clauderules).
Reviewed changes
Copilot reviewed 3 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| bin/sync-instructions | Rewrites Claude-only references while staging rule bodies so Copilot mirrors link to paths Copilot can open. |
| bin/design-tokens | New static scanner enforcing “token-only” color usage in lib/**, with comment stripping and an allowlist. |
| bin/check | Runs bin/design-tokens as part of the flutter scope (and in --fast). |
| .github/workflows/ci.yml | Adds a standalone “Design tokens” job to report raw color violations separately from Flutter analyze/test. |
| .github/instructions/design.instructions.md | Updates generated instruction text to reference Copilot-appropriate paths. |
| .design-token-allowlist | Introduces an allowlist (currently only lib/config/wind_theme.g.dart) with required per-entry rationale. |
Mirrored from the fork, where a code review caught it. The comment stripper looked for `/* */` before `//` and knew nothing about string literals, so a `/*` with no closing `*/` on the same line opened block mode that never closed, every line after it was emitted empty, and the gate printed its green line having scanned nothing. That is the worst shape a gate can fail in, because it is indistinguishable from success. This tree has the trigger: `lib/config/deeplink.dart:15` is `'paths': ['/*'],`, a `/*` inside a string, so everything after line 15 of that file was unchecked. The fork had four such files, two of them the very view files the job exists to police, and one was tripped by a doc comment naming a route glob. The stripper now scans each line left to right and acts on whichever of `//` and `/*` comes first outside a string, tracking `'` and `"` with backslash escapes. Dart's triple-quoted strings are still not tracked, and that gap is closed by an invariant rather than more parsing: `flutter analyze` guarantees every file here compiles, so an unterminated `/*` at end of file proves the stripper mis-parsed. It says so and exits 3, and the caller rescans the raw source. Over-reporting a comment costs a minute; under-reporting costs the gate its purpose. Verified: the gate still reports `callout.dart:64` for a planted `Colors.red` and is otherwise green, and `bin/check --fast` is 3/3.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
bin/design-tokens(new) scanslib/**/*.dartfor rawColor(0xandColors.*and exits non-zero with the offendingfile:line. It runs as abin/checkjob (static, so in--fasttoo) and as a new, non-blockingDesign tokensCI job. Separately,bin/sync-instructionsnow rewrites Claude-only references while staging a rule body.Why
Mirrored from the uptizm fork, per the mirroring rule in
AGENTS.md: a structural change there travels here in the same piece of work. This repository is the boilerplate, so tooling that catches a class of mistake belongs here before it belongs in any fork.Nothing measured the design-first claim.
design:lintvalidatesDESIGN.md's own YAML and never reads the Dart that is supposed to obey it;flutter analyzehas no opinion about a hardcoded colour. A component could ignore every semantic token and the gate would stay green.Upstream:
anilcancakir/uptizm#5(the gate) andanilcancakir/uptizm#7(the generator rewrite).Evidence
The gate was proven able to fail, then reverted:
The generator rewrite, before and after, with the source rule untouched:
design.instructions.md:9`CLAUDE.md``.github/copilot-instructions.md`design.instructions.md:95(`.claude/agents/component-visual-reviewer.md`)(a Claude Code agent definition; Copilot has no equivalent, so apply those criteria by hand)design.instructions.md:5GENERATED from .claude/rules/design.mdgit diff .claude/rules/design.mdis empty, andbin/sync-instructions --checkis clean and stable across repeated runs, so the substitutions are deterministic and CI cannot flap.bin/checkgreenTwo details that were paid for downstream, not guessed at here
Comments are stripped before the scan. A component's own docstring routinely says "no raw hex,
Color(0xFF...), orColors.*anywhere", so a naive grep flags the sentence claiming compliance. Three components in the fork did exactly that, and the job was red on arrival until the stripper existed.Colors.needs a left word boundary. Without one it matches the tail of any identifier ending inColors, so an ordinarykBrandColors.firstreads as a violation. That single false positive put a file on the fork's allowlist that had no real violation, which is a hole rather than an exemption: it would have silently permitted the first genuine one.Color(0xdeliberately has no boundary, soMaterialColor(0xFF008560in a generated theme is still caught.The allowlist has one entry, and that is the point
lib/config/wind_theme.g.dart, whichdesign:syncgenerates. That is the measured surface of this tree. A fork therefore starts from a clean list and adds an entry only when it has a real reason to, with the reason written next to the path.It prefers an exact file to a directory: a directory entry also exempts every file added to it later, under a reason that may not apply to them.
lib/config/was tried and rejected on exactly that ground downstream, where the directory holds fourteen files and only one is generated.What it does not measure
Stated in the script header rather than left to be discovered. It is regex and not an AST, so
Color.fromARGB,Color.fromRGBOandColor.fromwalk past it (zero occurrences today; the header says to add a rule the day one appears). A//inside a string literal truncates its line before the scan sees it. And a hardcoded pixel value, a colour token written without itsdark:pair, and a one-off widget where a registry component already exists are all outside what a regex can see. This is a floor, not the whole of design-first.Not required yet, on purpose
Design tokensis a new job with a new name, so it reports without blocking until someone adds it to branch protection. That is the right order for a gate that has never been green onmain;bin/checkis where it actually gates in the meantime.