fix: add layout rule exceptions for icons, backgrounds, images, text#100
Merged
fix: add layout rule exceptions for icons, backgrounds, images, text#100
Conversation
Frames whose children are all vector/shape types (VECTOR, BOOLEAN_OPERATION, ELLIPSE, LINE, STAR, REGULAR_POLYGON, RECTANGLE) are treated as icon-like and excluded from the no-auto-layout rule. Auto-layout is meaningless for these frames since they contain only visual elements, not layout elements. https://claude.ai/code/session_01BXadwG43WaXRGwckkERMbN
Change criteria from "all children are vector/shape" to "exactly one child that is a visual leaf type". A frame wrapping a single VECTOR, BOOLEAN_OPERATION, ELLIPSE, LINE, STAR, REGULAR_POLYGON, or RECTANGLE is exempt from no-auto-layout. Frames with 2+ children are still flagged. https://claude.ai/code/session_01BXadwG43WaXRGwckkERMbN
Broaden the exception from single-child-only to all-children-are-visual- leaf-types. Frames where every child is a vector/shape type (VECTOR, BOOLEAN_OPERATION, ELLIPSE, LINE, STAR, REGULAR_POLYGON, RECTANGLE) are exempt from no-auto-layout — covers icons with multiple paths. https://claude.ai/code/session_01BXadwG43WaXRGwckkERMbN
minWidth is rarely needed as content provides a natural minimum. Now only flags FILL containers missing maxWidth. Having maxWidth alone is sufficient to pass the check. https://claude.ai/code/session_01BXadwG43WaXRGwckkERMbN
Skip maxWidth check when: - Parent already has maxWidth (parent constrains stretch) - Root-level frame (represents the screen itself) - Only FILL child among siblings (intent is to fill parent) - Inside grid layout (grid controls sizing) - Inside flex wrap (wrap layout controls sizing per row) https://claude.ai/code/session_01BXadwG43WaXRGwckkERMbN
Move exception logic from inline checks to dedicated functions: - isAutoLayoutExempt: visual leaf children (icons, shapes) - isAbsolutePositionExempt: vectors, small decorations, component internals - isSizeConstraintExempt: maxWidth present, parent constrained, root level, only FILL child, grid, flex wrap, small elements https://claude.ai/code/session_01BXadwG43WaXRGwckkERMbN
New exceptions added to rule-exceptions.ts: - no-auto-layout: INSTANCE nodes (master manages layout) - absolute-position: full-size background elements (>= 90% of parent) - missing-size-constraint: TEXT nodes (content provides natural sizing) - fixed-size-in-auto-layout: image fills (intentional fixed thumbnails) Extracted isFixedSizeExempt from inline logic in fixed-size rule. Added dedicated tests for all exception functions. https://claude.ai/code/session_01BXadwG43WaXRGwckkERMbN
Contributor
|
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 (3)
📝 WalkthroughWalkthroughThis PR introduces centralized rule-exception utilities in a new module and refactors four structure rules to use these utilities instead of inline exemption logic, replacing hardcoded heuristics with consolidated, testable classification functions. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
- Add image fill exemption to isAbsolutePositionExempt (any node with IMAGE fill is exempt regardless of size) - Fix rule-engine depth weight test: use TEXT leaf instead of RECTANGLE to avoid triggering isAutoLayoutExempt https://claude.ai/code/session_01BXadwG43WaXRGwckkERMbN
…empt Image fills already cover background use cases — the size-based check (>= 90% of parent) is redundant and removed. https://claude.ai/code/session_01BXadwG43WaXRGwckkERMbN
Extract isVisualOnlyNode that covers: vector/shape types, image fills, and frames with only visual leaf children. Use it across all exempt functions to eliminate duplicated checks. https://claude.ai/code/session_01BXadwG43WaXRGwckkERMbN
These are now redundant — isVisualOnlyNode covers vectors, images, and visual-only frames. Removed isSmallRelativeToParent, isFullSizeRelativeToParent, and component parent check. isAbsolutePositionExempt now takes only node (no context needed). https://claude.ai/code/session_01BXadwG43WaXRGwckkERMbN
Pages typically don't contain component masters, so INSTANCE nodes should be flagged for missing auto-layout like any other container. https://claude.ai/code/session_01BXadwG43WaXRGwckkERMbN
…ling logic - Move isExcludedName calls from index.ts into isAbsolutePositionExempt and isFixedSizeExempt — all exception logic now lives in rule-exceptions.ts - Change missing-size-constraint sibling check: exempt when ALL siblings are FILL (e.g. list view), not when there's only one FILL child - Remove unused isExcludedName import from structure/index.ts https://claude.ai/code/session_01BXadwG43WaXRGwckkERMbN
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/core/rules/rule-exceptions.test.ts`:
- Around line 115-125: Add unit tests in src/core/rules/rule-exceptions.test.ts
to cover all exemption branches of isSizeConstraintExempt: create cases using
makeNode and makeContext that assert true when (1) node has maxWidth set, (2)
node is a "small" element (use whatever predicate your code uses to mark small
elements), (3) parent has maxWidth, (4) depth <= 1 (root-level frames), (5) node
is the sole FILL child among siblings, (6) parent layoutMode is "GRID", and (7)
existing TEXT node (already present); use the same pattern as the existing TEXT
test (makeNode/makeContext and expect(isSizeConstraintExempt(node,
ctx)).toBe(true)) so each exemption branch is exercised.
In `@src/core/rules/rule-exceptions.ts`:
- Around line 65-95: The sibling-based exemption in isSizeConstraintExempt must
only apply when siblings are explicitly provided: replace the loose truthy check
with an explicit Array.isArray(context.siblings) guard and compute fillSiblings
from that array (filter by layoutSizingHorizontal === "FILL"); only return true
when that array exists and fillSiblings.length <= 1 so an undefined siblings
field does not implicitly grant an exemption. Ensure you reference the existing
symbols: isSizeConstraintExempt, context.siblings, fillSiblings, and
layoutSizingHorizontal.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 454f95f5-4acf-4886-a0bc-daddd865e6ae
📒 Files selected for processing (6)
src/core/engine/rule-engine.test.tssrc/core/rules/rule-exceptions.test.tssrc/core/rules/rule-exceptions.tssrc/core/rules/structure/index.tssrc/core/rules/structure/no-auto-layout.test.tssrc/core/rules/structure/responsive-fields.test.ts
Comment on lines
+65
to
+95
| /** Nodes that don't need maxWidth even with FILL sizing */ | ||
| export function isSizeConstraintExempt(node: AnalysisNode, context: RuleContext): boolean { | ||
| // Already has maxWidth | ||
| if (node.maxWidth !== undefined) return true; | ||
|
|
||
| // Small elements — won't stretch problematically | ||
| if (node.absoluteBoundingBox && node.absoluteBoundingBox.width <= 200) return true; | ||
|
|
||
| // Parent already has maxWidth — parent constrains the stretch | ||
| if (context.parent?.maxWidth !== undefined) return true; | ||
|
|
||
| // Root-level frames — they represent the screen itself | ||
| if (context.depth <= 1) return true; | ||
|
|
||
| // Only FILL child among siblings — intent is to fill the parent entirely | ||
| if (context.siblings) { | ||
| const fillSiblings = context.siblings.filter((s) => s.layoutSizingHorizontal === "FILL"); | ||
| if (fillSiblings.length <= 1) return true; | ||
| } | ||
|
|
||
| // Inside grid layout — grid controls sizing | ||
| if (context.parent?.layoutMode === "GRID") return true; | ||
|
|
||
| // Inside flex wrap — wrap layout controls sizing per row | ||
| if (context.parent?.layoutWrap === "WRAP") return true; | ||
|
|
||
| // Text nodes — content length provides natural sizing | ||
| if (node.type === "TEXT") return true; | ||
|
|
||
| return false; | ||
| } |
Contributor
There was a problem hiding this comment.
Potential false exemption when siblings is undefined.
When context.siblings is undefined (which can happen per the RuleContext interface), the fillSiblings filter block is skipped entirely. However, this means a node that should be flagged (e.g., multiple FILL siblings exist but siblings wasn't populated) could incorrectly pass through to later checks and potentially get exempted by other conditions.
Consider whether this is the intended behavior. If siblings being undefined should not grant an exemption, you may want to only apply this exemption when siblings are explicitly provided and the condition is met.
Suggested clarification
// Only FILL child among siblings — intent is to fill the parent entirely
- if (context.siblings) {
+ if (context.siblings && context.siblings.length > 0) {
const fillSiblings = context.siblings.filter((s) => s.layoutSizingHorizontal === "FILL");
if (fillSiblings.length <= 1) return true;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core/rules/rule-exceptions.ts` around lines 65 - 95, The sibling-based
exemption in isSizeConstraintExempt must only apply when siblings are explicitly
provided: replace the loose truthy check with an explicit
Array.isArray(context.siblings) guard and compute fillSiblings from that array
(filter by layoutSizingHorizontal === "FILL"); only return true when that array
exists and fillSiblings.length <= 1 so an undefined siblings field does not
implicitly grant an exemption. Ensure you reference the existing symbols:
isSizeConstraintExempt, context.siblings, fillSiblings, and
layoutSizingHorizontal.
Add tests for all exemption conditions: maxWidth present, small elements, parent maxWidth, root-level, all-FILL siblings, GRID, flex wrap, mixed sizing (negative case). Addresses CodeRabbit review. https://claude.ai/code/session_01BXadwG43WaXRGwckkERMbN
Define PR lifecycle: draft first, subscribe for reviews, CodeRabbit rate limit handling, squash merge with branch deletion, user confirmation required for merge. https://claude.ai/code/session_01BXadwG43WaXRGwckkERMbN
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CLAUDE.md`:
- Line 238: Step 4 is a long, compound sentence — split it into 2–4 concise
sub-steps: (a) "After each push, watch for CodeRabbit's first comment." (b) "If
that comment contains a rate limit message, wait the specified duration." (c)
"After waiting, re-trigger the review by pushing an empty commit." Keep the
exact retry instruction text (git commit --allow-empty -m "chore: re-trigger
review") intact so readers know the exact command to run.
In `@src/core/rules/rule-exceptions.test.ts`:
- Around line 176-184: The test currently hits the sibling-FILL exemption before
the TEXT branch in isSizeConstraintExempt; update the test so the sibling
exemption does not trigger (e.g., make the other sibling's
layoutSizingHorizontal a non-FILL value or remove the sibling) so the function
evaluates node.type === "TEXT"; locate the test in rule-exceptions.test.ts (the
it("exempts TEXT nodes") block) and modify the siblings array to ensure only the
node under test is FILL while the sibling is FIXED/UNSET so the TEXT-specific
branch is actually exercised.
In `@src/core/rules/rule-exceptions.ts`:
- Around line 18-23: The hasImageFill function currently assumes each element in
node.fills is an indexable object; update it to guard each entry before reading
its "type": inside hasImageFill (and when iterating node.fills), skip entries
that are null/undefined or not objects (typeof !== "object" or Array.isArray)
and only then access the "type" property (also verify it's a string) and compare
to "IMAGE"; this prevents runtime throws when fills contains unknown values
while keeping the same overall logic.
- Around line 45-48: The exemption logic in isAutoLayoutExempt is incorrect:
replace the broad isVisualOnlyNode(node) check with a rule that returns true for
INSTANCE nodes and for image-backed frames that are truly visual-only (i.e.,
image/icon-like with no meaningful content children). Update isAutoLayoutExempt
to: return true if node.type === 'INSTANCE' OR (isVisualOnlyNode(node) AND the
node has no non-visual/content children), using the AnalysisNode children
metadata to detect real content children; keep all other nodes falling through
to the overlap/nested-container checks.
In `@src/core/rules/structure/index.ts`:
- Around line 139-145: The exemption check for full-bleed backgrounds is called
only with the child node, so it cannot compare the child's size to its
auto-layout parent; update the call in absolutePositionInAutoLayoutCheck to pass
the parent (context.parent) or the parent's bounds instead of only node, and
update the isAbsolutePositionExempt function signature/logic to accept the
parent (or parentBounds) and use that to compute the coverage percentage before
returning true; ensure callers of isAbsolutePositionExempt are adjusted
accordingly so the full-bleed background exemption works as intended.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 41624e3e-a981-4770-a214-851072c9cbff
📒 Files selected for processing (5)
CLAUDE.mdsrc/core/rules/rule-exceptions.test.tssrc/core/rules/rule-exceptions.tssrc/core/rules/structure/index.tssrc/core/rules/structure/responsive-fields.test.ts
CLAUDE.md
Outdated
| 1. Always create PRs as **draft** first — wait for user approval before marking ready | ||
| 2. When changes are needed, convert back to **draft** — mark ready again when done | ||
| 3. After creating a PR, **subscribe** with `subscribe_pr_activity` to monitor reviews and CI in real-time | ||
| 4. After each push, watch for CodeRabbit's first comment — if it contains a rate limit message, wait the specified duration then push an empty commit (`git commit --allow-empty -m "chore: re-trigger review"`) to re-trigger |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider breaking step 4 into sub-steps for better readability.
Step 4 packs multiple conditional actions (watch → detect → wait → retry) into a single long sentence. Breaking it into sub-steps or shorter sentences would make it easier to parse.
♻️ Proposed refactor for improved readability
-4. After each push, watch for CodeRabbit's first comment — if it contains a rate limit message, wait the specified duration then push an empty commit (`git commit --allow-empty -m "chore: re-trigger review"`) to re-trigger
+4. After each push, watch for CodeRabbit's first comment:
+ - If it contains a rate limit message, wait the specified duration
+ - Then push an empty commit to re-trigger: `git commit --allow-empty -m "chore: re-trigger review"`📝 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
| 4. After each push, watch for CodeRabbit's first comment — if it contains a rate limit message, wait the specified duration then push an empty commit (`git commit --allow-empty -m "chore: re-trigger review"`) to re-trigger | |
| 4. After each push, watch for CodeRabbit's first comment: | |
| - If it contains a rate limit message, wait the specified duration | |
| - Then push an empty commit to re-trigger: `git commit --allow-empty -m "chore: re-trigger review"` |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CLAUDE.md` at line 238, Step 4 is a long, compound sentence — split it into
2–4 concise sub-steps: (a) "After each push, watch for CodeRabbit's first
comment." (b) "If that comment contains a rate limit message, wait the specified
duration." (c) "After waiting, re-trigger the review by pushing an empty
commit." Keep the exact retry instruction text (git commit --allow-empty -m
"chore: re-trigger review") intact so readers know the exact command to run.
Comment on lines
+45
to
+48
| export function isAutoLayoutExempt(node: AnalysisNode): boolean { | ||
| if (isVisualOnlyNode(node)) return true; | ||
|
|
||
| return false; |
Contributor
There was a problem hiding this comment.
no-auto-layout exemption is both too broad and too narrow.
Using isVisualOnlyNode() here exempts any image-filled frame, including containers that still have real content children, while plain INSTANCE nodes can still fall through into the overlap/nested-container checks. That reverses the PR goal: icon-like visual-child frames and all instances should skip, not every image-backed container.
🎯 Proposed fix
export function isAutoLayoutExempt(node: AnalysisNode): boolean {
- if (isVisualOnlyNode(node)) return true;
-
- return false;
+ if (node.type === "INSTANCE") return true;
+ if (
+ node.type === "FRAME" &&
+ node.children &&
+ node.children.length > 0 &&
+ node.children.every((child) => isVisualLeafType(child.type))
+ ) {
+ return true;
+ }
+ return false;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core/rules/rule-exceptions.ts` around lines 45 - 48, The exemption logic
in isAutoLayoutExempt is incorrect: replace the broad isVisualOnlyNode(node)
check with a rule that returns true for INSTANCE nodes and for image-backed
frames that are truly visual-only (i.e., image/icon-like with no meaningful
content children). Update isAutoLayoutExempt to: return true if node.type ===
'INSTANCE' OR (isVisualOnlyNode(node) AND the node has no non-visual/content
children), using the AnalysisNode children metadata to detect real content
children; keep all other nodes falling through to the overlap/nested-container
checks.
…s, revert CLAUDE.md - isAbsolutePositionExempt: restore small-element-relative-to-parent check (badges/decorations < 25% of parent) that was removed in 2a5c745 - fixedSizeInAutoLayout: remove duplicate isFixedSizeExempt call - noAutoLayout: simplify redundant type guard (isContainerNode covers FRAME) - isSizeConstraintExempt: clarify comment for single/all-FILL siblings - Revert CLAUDE.md PR workflow changes (belongs in separate PR) - Add tests for small-decoration exemption
…ePositionExempt Size-based heuristics guess intent; name patterns reflect explicit designer intent. If a small element needs absolute positioning exemption, it should be named properly (badge, indicator, etc.) — which is already covered by EXCLUDED_NAME_PATTERN.
This was referenced Mar 26, 2026
This was referenced Mar 29, 2026
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
2 participants
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
rule-exceptions.ts로 분리하여 관리 용이하게 리팩토링Test plan
https://claude.ai/code/session_01BXadwG43WaXRGwckkERMbN
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Tests