Skip to content

fix(cli): make theming discoverable and its silent failure detectable - #5226

Open
josephfarina wants to merge 1 commit into
mainfrom
docs/agent-theme-guide-link
Open

josephfarina wants to merge 1 commit into
mainfrom
docs/agent-theme-guide-link

Conversation

@josephfarina

@josephfarina josephfarina commented Aug 19, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Two small changes addressing the same root cause: theming is hard to discover, and when it goes wrong it fails silently.

1. Point agents at the theme guide, the way we already do for layout

astryx docs theme is a thorough 15-section guide — quick start, defineTheme, component overrides, custom variants, runtime vs built themes, astryx theme build. Nothing in the injected AGENTS.md told an agent to open it.

Compare how the block treats the two subjects today. Layout gets a hard rule: "Frame first: read astryx docs layout before writing any page or screen." Theming gets one clause — "Brand/accent via astryx theme" — and astryx theme build is never mentioned at all. theme appears in the docs <topic> list, but reachable-on-demand is not the same as prompted, and an agent mid-task will not go looking.

Adds a matching "Theme first" rule and lists theme add|build in the CLI reference alongside swizzle. The rule triggers on restyling the same component twice, because a single override is legitimate — repetition is the actual signal that something belongs in the theme.

2. astryx theme add now surfaces the required build step

Following the current next-steps output exactly leaves you on the runtime-injection path with no idea a build step exists — astryx theme build is never mentioned.

That matters because the failure mode is invisible: edit a theme, skip the rebuild, and the stale artifact still reports __built, so the runtime skips style injection and the app renders the previous theme with no warning. The one warning that does exist fires on the correct path (runtime injection), so today the developer whose theme works gets nagged and the one whose theme is silently dead gets nothing.

Recommends the predev/prebuild script wiring rather than a bare command, since remembering to rebuild by hand is precisely the step that gets skipped.

Deliberately not changed

The SELF-CHECK line still prescribes xstyle as the remediation, which is arguably the deeper cause of agents avoiding the theme. That line was earned through the prompt-purity vibe tests — it cuts surviving raw CSS in complex runs by roughly 4x — so it should be changed as an A/B through the same harness, not rewritten blind. Tracking separately.

No CI theme build --check gate: no built theme artifacts are committed in this repo (only the .ts sources are tracked, and dev/build both run generate → build:theme first), so there is nothing here to drift. That risk lives in consumer projects, which is what the guidance above addresses.

Test plan

  • astryx init --features agents renders the new rule correctly in a scratch project
  • astryx theme add neutral renders the build guidance with the correct resolved path
  • Agent-docs unit tests pass
  • check:changesets and the full pre-commit gate pass
  • CI golden check (goldens are not tracked in this repo, so verifying on CI)

@vercel

vercel Bot commented Aug 19, 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 Sep 24, 2026 1:39am UTC

Request Review

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Aug 19, 2026
@josephfarina josephfarina changed the title docs(agents): point agents at the theme guide the way we do for layout fix(cli): make theming discoverable and its silent failure visible Aug 19, 2026
@github-actions

github-actions Bot commented Aug 19, 2026 •

Copy link
Copy Markdown
Contributor

PR Analysis Report

Preview availability: CI did not succeed, so no preview was published.

No new or modified components detected.

Bundle Size Summary

No component packages changed.

Accessibility Audit

Status: 36 accessibility violation(s) found — 2 critical, 34 serious.

Button pattern - 1 issue(s)
  • 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
    • Rule: color-contrast · Affects 1/18 stories · Learn more
    • WCAG: 1.4.3 (Level AA)
ChatComposer - 1 issue(s)
  • 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
    • Rule: color-contrast · Affects 1/19 stories · Learn more
    • WCAG: 1.4.3 (Level AA)
ChatComposerInput - 1 issue(s)
  • 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
    • Rule: color-contrast · Affects 1/16 stories · Learn more
    • WCAG: 1.4.3 (Level AA)
ChatReasoning - 1 issue(s)
  • 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
    • Rule: color-contrast · Affects 4/5 stories · Learn more
    • WCAG: 1.4.3 (Level AA)
ChatToolCalls - 1 issue(s)
  • 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
    • Rule: color-contrast · Affects 10/11 stories · Learn more
    • WCAG: 1.4.3 (Level AA)
Checkbox pattern - 1 issue(s)
  • 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
    • Rule: color-contrast · Affects 2/33 stories · Learn more
    • WCAG: 1.4.3 (Level AA)
CheckboxList - 1 issue(s)
  • 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
    • Rule: color-contrast · Affects 4/17 stories · Learn more
    • WCAG: 1.4.3 (Level AA)
ClickableCard - 1 issue(s)
  • 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
    • Rule: color-contrast · Affects 1/5 stories · Learn more
    • WCAG: 1.4.3 (Level AA)
CodeEditor - 1 issue(s)
  • 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
    • Rule: color-contrast · Affects 3/7 stories · Learn more
    • WCAG: 1.4.3 (Level AA)
CodeEditorPerf - 1 issue(s)
  • 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
    • Rule: color-contrast · Affects 2/2 stories · Learn more
    • WCAG: 1.4.3 (Level AA)
CodeEditorTheme - 1 issue(s)
  • 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
    • Rule: color-contrast · Affects 13/14 stories · Learn more
    • WCAG: 1.4.3 (Level AA)
CodeTheme - 1 issue(s)
  • 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
    • Rule: color-contrast · Affects 14/16 stories · Learn more
    • WCAG: 1.4.3 (Level AA)
DateRangeInput - 1 issue(s)
  • 🔴 critical: Ensure an element's role supports its ARIA attributes
    • Rule: aria-allowed-attr · Affects 1/21 stories · Learn more
    • WCAG: 4.1.2 (Level A)
FileInput - 1 issue(s)
  • 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
    • Rule: color-contrast · Affects 3/15 stories · Learn more
    • WCAG: 1.4.3 (Level AA)
GridMasonry - 1 issue(s)
  • 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
    • Rule: color-contrast · Affects 1/3 stories · Learn more
    • WCAG: 1.4.3 (Level AA)
Heading - 1 issue(s)
  • 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
    • Rule: color-contrast · Affects 1/16 stories · Learn more
    • WCAG: 1.4.3 (Level AA)
Icon - 1 issue(s)
  • 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
    • Rule: color-contrast · Affects 1/18 stories · Learn more
    • WCAG: 1.4.3 (Level AA)
LogStream - 1 issue(s)
  • 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
    • Rule: color-contrast · Affects 2/3 stories · Learn more
    • WCAG: 1.4.3 (Level AA)
MediaTheme Auto - 1 issue(s)
  • 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
    • Rule: color-contrast · Affects 1/5 stories · Learn more
    • WCAG: 1.4.3 (Level AA)
PowerSearch - 1 issue(s)
  • 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
    • Rule: color-contrast · Affects 2/26 stories · Learn more
    • WCAG: 1.4.3 (Level AA)
ProgressBar - 1 issue(s)
  • 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
    • Rule: color-contrast · Affects 2/18 stories · Learn more
    • WCAG: 1.4.3 (Level AA)
Radio group pattern - 1 issue(s)
  • 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
    • Rule: color-contrast · Affects 3/26 stories · Learn more
    • WCAG: 1.4.3 (Level AA)
RadioList - 1 issue(s)
  • 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
    • Rule: color-contrast · Affects 3/14 stories · Learn more
    • WCAG: 1.4.3 (Level AA)
RichTextEditor - 2 issue(s)
  • 🟠 serious: Ensure every ARIA input field has an accessible name
    • Rule: aria-input-field-name · Affects 2/18 stories · Learn more
    • WCAG: 4.1.2 (Level A)
  • 🔴 critical: Ensure every form element has a label
    • Rule: label · Affects 1/18 stories · Learn more
    • WCAG: 4.1.2 (Level A)
SelectableCard - 1 issue(s)
  • 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
    • Rule: color-contrast · Affects 1/4 stories · Learn more
    • WCAG: 1.4.3 (Level AA)
Stepper - 1 issue(s)
  • 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
    • Rule: color-contrast · Affects 1/30 stories · Learn more
    • WCAG: 1.4.3 (Level AA)
TableGroupedRows - 1 issue(s)
  • 🟠 serious: Ensure ARIA attributes are used as described in the specification of the element's role
    • Rule: aria-conditional-attr · Affects 3/3 stories · Learn more
    • WCAG: 4.1.2 (Level A)
TableTree - 1 issue(s)
  • 🟠 serious: Ensure ARIA attributes are used as described in the specification of the element's role
    • Rule: aria-conditional-attr · Affects 8/9 stories · Learn more
    • WCAG: 4.1.2 (Level A)
Text - 1 issue(s)
  • 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
    • Rule: color-contrast · Affects 1/23 stories · Learn more
    • WCAG: 1.4.3 (Level AA)
Theme - 1 issue(s)
  • 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
    • Rule: color-contrast · Affects 1/5 stories · Learn more
    • WCAG: 1.4.3 (Level AA)
PopArt - 1 issue(s)
  • 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
    • Rule: color-contrast · Affects 3/6 stories · Learn more
    • WCAG: 1.4.3 (Level AA)
Thumbnail - 1 issue(s)
  • 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
    • Rule: color-contrast · Affects 1/12 stories · Learn more
    • WCAG: 1.4.3 (Level AA)
Timestamp - 1 issue(s)
  • 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
    • Rule: color-contrast · Affects 1/20 stories · Learn more
    • WCAG: 1.4.3 (Level AA)
Token - 1 issue(s)
  • 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
    • Rule: color-contrast · Affects 2/10 stories · Learn more
    • WCAG: 1.4.3 (Level AA)
Tokenizer - 1 issue(s)
  • 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
    • Rule: color-contrast · Affects 2/24 stories · Learn more
    • WCAG: 1.4.3 (Level AA)

Visual Regression

19 of 422 shot(s) changed. View the report

A change here is a question, not a failure: check whether the after is the
picture you intended. Record that review on the PR. Baseline maintenance is an
explicit dispatch of CI; this report never rewrites the baseline or adds a release gate.

component story theme mode pixels
Slider Default probe light 15,617
Slider Default probe dark 15,615
Chat With Attachments probe light 12,964
Chat With Attachments probe dark 12,114
InputGroup With Typeahead probe light 11,012
InputGroup With Typeahead probe dark 9,189
ComplexSelector Tree list with search probe light 6,245
TextArea Vertical Form Alignment probe light 5,634
ComplexSelector Tree list with search probe dark 5,602
TextArea Vertical Form Alignment probe dark 5,047
ComplexSelector Fruit ripeness selector neutral light 4,007
ComplexSelector Fruit ripeness selector neutral dark 3,825
ToggleButton Group Single probe dark 1,538
ToggleButton Group Single probe light 1,538
MediaTheme On Dark neutral dark 838
Banner Collapsible Content (Expanded) probe dark 92
Banner Collapsible Content (Expanded) probe light 90
AlertDialog Delete neutral light 71
TableRowStatus Default neutral light 23
Slider — Default — probe light
BeforeAfterDiff
Before visual regression frame After visual regression frame Pixel difference frame
Slider — Default — probe dark
BeforeAfterDiff
Before visual regression frame After visual regression frame Pixel difference frame
Chat — With Attachments — probe light
BeforeAfterDiff
Before visual regression frame After visual regression frame Pixel difference frame

Generated by PR Enrichment workflow | View full report

@josephfarina josephfarina changed the title fix(cli): make theming discoverable and its silent failure visible fix(cli): make theming discoverable and its silent failure detectable Aug 19, 2026
@josephfarina
josephfarina marked this pull request as draft August 19, 2026 19:29
github-actions Bot added a commit that referenced this pull request Aug 19, 2026
github-actions Bot added a commit that referenced this pull request Aug 19, 2026
github-actions Bot added a commit that referenced this pull request Aug 24, 2026
@github-actions github-actions Bot added the needs:design-review Affects visuals — Design should review label Aug 27, 2026
github-actions Bot added a commit that referenced this pull request Aug 27, 2026
@josephfarina
josephfarina marked this pull request as ready for review September 1, 2026 21:23

@cixzhang cixzhang 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.

Thanks — making silent theme failures visible is the right problem, but this head can still give builders confident wrong answers.

  • theme add shows a source-theme import, then says edits require a build. Source imports are runtime-injected, so the generated JS/CSS are unused. Please present source/runtime and built/SSR as two complete paths.
  • doctor treats entry-file metadata as proof of freshness, yet later imports that same project module. Changing an imported token left doctor green while theme build --check exited 1; changing the entry made default doctor run a file-writing side effect despite its read-only contract. Keep the default pass non-evaluating and fail closed unless complete build provenance proves freshness.
  • Package-wide scans reuse root/first-package state. A wired app hid an unwired sibling's stale theme, while an app-local StyleX compiler still failed. Evaluate and aggregate each finding in its owning package.

[Reviewed by Robohands]

@cixzhang cixzhang 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.

Thanks — this head improves StyleX counting and comment handling, but my prior false-green cases still reproduce at current bytes.

  • Input discovery blanks executable template interpolations. ${require('./tokens')} produced complete: true; changing tokens left the digest unchanged, so doctor can report a stale theme as current.
  • The built-theme scan still returns “no built theme output found” before checking truncation. A 4,001-directory project with no artifact in the visited prefix returned INFO although the remainder was unexamined.
  • StyleX wiring treats any call of an imported plugin as configured. With const unused = stylex(); export default {plugins: []}, doctor returned INFO and generated docs recommended xstyle, although the app compiles no StyleX.

Please fail closed unless each dependency, output root, and active compiler path is actually verified.

[Reviewed by Robohands]

@cixzhang cixzhang 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.

Thanks — the old-head cases are fixed, but current bytes still turn unproved parsing into healthy answers.

  • A regex backtick before a later template hides a real token import; the graph reports complete and its digest does not change with that token.
  • An unused object's plugins: [stylex()] makes doctor report INFO and agent docs recommend xstyle, even when the exported config has plugins: [].
  • false && pnpm build:theme makes stale output look self-healing.

All three come from treating text as proof of active JavaScript or shell behavior. Please use syntax-aware discovery, or fail closed whenever active inputs, compiler wiring, or rebuild execution cannot be proved.

[Reviewed by Robohands]

@cixzhang cixzhang 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.

Thanks — the AST rewrite fixes the old-head cases, but current bytes still treat unproved execution as healthy.

  • isThemeBuildWired follows sibling scripts without preserving the caller’s shell gate. false && pnpm build:theme still returns wired, so stale output is called self-healing although no rebuild runs.
  • Theme input discovery misses valid import x = require('./tokens'); it reports the graph complete and the digest stays unchanged when that token file changes.
  • StyleX discovery accepts a serve-only return branch or an unrelated nested plugins key. Doctor reports INFO and generated docs recommend xstyle although production compiles none.

Please return healthy only when the active input, build, and compiler paths are proven; otherwise report them as unverifiable.

[Reviewed by Robohands]

@cixzhang cixzhang 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.

Thanks — the previous shell-gate and import-equals cases are fixed, but the current head still gives builders healthy answers for broken paths.

  • A theme can load a local token with a createRequire() alias; editing that token left the recorded digest unchanged, so doctor passed while theme build --check failed.
  • A function config with StyleX only in its serve return is treated as production wiring. doctor reports INFO and generated agent docs recommend xstyle, though the production branch has no compiler.
  • echo astryx theme build src/theme.ts counts as executable build wiring, so stale output is downgraded to self-healing INFO even though nothing rebuilds it.

Please return healthy only when each active input, production compiler path, and rebuild path is proved; otherwise report the result as unverifiable.

[Reviewed by Robohands]

@astracat-bot

astracat-bot Bot commented Sep 9, 2026 •

Copy link
Copy Markdown

AI review status for this pull request.

Review status Updated
🟡 Reviewed (for maintainers only) Sep 24, 2026, 1:37 AM UTC

Rebased onto current main (v0.6.x, +10 commits including the package-manager
check's own rework). Applied as the branch's net change rather than a nine-
commit replay, which conflicts at nearly every step against a main that has
moved this far; main's improved checkPackageManager doc and behaviour are kept
alongside the theme work rather than either side winning.

Twelve review rounds are folded in. The load-bearing ones:

- freshness proved from a recorded input digest, never from mtimes
- every input accounted for, by PARSING rather than pattern-matching: released
  packages fingerprinted name@version, linked and workspace source
  content-hashed, anything unresolvable reported unverifiable
- build wiring matched per exact source path, and a rebuild behind a shell gate
  is not wiring — the gate travels with a sibling script reference
- bounded scans report truncation instead of returning a clean pass
- StyleX wiring answers only for the EXPORTED config, through one shared
  detector so the diagnosis and the generated agent docs cannot disagree
- doctor looks wherever `theme build --out` can write, including dist/ and .next/

Verified end to end against a built app, not only in fixtures: edit an imported
token and it fails; touch the artifact so it looks newest and it still fails;
rebuild and it passes; wire the build into predev and it drops to info; gate
that predev behind `false &&` and it fails again.
@josephfarina
josephfarina force-pushed the docs/agent-theme-guide-link branch from f386a9e to a6b633c Compare September 24, 2026 01:31

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks — the direction is right and well-scoped: separating source from built themes, recording what a built theme was compiled from, and making doctor say "unverifiable" instead of guessing.

A few paths still return a healthy answer without proof, so this needs changes before approval:

  • createRequire alias missed — a theme that loads a token through a createRequire() alias is not followed, so editing that token leaves the recorded digest unchanged and doctor passes a stale theme.
  • Serve-only StyleX counted as production — a function build config that wires StyleX only in its serve return is treated as production wiring, so doctor and the generated agent docs say StyleX is active though the production build compiles none.
  • echo counted as rebuild wiring — echo astryx theme build src/theme.ts in a script counts as rebuild wiring, so stale output is downgraded to self-healing even though nothing rebuilds it.
  • Monorepo build base guessed — building from a workspace root records a root-relative source path that doctor later resolves from the artifact's package, so a stale theme becomes "source missing" instead of failing.
  • Default diagnostic executes docs despite claiming not to — the CSS check claims to evaluate no modules, but it dynamically imports component docs to read prop values. Please parse that metadata without importing, or isolate and document that boundary.

For each, please return healthy only when the active input, compiler, or rebuild path is proved — otherwise report it as unverifiable — and add a regression fixture for the case.

[Automated review]

This branch was successfully deployed

1 active deployment
Preview — a6b633c7 Deployed Sep 24, 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.

2 participants