fix: visual-diff marked boxes snap to quiet seams - #1
Conversation
… slice glyphs The raw changed-pixel bounding box put ring edges mid-glyph (a new trailing comma's topmost pixel set the box top mid-line, striking through unchanged words — observed on mtl-hockey#1's smoke diff). Edges now grow outward to the nearest uniform-background seam (rows capped at 48px, columns at 480px so desktop crops widen to whole readable lines), stroke drawn just outside the snapped bounds; busy layouts with no seam keep the old capped behavior. Verified locally against the exact failing screenshots. README also gains the stay-public caveat for the cross-repo tooling checkout. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 22 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe visual diff job snaps changed-region boxes to quiet seams in the candidate image. It uses the snapped boxes for annotations and crops. Reports now show before and after images without displayed pixel-diff images. Tests cover edge and gap cases. The README documents the checkout token and repository visibility requirements. ChangesVisual diff annotations
Checkout trust model
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant DiffJob
participant snapBox
participant ImageReports
DiffJob->>snapBox: changed-region box and candidate image
snapBox->>DiffJob: snapped box or raw edges
DiffJob->>ImageReports: annotations, crops, and report references
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Line 67: Update the README statement about the pr-preview diff job to match
the supported access model: either document and implement a workflow_call secret
passed as the second checkout’s with.token, or remove the explicit-token
fallback and state that SiliconSaga/volundr must remain public. Ensure the
documentation does not imply the caller’s GITHUB_TOKEN can access a separate
private repository.
In `@visual-diff/diff.mjs`:
- Around line 149-160: Update the seek logic and its edge checks so a candidate
boundary is accepted only when BOX_STROKE consecutive rows or columns on the
exterior side are quiet, rather than relying on a single quiet row or column.
Ensure both insertion and deletion paths validate this exterior quiet band
before stopping, while preserving bounds handling and add coverage for one- to
three-pixel gaps.
🪄 Autofix
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4119a5b3-f68b-4a5f-a3cb-848a32212844
📒 Files selected for processing (2)
README.mdvisual-diff/diff.mjs
…erased Owner feedback on the smoke diff: magenta tint over newly-added glyphs reads as deletion. Comment and report now carry marked + before/after only; the mask PNG is still written for debugging. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
…s, honest README Edges now require the full exterior stroke band quiet (a 1px seam no longer passes; deletion's d=0 edge is validated too) with raw-edge fallback; snapping extracted to snap.mjs with node --test coverage for insertion/deletion and 1-3px gaps; README states volundr must remain public (a caller's GITHUB_TOKEN cannot read a different private repo). Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@visual-diff/snap.mjs`:
- Around line 15-37: Update isQuietRow and isQuietCol in visual-diff/snap.mjs to
require that candidate bands represent unchanged background content, rather than
accepting any uniformly colored band; compare alpha alongside RGB in the pixel
predicate. In visual-diff/snap.test.mjs lines 28-37, assert the selected stroke
band is background content and add coverage for a uniform dark unchanged bar
that must not be selected.
🪄 Autofix
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b2be563f-d0ef-4772-83c8-19cb05590030
📒 Files selected for processing (4)
visual-diff/diff.mjsvisual-diff/package.jsonvisual-diff/snap.mjsvisual-diff/snap.test.mjs
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
…iform Review round 2: a solid rule or banner interior is uniform but is content — bands now qualify only when they match the local background, estimated by 8-point majority vote on the crop perimeter (follows dark-themed pages; no majority → no snapping). Alpha included in comparisons; new test proves a stroke-thick uniform dark rule is never selected, asserted on raw pixels rather than the predicate under test. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
2 participants
Summary
Test plan
Related