chore: full calibration run + add-rule pipeline test#18
Merged
Conversation
- nested-instance-override: -5 → -3 (risk) — Critic revised; 2-case medium-confidence evidence, 60% reduction capped at midpoint - fixed-width-in-responsive-context: -4 → -2 (risk) — Approved; medium confidence, 2 cases, 50% reduction meets threshold, severity unchanged - empty-frame: -2 → -3 (missing-info) — Critic revised; direction supported, conservative midpoint, severity unchanged Source: calibration against fixtures Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- nested-instance-override: -3 → -2 (Critic revised; high confidence, 3 cases, severity kept as risk) Source: calibration agent debate session
- fixed-width-in-responsive-context: severity risk → missing-info (score -2 unchanged). Rule flags a missing design decision (no explicit breakpoint intent declared), not an imminent implementation breakage; reclassified to match information-gap semantics. 3 confirmed cases, high confidence. - nested-instance-override: severity risk → missing-info (score -2 unchanged). Overrides inside nested instances represent absent handoff context that AI must guess, not structural breakage risk; reclassified accordingly. 3 confirmed cases, high confidence. Source: calibration against fixture (cross-run evidence) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Detects sibling FRAME nodes with identical structural fingerprints (type:layoutMode:children to depth 3) that aren't componentized. Catches copy-paste patterns that missing-component misses (different names, same structure). Value is design quality signal for AI token efficiency on large pages, not rendering accuracy. - severity: suggestion, score: -3 - minRepetitions: 2, maxFingerprintDepth: 3 - Skips INSTANCE subtrees and COMPONENT_SET parents - 0% false positive rate across 6 fixtures - 13 unit tests Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
cac library passes single variadic arg as string, not array. Changed from <categories...> to <category> with Array.isArray guard. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdded a new component rule Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/core/rules/rule-config.ts (1)
123-125:⚠️ Potential issue | 🟡 MinorUpdate the component rules count comment.
The comment says "(7 rules)" but with the addition of
"repeated-frame-structure", there are now 8 component rules in this section.📝 Proposed fix
// ============================================ - // Component (7 rules) + // Component (8 rules) // ============================================🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/rules/rule-config.ts` around lines 123 - 125, Update the component rules count comment to reflect the new rule: change the comment block "// Component (7 rules)" to indicate 8 rules since "repeated-frame-structure" was added; locate the comment near the component rules header and update the count to "(8 rules)" so it matches the actual rules in the section.
🧹 Nitpick comments (1)
src/cli/index.ts (1)
664-667: Type annotation doesn't reflect runtime behavior.The parameter is typed as
string, but theArray.isArrayguard and cast tostring[]suggestcacmight pass either type at runtime. Consider using a union type to accurately reflect this behavior and improve type safety.♻️ Proposed improvement
- .action((category: string) => { - const categories = Array.isArray(category) - ? (category as string[]) - : [category]; + .action((category: string | string[]) => { + const categories = Array.isArray(category) ? category : [category];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/index.ts` around lines 664 - 667, The action callback's parameter is annotated as string but runtime uses Array.isArray, so change the parameter type to a union (category: string | string[]) in the .action handler to match actual runtime values; then remove the forced cast to string[] and narrow with the existing Array.isArray check to produce const categories = Array.isArray(category) ? category : [category] (refer to the .action callback and the category parameter in src/cli/index.ts).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/core/rules/rule-config.ts`:
- Around line 123-125: Update the component rules count comment to reflect the
new rule: change the comment block "// Component (7 rules)" to indicate 8 rules
since "repeated-frame-structure" was added; locate the comment near the
component rules header and update the count to "(8 rules)" so it matches the
actual rules in the section.
---
Nitpick comments:
In `@src/cli/index.ts`:
- Around line 664-667: The action callback's parameter is annotated as string
but runtime uses Array.isArray, so change the parameter type to a union
(category: string | string[]) in the .action handler to match actual runtime
values; then remove the forced cast to string[] and narrow with the existing
Array.isArray check to produce const categories = Array.isArray(category) ?
category : [category] (refer to the .action callback and the category parameter
in src/cli/index.ts).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 69bdeb8f-8e1a-4fb7-badf-796fd121d06d
📒 Files selected for processing (5)
src/cli/index.tssrc/core/contracts/rule.tssrc/core/rules/component/index.tssrc/core/rules/component/repeated-frame-structure.test.tssrc/core/rules/rule-config.ts
- Update component rules count comment from 7 to 8 - Fix discovery-prune-evidence action param type to string | string[] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/cli/index.ts (1)
661-672: Empty check is unreachable with the new command signature.With
<category>being a required (non-optional) positional argument, cac will not invoke the action without a value. After normalization on line 665,categorieswill always have at least one element, making thecategories.length === 0check on line 666 dead code.Additionally, changing from variadic
<categories...>to singular<category>means users can no longer prune multiple categories in one command invocation. Verify this is the intended behavior.🔧 Suggested fix: remove unreachable code
.action((category: string | string[]) => { const categories = Array.isArray(category) ? category : [category]; - if (categories.length === 0) { - console.log("No categories specified — nothing to prune."); - return; - } pruneDiscoveryEvidence(categories); console.log(`Pruned discovery evidence for categories: ${categories.join(", ")}`); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/index.ts` around lines 661 - 672, The empty-length guard is dead because the CLI uses a required positional "<category>" so after normalizing into const categories = Array.isArray(category) ? category : [category] you'll always have at least one element; remove the unreachable if (categories.length === 0) { ... } branch and its console.log, and either (A) keep the current singular behavior and call pruneDiscoveryEvidence(categories) as-is, or (B) if you want to support pruning multiple categories in one invocation, change the command signature back to "discovery-prune-evidence <categories...>" so categories can be an array—update the command declaration accordingly and ensure pruneDiscoveryEvidence accepts an array.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/cli/index.ts`:
- Around line 661-672: The empty-length guard is dead because the CLI uses a
required positional "<category>" so after normalizing into const categories =
Array.isArray(category) ? category : [category] you'll always have at least one
element; remove the unreachable if (categories.length === 0) { ... } branch and
its console.log, and either (A) keep the current singular behavior and call
pruneDiscoveryEvidence(categories) as-is, or (B) if you want to support pruning
multiple categories in one invocation, change the command signature back to
"discovery-prune-evidence <categories...>" so categories can be an array—update
the command declaration accordingly and ensure pruneDiscoveryEvidence accepts an
array.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 546c0d7e-1fba-46d9-9ccf-1ca195d242a5
📒 Files selected for processing (2)
src/cli/index.tssrc/core/rules/rule-config.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/core/rules/rule-config.ts
Required positional arg guarantees categories is never empty. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Mar 24, 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
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
/add-rule파이프라인으로repeated-frame-structure룰 구현 및 디버깅discovery-prune-evidenceCLI 버그 수정캘리브레이션 결과
적용된 rule-config.ts 변경:
nested-instance-override: -5 → -2, severity risk → missing-infofixed-width-in-responsive-context: -4 → -2, severity risk → missing-infoempty-frame: -2 → -3add-rule 파이프라인 테스트
repeated-frame-structure룰 구현:파이프라인 발견 사항:
repeated-frame-structure는 추후missing-component통합 예정 (refactor: missing-component 룰 고도화 — 이름+구조+컴포넌트 존재 여부 통합 #17)버그 수정
discovery-prune-evidenceCLI: cac이 단일 variadic arg를 string으로 전달하는 문제 수정생성된 이슈
Test plan
pnpm test:run— 261 tests passedpnpm lint— cleanpnpm build— clean🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Changes
Tests