Skip to content

feat: expose ancestor type chain in RuleContext (#197)#199

Merged
let-sunny merged 2 commits intomainfrom
claude/review-issues-ndPdG
Mar 30, 2026
Merged

feat: expose ancestor type chain in RuleContext (#197)#199
let-sunny merged 2 commits intomainfrom
claude/review-issues-ndPdG

Conversation

@let-sunny
Copy link
Copy Markdown
Owner

@let-sunny let-sunny commented Mar 30, 2026

Summary

  • RuleContext에 ancestorTypes: string[] 추가 — 루트부터 부모까지의 노드 타입 체인
  • isInsideInstance()를 immediate parent 체크에서 full ancestor chain 체크로 개선
  • INSTANCE > FRAME > FRAME 같은 깊은 중첩에서도 정확하게 INSTANCE 서브트리 감지

Test plan

  • 전체 테스트 673개 pass
  • tsc --noEmit 타입 체크 pass
  • 실제 fixture로 분석 결과 비교 확인

Closes #197

https://claude.ai/code/session_01QreN5CXLt6o2bbKmRFYz8x

Summary by CodeRabbit

  • Bug Fixes
    • Rules now see the full ancestor-type chain, improving detection of nodes nested under particular ancestor types at any depth (not just direct parents).
  • Tests
    • Tests updated to initialize and validate ancestor-type context so rule behavior is exercised against nested ancestor scenarios.

Add ancestorTypes[] to RuleContext so rules can inspect the full ancestor
type chain instead of only the immediate parent. Update isInsideInstance
in the component rule to walk the entire chain for accurate INSTANCE
subtree detection.

https://claude.ai/code/session_01QreN5CXLt6o2bbKmRFYz8x
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 30, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 2b91727f-85a8-471e-94c1-98f7a1ba9701

📥 Commits

Reviewing files that changed from the base of the PR and between d79f955 and 28e009c.

📒 Files selected for processing (1)
  • src/core/rules/component/missing-component.test.ts

📝 Walkthrough

Walkthrough

The PR adds an ancestorTypes: string[] field to RuleContext and threads an ancestor-type chain through the rule engine traversal. The engine builds and passes ancestorTypes to per-node contexts so rules can inspect ancestor types at any depth.

Changes

Cohort / File(s) Summary
Rule Context Contract
src/core/contracts/rule.ts
Added required ancestorTypes: string[] to RuleContext (ancestor node types from root to parent, excludes current node).
Rule Engine Core
src/core/engine/rule-engine.ts
traverseAndCheck(...) signature now accepts ancestorTypes: string[]; analyze() seeds with []. Engine appends current node.type to create child ancestor chains before recursing.
Component Rule Logic
src/core/rules/component/index.ts
isInsideInstance() now reads context.ancestorTypes.includes("INSTANCE") instead of checking parent?.type, so skip logic applies if any ancestor is INSTANCE.
Tests & Test Helpers
src/core/rules/test-helpers.ts, src/core/rules/component/missing-component.test.ts, src/core/rules/rule-exceptions.test.ts
Test context builders now initialize ancestorTypes: []. The component test sets ancestorTypes: ["DOCUMENT","INSTANCE","FRAME"] for the Stage 3 skip scenario to exercise ancestor-chain behavior.

Sequence Diagram(s)

sequenceDiagram
    participant Analyzer as RuleEngine.analyze()
    participant Traverser as traverseAndCheck()
    participant Rule as Rule.check()
    participant Children as recurse into children

    Analyzer->>Traverser: call with file root, ancestorTypes = []
    Traverser->>Rule: build RuleContext (ancestorTypes from param) and invoke rule checks
    Rule-->>Traverser: returns findings (if any)
    Traverser->>Traverser: append current node.type -> childAncestorTypes
    Traverser->>Children: recurse with childAncestorTypes
    Children-->>Traverser: child results
    Traverser-->>Analyzer: aggregated AnalysisResult
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 I hopped the node tree, leaf to root in cheer,
Counting ancestor types now crystal clear.
No longer just parent — the whole chain I see,
Skips and checks align, from root down to me. 🥕🌿

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely describes the main change: exposing an ancestor type chain in RuleContext, which is the primary feature of this PR.
Linked Issues check ✅ Passed The PR successfully implements both objectives from issue #197: ancestor type chain is exposed via RuleContext.ancestorTypes, and the component rule is updated to check the full ancestor chain.
Out of Scope Changes check ✅ Passed All changes are within scope of issue #197 requirements. The PR modifies only the rule engine, RuleContext interface, component rule logic, and related test files to support ancestor type chain checking.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/review-issues-ndPdG

Comment @coderabbitai help to get the list of available commands and usage tips.

@let-sunny let-sunny marked this pull request as ready for review March 30, 2026 11:30
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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/component/missing-component.test.ts`:
- Around line 332-335: The test currently sets parent: instanceParent so it
still tests immediate-parent INSTANCE behavior; change the immediate parent
passed to makeContext to a non-INSTANCE node (e.g., use an existing frame/group
node like frameB or create a new node with type "FRAME" or "GROUP") and keep
"INSTANCE" only inside ancestorTypes (ancestorTypes:
["DOCUMENT","FRAME","INSTANCE"]) so the code exercises non-immediate INSTANCE
ancestry; update any variable names (replace parent: instanceParent with parent:
frameX) and ensure the test assertions remain the same to validate the new
behavior for the rule under test (referencing makeContext, parent, and
ancestorTypes).
🪄 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: 0a0f7320-3432-4c94-9fc3-620b9feef007

📥 Commits

Reviewing files that changed from the base of the PR and between 921bcd2 and d79f955.

📒 Files selected for processing (6)
  • src/core/contracts/rule.ts
  • src/core/engine/rule-engine.ts
  • src/core/rules/component/index.ts
  • src/core/rules/component/missing-component.test.ts
  • src/core/rules/rule-exceptions.test.ts
  • src/core/rules/test-helpers.ts

Change the isInsideInstance test to use a FRAME parent with INSTANCE only
in ancestorTypes, ensuring the test actually exercises full chain check.

https://claude.ai/code/session_01QreN5CXLt6o2bbKmRFYz8x
@let-sunny let-sunny merged commit 1e97d14 into main Mar 30, 2026
3 checks passed
@let-sunny let-sunny deleted the claude/review-issues-ndPdG branch March 30, 2026 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

chore: resolve ancestor type TODO in component rule

2 participants