feat: add styling_preflight single-call validation tool#163
Merged
Conversation
Combines component API discovery, CSS reference resolution, Shadow DOM validation, and theme checking into one call. Returns parts/tokens/slots API surface, valid/invalid status for all references, issues, correct CSS snippet, and pass/fail verdict. 16 tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughA new Changes
Sequence DiagramsequenceDiagram
participant Client as MCP Client
participant Tool as handleStylingCall
participant Handler as runStylingPreflight
participant CssApi as resolveCssApi
participant ShadowDom as checkShadowDomUsage
participant Theme as checkThemeCompatibility
participant Verdict as buildVerdict
Client->>Tool: styling_preflight request<br/>(cssText, tagName, htmlText)
Tool->>Handler: runStylingPreflight({css, html, meta})
Handler->>CssApi: resolveCssApi(css, metadata)
CssApi-->>Handler: CssApiResolution
Handler->>ShadowDom: checkShadowDomUsage(css, html)
ShadowDom-->>Handler: shadow DOM issues
Handler->>Theme: checkThemeCompatibility(css, metadata)
Theme-->>Handler: theme issues
Handler->>Verdict: buildVerdict(errorCount, warningCount, invalidRefs)
Verdict-->>Handler: verdict string
Handler-->>Tool: PreflightResult{<br/>componentApi,<br/>resolution,<br/>issues,<br/>correctSnippet,<br/>verdict}
Tool-->>Client: MCP success response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/handlers/styling-preflight.ts`:
- Around line 65-93: The two empty catch blocks around checkShadowDomUsage and
checkThemeCompatibility are swallowing exceptions and allowing an incomplete
analysis to return a clean preflight; change each catch to capture the error
(catch (err)) and either rethrow or—preferably—append a new error issue to the
same issues array (use severity: 'error', category: 'shadowDom' or
'themeCompat', message: err.message or String(err), and optional suggestion) and
mark the overall result as failed (e.g., set the preflight status variable or
flag used by the function); update the catch bodies in the blocks surrounding
checkShadowDomUsage(...) and checkThemeCompatibility(...) and ensure the calling
code respects the failure flag or thrown error.
- Around line 141-143: The verdict text currently lumps all warnings as
"theme/dark mode risk"; inspect the collected issues (variable issues) and
compute counts by type (e.g., theme/dark-mode issues vs shadowDom/selector
issues) and then push an appropriate summary string into parts (instead of the
single generic message)—for example pluralized messages for theme/dark-mode
risks and separate messages for Shadow DOM or selector warnings; update the
logic that builds parts (the block using warnings and parts) to check issue.kind
or the specific fields that identify shadowDom/selector items and emit accurate
labels for each category.
In `@packages/core/src/tools/styling.ts`:
- Around line 1013-1018: The test suite currently calls runStylingPreflight()
directly and misses exercising the public MCP entry, so add one integration test
that invokes handleStylingCall() with a simulated call for the
'styling_preflight' tool: construct args matching StylingPreflightArgsSchema
(cssText, tagName, htmlText), pass the same cem value used in production, and
assert the returned value is the wrapped success response (created via
createSuccessResponse) containing the JSON-stringified runStylingPreflight
result; this ensures tool registration, argument parsing
(StylingPreflightArgsSchema.parse), and response wrapping are validated
end-to-end rather than only testing runStylingPreflight in isolation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a4b36466-bca3-4f99-81a2-58cd48113edc
📒 Files selected for processing (4)
.changeset/styling-preflight.mdpackages/core/src/handlers/styling-preflight.tspackages/core/src/tools/styling.tstests/handlers/styling-preflight.test.ts
Comment on lines
+65
to
+93
| try { | ||
| const shadowResult = checkShadowDomUsage(css, meta.tagName, meta); | ||
| for (const issue of shadowResult.issues) { | ||
| issues.push({ | ||
| severity: issue.severity === 'error' ? 'error' : 'warning', | ||
| category: 'shadowDom', | ||
| message: issue.message, | ||
| line: issue.line, | ||
| suggestion: issue.suggestion, | ||
| }); | ||
| } | ||
| } catch { | ||
| // Shadow DOM check failed — skip | ||
| } | ||
|
|
||
| // 3. Run theme compatibility check | ||
| try { | ||
| const themeResult = checkThemeCompatibility(css); | ||
| for (const issue of themeResult.issues) { | ||
| issues.push({ | ||
| severity: 'warning', | ||
| category: 'themeCompat', | ||
| message: issue.message, | ||
| line: issue.line, | ||
| }); | ||
| } | ||
| } catch { | ||
| // Theme check failed — skip | ||
| } |
There was a problem hiding this comment.
Do not return a clean preflight when a sub-check crashes.
Both catch blocks suppress validator failures and continue. If either checker throws on an unexpected input shape, this function can still return pass even though the analysis was incomplete.
⚠️ Proposed fix
- } catch {
- // Shadow DOM check failed — skip
+ } catch {
+ issues.push({
+ severity: 'warning',
+ category: 'internal',
+ message: 'Shadow DOM validation could not be completed; preflight result is incomplete.',
+ });
}
@@
- } catch {
- // Theme check failed — skip
+ } catch {
+ issues.push({
+ severity: 'warning',
+ category: 'internal',
+ message: 'Theme compatibility validation could not be completed; preflight result is incomplete.',
+ });
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| try { | |
| const shadowResult = checkShadowDomUsage(css, meta.tagName, meta); | |
| for (const issue of shadowResult.issues) { | |
| issues.push({ | |
| severity: issue.severity === 'error' ? 'error' : 'warning', | |
| category: 'shadowDom', | |
| message: issue.message, | |
| line: issue.line, | |
| suggestion: issue.suggestion, | |
| }); | |
| } | |
| } catch { | |
| // Shadow DOM check failed — skip | |
| } | |
| // 3. Run theme compatibility check | |
| try { | |
| const themeResult = checkThemeCompatibility(css); | |
| for (const issue of themeResult.issues) { | |
| issues.push({ | |
| severity: 'warning', | |
| category: 'themeCompat', | |
| message: issue.message, | |
| line: issue.line, | |
| }); | |
| } | |
| } catch { | |
| // Theme check failed — skip | |
| } | |
| try { | |
| const shadowResult = checkShadowDomUsage(css, meta.tagName, meta); | |
| for (const issue of shadowResult.issues) { | |
| issues.push({ | |
| severity: issue.severity === 'error' ? 'error' : 'warning', | |
| category: 'shadowDom', | |
| message: issue.message, | |
| line: issue.line, | |
| suggestion: issue.suggestion, | |
| }); | |
| } | |
| } catch { | |
| issues.push({ | |
| severity: 'warning', | |
| category: 'internal', | |
| message: 'Shadow DOM validation could not be completed; preflight result is incomplete.', | |
| }); | |
| } | |
| // 3. Run theme compatibility check | |
| try { | |
| const themeResult = checkThemeCompatibility(css); | |
| for (const issue of themeResult.issues) { | |
| issues.push({ | |
| severity: 'warning', | |
| category: 'themeCompat', | |
| message: issue.message, | |
| line: issue.line, | |
| }); | |
| } | |
| } catch { | |
| issues.push({ | |
| severity: 'warning', | |
| category: 'internal', | |
| message: 'Theme compatibility validation could not be completed; preflight result is incomplete.', | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/handlers/styling-preflight.ts` around lines 65 - 93, The
two empty catch blocks around checkShadowDomUsage and checkThemeCompatibility
are swallowing exceptions and allowing an incomplete analysis to return a clean
preflight; change each catch to capture the error (catch (err)) and either
rethrow or—preferably—append a new error issue to the same issues array (use
severity: 'error', category: 'shadowDom' or 'themeCompat', message: err.message
or String(err), and optional suggestion) and mark the overall result as failed
(e.g., set the preflight status variable or flag used by the function); update
the catch bodies in the blocks surrounding checkShadowDomUsage(...) and
checkThemeCompatibility(...) and ensure the calling code respects the failure
flag or thrown error.
Comment on lines
+141
to
+143
| if (warnings > 0) { | ||
| parts.push(`${warnings} warning${warnings > 1 ? 's' : ''} (theme/dark mode risk)`); | ||
| } |
There was a problem hiding this comment.
The verdict text is mislabeling non-theme warnings.
issues can contain shadowDom warnings, but the summary always describes every warning as a theme/dark-mode risk. That makes the returned verdict inaccurate for selector and Shadow DOM findings.
📝 Proposed fix
if (warnings > 0) {
- parts.push(`${warnings} warning${warnings > 1 ? 's' : ''} (theme/dark mode risk)`);
+ parts.push(`${warnings} warning${warnings > 1 ? 's' : ''}`);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (warnings > 0) { | |
| parts.push(`${warnings} warning${warnings > 1 ? 's' : ''} (theme/dark mode risk)`); | |
| } | |
| if (warnings > 0) { | |
| parts.push(`${warnings} warning${warnings > 1 ? 's' : ''}`); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/handlers/styling-preflight.ts` around lines 141 - 143, The
verdict text currently lumps all warnings as "theme/dark mode risk"; inspect the
collected issues (variable issues) and compute counts by type (e.g.,
theme/dark-mode issues vs shadowDom/selector issues) and then push an
appropriate summary string into parts (instead of the single generic
message)—for example pluralized messages for theme/dark-mode risks and separate
messages for Shadow DOM or selector warnings; update the logic that builds parts
(the block using warnings and parts) to check issue.kind or the specific fields
that identify shadowDom/selector items and emit accurate labels for each
category.
Comment on lines
+1013
to
+1018
| if (name === 'styling_preflight') { | ||
| const { cssText, tagName, htmlText } = StylingPreflightArgsSchema.parse(args); | ||
| const meta = parseCem(tagName, cem); | ||
| const result = runStylingPreflight({ css: cssText, html: htmlText, meta }); | ||
| return createSuccessResponse(JSON.stringify(result, null, 2)); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add one integration test through handleStylingCall().
The new suite only calls runStylingPreflight() directly. A regression in tool registration, argument parsing, or response wrapping here would still pass CI even though the public MCP tool is broken.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/tools/styling.ts` around lines 1013 - 1018, The test suite
currently calls runStylingPreflight() directly and misses exercising the public
MCP entry, so add one integration test that invokes handleStylingCall() with a
simulated call for the 'styling_preflight' tool: construct args matching
StylingPreflightArgsSchema (cssText, tagName, htmlText), pass the same cem value
used in production, and assert the returned value is the wrapped success
response (created via createSuccessResponse) containing the JSON-stringified
runStylingPreflight result; this ensures tool registration, argument parsing
(StylingPreflightArgsSchema.parse), and response wrapping are validated
end-to-end rather than only testing runStylingPreflight in isolation.
Adds resolve_css_api and styling_preflight to the expected tools list (68 core + 2 token). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
styling_preflightMCP tool — one call that returns everything an agent needsWhy this matters
Trip-planner agents were hallucinating part names and token names because they skipped calling
get_component_quick_refbefore writing CSS. Now they callstyling_preflightonce and get the full picture: what the component supports, what's wrong with their CSS, and a correct snippet to reference.Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit