Skip to content

feat(cli): experimental theme: sixth to eighth passes, and lists derived from css-tree, Core docs and the stable font list - #11350

Open
josephfarina wants to merge 33 commits into
mainfrom
themes-v2-r10
Open

josephfarina wants to merge 33 commits into
mainfrom
themes-v2-r10

Conversation

@josephfarina

@josephfarina josephfarina commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor

Sixth to eighth passes for astryx experimental theme, plus a cleanup that replaces the editor's hand-kept lists with the sources the repo already has. Found by the final hunt passes and seeded chaos runs over every experimental theme command, subcommand and option.

Mostly experimental code (packages/cli/api/theme-edit/, the experimental command copies). Shared files touched, all behavior-neutral for stable commands unless noted:

  • api/theme/build/font-warning.mjs: unchanged (byte-identical to main); the editor reads main's list through collectUnloadedFonts and keeps its own small map of fonts that ship on one system.
  • api/theme/palette/generate/generator.mjs: exports its config key lists and modesForStrategy, which the editor reads instead of retyping.
  • foundation/release/cache.mjs: an opt-in leased option on tryLock, passed only by the editor's folder lock; the release cache's own behavior (incl. the identity takeover from the merged lock fix) is unchanged. lock-lease.mjs: foreignLockIsSilent takes an optional window (default unchanged). folder-lock.mjs: editor-only.
  • clients/cli/commands/theme.mjs: the experimental compile no longer holds the theme lock under --watch.
  • Response docs: two experimental JSON fields: sheetStale (optional) on set/undo/new and themeMissing (always present) on preview list, new refusal wording.

The Core playground defaults that make more components render in the docs playground are split into their own PR; the editor derives a demo from a component's props when its doc has no defaults, so this PR does not depend on it.

One source instead of hand lists

  • CSS property names and value grammar from css-tree (already a dependency; cost recorded when it was added). Legacy -ms- grammar branches are left out, so filter banana is refused.
  • System fonts judged by the stable build's own list (unchanged); open fonts derived as @fontsource/<name> with two listed exceptions.
  • Neutral stops read from the shipped neutral theme source; palette key lists from the generator.
  • Component demos from each .doc.mjs playground; when a doc has none (older Core), a demo is derived from its props.
  • how part words from Core's catalog target names and the shared edit distance; synonyms that are not target names live in one table.
  • Shared test helpers in test-utils/; duplicate hash, empty-folder, install-line and void-tag helpers replaced by the foundation ones.

Editor core (sixth to eighth passes)

  • Locks: a lock is never taken from a holder alive on this host; a holder in another process space is taken over only after its lease goes silent; every write under the lock is fenced, and a command that lost its lock refuses without rolling back the other command's files.
  • Recovery: new checks everything before writing, keeps user files and history, and recovers settings of the wrong shape; a refused new changes nothing (tree-hash test).
  • Corners: pill never makes anything less round; every container gets the container radius; steps read every setting that reaches the part; undo restores the radii a step was built with.
  • --state focus on a wrapper part suggests focus-within; refusals for a bad argument, a damaged or missing theme file, a busy theme and a missing project lead with a command that runs (a behavior test runs each first suggestion).
  • show without React lists settings and builds; a cut-short CSS file is rebuilt; sheet.html says when it is stale.

Colors (sixth to eighth passes)

  • Every part property checked by its CSS grammar; time and easing tokens take one value; part widths and durations are never negative.
  • An exact secondary text color reaches the tinted parts; mode switches list both modes; an older theme keeps its shades until a color changes, and the palette is brought up to date before the build reads it.
  • Code fonts fall back to monospace; Roboto installs @fontsource/roboto again.

editor how

  • A CSS property named in a question is set as named, with the typed value whole (colon or not); ratios ("halve"), reset words, direction words and description clauses read the right way round; more part words and variant words (error toasts, read-only).
  • Fresh question sets written to break it went from 51% to 79% (previous fresh set) and the older sets held at 96/89/92/94%.

Preview

  • Every component renders on the repo's Core and on an older installed Core (121 cells); overlays show once; state labels match; the Text cell suggests a command that works; folders named like flags get cd ./-k.

Final review round (follow-up commits)

  • Colors: typed CSS math for every Values 4 function (sizes, times, numbers, colors); no CSS-wide keywords on tokens; no percentage where the reading properties take none; one-system fonts named as such; a hand-edited generated settings module (LF or CRLF) is built over; font-warning.mjs is byte-identical to main (the editor keeps its own one-system map over main's list).
  • How: border shorthands completed or mapped to the right longhand; layer opacity; negation; motion; pt sizes; heading color through the text token; divider; reset to the base theme.
  • Preview: a route handler or another page at the preview's URL is refused; Next's build folders are refused; tall parts are framed; the pageExtensions refusal suggests commands.
  • Core: undo history survives recovery after a stopped new; file-name theme names; undo suggestions; radius tokens and monotonic corners; --state values Core never styles are refused with the working form; build output is fenced like every other write under the lock (one stillHolds() call in api/integration/add-helpers.mjs's applyWrites, which does nothing outside the editor's lock); a no-op change mends a stale build.
  • Changesets corrected where they contradicted the code; sheetStale typed and documented; the dead refuseWithoutReact removed.

Tests

Full gate on current main: 7,633 tests pass (theme-edit, experimental commands, api/theme, foundation, packages/cli/test), tsc strict, structure, package boundaries, changesets. One known timing flake in packed-gather-cache under full parallel load (passes alone; untouched here).

…red test helpers; reuse foundation helpers; drop dead code
…ry part property by its grammar, one-value time and easing tokens, type dials that do what they say
… load, mode switches list both modes, part widths and durations never negative, a theme keeps its shades until a color changes
…rts on their own; overlays once; state labels; preview list marks a gone theme; stray page named; failing browser named
… a live holder, fenced writes, recovery keeps user files, React checked first, refusals that run)
…overy changes nothing until checked, no rollback after a lost lock, pill and container radii, focus-within, show without React)
…up to date before the build reads them, legacy filter grammar left out, code fonts fall back to monospace
…nd, divider width, cd for a flag-like folder)
…built with; an earlier build keeps its radii until corners or the base change)
@vercel

vercel Bot commented Nov 12, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
astryx Ready Ready Preview Nov 12, 2026 7:59am UTC

Request Review

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Nov 12, 2026
@astracat-bot

astracat-bot Bot commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor

AI review status for this pull request.

Review status Updated
🔄 In progress (for maintainers only) Nov 12, 2026, 7:59 AM UTC

@github-actions

github-actions Bot commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor

PR Analysis Report

🔎 Preview · ab55741

Storybook · Sandbox · built from this PR's head

Package size: @astryxdesign/cli 3.99 MB packed (+43.8 kB); no new dependencies.

API changes: cli +2 · ~1 · −0

3 API changes (advisory)
Package Where Signature Usage
+ cli ./api/experimental · Types · type ThemeEditChangeData sheetStale?: string | undefined import type {ThemeEditChangeData} from '@astryxdesign/cli/api/experimental'; const x: ThemeEditChangeData = {sheetStale: …}
+ cli ./api/experimental · Types · type ThemeEditPreviewFields sheetStale?: string | undefined import type {ThemeEditPreviewFields} from '@astryxdesign/cli/api/experimental'; const x: ThemeEditPreviewFields = {sheetStale: …}
~ cli ./api/experimental · Types · type ThemeEditPreviewListResponse data: { folder: string | null; theme: string | null; pages: Array<{ id: string; label: string }>; notes: string[] } → data: { folder: string | null; theme: string | null; themeMissing: boolean; pages: Array<{ id: string; label: string }>; notes: string[] } import type {ThemeEditPreviewListResponse} from '@astryxdesign/cli/api/experimental'; const x: ThemeEditPreviewListResponse = {data: …}

Not run on this PR: add ci:run-visual, ci:run-a11y or ci:run-rtl to run them.


Generated by PR Enrichment workflow | View full report

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approval needs an engineering-owner decision, and these changes are needed either way. No current spec record defines how astryx experimental theme editor behaves. This pull request changes that behavior and adds two experimental result fields (sheetStale, themeMissing). An engineering owner records the decision by commenting /approve-spec <full head SHA> on this pull request, or names the record that must define it first; the author may do so as an engineering owner.

These need fixing whatever that decision is:

  • The shared font list adds Garamond, Comic Sans MS, Noto Sans, Ubuntu and Cantarell (font-warning.mjs:86-92), so stable astryx theme build and astryx doctor stop warning about them, even though the description says that output is unchanged and the change ships under an [experimental] changeset, which AST-017 FR22 keeps off stable commands.
  • Three changeset bullets say the opposite of what this head does and of later changesets in the same pull request, so remove or reword them: experimental-theme-hunt6.md says a command that lost its lock undoes its change and that show checks for React before it writes, and experimental-theme-how-answers-7.md says how cannot name combobox or accordion.
  • "Every editor refusal leads with a command that runs" is not true, because set <theme> font more (_check.mjs:1449-1453), a themes path that is a file or link (_edit.mjs:211-215) and a next.config without tsx (_preview-app.mjs:380-384) refuse with no command, so add one or narrow the claim.
  • The new ERR_RENDER_FAILED description says a missing React raises it, but a missing React raises ERR_RENDER_MISSING_DEPENDENCY or nothing, and refuseWithoutReact (_parts.mjs:270) is called only by a test whose title says it tests show.
  • set and single-setting undo return sheetStale, but neither ThemeEditChangeData (theme-edit.type.mjs:76-98) nor the themeEdit.set, undo and new response docs declare it, although the description says the response docs add it.
  • The math check (_css.mjs:208-218) refuses valid calc() values that use pow(), sqrt(), hypot(), exp(), log() or the trig functions, such as --radius-element 'calc(4px * pow(2, 2))', which the base accepted.
  • Not every write under the lock is fenced, as the description says: markSheetStale writes sheet.html directly (_context.mjs:376), and commitTheme checks the lock only before and after the build, which writes its outputs in between.
  • theme-hunt6.test.mjs:47 and theme-hunt7.test.mjs:40 copy the refusal() helper that this pull request moves into test-utils/theme-edit-fixture.mjs, so import it instead.
Claims and files checked
  • Description, shared font list leaves the build warning unchanged: false (font-warning.mjs:86-92, :120-123).
  • Description, the palette generator exports its key lists and mode helper for the editor: verified (generator.mjs:110-137).
  • Description, the leased lock option is opt-in and the release cache is unchanged: verified (cache.mjs:704-712, folder-lock.mjs:137).
  • Description, the silence check takes an optional window with the old default: verified (lock-lease.mjs:63-80).
  • Description, the experimental compile skips the theme lock under --watch: verified (theme.mjs:1072).
  • Description, the response docs gain one optional field, sheetStale: false (response-types.doc.mjs:117-139, theme-edit.type.mjs:76-98).
  • Description, demos are derived from a doc's props when it has no playground: verified (_parts.mjs demoProps, theme-catalog.test.mjs).
  • Description, CSS names and grammar come from css-tree with the legacy filter branches left out: verified (_css.mjs).
  • Description, system fonts come from the build's list and open-font packages are derived with two exceptions: verified (_tables.mjs).
  • Description, neutral stops and palette keys are read from their sources: verified (_palette.mjs:272-416).
  • Description, part words come from Core's catalog with the shared edit distance: verified (_how-question.mjs partLexicon, corrected).
  • Description, shared test helpers moved to test-utils: partly; two new tests still copy one (theme-hunt6.test.mjs:47, theme-hunt7.test.mjs:40).
  • Description, a live holder on this computer keeps its lock and another process space loses it only after silence: verified (folder-lock.mjs:59-66, cache.mjs:704-722).
  • Description, every write under the lock is fenced: false (_context.mjs:376, _settings.mjs:2577-2585).
  • Description, a command that lost its lock refuses without rolling back: verified (_settings.mjs:2611-2615, theme-hunt7-lock.test.mjs).
  • Description, a recovering new checks first, keeps user files and history, and recovers wrong-shape settings: verified (_edit.mjs, theme-hunt6.test.mjs, theme-hunt7.test.mjs).
  • Description, the corner rules for pill, containers, steps and undo radii: verified (_settings.mjs radiusAt, radiusTokenOf; theme-hunt7.test.mjs, theme-hunt8.test.mjs).
  • Description, focus on a wrapper part suggests focus-within: verified (_check.mjs:881-897).
  • Description, every editor refusal leads with a command that runs: false (_check.mjs:1449-1453, _edit.mjs:211-215, _preview-app.mjs:380-384).
  • Description, show without React, a rebuilt cut-short stylesheet and the stale sheet banner: verified (_edit.mjs:1175-1186, _settings.mjs:2393, _context.mjs:363-381).
  • Description, grammar checks with one time or easing per token and no negative widths or durations: verified, but valid math functions are refused (_css.mjs:208-218).
  • Description, secondary text on tinted parts, both modes listed, kept shades and a refreshed palette: verified (_settings.mjs compileTheme, commitTheme, buildTheme).
  • Description, code fonts fall back to monospace and Roboto gets its install line: verified (_settings.mjs compileTheme, theme-palette.test.mjs).
  • Description, the listed how readings: covered by theme-how.test.mjs cases; not run here.
  • Description, the question-accuracy percentages and the 121-cell count: not verifiable from the repository.
  • Description, the full test gate passes: CI's test shards pass at this head; the 7,633 count is not verified.
  • Changeset catalog-from-docs: verified.
  • Changeset colors6: verified, except that the math rule also refuses valid functions.
  • Changeset colors7: verified.
  • Changeset colors8: verified.
  • Changeset how-answers-7: false in one bullet (combobox and accordion), the rest covered by tests not run here.
  • Changeset how-answers-8: covered by tests not run here.
  • Changeset how-catalog-parts: verified.
  • Changeset hunt5 (edited bullet): verified.
  • Changeset hunt5b (edited bullet): verified.
  • Changeset hunt6: false in three bullets (lock undo, React check, every refusal).
  • Changeset hunt7: verified.
  • Changeset hunt8: verified.
  • Changeset preview-older-core: verified.
  • Changeset preview-ten: verified.
  • Changeset theme-editor-css-tree: false, a stable build change under an experimental note; the six newer properties are not verified here.
  • Files with findings: font-warning.mjs, theme-editor-css-tree.md, experimental-theme-hunt6.md, experimental-theme-how-answers-7.md, _check.mjs, _edit.mjs, _preview-app.mjs, error-codes.doc.mjs, _parts.mjs, theme-edit.type.mjs, response-types.doc.mjs, _css.mjs, _color.mjs, _context.mjs, _settings.mjs, theme-hunt6.test.mjs, theme-hunt7.test.mjs, theme-palette.test.mjs, theme-editor-refusals.behavior.test.mjs.
  • Files clear: the other twelve changesets; _adapter.mjs, _catalog.mjs, _css-properties.mjs (deleted), _how-commands.mjs, _how-question.mjs, _how.mjs, _overview.mjs, _palette.mjs, _preview.mjs, _project.mjs, _tables.mjs, themeEditPreviewList.doc.mjs; generator.mjs; experimental-theme-editor-new.doc.mjs, experimental/theme.mjs, commands/theme.mjs; doc-loads.test.mjs; cache.mjs, folder-lock.mjs, lock-lease.mjs; test-utils/theme-edit-fixture.mjs; and the tests theme-catalog, theme-edit, theme-how, theme-hunt, theme-hunt3, theme-hunt4, theme-hunt5, theme-hunt7-lock, theme-hunt8, theme-picture, theme-preview-safety, theme-preview, theme-stress.

Automated review (maintainer link)

Comment thread .changeset/experimental-theme-hunt6.md Resolved
Comment thread .changeset/experimental-theme-hunt6.md Resolved
Comment thread packages/cli/api/theme-edit/_parts.mjs Resolved
Comment thread packages/cli/api/theme-edit/_css.mjs Resolved
…rds on tokens, one-system fonts warn, var() refused in the font dial, a hand-edited settings module is built over
…, math inside colors checked, no percentage where readers take none, fonts judged by the theme build's own list, one-system fonts named, CRLF-safe module seal
…w's URL, Next's build folder, tall parts framed)
…ame theme names, undo suggestions, radius tokens and monotonic corners, states Core never takes refused, fenced build output, stale build mended by a no-op change)
@josephfarina

Copy link
Copy Markdown
Contributor Author

/approve-spec efc7314

@josephfarina

Copy link
Copy Markdown
Contributor Author

/approve-spec 87096ca

@astracat-bot astracat-bot Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Changes requested. An engineering owner's /approve-spec on this head settles the direction of the experimental editor behavior and its three new result fields. The earlier stable font-list change, the superseded changeset bullets, the math function list, the sheet and build fencing and the copied test helper are all fixed. These still need fixing:

  • The new how answers for renaming a theme or making a second one print new <theme>-2 --like ${env.data.like} (_how.mjs:910, :930), which becomes --like undefined for a theme with no base and is refused by new, so print --like none or leave the flag out there (the reset answer at _how-question.mjs:2273 prints "(undefined)" the same way).
  • Miss: a new that lost its lock still moves the damaged settings back, deletes .astryx-new.json and removes the folder in its catch (_edit.mjs:326-335), and its marker write and rename skip the fence, which contradicts the description and experimental-theme-hunt6.md line 7, so rethrow the lost-lock refusal before rolling back and fence those writes, or narrow both claims.
  • Miss: experimental-theme-hunt4.md line 10 says show writes nothing without React, but show now builds without React (_edit.mjs:1209-1216, experimental-theme-hunt7.md line 10), so reword it.
  • experimental-theme-colors9.md line 9 promises the "app must load them" warning for one-system fonts that experimental-theme-colors10.md replaces, and experimental-theme-hunt9.md line 8 says a theme typed as a file name is read as the theme, though it is refused with a show <theme> suggestion (_settings.mjs:261-283), so reword both.
  • "Every editor refusal leads with a command that runs" (experimental-theme-hunt6.md line 10 and the description) is still not true, because no package.json, an invalid package.json, Yarn Plug'n'Play, a folder where an output file goes and a write error refuse with no suggestion (_context.mjs:52-68, _adapter.mjs:111-115, _palette.mjs:61-66, _settings.mjs:2719-2724), and show's render-failure refusal suggests the same show (_edit.mjs:1219-1230), so narrow the claim to what the behavior test covers or add a working command to each.

CI's required test check is red only because the test-node (1/4) shard, which runs many of the editor's tests, hit its 15-minute job limit after all its tests passed, so it needs a rerun or a faster editor suite. One smaller point: the description says the response docs gain sheetStale, but response-types.doc.mjs gains themeMissing, and its set, undo and new entries still do not list sheetStale.

Claims and files checked
  • Description, the build font list is byte-identical to main: verified (font-warning.mjs is not changed).
  • Description, the generator exports its key lists and mode helper: verified (generator.mjs:110-137).
  • Description, the leased lock option is opt-in and the release cache is unchanged: verified (cache.mjs:704-709, folder-lock.mjs:137).
  • Description, the silence check takes an optional window with the old default: verified (lock-lease.mjs:63-80).
  • Description, the experimental compile skips the theme lock under --watch: verified (theme.mjs:1067-1074).
  • Description, the response docs gain one optional field, sheetStale: partly (the response docs gain themeMissing; sheetStale is in the types and the set and undo docs).
  • Description, playground defaults moved out and demos derived from props: verified (_parts.mjs, theme-catalog.test.mjs).
  • Description, CSS names and grammar from css-tree with legacy filter branches left out: verified (_css.mjs).
  • Description, system fonts from the build's list and open fonts by package name: verified (_tables.mjs:147-151).
  • Description, neutral stops and palette keys read from their sources: verified (_palette.mjs:281-331).
  • Description, part words from Core's catalog and the shared edit distance: verified (_how-question.mjs).
  • Description, shared test helpers moved to test-utils: verified (all eleven test files import refusal).
  • Description, a live holder on this computer keeps its lock and another process space loses it only after silence: verified (folder-lock.mjs:59-66, :137-150).
  • Description, every write under the lock is fenced and a lost-lock command does not roll back: false for new (_edit.mjs:298-335).
  • Description, recovery by new checks first and keeps user files and history: verified (_edit.mjs, theme-hunt9.test.mjs).
  • Description, corner rules for pill, containers, steps and undo: verified (_settings.mjs radiusAt, theme-hunt8.test.mjs, theme-hunt9.test.mjs).
  • Description, focus on a wrapper part suggests focus-within: verified (_check.mjs:932-946).
  • Description, every editor refusal leads with a command that runs: false (see the findings).
  • Description, show without React, a rebuilt cut-short stylesheet and the stale sheet banner: verified (_edit.mjs:1209-1239, _context.mjs:379-399).
  • Description, every part property checked by grammar, one-value time and easing tokens, no negative widths or durations: verified (_css.mjs, _check.mjs).
  • Description, secondary text, both modes listed, kept shades and a refreshed palette: verified (_settings.mjs commitTheme).
  • Description, code fonts fall back to monospace and Roboto gets its package: verified (theme-palette.test.mjs).
  • Description, the listed how readings: covered by theme-how.test.mjs; not run here.
  • Description, question-accuracy figures and the 121-cell count: not verifiable from the repository.
  • Description, typed math, no CSS-wide keywords on tokens, no percentage where readers take none, one-system fonts named, hand-edited module built over: verified (_css.mjs, _check.mjs:334-376, _context.mjs:227-238, _settings.mjs moduleUnedited).
  • Description, preview refusals for route handlers, build folders, tall parts and pageExtensions: verified (_preview-app.mjs:139-436, _preview.mjs:694).
  • Description, the ninth-pass Core fixes and fenced build output: verified (add-helpers.mjs:383, _settings.mjs, theme-hunt9.test.mjs).
  • Description, changesets corrected where they contradicted the code: false (hunt4, colors9, hunt9).
  • Description, sheetStale typed and documented and the dead React check removed: verified (theme-edit.type.mjs:54, :99).
  • Description, the full test gate passes: partly (every CI test step passed, but one test shard hit its 15-minute job limit, so the required check is red).
  • Changesets catalog-from-docs, colors6, colors7, colors8, colors10, how-catalog-parts, hunt5, hunt5b, hunt7, hunt8, preview-older-core, preview-ten, preview-eleven, preview-twelve, theme-editor-css-tree: verified.
  • Changesets how-answers-7, how-answers-8, how-answers-9: covered by tests not run here.
  • Changeset colors9: false in one bullet (one-system font warning).
  • Changeset hunt4: false in one bullet (show without React).
  • Changeset hunt6: false in two bullets (no rollback after a lost lock; every refusal).
  • Changeset hunt9: false in one bullet (file-name theme).
  • Files with findings: _how.mjs, _how-question.mjs, _edit.mjs, _settings.mjs, _context.mjs, _adapter.mjs, _palette.mjs, response-types.doc.mjs, and the hunt4, hunt6, hunt9 and colors9 changesets.
  • Files clear: the other eighteen changesets; add-helpers.mjs, cache.mjs, folder-lock.mjs, lock-lease.mjs, generator.mjs, commands/theme.mjs, experimental/theme.mjs, the three command docs, error-codes.doc.mjs, doc-loads.test.mjs, test-utils/theme-edit-fixture.mjs; _catalog.mjs, _check.mjs, _color.mjs, _compiler.mjs, _css.mjs, _css-properties.mjs (deleted), _how-commands.mjs, _overview.mjs, _parts.mjs, _preview-app.mjs, _preview.mjs, _project.mjs, _tables.mjs, theme-edit.type.mjs, the five theme-edit API docs; and the tests theme-catalog, theme-edit, theme-editor-refusals.behavior, theme-how, theme-hunt, theme-hunt3 to theme-hunt9, theme-hunt7-lock, theme-palette, theme-picture, theme-preview-safety, theme-preview, theme-stress.

Automated review (maintainer link)

Comment thread packages/cli/api/theme-edit/_how.mjs Resolved
Comment thread packages/cli/api/theme-edit/_edit.mjs Resolved
Comment thread .changeset/experimental-theme-hunt4.md Resolved
Comment thread .changeset/experimental-theme-hunt9.md Resolved
…a new that lost its lock rolls nothing back, refusals with commands that run, changesets match the behavior, sheetStale listed)
@josephfarina

Copy link
Copy Markdown
Contributor Author

/approve-spec 2d2ecc0

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Changes requested. An engineering owner's /approve-spec on this head settles the direction of the experimental editor behavior and its three new result fields. These are fixed: the --like undefined answers, the rollback in a new that lost its lock, the hunt4, colors9 and hunt9 changeset bullets, and the response docs for sheetStale. These still need fixing:

  • The description still says "every editor refusal leads with a command that runs", but three refusals suggest no command: a lock folder that cannot be written, a lock that cannot be taken, and a write error under the lock (_adapter.mjs:166-170, :252-256, :261-265); use the narrowed wording from experimental-theme-hunt6.md line 10 there, or add a command to each.
  • Miss: the description's shared-files list says api/theme/build/font-warning.mjs "becomes the one system-font list, split into fonts on every desktop and fonts only some systems ship" and is pinned by a test, but this pull request does not change that file (its own final-round section says so), so drop or correct that bullet.
  • Miss: experimental-theme-colors6.md line 7 says a font only some systems ship gets its install line "or a warning that the app must load it", but Gill Sans and Calibri have no package and now get "ships only on macOS" or "ships only on Windows" (_context.mjs:240-247, experimental-theme-colors10.md line 12), so drop that clause as colors9 now does.

The first two need only a description edit. Two smaller points: show's new marker removal and new's removals after its build skip the lock check (_edit.mjs:1246, :363, :378), and the description's response-docs line leaves out themeMissing.

Claims and files checked
  • Description, the build's font list becomes a split one-system list pinned by a test: false (the file is unchanged).
  • Description, the build's font list is byte-identical to main: verified (not in the diff).
  • Description, the palette generator exports its key lists and mode helper for the editor: verified (generator.mjs:110-137, _palette.mjs:30-35).
  • Description, the leased lock option is opt-in and the release cache is unchanged: verified (cache.mjs:704-709, folder-lock.mjs:137).
  • Description, the silence check takes an optional window with the old default: verified (lock-lease.mjs:63-80).
  • Description, the experimental compile no longer holds the theme lock under --watch: verified (theme.mjs:1073).
  • Description, the response docs gain one optional field, sheetStale: partly (they also document themeMissing, response-types.doc.mjs:156).
  • Description, demos come from doc playgrounds or a doc's props: verified (_parts.mjs, theme-catalog.test.mjs).
  • Description, CSS names and grammar come from css-tree with legacy filter branches left out: verified (_css.mjs).
  • Description, system fonts come from the build's list and open fonts by package name with two exceptions: verified (_tables.mjs:143-151, :254-257).
  • Description, neutral stops and palette keys are read from their sources: verified (_palette.mjs:288-331).
  • Description, part words come from Core's catalog with the shared edit distance: verified (_how-question.mjs).
  • Description, shared test helpers live in test-utils: verified (test-utils/theme-edit-fixture.mjs).
  • Description, a live holder on this computer keeps its lock and another process space loses it only after silence: verified (folder-lock.mjs:59-66, :137-150).
  • Description, every write under the lock is fenced and a command that lost its lock does not roll back: verified for writes and for new (_edit.mjs:302, :311, :331); three removals skip the check (non-blocking).
  • Description, new checks first, keeps user files and history, and recovers wrong-shape settings: verified (_edit.mjs:189-340).
  • Description, the corner rules for pill, containers, steps and undo: carried from the earlier pass (_settings.mjs, theme-hunt8.test.mjs).
  • Description, focus on a wrapper part suggests focus-within: verified (_check.mjs:932-946, theme-hunt7.test.mjs M3).
  • Description, every editor refusal leads with a command that runs: false (_adapter.mjs:166-170, :252-256, :261-265).
  • Description, show without React, a rebuilt cut-short stylesheet and the stale-sheet banner: verified (_edit.mjs:1214-1221, :1302-1306; _settings.mjs:2463-2468; _context.mjs:389-403).
  • Description, part properties checked by grammar, one-value time and easing tokens, no negative widths or durations: carried from the earlier pass (_css.mjs, _check.mjs).
  • Description, secondary text, both modes listed, kept shades and a refreshed palette: carried from the earlier pass (_settings.mjs).
  • Description, code fonts fall back to monospace and Roboto gets its package: carried from the earlier pass (theme-palette.test.mjs).
  • Description, the listed how readings: covered by theme-how.test.mjs; not run here.
  • Description, question-accuracy figures and the 121-cell count: not verifiable from the repository.
  • Description, typed math, no CSS-wide keywords on tokens, no percentage where readers take none, one-system fonts named, hand-edited module built over: verified (theme-palette.test.mjs:1872-1899, _context.mjs:240-247).
  • Description, the preview refusals: carried from the earlier pass (_preview-app.mjs).
  • Description, undo history kept, file-name theme names refused with a suggestion, build output fenced: verified (_settings.mjs:261-283, add-helpers.mjs:383).
  • Description, changesets corrected where they contradicted the code: false (colors6 line 7).
  • Description, sheetStale typed and documented: verified (theme-edit.type.mjs:54, :99; response-types.doc.mjs:120, :126, :138).
  • Description, the full test gate passes: not verified here; CI for this head is its evidence.
  • Changesets catalog-from-docs, colors7, colors8, colors9, colors10, how-catalog-parts, hunt4, hunt5, hunt5b, hunt6, hunt7, hunt8, hunt9, preview-older-core, preview-ten, preview-eleven, preview-twelve, theme-editor-css-tree: verified.
  • Changesets how-answers-7, how-answers-8, how-answers-9: covered by tests not run here.
  • Changeset colors6: false in one bullet (line 7).
  • Files with findings: experimental-theme-colors6.md, _adapter.mjs; non-blocking notes on _edit.mjs, _preview-app.mjs, _check.mjs.
  • Files changed since the last review and checked in full: the hunt4, hunt6, hunt9 and colors9 changesets, _adapter.mjs, _context.mjs, _edit.mjs, _how-question.mjs, _how.mjs, _palette.mjs, _settings.mjs, theme-hunt9.test.mjs, response-types.doc.mjs.
  • Files clear: every other changed file. The shared modules (add-helpers.mjs, cache.mjs, folder-lock.mjs, lock-lease.mjs, generator.mjs, commands/theme.mjs, experimental/theme.mjs), the command and response docs, and test-utils/theme-edit-fixture.mjs were reread at this head. The other editor modules and tests are unchanged since the last review and keep its result.

Automated review (maintainer link)

Comment thread packages/cli/api/theme-edit/_edit.mjs Resolved
…y where it applies, --only spacing steps from the group, refusals keep scope, tabs, compile finds editor themes from the file, Windows device names)
… the dial, a font shorthand on a part is split, no MDN notes as values, part colors suggest a color
…s out of the project, basePath, Core stylesheet note, palette stand-in for a kept Colors page)
@josephfarina

Copy link
Copy Markdown
Contributor Author

/approve-spec ab55741

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Changes requested. An engineering owner's /approve-spec on this head settles the direction of the experimental editor behavior and its three new result fields, and the earlier review's points are fixed. These still need fixing:

  • experimental-theme-hunt9.md line 10 and the description promise that a --state a part never takes is refused, but the hunt10 pass removed that refusal for disabled (_check.mjs:988-995), so set <theme> background red --on card --state disabled is saved with no effect, as on main: restore the refusal where neither the part's states nor its own styles mention :disabled or aria-disabled (with a card test), or reword hunt9 line 10, the description's "--state values Core never styles are refused" and "changesets corrected where they contradicted the code", and the comment at _check.mjs:968-970.
  • The Windows device-name test is copied twice into _settings.mjs (windowsReserved at line 119 and the slugFor replace at line 137) beside the one reservedFolder already has (_preview-app.mjs:262), and the copies already differ on extensions and letter case, so export one helper and call it from all three.
  • The description calls themeMissing an optional field, but preview list always returns it and the type marks it required (theme-edit.type.mjs:230), so describe it as a boolean that is always present.

Two smaller points: the unreleased experimental-theme-how-answers-5.md on the base still says a link has no color of its own per state, which how now does; and a theme-how.test.mjs comment says "no command" above an assertion that expects one.

Claims and files checked
  • Description, a lock is never taken from a holder alive on this computer, and one in another process space only after its lease goes silent: verified (folder-lock.mjs:137-150, cache.mjs:690-722).
  • Description, every write under the lock is fenced, and a command that lost its lock does not roll back: verified, removals included (_edit.mjs:331, :363, :379; _settings.mjs:2150).
  • Description, new checks first, keeps user files and history, and recovers wrong-shape settings: verified (_edit.mjs:189-385, theme-hunt7.test.mjs).
  • Description, the corner rules for pill, containers, steps and undo radii: verified (_settings.mjs radiusAt, commitTheme; theme-hunt8.test.mjs).
  • Description, focus on a wrapper part suggests focus-within, and the listed refusal kinds lead with a command that runs: verified (_check.mjs:950-966, theme-editor-refusals.behavior.test.mjs).
  • Description, show without React, a rebuilt cut-short stylesheet and the stale-sheet banner: verified (_edit.mjs:1219-1310, _settings.mjs staleBuild, _context.mjs markSheetStale).
  • Description, grammar checks for part properties, one-value time and easing tokens, no negative widths or durations: verified (_css.mjs, _check.mjs).
  • Description, secondary text, both modes listed, kept shades, palette brought up to date first: verified (_settings.mjs compileTheme, buildTheme).
  • Description, code fonts fall back to monospace and Roboto gets its package: verified (_settings.mjs font fallbacks, _tables.mjs open fonts).
  • Description, the listed how readings and the preview refusals: present in code and covered by tests; tests not run here.
  • Description, question-accuracy figures, the 121-cell count and the full test gate: not verifiable from the repository; CI for this head is the evidence.
  • Description, typed math, no CSS-wide keywords on tokens, no percentage where readers take none, one-system fonts named, hand-edited module built over: verified (_css.mjs, _check.mjs, _tables.mjs, _settings.mjs moduleUnedited).
  • Description, the build's font list is unchanged and identical to main: verified (not in the diff, unchanged on main).
  • Description, --state values Core never styles are refused with the working form: false for disabled on a styled part that is never disabled (_check.mjs:988-995).
  • Description, build output fenced through one stillHolds() in applyWrites, a no-op change mends a stale build: verified (add-helpers.mjs:383, _settings.mjs commitTheme).
  • Description, changesets corrected where they contradicted the code: false (experimental-theme-hunt9.md line 10).
  • Description, sheetStale typed and documented, dead refuseWithoutReact removed: verified.
  • Description, the shared-file notes (generator exports, opt-in leased, optional silence window, editor-only folder lock, compile --watch): verified.
  • Description, two optional JSON fields: partly false (themeMissing is always present).
  • Description, one source instead of hand lists (css-tree without -ms- branches, the build's font list, @fontsource names with two exceptions, neutral stops, generator keys, demos from docs, Core's target names, shared test helpers): verified.
  • Changesets catalog-from-docs, colors6, colors7, colors8, colors9, colors10, colors11, hunt4, hunt5, hunt5b, hunt6, hunt7, hunt8, hunt10, how-catalog-parts, preview-ten, preview-eleven, preview-twelve, preview-thirteen, preview-older-core, theme-editor-css-tree: verified against the code.
  • Changesets how-answers-7 to how-answers-10: covered by the how question tests; tests not run here.
  • Changeset hunt9: false in line 10 (no refusal of disabled ships, and a disabled button is now styled).
  • Files with findings: _check.mjs, _settings.mjs, .changeset/experimental-theme-hunt9.md; non-blocking: theme-how.test.mjs.
  • Files clear: every other changed file, including the shared modules (add-helpers.mjs, cache.mjs, folder-lock.mjs, lock-lease.mjs, generator.mjs, commands/theme.mjs, experimental/theme.mjs), the command and response docs, and test-utils/theme-edit-fixture.mjs.

Automated review (maintainer link)

// focusable element is inside it, or beside it).
const focusable = (/** @type {string} */ t) =>
/:focus(?!-within)/.test(JSON.stringify(own(t)));
const dead = parts.filter(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This refusal now covers focus and active but no longer covers disabled. The hunt9 pass had that clause, and hunt10 removed it. So set <theme> background red --on card --state disabled is saved again, as on main, and .astryx-card:is(:disabled,[aria-disabled="true"]) never matches. Meanwhile experimental-theme-hunt9.md line 10 and the description say such a state is refused. Keep refusing disabled where neither states(t) nor the part's own styles mention :disabled or aria-disabled, and add a card case. Or reword those claims. The comment at lines 968-970 still describes the removed rule.

- The undo history is never deleted by a recovering `new`, and a stopped `new`'s leftover marker is removed by the next change.
- A theme typed as a file name (`t.theme.ts`) is refused with a suggestion of the theme it names. An undo that is refused suggests undos.
- Undoing a radius token rebuilds the corners without it. Inner corners never get smaller going up the scale. A step never moves a part the other way, starts from an exact border-radius, and for a variant starts from the part's plain setting.
- A `--state` that a part never takes (a disabled button, which Core marks with an attribute) is refused and says what to do.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No refusal of disabled ships at this head. The disabled button this line cites is now styled through aria-disabled (experimental-theme-hunt10.md line 7, theme-hunt10.test.mjs), and a part that is never disabled, such as a card, is accepted (_check.mjs:988-995). Reword this line, or restore the refusal so it holds.

* so cannot be checked out there.
* @param {string} s
*/
export const windowsReserved = s =>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

reservedFolder already has this check (_preview-app.mjs:262, which also catches con.txt and capitals), and line 137 repeats it again in slugFor. Export one helper and call it from reservedFolder, slugOk and slugFor, so a fix reaches all three.

…ile guards only editor files and follows links, a merge snippet that runs, theme lists, end of the corners scale, property suggestions that work, disabled and focus-within only where they apply)
…at the top-level comma, --mode refused on light-dark(), every color() space read
…group root layouts, basePath without comments)
@josephfarina

Copy link
Copy Markdown
Contributor Author

/approve-spec 0d32982

This branch was successfully deployed

1 active deployment
Preview — 0d32982a Deployed Nov 12, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot. needs:design-review Affects visuals — Design should review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant