chore(ui): prune the unused vendored color tokens - #183
Open
renefloor wants to merge 4 commits into
Open
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## feat/update-color-tokens #183 +/- ##
=========================================================
Coverage 68.57% 68.57%
=========================================================
Files 210 210
Lines 8593 8593
=========================================================
Hits 5893 5893
Misses 2700 2700 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
renefloor
force-pushed
the
chore/prune-unused-tokens
branch
from
September 10, 2026 12:00
18f6421 to
f735efa
Compare
renefloor
added this pull request to stack #185
September 10, 2026 12:29
renefloor
force-pushed
the
chore/prune-unused-tokens
branch
2 times, most recently
from
September 10, 2026 12:42
72ebd12 to
b1c76eb
Compare
backgroundElevationElevation0..3 are unread, and the names no longer exist upstream — the core/chat/video namespace split renamed them to background/core/elevation-N, which generates as backgroundCoreElevation0..3. Carrying an unread constant under a name the source of truth has retired is worse than not carrying it: it reads as the current name to anyone checking whether a token is vendored. The colorScheme fields that mirror them (backgroundElevation0..3) resolve through chrome[0] and are unaffected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
124 unread color constants for badge, button, avatar, control, chat, input, reaction and presence components. They are dead by architecture, not by neglect: this package vendors only the root semantics, and a component's values are re-derived from those in its own defaults. StreamBadgeNotification reads colorScheme.accentError rather than a badgeBgError constant, so upstream's derived tokens have no counterpart here on purpose. Keeping them invited the opposite reading — that a component should reach for a token constant — which is the one thing a component must not do, since a constant bypasses the seedable color scheme and a custom brand or chrome would stop applying. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
31 unread constants whose StreamColorScheme field resolves through the generated brand or chrome swatch instead — textPrimary through chrome.shade900, accentPrimary through brand.shade500, backgroundCoreApp through chrome[0]. That indirection is deliberate: the swatches are regenerated from a seed color, so resolving a field through the swatch is what lets StreamColorScheme.light(brand: ...) actually repaint the SDK, where a baked hex would silently ignore it. The constant is therefore redundant by construction, and the field is the single source. Verified before deleting that this is redundancy and not a competing value: all 29 mechanically resolvable pairs hold the same color as the swatch shade their field reads, in both light and dark. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
renefloor
force-pushed
the
chore/prune-unused-tokens
branch
from
September 10, 2026 14:39
b1c76eb to
c662af3
Compare
The sync-state note named `backgroundElevationElevation0` and `avatarPaletteBg1` as constants that survive only here, and said `light/` and `dark/` do not hold the same set. This PR deletes the first two and makes the sets identical, so both claims are now false. The relationship they described is also simpler than it was: with the unread constants gone, the vendored set is a strict subset of upstream — every name here exists upstream under the same spelling. That is worth stating, since it is the property a future sync can actually check. The naming table had the same problem one layer down: it taught the group-segment transform using `borderUtilityWarning` and `backgroundCoreOnAccent`, both deleted here. Swapped for `borderUtilitySelected` and `backgroundCoreHighlight`, which survive precisely because a field reads them — so the table now demonstrates the rule it sits next to rather than contradicting it. Also two American-English fixes: `recognise` (mine, from the previous PR) and `Multicolour`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
renefloor
marked this pull request as ready for review
September 11, 2026 08:24
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.
Stacked on #182 — review and merge that first; this PR's base is
feat/update-color-tokens, so its diff is only the cleanup.stream_tokens.dartcarried 502 constants of which 163 were read. This PR removes the 159 unread color constants (158 inlight/, 159 indark/— see the asymmetry note below). The dimension constants are deliberately left alone — see the note at the bottom.The unread constants were unreachable as well as unread: the files sit under
internal/, are excluded from the barrel contract, and are imported by exactly two files —stream_colors.dart(light only) andstream_color_scheme.dart(both). Nothing public is touched.One commit per category, so a bisect stays meaningful:
c46b2dabackgroundElevationElevation0..3— unread, and the names no longer exist upstream after the core/chat/video namespace split.1f12b4bStreamColorSchemefield.StreamBadgeNotificationreadscolorScheme.accentError, not abadgeBgErrorconstant.c662af3textPrimary ??= chrome.shade900). Verified against the resolved swatch shade in both modes before deleting — redundancy, not a competing value.38f7062Also fixes a light/dark asymmetry
dark/carriedborderUtilityFocus(0x4078A8FF) with no counterpart inlight/— 501 light vs 502 dark. Upstream has onlyborder/utility/**focused**({brand.150}), which generates#C3D9FFlight /#19418Ddark, exactly whatborderFocus ??= brand.shade150already resolves to. So it was a stale name carrying a stale value, and nothing read it.Both files now declare 343 identical constants. That symmetry is what lets the
check:tokensguard in the follow-up PR pass.Verification
analyzeis the load-bearing check here: these arestatic constmembers reached only through a prefixed import, so a deleted-but-used constant is anundefined_gettercompile error, not a lint. It passes across all four packages, and--ignore="*example*"does not exclude the gallery, so that is analyzed too.The deletion is exactly minimal rather than approximate: 163 color constants remain and all 163 have a reader. No deleted name has a reference anywhere in the repo — including
apps/,scripts/, tests, and the tracked generated output.format:verifyandcheck:barrelspass.flutter testis+397 -56, but that is a null result rather than evidence: no test, golden, or source file changes, so the count could not have moved. The 56 are the pre-existing macOS-golden drift.Note if you grep this yourself: a bare-name search for
accentPrimaryortextPrimaryreturns ~1100 hits, because those are alsoStreamColorSchemefield names. Every hit iscolorScheme.X; the only reference form for a token isStreamTokens.X.Scope: colors only
The "every constant has a reader" property now holds for colors. 180 dimension constants (
size*,space*,radius*,font*,lineHeight*,typography*, and the component dimension groups) remain unread, deliberately.An earlier revision of this branch deleted those too, on the grounds that the package does not sync dimensions from upstream. That was wrong — those values mirror upstream exactly, ordering included, and typography specifically comes from the
webflavor, the only one carrying theGeistfamily this package ships.They are unreferenced for a different reason:
StreamSpacing,StreamRadiusandStreamTokensTypographyhard-code the same numbers instead of reading the constants. That is the swatch-versus-baked-hex hazard one layer up — nothing ties the class to the constant it mirrors, so the two can drift silently.Deleting them would remove the evidence of that gap rather than the gap, so it is handled separately by wiring those classes to the constants. The
check_tokens.dartguard that fails CI on an unreferenced token moved to that PR too, since it can only pass once the dimensions are wired.This also resolves an apparent tension in the skill: "do not vendor
button/*" governs button colors, which this PR drops, while the dimensions carve-out keepsbuttonPaddingX*and friends.No changelog entry
Deliberate. The policy is conditional on changing package behavior, and nothing read these constants.
StreamTokensis internal — private constructor, underinternal/, listed incheck_barrels.yaml'sinternal_dirs, and exported from no barrel — so the removals are outside the deprecation policy too.