Repository navigation
fix(cli): make theming discoverable and its silent failure detectable - #5226
josephfarina wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR Analysis Report
No new or modified components detected. Bundle Size SummaryNo component packages changed. Accessibility AuditStatus: 36 accessibility violation(s) found — 2 critical, 34 serious. Button pattern - 1 issue(s)
ChatComposer - 1 issue(s)
ChatComposerInput - 1 issue(s)
ChatReasoning - 1 issue(s)
ChatToolCalls - 1 issue(s)
Checkbox pattern - 1 issue(s)
CheckboxList - 1 issue(s)
ClickableCard - 1 issue(s)
CodeEditor - 1 issue(s)
CodeEditorPerf - 1 issue(s)
CodeEditorTheme - 1 issue(s)
CodeTheme - 1 issue(s)
DateRangeInput - 1 issue(s)
FileInput - 1 issue(s)
GridMasonry - 1 issue(s)
Heading - 1 issue(s)
Icon - 1 issue(s)
LogStream - 1 issue(s)
MediaTheme Auto - 1 issue(s)
PowerSearch - 1 issue(s)
ProgressBar - 1 issue(s)
Radio group pattern - 1 issue(s)
RadioList - 1 issue(s)
RichTextEditor - 2 issue(s)
SelectableCard - 1 issue(s)
Stepper - 1 issue(s)
TableGroupedRows - 1 issue(s)
TableTree - 1 issue(s)
Text - 1 issue(s)
Theme - 1 issue(s)
PopArt - 1 issue(s)
Thumbnail - 1 issue(s)
Timestamp - 1 issue(s)
Token - 1 issue(s)
Tokenizer - 1 issue(s)
Visual Regression19 of 422 shot(s) changed. View the report A change here is a question, not a failure: check whether the after is the
Generated by PR Enrichment workflow | View full report |
cixzhang
left a comment
There was a problem hiding this comment.
Thanks — making silent theme failures visible is the right problem, but this head can still give builders confident wrong answers.
theme addshows 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.doctortreats entry-file metadata as proof of freshness, yet later imports that same project module. Changing an imported token leftdoctorgreen whiletheme build --checkexited 1; changing the entry made defaultdoctorrun 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
left a comment
There was a problem hiding this comment.
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')}producedcomplete: true; changingtokensleft the digest unchanged, sodoctorcan 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: []},doctorreturned INFO and generated docs recommendedxstyle, 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
left a comment
There was a problem hiding this comment.
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()]makesdoctorreport INFO and agent docs recommendxstyle, even when the exported config hasplugins: []. false && pnpm build:thememakes 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
left a comment
There was a problem hiding this comment.
Thanks — the AST rewrite fixes the old-head cases, but current bytes still treat unproved execution as healthy.
isThemeBuildWiredfollows sibling scripts without preserving the caller’s shell gate.false && pnpm build:themestill 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
pluginskey. Doctor reports INFO and generated docs recommendxstylealthough 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]
80af308 to
f386a9e
Compare
cixzhang
left a comment
There was a problem hiding this comment.
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, sodoctorpassed whiletheme build --checkfailed. - A function config with StyleX only in its
servereturn is treated as production wiring.doctorreports INFO and generated agent docs recommendxstyle, though the production branch has no compiler. echo astryx theme build src/theme.tscounts 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]
|
AI review status for this pull request.
|
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.
f386a9e to
a6b633c
Compare
There was a problem hiding this comment.
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:
createRequirealias missed — a theme that loads a token through acreateRequire()alias is not followed, so editing that token leaves the recorded digest unchanged anddoctorpasses a stale theme.- Serve-only StyleX counted as production — a function build config that wires StyleX only in its
servereturn is treated as production wiring, sodoctorand the generated agent docs say StyleX is active though the production build compiles none. echocounted as rebuild wiring —echo astryx theme build src/theme.tsin 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
doctorlater 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]









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 themeis a thorough 15-section guide — quick start,defineTheme, component overrides, custom variants, runtime vs built themes,astryx theme build. Nothing in the injectedAGENTS.mdtold an agent to open it.Compare how the block treats the two subjects today. Layout gets a hard rule: "Frame first: read
astryx docs layoutbefore writing any page or screen." Theming gets one clause — "Brand/accent viaastryx theme" — andastryx theme buildis never mentioned at all.themeappears in thedocs <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|buildin the CLI reference alongsideswizzle. 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 addnow surfaces the required build stepFollowing the current next-steps output exactly leaves you on the runtime-injection path with no idea a build step exists —
astryx theme buildis 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/prebuildscript wiring rather than a bare command, since remembering to rebuild by hand is precisely the step that gets skipped.Deliberately not changed
The
SELF-CHECKline still prescribesxstyleas 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 --checkgate: no built theme artifacts are committed in this repo (only the.tssources are tracked, anddev/buildboth rungenerate→build:themefirst), 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 agentsrenders the new rule correctly in a scratch projectastryx theme add neutralrenders the build guidance with the correct resolved pathcheck:changesetsand the full pre-commit gate pass