fix: rule condition review and updates (#152)#163
Conversation
- Section: remove children check (Section in UI tree is always wrong) - fixed-size-in-auto-layout: remove 48px threshold, use isVisualOnlyNode - missing-size-constraint: implement wrap/grid subTypes, remove over-generous exemptions (200px, sibling FILL, TEXT, grid, wrap) - missing-component: minRepetitions 3→2 - detached-instance: includes() → word boundary regex, exact case - raw-value: opacity 1.0 no longer flagged - irregular-spacing: gridBase 4→2 - Merge default-name into non-semantic-name (18→17 rules) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe standalone Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
FIXED children in wrap/grid already have explicit size — min-width not needed. Only FILL children need constraints to control responsive behavior. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/core/rules/structure/index.ts (1)
292-299:⚠️ Potential issue | 🟠 MajorKeep
SECTIONreporting scoped to non-empty sections.Flagging every
SECTIONwill turn empty or purely organizational sections into false positives, even though this rule’s rationale only applies when the section is actually being used as a container. Please restore the child-presence guard here.Based on learnings, the `non-layout-container` rule "intentionally flags non-empty `SECTION` nodes (`children.length > 0`) because Figma `SECTION` is not a layout container by design — using it structurally with children is treated as semantic misuse."Suggested fix
- if (node.type === "SECTION") { + if (node.type === "SECTION" && (node.children?.length ?? 0) > 0) { return { ruleId: nonLayoutContainerDef.id, subType: "section" as const, nodeId: node.id, nodePath: context.path.join(" > "), message: nonLayoutContainerMsg.section(node.name), }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/rules/structure/index.ts` around lines 292 - 299, The SECTION branch currently flags every SECTION; restrict it to only non-empty sections by checking node.children (e.g., node.children?.length > 0) before returning the non-layout-container violation. Update the SECTION handling in the rule (where nonLayoutContainerDef.id and nonLayoutContainerMsg.section(...) are used) to guard on child presence so empty/organizational SECTION nodes are not reported.docs/REFERENCE.md (1)
20-40:⚠️ Potential issue | 🟡 MinorBring the reference snippets in sync with the shipped config.
The rule table now reflects the
default-namemerge, but the schema and example configs still referencedefault-name, and the field table still documentsgridBaseas4. Those snippets are now misleading to copy verbatim. Switch the examples tonon-semantic-nameonly, drop the duplicate disable in the relaxed example, and update the documented default grid base to2.Also applies to: 71-82, 143-163
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/REFERENCE.md` around lines 20 - 40, Update the example JSON and field docs to match the shipped config: replace any occurrences of the rule name "default-name" with "non-semantic-name" in the "rules" object and example configs (remove the duplicate disable entry in the relaxed example), and change the documented default value for `gridBase` from 4 to 2; ensure the Fields table and example snippets reference `excludeNodeTypes`, `excludeNodeNames`, `gridBase`, and `rules` consistently with these edits so the examples are copy-paste accurate.src/cli/docs.ts (1)
90-104:⚠️ Potential issue | 🟡 MinorFix the stale
gridBasedefault in the CLI docs.This guide still says
gridBasedefaults to4, butRULE_CONFIGS["irregular-spacing"].options.gridBaseis now2. Please update the prose here, and the sample JSON if it is meant to reflect defaults, socanicode docs configmatches runtime behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/docs.ts` around lines 90 - 104, Update the docs text and example JSON to reflect the current default gridBase of 2 instead of 4: change the prose line that currently reads "gridBase: spacing grid unit, default 4" to state default 2, and update the sample JSON value for "gridBase" from 4 to 2 so it matches RULE_CONFIGS["irregular-spacing"].options.gridBase; ensure any nearby explanatory text referencing the default is consistent with the RULE_CONFIGS["irregular-spacing"].options.gridBase setting.
🤖 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/engine/scoring.test.ts`:
- Line 307: The test fixture for the "minor" category in
src/core/engine/scoring.test.ts incorrectly duplicates the rule ID
"non-semantic-name", which inflates density because calculateScores() collapses
diversity by ruleId; update the fixture to use only the three unique minor IDs
(e.g., "non-standard-naming", "non-semantic-name",
"inconsistent-naming-convention") and, if you need the same pressure, increase
the inner iteration count in the test loop that builds occurrences (the code
that generates many minor occurrences for the calculateScores() run) rather than
duplicating a ruleId.
In `@src/core/rules/component/index.ts`:
- Around line 301-303: Static analysis flagged a ReDoS risk but the escape used
when building pattern from component.name is correct and safe; update the code
around the loop that iterates over components (the for (const [, component] of
Object.entries(components)) block) to add a brief inline comment explaining that
component.name is escaped with component.name.replace(/[.*+?^${}()|[\]\\]/g,
"\\$&") to force a literal match, that the RegExp is wrapped in \b...\b so no
quantifiers/alternations exist (hence no catastrophic backtracking), and note
the edge-case that JavaScript \b only recognizes ASCII word characters so
non‑ASCII component names (CJK, Cyrillic) may not behave as expected.
- Around line 298-303: The component name matching currently builds a
case-sensitive RegExp in the loop (pattern variable) while the missing-component
rule uses case-insensitive matching; make the behavior consistent and add tests:
either change the RegExp construction in the loop that uses component.name to
include the 'i' flag (or use RegExp.prototype.test with case-normalized strings)
so it matches missing-component’s case-insensitive approach, or update the
missing-component logic to be case-sensitive instead; then add unit tests
exercising both "Button" vs "button" cases to the rule test suite to document
the chosen behavior and update a brief comment near the pattern creation and the
missing-component rule to state the intended case-sensitivity.
In `@src/core/rules/naming/non-semantic-name.test.ts`:
- Around line 10-27: The test currently only asserts that
nonSemanticName.check(node, makeContext()) returns a result with subType
defined; update the test to parameterize node.type alongside name and assert the
exact subtype returned by getDefaultNameSubType: change the it.each input to
tuples like [nodeType, name, expectedSubType] (use makeNode({ name, type }) and
makeContext()), call nonSemanticName.check(node, makeContext()), then
expect(result).not.toBeNull(), expect(result!.ruleId).toBe("non-semantic-name"),
and expect(result!.subType).toBe(expectedSubType) so the test verifies the
precise default-name -> subtype mapping used by getDefaultNameSubType rather
than only checking presence.
In `@src/core/rules/structure/index.ts`:
- Around line 224-258: The code never emits subType "min-width" for a general
FILL child; restore the general min-width path before the max-width fallback by
adding a check in the FILL-handling branch that, after
isSizeConstraintExempt(node, context) is checked, returns the
missingSizeConstraintDef with subType: "min-width" when
node.layoutSizingHorizontal === "FILL" and node.minWidth === undefined (use
missingSizeConstraintMsg.minWidth(node.name) for the message and include
nodeId/nodePath as in the other returns), placing this check before the existing
"max-width" return so the collapsed-on-small-screens case is reachable.
---
Outside diff comments:
In `@docs/REFERENCE.md`:
- Around line 20-40: Update the example JSON and field docs to match the shipped
config: replace any occurrences of the rule name "default-name" with
"non-semantic-name" in the "rules" object and example configs (remove the
duplicate disable entry in the relaxed example), and change the documented
default value for `gridBase` from 4 to 2; ensure the Fields table and example
snippets reference `excludeNodeTypes`, `excludeNodeNames`, `gridBase`, and
`rules` consistently with these edits so the examples are copy-paste accurate.
In `@src/cli/docs.ts`:
- Around line 90-104: Update the docs text and example JSON to reflect the
current default gridBase of 2 instead of 4: change the prose line that currently
reads "gridBase: spacing grid unit, default 4" to state default 2, and update
the sample JSON value for "gridBase" from 4 to 2 so it matches
RULE_CONFIGS["irregular-spacing"].options.gridBase; ensure any nearby
explanatory text referencing the default is consistent with the
RULE_CONFIGS["irregular-spacing"].options.gridBase setting.
In `@src/core/rules/structure/index.ts`:
- Around line 292-299: The SECTION branch currently flags every SECTION;
restrict it to only non-empty sections by checking node.children (e.g.,
node.children?.length > 0) before returning the non-layout-container violation.
Update the SECTION handling in the rule (where nonLayoutContainerDef.id and
nonLayoutContainerMsg.section(...) are used) to guard on child presence so
empty/organizational SECTION nodes are not reported.
🪄 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: 9f5cfcdf-2b20-442f-b6e7-7de97a2efbde
📒 Files selected for processing (18)
docs/REFERENCE.mdsrc/cli/docs.tssrc/core/contracts/rule.tssrc/core/engine/rule-engine.test.tssrc/core/engine/scoring.test.tssrc/core/engine/scoring.tssrc/core/rules/component/index.tssrc/core/rules/component/missing-component.test.tssrc/core/rules/naming/default-name.test.tssrc/core/rules/naming/index.tssrc/core/rules/naming/non-semantic-name.test.tssrc/core/rules/rule-config.tssrc/core/rules/rule-exceptions.test.tssrc/core/rules/rule-exceptions.tssrc/core/rules/rule-messages.tssrc/core/rules/structure/index.tssrc/core/rules/structure/responsive-fields.test.tssrc/core/rules/token/index.ts
💤 Files with no reviewable changes (2)
- src/core/rules/naming/default-name.test.ts
- src/core/contracts/rule.ts
| // Check if there's a component in the file with a matching name (word boundary) | ||
| const components = context.file.components; | ||
| const nodeName = node.name.toLowerCase(); | ||
|
|
||
| for (const [, component] of Object.entries(components)) { | ||
| if (nodeName.includes(component.name.toLowerCase())) { | ||
| const pattern = new RegExp(`\\b${component.name.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")}\\b`); | ||
| if (pattern.test(node.name)) { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Search for test cases or comments documenting case-sensitivity behavior for detached-instance
# Check test file for case-sensitivity tests
rg -n -i 'case|lowercase|uppercase|insensitive' src/core/rules/component/detached-instance.test.ts
# Check for any documentation about the case-sensitivity change
rg -n -C2 'exact case|case.?sensitiv' --type=mdRepository: let-sunny/canicode
Length of output: 44
🏁 Script executed:
# First, check if the test file exists and find related files
fd -t f 'detached-instance' --type f
# Also check the structure of the component rules directory
fd -t f . src/core/rules/component/ --type fRepository: let-sunny/canicode
Length of output: 308
🏁 Script executed:
# Read the actual rule implementation to understand the case-sensitivity logic
cat -n src/core/rules/component/index.ts | head -320 | tail -30Repository: let-sunny/canicode
Length of output: 1308
🏁 Script executed:
# Search for any PR context or documentation about case-sensitivity
rg -n 'exact case|case.?sensitiv|toLowerCase' src/core/rules/component/
# Also check rule-config.ts for any configuration
rg -n 'detached' src/core/rules/rule-config.ts -A3 -B3Repository: let-sunny/canicode
Length of output: 755
🏁 Script executed:
# Read the detached-instance test file to check test coverage
cat -n src/core/rules/component/detached-instance.test.tsRepository: let-sunny/canicode
Length of output: 2398
🏁 Script executed:
# Check for any documentation or PR comments about case-sensitivity
rg -n 'detached|instance' --type=md -A 2 -B 2Repository: let-sunny/canicode
Length of output: 2015
🏁 Script executed:
# Compare the two rule implementations side by side for context
cat -n src/core/rules/component/index.ts | sed -n '120,145p'Repository: let-sunny/canicode
Length of output: 1281
Clarify case-sensitivity approach and add test coverage for this rule.
The implementation uses case-sensitive matching, but this choice is neither documented in tests nor documented in comments. More importantly, the missing-component rule in the same file uses case-insensitive matching (lines 125, 139-141), creating an inconsistency.
The test suite has no coverage for case-sensitivity variations (e.g., frame "button" vs component "Button"). Add a test case demonstrating the case-sensitive behavior to make the design choice explicit, or align both rules on a consistent approach.
🧰 Tools
🪛 ast-grep (0.41.1)
[warning] 301-301: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(\\b${component.name.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")}\\b)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core/rules/component/index.ts` around lines 298 - 303, The component name
matching currently builds a case-sensitive RegExp in the loop (pattern variable)
while the missing-component rule uses case-insensitive matching; make the
behavior consistent and add tests: either change the RegExp construction in the
loop that uses component.name to include the 'i' flag (or use
RegExp.prototype.test with case-normalized strings) so it matches
missing-component’s case-insensitive approach, or update the missing-component
logic to be case-sensitive instead; then add unit tests exercising both "Button"
vs "button" cases to the rule test suite to document the chosen behavior and
update a brief comment near the pattern creation and the missing-component rule
to state the intended case-sensitivity.
| for (const [, component] of Object.entries(components)) { | ||
| if (nodeName.includes(component.name.toLowerCase())) { | ||
| const pattern = new RegExp(`\\b${component.name.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")}\\b`); | ||
| if (pattern.test(node.name)) { |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Regex escaping is correct; static analysis ReDoS warning is a false positive here.
The escape pattern correctly covers all standard regex metacharacters (. * + ? ^ $ { } ( ) | [ ] \), and the resulting pattern is purely literal characters within \b...\b — no quantifiers or alternations that could cause catastrophic backtracking. The fixtures with names like "Menu/16" and "Platform=Desktop" will be handled correctly.
One edge case worth noting: \b in JavaScript only recognizes ASCII word characters [a-zA-Z0-9_]. If component names contain non-ASCII characters (e.g., CJK, Cyrillic), word boundary matching may behave unexpectedly. This is likely acceptable given the design tool context, but worth documenting if internationalized component naming is expected.
📝 Optional: Add brief comment explaining the escaping rationale
for (const [, component] of Object.entries(components)) {
+ // Escape regex metacharacters; resulting pattern is pure literals within word boundaries (no ReDoS risk)
const pattern = new RegExp(`\\b${component.name.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")}\\b`);
if (pattern.test(node.name)) {📝 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.
| for (const [, component] of Object.entries(components)) { | |
| if (nodeName.includes(component.name.toLowerCase())) { | |
| const pattern = new RegExp(`\\b${component.name.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")}\\b`); | |
| if (pattern.test(node.name)) { | |
| for (const [, component] of Object.entries(components)) { | |
| // Escape regex metacharacters; resulting pattern is pure literals within word boundaries (no ReDoS risk) | |
| const pattern = new RegExp(`\\b${component.name.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")}\\b`); | |
| if (pattern.test(node.name)) { |
🧰 Tools
🪛 ast-grep (0.41.1)
[warning] 301-301: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(\\b${component.name.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")}\\b)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core/rules/component/index.ts` around lines 301 - 303, Static analysis
flagged a ReDoS risk but the escape used when building pattern from
component.name is correct and safe; update the code around the loop that
iterates over components (the for (const [, component] of
Object.entries(components)) block) to add a brief inline comment explaining that
component.name is escaped with component.name.replace(/[.*+?^${}()|[\]\\]/g,
"\\$&") to force a literal match, that the RegExp is wrapped in \b...\b so no
quantifiers/alternations exist (hence no catastrophic backtracking), and note
the edge-case that JavaScript \b only recognizes ASCII word characters so
non‑ASCII component names (CJK, Cyrillic) may not behave as expected.
…ase-sensitivity - Remove duplicate non-semantic-name in scoring test fixture - Pin exact subType per node type in non-semantic-name tests - Add case-sensitive + word boundary tests for detached-instance - min-width general path: rejected (by design, wrap/grid only) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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/naming/non-semantic-name.test.ts`:
- Around line 39-46: The test currently only asserts that result!.subType is not
"shape-name", which allows false positives; update the assertion in the it.each
block that builds a FRAME node using makeNode and calls
nonSemanticName.check(node, makeContext()) to require the exact subtype "frame"
(e.g., expect(result!.subType).toBe("frame")) so the test asserts the precise
overlap handling for FRAME nodes rather than any non-"shape-name" subtype.
- Line 20: Remove the unnecessary `as any` casts used when creating test nodes:
update the call to makeNode({ name, type: type }) (remove `as any`) and remove
the `as any` cast where the `"ELLIPSE"` literal is used; both `type` values
already match the AnalysisNodeTypeSchema/enum via the test's `as const`
parametrization so just pass the values directly and ensure the tests still
compile against makeNode and AnalysisNodeTypeSchema without the casts.
🪄 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: bfe6cfb0-b55b-4793-aa26-5843eb230aef
📒 Files selected for processing (3)
src/core/engine/scoring.test.tssrc/core/rules/component/detached-instance.test.tssrc/core/rules/naming/non-semantic-name.test.ts
| { name: "Component 1", type: "COMPONENT", expectedSubType: "component" }, | ||
| { name: "Instance 3", type: "INSTANCE", expectedSubType: "instance" }, | ||
| ] as const)("flags default name $name → subType $expectedSubType", ({ name, type, expectedSubType }) => { | ||
| const node = makeNode({ name, type: type as any }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify where `as any` is used and inspect makeNode typing contract.
rg -n "as any" src/core/rules/naming/non-semantic-name.test.ts -C2
rg -n "export (const|function) makeNode|makeNode\\s*\\(" src/core/rules/test-helpers.ts -C5
rg -n "type\\s*:\\s*z\\.enum|name\\s*:\\s*z\\.string|AnalysisNode" src/core/contracts/figma-node.ts -C3Repository: let-sunny/canicode
Length of output: 3461
🏁 Script executed:
# Find all enum values in AnalysisNodeTypeSchema to confirm ELLIPSE is valid
rg -A 50 "AnalysisNodeTypeSchema = z.enum" src/core/contracts/figma-node.ts | head -60
# Look at the complete test case array to see how type is defined
cat -n src/core/rules/naming/non-semantic-name.test.ts | head -30Repository: let-sunny/canicode
Length of output: 3176
🏁 Script executed:
# Check if there are any TypeScript errors in the test file when running type checking
# Also look at how makeNode is imported and used to understand type inference
cat -n src/core/rules/naming/non-semantic-name.test.ts | sed -n '1,65p'
# Check the exact import statement
rg -n "import.*makeNode" src/core/rules/naming/non-semantic-name.test.tsRepository: let-sunny/canicode
Length of output: 3493
Remove as any casts to maintain type safety in tests.
Lines 20 and 59 use unnecessary as any casts that bypass TypeScript's type checking without benefit. The parametrized test on line 19 uses as const, which preserves literal types for all test case values—all matching valid enum members in AnalysisNodeTypeSchema. Similarly, "ELLIPSE" on line 59 is a valid enum literal. Both casts can be safely removed.
Suggested fix
- ] as const)("flags default name $name → subType $expectedSubType", ({ name, type, expectedSubType }) => {
- const node = makeNode({ name, type: type as any });
+ ] as const)("flags default name $name → subType $expectedSubType", ({ name, type, expectedSubType }) => {
+ const node = makeNode({ name, type });
@@
- it("allows shape names on leaf shape primitives", () => {
- const node = makeNode({ type: "ELLIPSE" as any, name: "polygon" });
+ it("allows shape names on leaf shape primitives", () => {
+ const node = makeNode({ type: "ELLIPSE", name: "polygon" });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core/rules/naming/non-semantic-name.test.ts` at line 20, Remove the
unnecessary `as any` casts used when creating test nodes: update the call to
makeNode({ name, type: type }) (remove `as any`) and remove the `as any` cast
where the `"ELLIPSE"` literal is used; both `type` values already match the
AnalysisNodeTypeSchema/enum via the test's `as const` parametrization so just
pass the values directly and ensure the tests still compile against makeNode and
AnalysisNodeTypeSchema without the casts.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 participant
Summary
17개 룰 전수 검토 — 조건 수정, 면제 정리, subType 구현, 룰 통합.
Changes
non-layout-containerfixed-size-in-auto-layoutisVisualOnlyNode기반missing-size-constraintmissing-componentdetached-instanceincludes()→ word boundary regex, exact caseraw-valueirregular-spacingdefault-name+non-semantic-nameRule Reference
전체 룰 조건/subType/면제 문서: Wiki — Rule Reference
의사결정 기록: Wiki — Decision Log
Test plan
pnpm lint— cleanpnpm test:run— 550/550 passRelated
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Changes
Documentation