Skip to content

feat: update tokens from Tokens Studio release - #5284

Merged
eddiman merged 1 commit into
mainfrom
tokens-studio-release
Aug 11, 2026
Merged

feat: update tokens from Tokens Studio release#5284
eddiman merged 1 commit into
mainfrom
tokens-studio-release

Conversation

@github-actions

@github-actions github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Automated pull of the token state after a Tokens Studio release: raw token sets via studio tokens pull (sources in packages/eds-tokens/.studio.json), generated CSS (EDS-CSS export) into packages/eds-tokens/src/tokens/css/, DTCG (EDS-DTCG export) into src/tokens/dtcg/, TypeScript modules generated from the two into src/tokens/ts/, and the bundled src/tokens/css/variables.css (concatenation of the CSS export, per ADR-0010 — proposed in #5199). The semantic/*.css files are widened to :root, [data-color-scheme] by scripts/widen-semantic-scope.mjs before bundling (#5226) — that selector diff against the raw export is expected.

@pomfrida

pomfrida commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

@claude review

@claude

claude Bot commented Aug 6, 2026

Copy link
Copy Markdown

Claude finished @pomfrida's task in 2m 42s —— View job


Review: feat: update tokens from Tokens Studio release

  • Read the PR diff against origin/main
  • Check the Tokens Studio pipeline conventions
  • Inspect the workflow that produced this PR
  • Post review

TL;DR — this PR contains no token changes at all

Every one of the 20 changed lines is a timestamp or a hash derived from timestamps. Grouping the diff by key name:

Key Changed Meaning
generated_at 1 wall-clock of the studio tokens pull run
updated_at 1 wall-clock, dependency level
modified_at 17 wall-clock, one per token file
lockfile_hash 1 integrity hash over the above

Zero changes to content_hash, hash, size or tokens. The dependency content_hash is byte-identical:

content_hash = "sha256:3171780b3a4849a9d5bc255067153dd6741eafe62767548efb8c16af417d0a7d"   # unchanged

And no other file in the repo changed — no src/tokens/css/, no src/tokens/dtcg/, no src/tokens/ts/, no variables.css. The CSS/DTCG/TS export steps and generate:css-bundle all ran and produced identical output.


🔴 The important question: did the release actually get pulled?

.studio.json pins the source to a branch, not the release:

"ref": { "type": "branch", "name": "main" }

and tokens_studio_release.yaml:7-9 acknowledges why: "The payload does not include the release version, so the workflow pulls the current state of the sources."

That leaves two possible explanations for an empty pull, and they need different responses:

  1. The release genuinely contained no token changes (a re-release, a metadata-only release, or a release of already-synced state). Harmless — just close this PR.
  2. The pull raced the release — the dispatch fired before the released state was visible on the Studio main branch, so the workflow snapshotted the pre-release content. In that case the release's tokens silently never land in the repo, and nothing in the pipeline will tell you: the job succeeded, so the Slack alert at :115-122 (if: failure()) stays quiet, and the workflow comment notes the resulting PR gets no CI.

I'd confirm against the live Studio project which release fired this dispatch and whether its content matches sha256:3171780b… before closing. If it's case 2, the fix is to pin the pull to the released ref (or re-run the workflow), and the branch-ref race is worth tracking as a pipeline bug.


🟠 Merging this would cut an empty beta release

packages/eds-tokens/studio.lock is not in the exclude-paths list for packages/eds-tokens in .github/release-please-config.json:92-107. Combined with the hardcoded feat: commit type at tokens_studio_release.yaml:109, squash-merging this will:

  • bump @equinor/eds-tokens 3.0.0-beta.43.0.0-beta.5
  • add "✨ Added — update tokens from Tokens Studio release" to the changelog

…for a package whose published tarball is byte-identical. studio.lock isn't in package.json files (dist/*, build/*, commonjs/*, instructions/*, tokens.css, elements.css), so consumers get a version bump and a changelog entry promising token updates that don't exist.

Recommendation: close this PR rather than merge it, once you've ruled out the race above.


🟡 Pipeline hardening (the structural issue)

The root cause is that studio tokens pull rewrites the lockfile timestamps on every run regardless of content, so the tree is always dirty and peter-evans/create-pull-request always opens a PR. Every future release dispatch that changes nothing will produce this same PR — and because these PRs are cheap to rubber-stamp, the noise makes it easy to merge an empty beta by reflex.

A guard after the generation steps (:98-99, before the PR step) fixes it. create-pull-request already no-ops on a clean tree, so simply reverting a timestamp-only lockfile is enough — no if: needed on the PR step:

- name: Discard timestamp-only lockfile churn
  working-directory: packages/eds-tokens
  run: |
    # studio tokens pull always rewrites generated_at/updated_at/modified_at
    # (and lockfile_hash, which covers them). If nothing else in the repo
    # changed and every content hash/size is identical, there is nothing to
    # release — drop the lockfile so no empty PR is opened.
    if git diff --quiet -- . ':(exclude)packages/eds-tokens/studio.lock'; then
      norm() {
        sed -E 's/(generated_at|updated_at|modified_at|lockfile_hash) = ".*"/\1 = "X"/'
      }
      if diff -q \
          <(git show HEAD:packages/eds-tokens/studio.lock | norm) \
          <(norm < studio.lock) >/dev/null; then
        echo "Lockfile changed only in timestamps — no token changes to release."
        git checkout -- studio.lock
      fi
    fi

Normalising lockfile_hash is safe here: any genuine token change necessarily shows up first as a hash / size / content_hash diff, which this comparison does not mask. I verified locally that after this normalisation the two lockfiles are identical.

Two smaller follow-ups worth considering:

  • Notify on no-op, don't just go quiet. Given hypothesis 2 above, "release fired but pull produced nothing" is exactly the signal you want in Slack. Extending the act10ns/slack step beyond if: failure() to cover this case would close the observability gap the workflow comment already worries about.
  • The static PR body is misleading. :111 enumerates CSS, DTCG, TS and variables.css outputs on every PR, including this one where none of them changed. Reviewers reading the body would reasonably assume generated output is in the diff. Interpolating the actual changed paths would make these PRs self-describing.

Fix this →

Note: I can't edit files under .github/workflows/ myself — GitHub App permissions don't allow workflow modifications, so the workflow changes above need a human (or the linked Claude Code session running locally).


Summary

Nothing is wrong with the diff as a diff — it's valid, correctly generated, and the pipeline behaved as written. The finding is that it shouldn't exist: it's a no-op that would cost a beta version and a misleading changelog entry. Verify the release actually contained changes, then close, and add the guard so the next no-op release doesn't reach a PR.
• branch tokens-studio-release

@github-actions
github-actions Bot force-pushed the tokens-studio-release branch from 9ece440 to f5d4224 Compare August 6, 2026 10:23
@pomfrida

pomfrida commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

@claude review

@claude

claude Bot commented Aug 6, 2026

Copy link
Copy Markdown

Claude finished @pomfrida's task in 5m 58s —— View job


Review: feat: update tokens from Tokens Studio release

  • Read the PR diff against origin/main
  • Trace the change through raw → DTCG → CSS → TS → variables.css
  • Verify generated output is internally consistent
  • Check contrast / scale impact
  • Post review

The branch has been updated since my earlier review — that one landed while the diff was still timestamp-only churn. This diff contains a real, and good, token change. Disregard the "close this PR" recommendation above.


TL;DR — one source edit, correctly fanned out. This is a bug fix. 👍

Exactly one value changed at the source:

# packages/eds-tokens/src/tokens/raw/input/scale.json — input.scale.dark.6 ("border-subtle")
-  "$value": 0.47,
+  "$value": "0.595",

All 64 remaining changed lines are the deterministic fan-out of that single lightness value, plus lockfile bookkeeping. I traced and verified every hop.

Why this is a fix, not a tweak

The dark lightness ramp was non-monotonic at step 6:

step 3 4 5 6 7 8
before 0.47 0.52 0.58 0.47 ⬅ dip back to step 3 0.61 0.76
after 0.47 0.52 0.58 0.595 0.61 0.76

Step 6 was sitting exactly on step 3's lightness. Because chroma is derived from lightness via the gaussian foundation, that also forced step 6's chroma to equal step 3's in every palette (--eds-dark-blue-6 was byte-identical to --eds-dark-blue-3) — which is why the chroma changes in this diff are derived, not separately authored. Steps 1–11 are now monotonic, with this having been the sole violation.

The user-visible consequence, via --eds-border-neutral-strong: var(--eds-neutral-6) (semantic/default.css:77) and --eds-neutral-6: var(--eds-dark-north-sea-6) (color-scheme/dark.css:130):

dark-mode token value contrast vs bg-canvas #131414 WCAG 1.4.11 (3:1)
border-neutral-medium (neutral-4) #5d6b76 3.37:1 pass
border-neutral-strong before (neutral-6) #525c65 2.70:1 fail
border-neutral-strong after #6f8190 4.59:1 ✅ pass

So "strong" was both darker and lower-contrast than "medium" — an inversion that also failed the 3:1 non-text contrast minimum. This PR fixes both. Nice catch by whoever edited it in Studio.

Verification of the generated output

Check Result
Palettes affected 7 of 9 dark ramps (blue, gray, green, moss-green, north-sea, orange, red)
The 2 untouched dataviz-div / dataviz-seq — independent hex ramps, don't derive from input.scale.dark. Correctly excluded, and neither has the same duplication.
css/colors/default.css vs bundled css/variables.css diffs are line-for-line identical → generate:css-bundle output is in sync
DTCG channels vs CSS 4-dp rounding all 7 match (0.11827469…0.1183, 0.20161003…0.2016, …)
TS hex vs CSS oklch() converted all 7 independently — 6 exact matches; orange-6 lands on a 187.5/255 half-pixel boundary where my converter also disagrees by 1 on unchanged baseline values, so that's my rounding, not a pipeline inconsistency
ts/color-scheme/dark.ts (6 aliases) + ts/semantic/dark.ts (1 leaf) consistent with the palette change
No diff in dtcg/semantic/*, css/semantic/* correct — those hold aliases ("$value": "{accent.9}", var(--eds-neutral-6)), not resolved values
studio.lock only input/scale.json changed hash; size 8632 → 8635 = exactly 0.47"0.595" (+3 bytes). Dependency content_hash and lockfile_hash updated accordingly. ✅

🟡 Notes — none blocking

1. $value is now a string under $type: "number". "0.595" instead of 0.595 is a DTCG spec violation. Pre-existing pattern, not introduced here — 11 of the 30 values in scale.json are already quoted ("0.19", "0.226", "0.77", "0.72", …), and the quoted ones look like the manually-edited ones. The pipeline handles it fine (CSS emits 0.595, not "0.595") and nothing in this repo parses the raw file. Worth raising with the Studio team as data hygiene, since a stricter downstream DTCG consumer would choke on the mixed types.

2. feat: understates this, and the changelog won't mention it. The workflow hardcodes feat: (tokens_studio_release.yaml:109), so merging gives 3.0.0-beta.5 with "✨ Added — update tokens from Tokens Studio release". The bump itself is right — src/tokens/** is deliberately not in exclude-paths (release-please-config.json:92-107) — but semantically this is a fix: and beta consumers get no hint that dark-mode neutral borders visibly changed. Consider adding a manual changelog line, or letting the workflow interpolate the changed paths into the PR body (its current static body claims CSS/DTCG/TS/variables.css changed on every run, which happened to be false last time and is true here only by luck).

3. Light/dark parity is now lopsided at step 6. Same semantic token, other scheme:

border-neutral-strong value vs its canvas 3:1
dark (after this PR) #6f8190 4.59:1
light (unchanged) #b4b4b4 1.95:1

0.595 is the only sensible interpolation given dark step 7 = 0.61 — the real oddity is that the dark 5/6/7 cluster is compressed into a 0.03 lightness band while light spreads 5/6/7 across 0.10. So this PR is right, but it exposes that light-mode neutral borders sit well under the non-text contrast minimum. Out of scope here; worth its own issue.

4. $description drift on the scale steps. Step 6 is described border-subtle and step 7 border-medium, but semantic actually consumes neutral-2 → border-neutral-subtle, neutral-4 → border-neutral-medium, neutral-6 → border-neutral-strong. The hints are off by two rungs, which is plausibly part of why a step-6 dip survived unnoticed.

5. Blast radius inside this repo is nil. No /next component references --eds-border-neutral-*; the only neutral-6 usage (Foundation/elements.css:315) is the legacy --eds-color-neutral-6 namespace from the old pipeline. No test asserts these values, and nothing needs rebuilding. Impact is on beta token consumers only.

Fix this →

I can't edit .github/workflows/ myself — GitHub App permissions don't allow workflow modifications, so note 2 needs a human or the linked local session. See the FAQ.


Verdict

Ship it. One source value fixed a non-monotonic dark ramp and a real WCAG 1.4.11 failure; the generated CSS, DTCG, TS and bundled variables.css are all consistent with it, and the lockfile integrity checks out. The beta bump is warranted. The notes above are follow-ups for the token team and the pipeline, not changes to this diff.

(The timestamp-churn guard I suggested in the earlier revision of this comment still stands as pipeline hardening for future no-op releases — it just doesn't apply to this PR.)
• branch tokens-studio-release

@pomfrida

pomfrida commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Checked the content: this release changes exactly one thing — shade 6 in the seven dark palettes (0.47 → 0.595), fixing the duplicate from #5282.
For anyone tracking the Chip migration: the shade 2→3 jump and the resting-fill decision (#5282), and the missing success/info interactive tokens (#5283), are not part of this change — so #5225 stays in draft

@github-actions
github-actions Bot force-pushed the tokens-studio-release branch from f5d4224 to 03344d5 Compare August 7, 2026 13:11
@pomfrida

pomfrida commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

@claude review

@claude

claude Bot commented Aug 7, 2026

Copy link
Copy Markdown

Claude finished @pomfrida's task in 8m 17s —— View job


Review: feat: update tokens from Tokens Studio release

  • Read the PR diff against origin/main
  • Trace raw → DTCG → CSS → TS → variables.css for each change
  • Verify generated output consistency and lockfile integrity
  • Check contrast, cross-family consistency, blast radius
  • Post review

⚠️ The branch has changed again since my last review. It's now +639/−79 and contains four distinct source changes, not one. @pomfrida's summary above ("this release changes exactly one thing") described the previous revision — in particular, the success/info interactive tokens from #5283 are in this diff now, and there's a compact-density typography change too.


What actually changed at the source

Four edits in src/tokens/raw/, everything else is deterministic fan-out:

# File Change Status
1 input/scale.json:56 dark step 6 0.47"0.595" ✅ the #5282 duplicate-shade fix (verified last review)
2 semantic.json new success + info interactive families (bg, border, icon, text) — 24 tokens ✅ closes #5283, but see 🟠 1
3 semantic.json:1795 icon.accent {accent.12}{accent.10} ✅ consistency fix, see 🟠 3
4 density/compact.json:580 compact ui.sm.line-height lineheight-scale.200.100 (16px → 12px) 🟡 see note 4 — undocumented, worth confirming it's intentional

Pipeline integrity ✅

Check Result
variables.css vs per-file exports +45/−9 in the bundle == +7/−7 (colors) + +1/−1 (density) + +37/−1 (semantic). Exact match; every new token present in the bundle at the right scope.
Semantic scope widening :root, [data-color-scheme] still at variables.css:590widen-semantic-scope.mjs ran.
DTCG parity semantic/default.json +177/−1 == raw/semantic.json +177/−1; colors/default.json 13/13 = 7 lightness + 6 chroma (gray's chroma stays 0) ✅
ts/color-scheme/dark.ts all 6 step-6 aliases now sit strictly between 5 and 7 ✅
studio.lock exactly 3 content hashes moved. scale.json 8632 → 8635 (+3 = 0.47"0.595"), semantic.json 52654 → 56620, density/compact.json hash changed with no size change (.200..100. is byte-equal) ✅
New families vs danger/warning bg emphasis 9/10/11, bg muted 2/3/4, border emphasis 9/10/11, border muted 4/5/7 — all identical ✅

🟠 1. icon-interactive and text-interactive disagree for the new families only

semantic/default.css:190-198 vs :231-239:

family icon-interactive steps text-interactive steps match?
danger 10 / 11 / 12 10 / 11 / 12
warning 10 / 11 / 12 10 / 11 / 12
link 8 / 9 / 10 8 / 9 / 10
accent 11 / 12 / 13 10 / 11 / 12 ❌ (pre-existing)
info (new) 11 / 12 / 13 10 / 11 / 12
success (new) 11 / 12 / 13 10 / 11 / 12

Concretely in light mode: icon-interactive-success-default = #205c1f but text-interactive-success-default = #206d1f — an icon and its label in the same control render at different darknesses. For danger and warning they're byte-identical.

So the new families copied the accent pattern for icons and the danger/warning pattern for text. One of the two conventions is wrong; three of five status families now say icons match text, two say icons are one step further. Worth resolving in Studio before this ships to beta consumers, since changing it later is a breaking visual change.

🟠 2. The dark ramp has a second, unfixed dip — at step 12 — and the new tokens walk right through it

This PR fixes the step-6 dip. Looking at the full dark input scale (input/scale.json), there's another:

step 9 10 11 12 13
lightness 0.82 0.88 0.93 0.91 ⬅ backwards 0.99
$description bg-fill-emphasis-default …-hover …-active text-subtle text-strong

The light ramp is strictly monotonic (0.98 → 0.32, no dip), so dark is the odd one out. The $descriptions suggest 12/13 are meant as a separate text sub-ramp rather than a continuation — which would make 0.91 defensible in isolation. But the tokens added in this PR use 11/12/13 and 10/11/12 as interactive state progressions, so in dark mode:

  • icon-interactive-success: default #c7f7c3 (0.93) → hover #bcf2b8 (0.91, darker) → pressed #f1ffef (0.99)
  • text-interactive-success: default #aceba8 (0.88) → hover #c7f7c3 (0.93) → pressed #bcf2b8 (0.91, darker than hover)

Contrast against bg-canvas #131414 goes 15.40:1 → 14.53:1 on hover. Not a WCAG failure — both pass comfortably — but the hover/pressed step moves the wrong way perceptually in dark mode while moving correctly in light. It's the same "unnoticed dip" class as the bug this PR fixes, and this PR extends it from accent to two more families. Worth raising with the token team alongside the step-6 fix.

🟠 3. icon-accent now diverges from text-accent

semantic/default.css:180 — the change is clearly right in one direction: every other static icon token is step 10 (icon-danger, icon-info, icon-success, icon-warning = {family}-10), so icon-accent: accent-12 was the outlier and this aligns it. Contrast is fine either way (light #1f666d = 6.60:1 on white, down from 10.17:1; dark #ace3e9 = 13.11:1).

But --eds-text-accent (:221) is still accent-12, while text-danger/text-info/text-success/text-warning are all step 10. So the fix closes the icon-family asymmetry and opens an icon-vs-text one — icon-X == text-X for all four status families but not for accent. Should text-accent move to accent-10 in the same release?

🟡 4. Compact ui.sm line-height 16px → 12px — the only cross-density divergence in the system

density/compact.css:46 / ts/density/compact.ts:88. The density modes are a strict one-step shift of the same type scale, so the same font size appears in two or three modes. Line-heights agreed everywhere — until now:

font size compact comfortable relaxed agree?
10px sm12 (was 16) xs → 16 only after this change
12px md → 16 sm → 16 xs → 16
14px lg → 20 md → 20 sm → 20
16px xl → 24 lg → 24 md → 24
18px xl → 24 lg → 24

There is an internal-to-compact rationale: compact header.xs is already 10/12, so 10px is now uniformly 12px within compact, and 10/16 (ratio 1.60) was the loosest step in an otherwise 1.33–1.50 mode. But the resulting 1.20 ratio is the tightest in the whole system, and it's at or just under Inter's own default content box (~1.21em) — so at 10px the glyph box slightly exceeds the line box, which matters for any consumer clipping to the line height. 12px is still on the 4px baseline ✅.

Not blocking, but this one arrived with no explanation and reads more like a Studio slip than the other three. Worth a quick confirm with whoever made the edit.

🟡 5. Still no automated guard for any of this

packages/eds-tokens has "test": "vitest run" but zero test files — nothing asserts ramp monotonicity or family parity. The step-6 dip (#5282), the step-12 dip above, and the icon-vs-text step mismatch are all mechanically checkable from the generated ts/ modules, and all three slipped through human review at least once.

Fix this →

ℹ️ Two things carried over, not introduced here

  • The muted 2→3 jump (Dark palette scales are non-monotonic: step 6 duplicates step 3 #5282) now applies to two more families. Dark bg-interactive-info-muted: default #191d1f → hover #33607e is a near-black-to-mid-blue leap, same shape as danger (#221919#923a3c). Consistent with existing families, so the tracked decision just got wider blast radius. Likewise, light muted borders are pastels across the board (new success #bbe0b8 = 1.45:1 on white, alongside existing danger #ffc3c0, accent #bbdbdf) — same sub-3:1 shortfall I flagged for light border-neutral-strong last time, not a regression.
  • $value is a quoted string under $type: "number". "0.595" — 11 of 30 values in scale.json were already quoted, and the pipeline handles it. DTCG hygiene item for the Studio team.

ℹ️ Blast radius inside this repo: still nil

Zero references in packages/eds-core-react/src to --eds-icon-accent, --eds-text-accent, any --eds-{icon,text,background}-interactive-*, or any --eds-density-typography-*. /next components compute their own --eds-typography-ui-body-sm-* from the modular scale in Foundation/typography.css, not from the Studio density tokens — so change 4 doesn't touch any component today. Impact is on beta @equinor/eds-tokens consumers and the Chip migration (#5225) only.

Verification method: I cross-checked the generated output against the raw sources by hand (diff-line arithmetic, per-file greps, lockfile hash/size deltas) rather than re-running the generators — the sandbox here blocks the node scripts/*.mjs invocations. Everything I could check lines up; a local pnpm --filter @equinor/eds-tokens generate:css-bundle && generate:ts-tokens producing a clean tree would confirm the last mile.


Verdict

Mergeable, with two questions I'd want answered first — they're cheap to fix now and breaking to fix later:

  1. Should icon-interactive-{info,success} be 10/11/12 (matching danger/warning) rather than 11/12/13? (🟠 1)
  2. Is the compact ui.sm line-height drop from 16px to 12px intentional? (🟡 4)

Changes 1 and 3 are unambiguously good. The step-12 dark dip (🟠 2) and the missing test guard (🟡 5) are follow-ups for the token team, not blockers for this diff. The feat: bump to 3.0.0-beta.5 is warranted this time — there's real new API surface (24 new tokens) plus a fix, though the changelog line "✨ Added — update tokens from Tokens Studio release" still tells beta consumers nothing about the dark-mode neutral border shift or the new success/info families.
• branch tokens-studio-release

@github-actions
github-actions Bot force-pushed the tokens-studio-release branch from 03344d5 to e78eec2 Compare August 11, 2026 08:52
@millus

millus commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

@claude review

@claude

claude Bot commented Aug 11, 2026

Copy link
Copy Markdown

Claude finished @millus's task in 6m 59s —— View job


Review: feat: update tokens from Tokens Studio release

  • Read the PR diff against origin/main
  • Identify the source-level changes in src/tokens/raw/
  • Re-run the generators to verify the fan-out into CSS / TS / variables.css
  • Check lockfile integrity, ramp monotonicity, contrast, and blast radius
  • Post review

⚠️ The branch has grown again since the last review — it's now +1445/−281 (was +639/−79). Three of the four items I flagged last time have been fixed at the source, and a substantial new set of semantic-layer changes has landed that wasn't there before. @pomfrida's summary of the release ("exactly one thing") described a much earlier revision.


What changed at the source

Everything else in the diff is deterministic fan-out from these four raw files:

# File Change Status
1 raw/input/scale.json dark step 6 0.470.595 ✅ the #5282 duplicate-shade fix
2 raw/input/scale.json dark step 12 0.910.96 fixes the second dip I flagged last review
3 raw/input/scale.json 11 $values unquoted ("0.19"0.19) fixes the DTCG $type: number hygiene note
4 raw/semantic.json new success + info interactive families (bg / border / icon / text) ✅ closes #5283 — see 🟠 3
5 raw/semantic.json icon.accent {accent.12}{accent.10} ✅ but see 🟠 4
6 raw/semantic.json muted ramps shifted one step up (2/3/43/4/5) across accent, danger, warning, neutral-selected, and the static background.{danger,info,success,warning} 🟠 the #5282 resting-fill decision — see 🟠 1 and 🟠 2
7 raw/semantic.json background.interactive.neutral.muted 1/2/33/4/5; bg.disabled/read-only neutral-23; border.interactive.disabled and border.neutral.subtle neutral-24 🔴 see 🟠 1
8 raw/density/compact.json compact ui.sm.line-height 200100 (16px → 12px) 🟡 see note 5 — still unexplained

Pipeline integrity — verified by re-running the generators, not by inspection ✅

Unlike the last two reviews, the sandbox let me execute the build steps this time:

Check Result
node scripts/generate-css-bundle.mjs byte-identical to the committed variables.csszero drift
node scripts/generate-ts-tokens.mjs all 10 modules byte-identical — zero drift
node scripts/assert-no-light-dark.mjs passes
Semantic scope widening :root, [data-color-scheme] present at variables.css:590
DTCG parity dtcg/semantic/default.json +503/−25 == raw/semantic.json +503/−25
studio.lock exactly 3 hashes moved. scale.json 8632 → 8611 = −22 (11 quote pairs) +1 (0.470.595) — exact ✅
All 14 oklch() ramps, steps 1–13 strictly monotonic in lightness, zero duplicate values, in both schemes

Both ramp dips are gone, and the interactive progressions that ran backwards in dark mode last time now run the right way:

icon-interactive-accent  dark:  #c7f1f6 (0.93) → #d7fafd (0.96) → #f1ffff (0.99)   ✅ was 0.93 → 0.91 → 0.99

🟠 1. Three new value collisions at the semantic layer

This is the finding I'd want addressed before merge. The step-6/12 fixes remove duplicate values from the ramps; item 7 above re-introduces them one layer up, in the semantic tokens. All three are new — I diffed the resolved values against origin/main:

a. border-neutral-subtle is now byte-identical to border-neutral-medium

token before after
border.neutral.subtle #ebebeb (neutral-2) #d4d4d4 (neutral-4)
border.neutral.medium #d4d4d4 (neutral-4) #d4d4d4 (neutral-4)

Dark: both #5d6b76. Two named tiers of a three-tier scale now render identically in both schemes — exactly the class of bug #5282 was about.

b. An enabled neutral-muted control is now pixel-identical to a disabled one

token light dark
background.interactive.neutral.muted.default #e1e1e1 #525c65
background.interactive.disabled #e1e1e1 #525c65
background.interactive.readOnly #e1e1e1 #525c65
border.interactive.neutral.muted.default #d4d4d4 #5d6b76
border.interactive.disabled #d4d4d4 #5d6b76

Before, neutral.muted.default was canvas (#f8f8f8 / #131414) and disabled was neutral-2 — clearly distinct. Now the container is identical in fill and border; only the label colour separates the two states (#404040 at 7.93:1 vs #c4c4c4 at 1.33:1 on the same fill). That still reads, but "disabled" is now carried entirely by text colour, which is fragile for icon-only controls. Note this is a two-step move for neutral (1→3) where every other family moved one step (2→3) — the normalisation is right, the collision with disabled looks unintended.

c. accent.muted.pressed now equals accent.selected.default

#a2cdd2 light / #41878e dark — pressing a muted accent control renders exactly its selected resting state. Previously muted.pressed = accent-4, selected.default = accent-5. Possibly deliberate (press → select feels continuous), but worth confirming rather than inheriting from the one-step shift.

🟠 2. The muted shift is imperceptible in light and dramatic in dark

The dark ramp has a 0.244 lightness gap between steps 2 and 3; light has 0.03. So the same one-step semantic move produces wildly different results per scheme:

token light before → after (Δ vs canvas) dark before → after (Δ vs canvas)
background.danger #ffe1df#ffd4d2 (1.16 → 1.27:1) #221919#923a3c (1.07 → 2.54:1)
background.info #d9efff#cae6fa (→ 1.22:1) #191d1f#33607e (→ 2.74:1)
background.success #ddf2db#cfeacc (→ 1.21:1) #191d19#3c673a (→ 2.81:1)
background.warning #ffe6d3#fbdac1 (→ 1.24:1) #201b17#7e4e25 (→ 2.64:1)

In light mode a status banner stays a barely-there tint; in dark mode it goes from near-invisible to a solid mid-tone fill. Text on the new dark fills is fine (text.danger on background.danger = 4.81:1, all four families land 4.7–4.8:1), so this isn't a contrast problem — it's that the two schemes are no longer perceptually paired. Given #5282 was explicitly about the 2→3 jump, this looks like the intended resting-fill decision landing; I'd just want the dark-side magnitude to be a conscious choice rather than a side effect of the index shift.

🟠 3. icon-interactive vs text-interactive still disagree — for the two new families

Unchanged from my last review, but now baked into new public API:

family icon steps text steps match?
danger / warning 10 / 11 / 12 10 / 11 / 12
link 8 / 9 / 10 8 / 9 / 10
accent 11 / 12 / 13 10 / 11 / 12 ❌ pre-existing
info (new) 11 / 12 / 13 10 / 11 / 12
success (new) 11 / 12 / 13 10 / 11 / 12

Light mode: icon.interactive.success.default #205c1f vs text.interactive.success.default #206d1f — an icon and its label in the same control at different darknesses, where danger and warning are byte-identical. Three families say icons match text, two say icons are one step further. Cheap to align now, breaking later.

🟠 4. icon-accent now diverges from text-accent

Moving icon.accent to step 10 is right in one direction — every other static icon token is {family}-10, so accent was the outlier. But text.accent is still {accent.12}, and icon.X == text.X holds for all four status families:

light dark
icon.accent (10) #1f666d — 6.60:1 on white #ace3e9 — 13.11:1
text.accent (12) #20474b — 10.17:1 on white #d7fafd — 16.68:1

Should text.accent move to accent-10 in the same release, so the fix closes both asymmetries rather than trading one for the other?

🟡 5. Compact ui.sm line-height 16 → 12 — still the only cross-density divergence

Regenerated from the TS output:

compact       xs:9/12   sm:10/12  md:12/16  lg:14/20  xl:16/24
comfortable   xs:10/16  sm:12/16  md:14/20  lg:16/24  xl:18/24
relaxed                 xs:12/16  sm:14/20  md:16/24  lg:18/24

Every other font size resolves to the same line-height in every mode. 10px is now 12 in compact and 16 in comfortable. Internally coherent (compact xs is already 9/12, and 10/16 was the loosest step in the mode) and still on the 4px baseline ✅ — but 1.20 is the tightest ratio in the system and sits at Inter's own content box (~1.21em), so at 10px the glyph box slightly exceeds the line box. It arrived with no explanation across two revisions now; worth a one-line confirmation from whoever made the edit.

🟡 6. $description drift on the scale steps got worse

input/scale.json labels step 6 border-subtle, 7 border-medium, 8 border-strong. After this PR the semantic layer consumes neutral-4 → border-neutral-{subtle,medium} and neutral-6 → border-neutral-strong. The hints are now off by two-to-four rungs and actively misleading — plausibly part of why 🟠 1a wasn't caught in Studio.

🟡 7. Still no automated guard on the Studio output

packages/eds-tokens does now have two vitest files (src/__tests__/typography-shape.test.ts, density-aliases.test.ts), but both target the legacy build/ output — nothing covers src/tokens/**. Every finding above is mechanically checkable from the generated ts/ modules; the two ramp dips and the three new collisions each survived at least one human review.

Add the guard →


ℹ️ Resolved since the last review

  • Dark step-12 dip (0.93 → 0.91 → 0.99) — fixed to 0.96. All interactive hover/pressed progressions now move the right way in dark mode.
  • $value string-under-$type: "number" — all 11 quoted values in scale.json are now real numbers. input/scale.json is fully DTCG-clean.
  • text.primary / icon.primary dark got brighter as a side effect (#d6e3ee#e9f3fc, 14.13:1 → 16.43:1 on canvas). Improvement, but it's a global dark-mode text shift that isn't obvious from the PR body.

ℹ️ Blast radius inside this repo: nil

A grep across eds-core-react, eds-lab-react and eds-data-grid-react for --eds-border-neutral-*, --eds-background-interactive-*, --eds-border-interactive-*, --eds-{icon,text}-interactive-*, --eds-{icon,text}-accent, --eds-background-{danger,info,success,warning} and --eds-density-typography-* returns zero matches. /next components compute typography from their own modular scale in Foundation/typography.css, not the Studio density tokens. Impact is on beta @equinor/eds-tokens consumers and the Chip migration (#5225) only.


Verdict

The generated output is correct and fully reproducible — I re-ran both generators and got byte-identical files, the lockfile arithmetic checks out to the byte, and every ramp is now clean. The pipeline is not the problem.

What I'd resolve before merge is 🟠 1border-neutral-subtle == border-neutral-medium, and enabled-neutral-muted being pixel-identical to disabled. Those are new duplicates introduced by the muted shift, they're in public token API, and they're the same defect class this PR fixes in the ramps. 🟠 3 and 🟠 4 (icon/text step parity) are cheap now and breaking later, since the info/success families are brand new. 🟡 5 just needs a confirmation.

The feat: bump to 3.0.0-beta.5 is warranted — 24 new tokens plus two ramp fixes. The changelog line "✨ Added — update tokens from Tokens Studio release" still tells beta consumers nothing about the dark-mode status-background shift, which is the most visible change in this release.
• branch tokens-studio-release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants