test(release-assets): assert on each scaffold's own utilities, not a fixed list - #3412
Conversation
…fixed list Follow-up to review on #3407, which merged before these two landed. 1. The Tailwind assertion was partly vacuous (Copilot, valid). It matched a hard-coded set: min-h-screen, mx-auto, flex, font-bold. The ai-agent template declares none of them; its shell is styled with h-screen alone. So for that template the check passed on CSS contributed by framework dependencies, not on anything the scaffold asked for, which is exactly the failure mode #3407 exists to prevent. Adding h-screen to the list, as suggested, fixes ai-agent but leaves the same trap for the next template. The assertion now reads the utilities out of the scaffold's own markup and requires one of them in the compiled stylesheet, so it stays tied to the template and cannot drift. Measured per template, declared vs matched: ai-agent h-screen -> h-screen minimal 24 utilities -> all 24 docs-agent 18 -> all 18 agentic-workflow 57 -> all 57 multi-agent-system 31 -> all 31 coding-agent 35 -> all 35 saas-starter 79 -> all 79 Plain tokens only. Variants, arbitrary values and slashes compile to escaped selectors, and matching those is a CSS-escaping exercise this check does not need to take on. 2. Em dashes (CodeRabbit, valid). AGENTS.md line 55 prohibits them. I dismissed this on #3407 because the surrounding prose already used them, which was wrong: a pre-existing violation does not license a new one. Fixes the three I introduced, rewriting the sentences rather than swapping the character. The older ones elsewhere in css-compile.ts are left alone; they are outside what this change touches. 15 suites / 220 steps green.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 2 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR tightens the src/release-assets/ scaffolded-project release-asset build regression test so the “compiled Tailwind utility exists” assertion is derived from each scaffold’s own markup (avoiding vacuous passes on framework-provided CSS), and adjusts nearby prose to comply with the project’s no em/en dash rule.
Changes:
- Update
scaffolded-project-build.test.tsto extract plain Tailwind utility tokens from each scaffold’s TSX/JSX and assert at least one appears in the compiled CSS. - Minor doc comment punctuation updates in
scaffolded-project-build.test.tsandcss-compile.ts(replacing em dash usage in the touched lines).
Verification (from PR description):
- Reported:
src/release-assets/tests pass (15 suites), fmt/lint/deno checkclean. - Not executed in this review environment. Suggested spot-check if needed:
deno task test:unit(or the repo’s full unit test command).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/release-assets/scaffolded-project-build.test.ts | Makes the CSS utility assertion template-specific by extracting utility tokens from scaffold markup, and updates related comments. |
| src/release-assets/css-compile.ts | Small doc comment punctuation change clarifying the “explicit” optimizer policy without an em dash. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The comment claimed "plain tokens only" and listed variants, arbitrary values and slashes. The regex also drops negative utilities (-mt-4) and anything leading with a digit, which the comment did not say. Names the pattern instead so the exclusions are readable off it.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/release-assets/scaffolded-project-build.test.ts:133
scaffoldUtilityClasses()currently only extracts utilities fromclassName="..."inside.tsx/.jsxfiles. That misses real scaffold markup cases like.mdxfiles (e.g.minimal/app/about/page.mdx) andclassName={...}/template-literal usages (e.g.saas-starter/app/dashboard/page.tsx), which can make the utility list incomplete or even empty if templates evolve, weakening the intent of tying the assertion to the scaffold’s own markup.
function scaffoldUtilityClasses(
files: ReadonlyArray<{ path: string; content: string }>,
): string[] {
const found = new Set<string>();
for (const file of files) {
if (!/\.(tsx|jsx)$/.test(file.path)) continue;
for (const match of file.content.matchAll(/className="([^"]+)"/g)) {
for (const token of match[1]!.split(/\s+/)) {
if (/^[a-z][a-z0-9-]*$/.test(token)) found.add(token);
}
}
}
return [...found];
}
Follow-up to review feedback on #3407, which merged before these two comments landed. Both were valid.
1. The Tailwind assertion was partly vacuous (Copilot)
It matched a hard-coded set:
min-h-screen,mx-auto,flex,font-bold. Theai-agenttemplate declares none of them — its shell is styled withh-screenalone (cli/templates/files/ai-agent/app/page.tsx:10).So for that template the check passed on CSS contributed by framework dependencies, not on anything the scaffold asked for. That is exactly the failure mode #3407 exists to prevent, in #3407's own test.
Adding
h-screento the list — as suggested — fixesai-agentbut leaves the same trap for the next template. Instead the assertion now reads utilities out of the scaffold's own markup and requires one of them in the compiled stylesheet, so it stays tied to the template and cannot drift.Measured per template, declared vs matched:
ai-agenth-screenh-screenminimaldocs-agentagentic-workflowmulti-agent-systemcoding-agentsaas-starterPlain tokens only. Variants (
hover:), arbitrary values (w-[3px]) and slashes (bg-black/50) compile to escaped selectors, and matching those is a CSS-escaping exercise this check doesn't need to take on.2. Em dashes (CodeRabbit)
AGENTS.md:55prohibits em and en dashes. I dismissed this on #3407 on the grounds that the surrounding prose already used them — that was wrong. A pre-existing violation doesn't license a new one, and the rule is documented.Fixes the three I introduced, rewriting the sentences rather than swapping the character so they still read. The older ones elsewhere in
css-compile.tsare left alone, being outside what this change touches.Verification
src/release-assets/: 15 suites, 220 steps, 0 failed. fmt / lint /deno checkclean. No production code changes beyond one comment line.