feat: fixture-done --dry-run + latest run auto-resolve (#30)#42
Merged
feat: fixture-done --dry-run + latest run auto-resolve (#30)#42
Conversation
Move pure functions from visual-compare.ts to visual-compare-helpers.ts: - getFigmaCachePath, isCacheFresh, inferDeviceScaleFactor, padPng, compareScreenshots - FIGMA_CACHE_DIR, FIGMA_CACHE_TTL_MS, SCALE_ROUNDING_TOLERANCE, UNITY_SCALE_TOLERANCE Tests now import real implementation instead of mirrored copies. Added 6 new inferDeviceScaleFactor tests (2x, 3x, 1x, zero, fractional). Closes #33 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…note 1. isCacheFresh: try/catch instead of existsSync+statSync (TOCTOU race) 2. compareScreenshots: floor instead of round for similarity (100 reserved for exact match only, diffPixels===0) 3. @/ alias for visual-compare-helpers imports 4. padPng: note about theoretical magenta collision 5. Remove unused existsSync import Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Codebase uses relative paths (217 files) not @/ alias (21 files). Revert review-driven alias change and update CLAUDE.md to reflect actual convention: "Use relative paths for imports (not @/* alias)". Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ents/ Previous revert (e29313c) only fixed visual-compare files. 12 files in src/agents/ still used @/ alias (25 occurrences). Convert all to relative paths to match codebase convention. All 323 tests pass, build succeeds. https://claude.ai/code/session_01F4wtDqAS91kpr3zFoF9tvN
…summary (#30) - Add resolveLatestRunDir(): auto-finds latest calibration run by fixture name - Add checkConvergence(): returns detailed summary with decision counts (applied/revised/rejected/kept, strict/lenient mode, reason string) - fixture-done: auto-resolve --run-dir from latest run when omitted - fixture-done --dry-run: show convergence judgment without moving files - Enhanced CLI output with mode, counts, and verdict - 9 new tests for resolveLatestRunDir and checkConvergence Closes #30 https://claude.ai/code/session_01F4wtDqAS91kpr3zFoF9tvN
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughRefactors alias imports to relative paths, extracts visual-compare helpers into a new module, adds run-directory utilities ( Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI
participant RunDir as RunDirectory
participant Convergence as ConvergenceCheck
participant FS as FileSystem
User->>CLI: fixture-done [--dry-run] [--force] [--run-dir]
activate CLI
CLI->>CLI: extractFixtureName(fixturePath)
alt --force set or explicit --run-dir
CLI->>Convergence: checkConvergence(explicit run-dir, options)
else
CLI->>RunDir: resolveLatestRunDir(fixtureName)
activate RunDir
RunDir->>FS: listCalibrationRuns()
RunDir-->>CLI: latestRunDir | null
deactivate RunDir
CLI->>Convergence: checkConvergence(latestRunDir, options)
end
activate Convergence
Convergence->>FS: read debate.json
Convergence->>Convergence: count decisions (applied,revised,rejected), compute kept/total, apply strict/lenient rules
Convergence-->>CLI: ConvergenceSummary {converged, mode, counts, reason}
deactivate Convergence
alt Converged
alt --dry-run
CLI->>User: "Would move files" (no FS changes)
else
CLI->>FS: move fixture files
CLI->>User: "Files moved"
end
else Not converged
CLI->>User: "Not converged" (mode, reason, suggestion for --force/--lenient-convergence)
end
deactivate CLI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Owner
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
![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: 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/cli/index.ts`:
- Around line 664-688: The error message when no run directory is found is
unclear; update the message emitted when !options.force && !runDir to indicate
the CLI attempted to auto-resolve a matching run directory but failed. Locate
the block using options.force, runDir and checkConvergence (the code that prints
the convergence summary and exits) and change the console.error text to
something like: "Error: no run directory found (auto-resolution attempted but no
matching run). Specify --run-dir, or use --force to skip check." Ensure the
message references auto-resolution and still suggests --run-dir or --force,
keeping fixturePath/log context unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5682e6d3-a366-424c-b500-8357acab54f9
📒 Files selected for processing (19)
CLAUDE.mdsrc/agents/analysis-agent.test.tssrc/agents/analysis-agent.tssrc/agents/code-renderer.tssrc/agents/contracts/analysis-agent.tssrc/agents/contracts/evaluation-agent.tssrc/agents/contracts/tuning-agent.tssrc/agents/evaluation-agent.tssrc/agents/orchestrator.test.tssrc/agents/orchestrator.tssrc/agents/report-generator.test.tssrc/agents/report-generator.tssrc/agents/run-directory.test.tssrc/agents/run-directory.tssrc/agents/tuning-agent.tssrc/cli/index.tssrc/core/engine/visual-compare-helpers.tssrc/core/engine/visual-compare.test.tssrc/core/engine/visual-compare.ts
Comment on lines
664
to
688
| if (!options.force) { | ||
| if (!options.runDir) { | ||
| console.error("Error: --run-dir required to check convergence (or use --force to skip check)"); | ||
| if (!runDir) { | ||
| console.error("Error: no run directory found. Specify --run-dir, or use --force to skip check."); | ||
| process.exit(1); | ||
| } | ||
| if (!isConverged(resolve(options.runDir), { lenient: options.lenientConvergence })) { | ||
| const debate = parseDebateResult(resolve(options.runDir)); | ||
| const summary = debate?.arbitrator?.summary ?? debate?.skipped ?? "no debate.json found"; | ||
| console.error(`Error: fixture has not converged (${summary}). Use --force to override.`); | ||
| const summary = checkConvergence(runDir, { lenient: options.lenientConvergence }); | ||
| console.log(`\nConvergence check (${summary.mode}):`); | ||
| console.log(` ${summary.reason}`); | ||
| if (summary.total > 0) { | ||
| console.log(` applied=${summary.applied} revised=${summary.revised} rejected=${summary.rejected} kept=${summary.kept}`); | ||
| } | ||
|
|
||
| if (options.dryRun) { | ||
| console.log(`\n[dry-run] Would ${summary.converged ? "move" : "NOT move"} fixture: ${fixturePath}`); | ||
| return; | ||
| } | ||
|
|
||
| if (!summary.converged) { | ||
| console.error(`\nError: fixture has not converged. Use --force to override or --lenient-convergence.`); | ||
| process.exit(1); | ||
| } | ||
| } else if (options.dryRun) { | ||
| console.log(`[dry-run] --force: would move fixture without convergence check: ${fixturePath}`); | ||
| return; | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider improving the error message for clarity.
When auto-resolution fails (no matching runs exist), the error at line 666 says "no run directory found" without indicating that auto-resolution was attempted. Users might not realize the CLI tried to find a matching run.
Consider:
if (!runDir) {
- console.error("Error: no run directory found. Specify --run-dir, or use --force to skip check.");
+ console.error(`Error: no run directory found for fixture "${fixtureName}". Specify --run-dir, or use --force to skip check.`);
process.exit(1);
}🤖 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 - 688, The error message when no run
directory is found is unclear; update the message emitted when !options.force &&
!runDir to indicate the CLI attempted to auto-resolve a matching run directory
but failed. Locate the block using options.force, runDir and checkConvergence
(the code that prints the convergence summary and exits) and change the
console.error text to something like: "Error: no run directory found
(auto-resolution attempted but no matching run). Specify --run-dir, or use
--force to skip check." Ensure the message references auto-resolution and still
suggests --run-dir or --force, keeping fixturePath/log context unchanged.
Addresses CodeRabbit review: clarifies that auto-resolution was attempted but no matching run was found for the given fixture. https://claude.ai/code/session_01F4wtDqAS91kpr3zFoF9tvN
This was referenced Mar 25, 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
fixture-done --dry-run: convergence 판정 결과만 출력, 실제 파일 이동 없음--run-dir생략 시 fixture명 기준 최신 run directory 자동 resolve새 함수
resolveLatestRunDir(fixtureName)—logs/calibration/<name>--*중 최신 반환checkConvergence(runDir, options)— 상세ConvergenceSummary반환Test plan
resolveLatestRunDir— 최신 선택, 미매칭 null, 빈 상태 null (3개)checkConvergence— 카운트 정확성, strict/lenient 분기, skipped, no debate, no arbitrator (6개)Closes #30
https://claude.ai/code/session_01F4wtDqAS91kpr3zFoF9tvN
Summary by CodeRabbit
New Features
Refactor
Tests