tech-debt(unic-pr-review): regression fixture for dedupe-ordering catch (PR #5612)#221
Merged
orioltf merged 6 commits intoJun 8, 2026
Conversation
…ch (PR #5612) Add tests/dedupe-ordering-regression.test.mjs as a parity/quality-regression guard. The fixture encodes the unconditional ref/state reset placed before an early-return guard pattern from AnalyticsTracker.tsx in PR #5612. Assertions verify the pattern ordering and that a reference finding at confidence 70 (Minor band, 60-79 per ADR-0002) validates correctly via parseFinding. unic-pr-review correctly surfaced this at Minor severity; pr-review v1.4.0 missed it (false negative). This fixture prevents that quality win from silently regressing as the plugin evolves. Closes #218 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…format) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Member
Author
🔍 Comprehensive PR ReviewPR: #221 SummaryPR #221 adds Verdict: ✅
🟢 Low Issues (For Consideration)All carry a keep as-is recommendation. View 4 low-priority observations
✅ What's Good
|
…disconnection in fixture Add a note to the REFERENCE_FINDING JSDoc making the fixture boundary explicit: the finding shape is the expected detector output, not runtime- derived from the diff constant. Prevents future maintainers from assuming cross-validation that does not exist at the unit level.
Member
Author
⚡ Self-Fix Report (Aggressive)Status: COMPLETE Fixes Applied (1 total)
View all fixes
Tests Added(none) — 522/522 existing tests pass Skipped (3)
Suggested Follow-up Issues(none) Validation✅ Type check | ✅ Lint | ✅ Tests (522 passed) Self-fix by Archon · aggressive mode · fixes pushed to |
…n-null assertion Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
3 tasks
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new deterministic regression/eval test fixture to lock in unic-pr-review’s expected “dedupe-ordering” finding behavior (Minor severity band, validator acceptance, and basic title/body invariants), based on the defect first adjudicated in PR #5612 and tracked in #218.
Changes:
- Introduces
dedupe-ordering-regression.test.mjsto encode the dedupe-ordering pattern as a canonical diff snippet plus a reference “ground-truth” Finding. - Adds assertions to ensure (a) reset-before-guard ordering is present in the fixture, (b) confidence 70 buckets to
minor, and (c)parseFindingaccepts the reference finding and derivesseverity: 'minor'.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address Copilot review on the dedupe-ordering regression fixture: - Make the JSDoc explicit that the reset lives in the effect-setup body (not inside the routeChangeComplete callback), and that the fix is to move it to cleanup teardown — not to relocate it into the callback. This preempts the reviewer's suggested code move, which would misrepresent the real PR #5612 defect and contradict the REFERENCE_FINDING.suggestion. - Tighten the suggestion wording: the reset fires on every effect run (gated by [router]), not "every render cycle". - Drop em dashes from authored prose per the org typography rule. No assertion or fixture-data change; 522/522 tests still pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ry in fixture From the toolkit comment-analyzer pass on PR #221: - Anchor the "38/46" line numbers as references to the original PR #5612 source, not this fixture's own lines, so a maintainer won't try to map them onto this file. - Show the routeChangeComplete callback wrapper in the depiction and indent the guard lines inside it, so "the reset (setup) runs before the guard (callback)" is legible from the depiction alone, not just the diff string below it. Documentation only; assertions and fixture data unchanged. 522/522 pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
orioltf
deleted the
archon/task-feature-unic-pr-review-218-dedupe-ordering-regress
branch
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
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
tests/dedupe-ordering-regression.test.mjsas a parity/quality-regression guard for the dedupe-ordering catch first identified in PR #5612lastFiredPathRef.current = nullplaced before thecancelledearly-return guard, where a same-key SSP-redirect re-fires a duplicatepage_viewparseFindingacceptance atseverity: 'minor', title regex, body regexWhy
unic-pr-review correctly surfaced this at Minor severity; pr-review v1.4.0 missed it entirely (false negative). This is a quality win worth locking in so it cannot silently regress as the plugin evolves.
Test plan
pnpm --filter unic-pr-review test— 522 pass (5 new cases included), 0 failpnpm --filter unic-pr-review typecheck— exits 0, no errorsplugin.json/CHANGELOG.mdchanges (test-only per issue acceptance criteria)apps/claude-code/pr-review/Closes #218
🤖 Generated with Claude Code