Skip to content

feat: implement explicit dark theme foundations - #39

Open
feuersteiner wants to merge 11 commits into
mainfrom
issue-4-dark-theme
Open

feat: implement explicit dark theme foundations#39
feuersteiner wants to merge 11 commits into
mainfrom
issue-4-dark-theme

Conversation

@feuersteiner

Copy link
Copy Markdown
Owner

Summary

  • replace runtime light/dark theme generation with explicit dark semantic tokens
  • remove the theme toggle and global styling outside the theme foundation
  • document the static token contract in the README, theme page, and llms.txt

Closes #4

Validation

  • bun run check
  • bun run build
  • targeted Prettier and ESLint checks for changed files

Verification note

bun run lint still fails on the pre-existing formatting issue in src/lib/base/toggle/toggle.svelte; this PR does not modify that file.

@feuersteiner feuersteiner left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review round 1 — gpt-5.6-sol (high) — head 4c50e4c

Findings:

  • [P2] Preserve a visible focus indicator in forced-colors mode.
  • [P3] Remove the escaped Markdown backticks around <Theme />.

Comment thread src/lib/base/theme/theme.svelte Outdated
Comment thread README.md Outdated
@feuersteiner

Copy link
Copy Markdown
Owner Author

Automated review round tracker

  • Round: 1
  • Reviewed head: 4c50e4cf45b94a0536947b9bf381416628c726da
  • Signature: Automated review round 1 — gpt-5.6-sol (high) — head 4c50e4c
  • Outcome: COMMENT — 2 actionable findings
  • Findings: preserve focus visibility in forced-colors mode; fix escaped README inline-code delimiters.
  • Tests: bun run check passed; bun run build passed (including packaging/publint); bun run lint reached the documented pre-existing Prettier failure in src/lib/base/toggle/toggle.svelte.
  • Actionable feedback remains: yes.

@feuersteiner

Copy link
Copy Markdown
Owner Author

Review round 1 addressed — gpt-5.6-terra (high)

  • Fixed [P2]: restored a visible 2px focus outline with a forced-colors Highlight fallback.
  • Fixed [P3]: corrected README inline-code delimiters for <Theme />.
  • Tests: bun run check, bun run build, and bunx prettier --check README.md src/lib/base/theme/theme.svelte passed.
  • Declined items: none.
  • Unresolved blockers: none.

@feuersteiner feuersteiner left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review round 2 — gpt-5.6-sol (high) — head b8aa8d4

Finding:

  • [P2] Document only the spacing tokens that actually exist.

Checks: bun run check, bun run build, changed-file Prettier, compiled theme/forced-colors CSS, semantic-token usage, and contrast values passed. bun run lint reaches the documented pre-existing Prettier failure in src/lib/base/toggle/toggle.svelte.

Comment thread src/routes/theming/snippets.ts Outdated
@feuersteiner

Copy link
Copy Markdown
Owner Author

Automated review round tracker

  • Round: 2
  • Reviewed head: b8aa8d4fe1ec4874e7413b01a90696120bb9a9ee
  • Signature: Automated review round 2 — gpt-5.6-sol (high) — head b8aa8d4
  • Outcome: COMMENT — 1 actionable finding
  • Finding: the theming docs imply --space-5 and --space-7 exist, while the theme defines only --space-1, --space-2, --space-3, --space-4, --space-6, and --space-8.
  • Tests: bun run check passed; bun run build passed (including packaging/publint); changed-file Prettier passed; compiled theme and forced-colors CSS, semantic-token usage, and contrast values were checked. bun run lint reached the documented pre-existing Prettier failure in src/lib/base/toggle/toggle.svelte.
  • Actionable feedback remains: yes.

@feuersteiner

Copy link
Copy Markdown
Owner Author

Review round 2 addressed — gpt-5.6-terra (high)

Fixed items:

  • Enumerated only the six defined spacing tokens in the CSS-variable snippet and theming-page foundation-token list.

Tests:

  • bun run check
  • bun run build
  • bun run prepack (package + publint)
  • bunx publint
  • bunx prettier --check src/routes/theming/snippets.ts src/routes/theming/+page.svelte

Declined items: none.
Unresolved blockers: none.

@feuersteiner feuersteiner left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review round 3 — gpt-5.6-sol (high) — head 385ce69

Findings:

  • [P2] Remove the now-unused colord production dependency from package.json and bun.lock. This PR deletes its only imports, while parent issue #1 requires Svelte to be the sole runtime dependency.
  • [P2] Complete the published token contract. src/routes/theming/snippets.ts and the theming page omit the primary/secondary/status aliases and their soft/contrast variants that Theme defines and public components consume.
  • [P2] Finish the “components consume semantic tokens rather than raw colors” acceptance criterion. Button and IconButton still mix states with raw black, and Slider still embeds raw rgba shadow colors instead of theme tokens.

Checks: bun run check; bun run build (including package and publint); changed-file Prettier; compiled theme/forced-colors CSS and package exports inspected. bun run lint reaches only the documented pre-existing Prettier failure in src/lib/base/toggle/toggle.svelte.

@feuersteiner

Copy link
Copy Markdown
Owner Author

Automated review round tracker

  • Round: 3
  • Reviewed head: 385ce69
  • Signature: Automated review round 3 — gpt-5.6-sol (high) — head 385ce69
  • Outcome: COMMENT — 3 actionable findings
  • Findings: remove the unused colord production dependency; document all public component color aliases/variants; replace remaining raw component colors with semantic theme tokens.
  • Tests: bun run check passed; bun run build passed, including packaging and publint; changed-file Prettier passed; compiled theme and forced-colors CSS plus package exports were inspected. bun run lint reached only the documented pre-existing formatting failure in src/lib/base/toggle/toggle.svelte.
  • Actionable feedback remains: yes.

@feuersteiner

Copy link
Copy Markdown
Owner Author

Review round 3 addressed — gpt-5.6-terra (high)

Fixed items:

  • Removed unused colord from package.json and bun.lock.
  • Documented every public primary, secondary, and status color alias plus defined soft, saturated, and contrast variants in the theming page and CSS-variable snippet.
  • Replaced Button and IconButton raw black state mixing with --color-background-saturated; moved Slider thumb shadows to semantic --shadow-control tokens while preserving the prior shadow values.

Tests:

  • bun install --frozen-lockfile
  • bun run check
  • bun run build
  • bun run prepack
  • bunx publint
  • targeted ESLint and Prettier checks
  • token searches confirming no colord, raw black, or rgba() remain in the reviewed components

Declined items: none.
Unresolved blockers: none.

Round-3 feedback was supplied in the review body; GraphQL reports no unresolved inline review threads to reply to or resolve.

@feuersteiner feuersteiner left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review round 4 — gpt-5.6-sol (high) — head 940e7f4

Findings:

  • [P2] Preserve the documentation shell's centering after removing global body alignment.
  • [P2] Complete the published static token contract; several defined and component-consumed tokens remain undocumented.
  • [P2] Raise the danger-on-danger-soft contrast from 4.40:1 to at least 4.5:1 for normal button text.

Checks: exact head reverified; bun install --frozen-lockfile, bun run check, bun run build (including package and publint), changed-file Prettier, and targeted ESLint for materially changed implementation files passed. bun run lint reaches the documented pre-existing Prettier failure in src/lib/base/toggle/toggle.svelte; broader targeted ESLint also reports pre-existing unchanged-line issues in src/routes/+page.svelte and src/routes/types/+page.svelte. Dependency/lock cleanup, runtime-theme removal, semantic color replacement, compiled theme/forced-colors CSS, raw-color/security searches, package exports, and token contrast values were inspected.

Comment thread src/routes/+layout.svelte
Comment thread src/routes/theming/snippets.ts
Comment thread src/lib/base/theme/theme.svelte
@feuersteiner

Copy link
Copy Markdown
Owner Author

Automated review round tracker

  • Round: 4
  • Reviewed head: 940e7f44743cf273a39d996b84b24d4ecf2ff16e
  • Signature: Automated review round 4 — gpt-5.6-sol (high) — head 940e7f4
  • Outcome: COMMENT — 3 actionable findings
  • Findings: preserve documentation-shell centering after global body-layout removal; publish the complete static token contract; raise danger-on-danger-soft text contrast from 4.40:1 to at least 4.5:1.
  • Tests: bun install --frozen-lockfile, bun run check, bun run build (including package and publint), changed-file Prettier, and targeted ESLint for materially changed implementation files passed. bun run lint reached the documented pre-existing Prettier failure in src/lib/base/toggle/toggle.svelte; broader targeted ESLint also reported pre-existing unchanged-line issues in src/routes/+page.svelte and src/routes/types/+page.svelte.
  • Additional review: dependency/lock cleanup, runtime-theme removal, semantic replacements, compiled theme/forced-colors CSS, raw-color/security searches, package exports, and token contrast values inspected.
  • Actionable feedback remains: yes.

@feuersteiner

Copy link
Copy Markdown
Owner Author

Review round 4 addressed — gpt-5.6-terra (high)

Fixed items:

  • Restored documentation-shell centering with scoped margin-inline: auto on the root layout.
  • Enumerated all 66 Theme tokens in the CSS-variable snippet and theming page, including background/foreground variants and compatibility/foundation tokens.
  • Updated --color-danger to #f2767e; danger text on --color-danger-soft now measures 4.77:1.

Tests:

  • bun install --frozen-lockfile
  • bun run check
  • bun run build
  • bun run prepack
  • bunx publint
  • changed-file Prettier and targeted ESLint
  • token-contract/component-consumer validation (66 defined tokens documented in both surfaces; 31 component-consumed public tokens resolve)
  • WCAG contrast validation (4.77:1)

Declined items: none.
Unresolved blockers: none.

Replied to and resolved all three round-4 inline threads.

@feuersteiner feuersteiner left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review round 5 — gpt-5.6-sol (high) — head b449d1f

Finding:

  • [P2] Preserve normal-text contrast in Button and IconButton hover/active states. The resting danger-soft pair now passes at 4.77:1, but the interaction mixes reduce several states below 4.5:1; danger measures about 3.82:1 on soft hover, 3.01:1 on soft active, and 3.82:1 on filled active. The same pattern affects all semantic soft hover/active states and most filled active states. Adjust the tokens or state treatment in both button implementations and validate every semantic color/state.

Checks: exact head reverified; bun install --frozen-lockfile, bun run check, bun run build (including package and publint), changed-file Prettier, and targeted ESLint passed. bun run lint reaches only the documented pre-existing formatting issue in src/lib/base/toggle/toggle.svelte. Browser verification confirmed the 960px shell is centered in a 1280px viewport, dark computed styles and forced-colors focus CSS are present, and exactly 66 computed theme tokens are installed. Both documentation surfaces exactly match all 66 defined tokens; component theme-token consumers resolve; dependency/runtime-theme/raw-color/security searches and all resting semantic contrast pairs were inspected.

Comment thread src/lib/base/button/button.svelte Outdated
@feuersteiner

Copy link
Copy Markdown
Owner Author

Automated review round tracker

  • Round: 5
  • Reviewed head: b449d1f6987bdb2b8e6562e4d089508904037455
  • Signature: Automated review round 5 — gpt-5.6-sol (high) — head b449d1f
  • Outcome: COMMENT — 1 actionable finding
  • Finding: preserve at least 4.5:1 normal-text contrast across Button and IconButton semantic hover/active states; danger is approximately 3.82:1 on soft hover, 3.01:1 on soft active, and 3.82:1 on filled active, with the same broader failure pattern across other semantic colors.
  • Tests: bun install --frozen-lockfile, bun run check, bun run build (including package and publint), changed-file Prettier, and targeted ESLint passed. Browser validation confirmed the 960px shell is centered at a 1280px viewport, exactly 66 computed theme tokens install, and dark/focus styles compile. Token definitions match both 66-token documentation surfaces exactly; component token consumers, runtime-theme/dependency/raw-color/security searches, and resting contrast pairs were inspected. bun run lint reached only the documented pre-existing Prettier failure in src/lib/base/toggle/toggle.svelte.
  • Actionable feedback remains: yes.

@feuersteiner

Copy link
Copy Markdown
Owner Author

Review round 5 addressed — gpt-5.6-terra (high)

Fixed items:

  • Updated Button and IconButton filled hover/active states to blend toward --color-foreground-saturated, retaining distinct brighter feedback without reducing contrast.
  • Updated soft hover/active states to blend toward --color-background, preserving semantic text separation.
  • Reduced ghost active overlays from 25% to 20%; this also closes secondary and danger ghost-active edge cases.

Tests:

  • bun install --frozen-lockfile
  • bun run check
  • bun run build
  • bun run prepack
  • bunx publint
  • targeted ESLint and Prettier checks
  • browser validation of all 108 Button/IconButton semantic color × variant × rest/hover/active combinations (minimum 4.71:1)

Declined items: none.
Unresolved blockers: none.

Replied to and resolved the round-5 review thread.

@feuersteiner feuersteiner left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review round 6 — gpt-5.6-sol (high) — head 5bda3b1

Finding:

  • [P2] Cover the public foreground/background aliases in the interaction-state treatment. The documented 108-state validation covers only six palette colors, while the public Color type and component demos expose eight. background ghost Button/IconButton instances render #181818 content on the transparent #181818 page at rest and reach only approximately 1.24:1/1.61:1 on hover/active; foreground soft also changes only about #111111#121212#131313.

Checks: exact head reverified; bun install --frozen-lockfile, bun run check, bun run build, bun run prepack, publint, changed-file Prettier, and targeted ESLint passed. Both component demos, all eight color aliases across filled/soft/ghost rest-state rendering, compiled interaction CSS, all 144 color × variant × state × component contrast calculations, 66-token definitions/documentation, token consumers, layout/focus CSS, dependencies, runtime-theme removal, raw component colors, and security-sensitive patterns were inspected. bun run lint reaches only the documented pre-existing Prettier failure in src/lib/base/toggle/toggle.svelte.

Comment thread src/lib/base/button/button.svelte
@feuersteiner

Copy link
Copy Markdown
Owner Author

Automated review round tracker

  • Round: 6
  • Reviewed head: 5bda3b173539983f4d0b1ff88b66f79ea48aec1d
  • Signature: Automated review round 6 — gpt-5.6-sol (high) — head 5bda3b1
  • Outcome: COMMENT — 1 actionable finding
  • Finding: cover the public foreground/background Button and IconButton aliases in interaction-state validation and treatment; background.ghost is effectively invisible (1.00:1 rest, ~1.24:1 hover, ~1.61:1 active), and foreground.soft has negligible hover/active differentiation.
  • Tests: bun install --frozen-lockfile, bun run check, bun run build, bun run prepack, bunx publint, changed-file Prettier, and targeted ESLint passed. Both rendered component demos, compiled interaction CSS, all 144 color × variant × state × component contrast calculations, the exact 66-token contract/documentation, token consumers, theme/global layout/focus behavior, dependency/runtime-theme/raw-color/security searches, and package exports were inspected. bun run lint reached only the documented pre-existing Prettier failure in src/lib/base/toggle/toggle.svelte.
  • Actionable feedback remains: yes.

@feuersteiner

Copy link
Copy Markdown
Owner Author

Review round 6 addressed — gpt-5.6-terra (high)

Fixed items:

  • Restored visible background.ghost Button and IconButton content, including nested icons, with the foreground token.
  • Added distinct foreground.soft hover and active backgrounds that retain text contrast.
  • Made Color a canonical COLOR_VALUES contract; a shared exhaustive Record<Color, ...> mapping now drives both components.
  • Added validate:button-colors, which reads the public contract and validates all 144 Button/IconButton color × variant × rest/hover/active combinations.

Tests:

  • bun install --frozen-lockfile
  • bun run check
  • bun run build
  • bun run prepack
  • bunx publint
  • bun run validate:button-colors (minimum contrast 4.73:1; minimum interaction delta 0.016)
  • changed-file Prettier and targeted ESLint
  • rendered Button and IconButton demo checks: all 48 rest combinations; repaired aliases measure 11.76:1 in their demo context

Declined items: none.
Unresolved blockers: none.

Replied to and resolved the round-6 inline thread.

@feuersteiner feuersteiner left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review round 7 — gpt-5.6-sol (high) — head f021bf7

Finding:

  • [P2] Make validate:button-colors exercise the actual Button and IconButton source of truth. The current script models the expected mapping and state formulas independently, so real component regressions can still pass.

Checks: exact head reverified; bun install --frozen-lockfile, bun run validate:button-colors, bun run check, bun run build (including package and publint), changed-file Prettier, targeted ESLint, token-contract equality, runtime-theme/security searches, and rendered Button/IconButton demos passed. All 48 rendered rest combinations resolve correctly (minimum measured contrast 4.77:1); compiled interaction CSS and the 144 modeled state calculations were inspected. bun run lint reaches only the documented pre-existing formatting failure in src/lib/base/toggle/toggle.svelte.

Comment thread scripts/validate-button-color-states.mjs Outdated
@feuersteiner

Copy link
Copy Markdown
Owner Author

Automated review round tracker

  • Round: 7
  • Reviewed head: f021bf769803e9ada4187081d1cc9e6546c065c4
  • Signature: Automated review round 7 — gpt-5.6-sol (high) — head f021bf7
  • Outcome: COMMENT — 1 actionable finding
  • Finding: validate:button-colors reimplements expected mappings and interaction formulas instead of exercising the actual Button and IconButton declarations; a temporary background.ghost contrast regression still passed unchanged, so the claimed 144-state guard can miss implementation drift.
  • Tests: bun install --frozen-lockfile, bun run validate:button-colors, bun run check, bun run build (including packaging and publint), changed-file Prettier, and targeted ESLint passed. bun run lint reached only the documented pre-existing formatting failure in src/lib/base/toggle/toggle.svelte.
  • Additional review: all 48 rendered Button/IconButton rest combinations resolved the intended eight-color mappings and measured at least 4.77:1; compiled interaction CSS, 66-token definitions/documentation, token consumers, theme/global layout/focus behavior, dependency/runtime-theme/raw-color/security searches, and package output were inspected.
  • Actionable feedback remains: yes.

@feuersteiner

Copy link
Copy Markdown
Owner Author

Review round 7 addressed — gpt-5.6-terra (high)

  • Fix: replaced the modeled palette matrix with a validator that compiles Button and IconButton CSS, reads their emitted declarations plus the exported mapping, and evaluates every public color, variant, and rest/hover/active DOM state.
  • Tests: frozen install; mutation proof for both background.ghost regressions; 144-state contrast/differentiation validation (minimum 4.73:1); check, build, prepack, publint, Prettier, and targeted ESLint all pass.
  • Declines: none.
  • Blockers: none.

@feuersteiner feuersteiner left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review round 8 — gpt-5.6-sol (high) — head bb96652

Findings:

  • [P2] Use browser-accurate selector/state/cascade evaluation; the current hand parser accepts disabled-only hover rules and ignores specificity, so runtime interaction/contrast regressions can pass.
  • [P2] Assert semantic alias identity; importing the mapping as truth lets danger map to primary (or primary to secondary) while the validator still passes all 144 cells.

Checks: exact head reverified; frozen install, validate:button-colors --mutation-proof, check, build, prepack, publint, changed-file Prettier, targeted ESLint, 66-token contract equality, runtime rest-state contrast for all 48 Button/IconButton combinations, focus/forced-colors and font-scaling inspection, dependency/runtime-theme/raw-color/security searches, and package output passed. Four independent adversarial mutations all returned status 0. bun run lint reaches only the documented pre-existing formatting failure in src/lib/base/toggle/toggle.svelte.

Comment thread scripts/validate-button-color-states.mjs Outdated
Comment thread scripts/validate-button-color-states.mjs Outdated
@feuersteiner

Copy link
Copy Markdown
Owner Author

Automated review round tracker

  • Round: 8
  • Reviewed head: bb9665251725cdc6575a2926c00ea2927523602a
  • Signature: Automated review round 8 — gpt-5.6-sol (high) — head bb96652
  • Outcome: COMMENT — 2 actionable findings
  • Findings: replace the hand-modeled selector/cascade evaluation with browser-accurate computed state validation; assert semantic alias identity so valid-contrast remaps cannot pass.
  • Mutation evidence: danger → primary, primary → secondary, enabled soft-hover → disabled-only, and a higher-specificity 1:1 danger-filled hover override all still exited 0 with 144 combinations reported valid.
  • Tests: frozen install; bun run validate:button-colors --mutation-proof; bun run check; bun run build; bun run prepack; bunx publint; changed-file Prettier; targeted ESLint; exact 66-token contract/documentation equality; browser runtime rest contrast across all 48 Button/IconButton combinations (minimum 4.77:1); focus/forced-colors, scalable sizing, dependency/runtime-theme/raw-color/security, and package-output inspection. bun run lint reached only the documented pre-existing formatting failure in src/lib/base/toggle/toggle.svelte.
  • Actionable feedback remains: yes.

@feuersteiner feuersteiner left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review round 9 — gpt-5.6-sol (high) — head d70b271

Findings:

  • [P2] Exercise actual Svelte Button and IconButton instances in the Chromium matrix; the current synthetic fixtures bypass component markup, prop wiring, inline token installation, and nested icons.
  • [P2] Validate effective semantic alias declarations; the current first-match parser accepts a correct declaration followed by a later remapping that wins in inline-style order.

Checks: exact head reverified; frozen install; Chromium validator and built-in mutation proof; four exact round-8 mutations independently injected and rejected with browser/identity evidence; check; build; prepack; publint; changed-file Prettier; targeted ESLint; repeated baseline validation; 66-token definition/documentation equality; actual built Button/IconButton instances and nested icon colors; dark scheme, focus, and forced-colors behavior; raw-color, runtime-theme, dependency, package-output, portability, and security inspection. Independent component-binding and duplicate-alias mutations both still returned status 0.

Comment thread scripts/validate-button-color-states.mjs Outdated
Comment thread scripts/validate-button-color-states.mjs Outdated
@feuersteiner

Copy link
Copy Markdown
Owner Author

Automated review round tracker

  • Round: 9
  • Reviewed head: d70b2716802d3caa6aac6e4ea3e4c0cb28c07cbf
  • Signature: Automated review round 9 — gpt-5.6-sol (high) — head d70b271
  • Outcome: COMMENT — 2 actionable findings
  • Findings: mount actual Svelte Button/IconButton instances so Chromium validates public markup, prop wiring, inline token installation, and nested icons; validate final/effective semantic alias declarations instead of first textual matches.
  • Mutation evidence: all four exact round-8 mutations were independently injected and rejected. Removing the real Button token-style binding still exited 0 with 288 rows valid; appending a later danger → primary declaration triplet also exited 0 because the oracle read the earlier valid triplet while Chromium applied the later one.
  • Tests: bun install --frozen-lockfile; bun scripts/validate-button-color-states.mjs --mutation-proof (288 rows, minimum 4.71:1 contrast, 0.006 interaction delta); bun run check; bun run build; bun run prepack; bunx publint; changed-file Prettier; targeted ESLint; exact 66-token definition/snippet/page equality; built Button/IconButton and nested-icon runtime inspection; dark/focus/forced-colors browser inspection; diff, dependency, package, portability, raw-color, runtime-theme, and security inspection. The dependency audit reports existing toolchain advisories; the newly added Playwright packages are not named by them.
  • Actionable feedback remains: yes.

@feuersteiner feuersteiner left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review round 10 — gpt-5.6-sol (high) — head 56f6ccd

Findings:

  • [P2] Validate the rendered nested Icon foreground, not only the button element’s computed color. renderedState reads getComputedStyle(element).color, but IconButton’s only visible content is a child Icon whose color classes override inheritance. Independently changing IconButton to contrastMode={false} makes a filled primary icon render primary-on-primary (about 1:1), yet the command still exits 0 with all 288 cells valid and reports 4.71:1 minimum contrast. Inspect each rendered icon (and its effective background) so icon prop/class regressions fail.
  • [P2] Compare semantic identity against the effective computed cascade. The oracle reads custom properties from element.style, so stylesheet declarations that win over inline styles are invisible to it. An independently injected .danger rule remapping all three aliases to primary with !important changed Chromium’s effective tokens but still exited 0 with all 288 cells valid. Read the three properties from getComputedStyle(element) (while retaining the independent alias oracle) so higher-priority stylesheet remaps are rejected.

Checks: exact head reverified; frozen install; actual 288-cell Chromium baseline; built-in mutation proof; both exact round-9 mutations independently injected and rejected; check; build; prepack; publint; changed-file Prettier; targeted ESLint; exact 66-token definition/snippet/page equality; focus/forced-colors, scalable sizing, runtime-theme removal, semantic colors, package output, diff integrity, and security-sensitive additions inspected. Full lint reaches only the documented pre-existing Prettier failure in src/lib/base/toggle/toggle.svelte. bun audit reports the same 55 existing toolchain advisories on base and head; Playwright is not named.

@feuersteiner

Copy link
Copy Markdown
Owner Author

Automated review round tracker

  • Round: 10
  • Reviewed head: 56f6ccdf0f335b51af3f908f1735a6d70869a625
  • Signature: Automated review round 10 — gpt-5.6-sol (high) — head 56f6ccd
  • Outcome: COMMENT — 2 actionable findings
  • Findings: validate the nested Icon’s actual rendered foreground/background so IconButton prop regressions cannot hide behind the button element’s unused computed color; compare semantic aliases with the effective computed CSS cascade so stylesheet/!important remaps cannot hide behind the inline declaration source.
  • Mutation evidence: both exact round-9 mutations were independently injected and rejected. Independently setting IconButton contrastMode={false} still exited 0 with 288 cells and a reported 4.71:1 minimum despite a primary-on-primary icon; an independent .danger !important remap to primary also exited 0 with all 288 cells valid.
  • Tests: bun install --frozen-lockfile; bun run validate:button-colors --mutation-proof (288 actual rendered cells, 4.71:1 minimum contrast, 0.015 minimum interaction delta); repeated baseline validation; bun run check; bun run build; bun run prepack; bunx publint; changed-file Prettier; targeted ESLint; exact 66-token definition/snippet/page equality; issue [Blocker] Implement explicit dark theme foundations #4 acceptance, actual component markup/class/prop wiring, focus/forced-colors, scalable sizing, package output, diff integrity, raw-color/runtime-theme/security-sensitive additions inspected. Full lint reaches only the pre-existing Prettier failure in src/lib/base/toggle/toggle.svelte. bun audit reports the same 55 existing toolchain advisories on base and head; Playwright is not named.
  • Review: feat: implement explicit dark theme foundations #39 (review)
  • Actionable feedback remains: yes.

@feuersteiner feuersteiner left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review round 10 addressed — gpt-5.6-terra (high)

@feuersteiner feuersteiner left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review round 11 — gpt-5.6-sol (high) — head 51a1742

No findings.

Checks: exact head reverified; frozen install; built-in Chromium mutation proof (288 actual Svelte Button/IconButton cells, eight aliases, three variants, rest/hover/active plus disabled, minimum 4.71:1 contrast and 0.015 interaction delta); four independently isolated exact/adjacent Icon/Glyph and computed-cascade/!important mutations rejected; check; build; package/publint; changed-file Prettier; targeted ESLint; exact 66-token definition/documentation equality; dark theme, centered shell, focus/forced-colors, and 200% font-scaling runtime inspection; diff/security/package review. Full lint reaches only the pre-existing Prettier failure in src/lib/base/toggle/toggle.svelte. Base and head each report the same 55 existing toolchain advisories; Playwright is not named. GitHub does not permit approving a self-authored PR, so this is a signed no-findings COMMENT.

@feuersteiner

Copy link
Copy Markdown
Owner Author

Automated review round tracker

  • Round: 11
  • Reviewed head: 51a17421f9a4494bd9b90e0d8b79af595ed95d00
  • Signature: Automated review round 11 — gpt-5.6-sol (high) — head 51a1742
  • Outcome: COMMENT — no findings; GitHub does not permit self-approval.
  • Findings: none.
  • Mutation evidence: the built-in 17-case mutation proof passed. Four additional isolated mutations were independently injected and rejected: IconButton contrastMode={false}; direct primary Glyph contrast override; a valid-contrast .danger.filled !important semantic remap; and a state-only button.danger.filled:hover unused-alias !important remap. The last two prove effective computed cascade and per-state semantic identity checks.
  • Tests: bun install --frozen-lockfile; bun run validate:button-colors --mutation-proof (288 actual Svelte+Chromium cells, minimum 4.71:1 contrast and 0.015 interaction delta); bun run check; bun run build (including package and publint); changed-file Prettier; targeted ESLint; exact 66-token contract/documentation equality; runtime dark theme, centered 960px shell, focus/forced-colors, and 200% font scaling. Full lint reaches only the pre-existing formatting failure in src/lib/base/toggle/toggle.svelte. Base and head have the same 55 existing toolchain advisories; Playwright is not named.
  • Review: feat: implement explicit dark theme foundations #39 (review)
  • Actionable feedback remains: no.

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.

[Blocker] Implement explicit dark theme foundations

1 participant