feat(visual-agent): visual diff + change quantification#651
Conversation
Compare before/after route screenshots with pixelmatch, emit diff PNGs, changed-pixel percentages, and changed/unchanged/new/removed classification for the owner-led visual PR review pipeline. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Note Gittensory Gate skippedPR closed before full evaluation. No late first comment was created.
💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers. |
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
|
gittensory · advisory review Reviewed 5 changed file(s) — two independent AI reviewers. Changed files: Suggested action: ✅ Safe to merge — both reviewers found no blocking issues. Reviewer A · Suggestions
Worth double-checking
Reviewer B · Worth double-checking
|
New advisories for ws, tar, and js-yaml caused CI audit step to fail. Pin ws via overrides and refresh the lockfile so npm audit --audit-level=moderate passes. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Hi, @JSONbored Thanks |
JSONbored
left a comment
There was a problem hiding this comment.
PR #651 — feat(visual-agent): visual diff + change quantification
Author: kiannidev | CI: ✅ pass | Status: Concerns
ACTION: REQUEST CHANGES
Highlights:
- Internal tooling that produces visual-diff bytes — not a visible UI change, so the screenshot rule doesn't apply.
- Pure, deterministic, well-tested image comparison (unchanged/changed/new/removed, noise suppression, dimension mismatch, diff-image toggle).
- pixelmatch/pngjs are genuinely required by this feature (legitimate, scoped).
Change requests / reasons:
- merge conflicts
- Verify pixelmatch/pngjs (Node Buffer/zlib) are NOT imported into the Worker entry bundle — could break the deploy bundle. Confirm they're only on the agent path.
Move pixelmatch/pngjs to devDependencies for agent-path-only use, add worker entry boundary tests proving the Worker and MCP bin do not reach visual-diff, and cover the empty capture branch for coverage. Co-authored-by: Cursor <cursoragent@cursor.com>
Cover remaining visual capture summary branches and add score-breakdown API/MCP tests merged from main so CI stays above the 97% threshold. Co-authored-by: Cursor <cursoragent@cursor.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #651 +/- ##
==========================================
+ Coverage 96.66% 96.68% +0.01%
==========================================
Files 92 93 +1
Lines 13720 13769 +49
Branches 5001 5015 +14
==========================================
+ Hits 13263 13312 +49
Misses 88 88
Partials 369 369 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Add height-only mismatch, custom threshold, and removed-only summary cases so changed lines in visual-diff.ts meet the 97% patch gate. Co-authored-by: Cursor <cursoragent@cursor.com>
Use a full color swap so pixelmatch reliably detects change while still exercising the custom threshold option path. Co-authored-by: Cursor <cursoragent@cursor.com>
Replace divide-by-zero guard with Math.max and narrow measurable entries so codecov patch coverage hits 97% without dead ?? fallbacks. Co-authored-by: Cursor <cursoragent@cursor.com>
JSONbored
left a comment
There was a problem hiding this comment.
PR #651 — feat(visual-agent): visual diff + change quantification
Verdict: MERGE
Resolves issue #582: FULLY
CI: All green (validate, test x2, lint, workers, mcp, ui, security, Superagent Security Scan = success, codecov). gittensory/Context neutral.
Highlights:
- Implements exactly what #582 asks: compareRouteScreenshots + compareVisualCaptureSets using pixelmatch/pngjs, producing diff PNG bytes, per-route changed-pixel %, and changed/unchanged/new/removed classification + overall average and summary.
- Anti-alias tolerance via pixelmatch threshold and sub-threshold noise suppression via configurable changeThresholdPercent; dimension mismatch handled as deterministic "changed" (no crash).
- Strong unit tests (all four classifications, noise suppression, custom threshold, includeDiffImage toggle, width/height-only mismatch, mixed/empty summaries).
- Good safety boundary: worker-entry-boundary.test.ts asserts the Node-only deps (pixelmatch/pngjs/visual-agent) never become reachable from the Worker entry or the published MCP bin.
Concerns / required changes:
- Minor: includes test-only additions for the pre-existing /v1/scoring/explain-breakdown route and gittensory_explain_score_breakdown — tangential extra coverage, no unrelated production code. Harmless, slightly off-scope.
Completeness vs issue: Routes with real changes get a diff image + change %, unchanged marked unchanged, new/removed classified — all acceptance criteria met and tested.
|
Adding gittensor:feature label as a one-time goodwill gesture - please do NOT complete issues assigned to other people unless explicitly asked to do so, or else you will not get any points in the future. |
Summary
compareRouteScreenshotsandcompareVisualCaptureSetsinsrc/visual-agent/visual-diff.tsto compare before/after PNG captures per route usingpixelmatch.changed,unchanged,new,removed).Closes #582
Parent epic: #577
Test plan
npm run typechecktest/unit/visual-diff.test.tscover unchanged, changed, new/removed, noise suppression, dimension mismatch, and set summariesnpm run validate)Made with Cursor