tech-debt(unic-pr-review): regression fixture for dedupe-ordering catch (PR #5612) by orioltf · Pull Request #221 · unic/unic-agents-plugins

orioltf · 2026-06-05T15:23:53Z

Summary

Adds tests/dedupe-ordering-regression.test.mjs as a parity/quality-regression guard for the dedupe-ordering catch first identified in PR #5612
The fixture encodes the pattern: unconditional lastFiredPathRef.current = null placed before the cancelled early-return guard, where a same-key SSP-redirect re-fires a duplicate page_view
Five deterministic assertions: fixture ordering invariant, confidence-to-Minor bucket, parseFinding acceptance at severity: 'minor', title regex, body regex

Why

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 fail
pnpm --filter unic-pr-review typecheck — exits 0, no errors
No version bump, no plugin.json / CHANGELOG.md changes (test-only per issue acceptance criteria)
Clean-slate doctrine honored — no code from apps/claude-code/pr-review/

Closes #218

🤖 Generated with Claude Code

…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>

orioltf · 2026-06-05T15:33:53Z

🔍 Comprehensive PR Review

PR: #221
Reviewed by: 2/5 specialized agents (code-review, comment-quality)
Date: 2026-06-05

Summary

PR #221 adds dedupe-ordering-regression.test.mjs — a parity guard for the false-negative caught in PR #5612. The code is clean, CI-ready, and follows all project conventions. Both completed reviewers return APPROVE with zero CRITICAL/HIGH/MEDIUM findings.

Verdict: ✅ APPROVE

Severity	Count
🔴 CRITICAL	0
🟠 HIGH	0
🟡 MEDIUM	0
🟢 LOW	4

🟢 Low Issues (For Consideration)

All carry a keep as-is recommendation.

View 4 low-priority observations

Issue	Location	Agent	Recommendation
Test 2 duplicates `bucketBySeverity(70)` coverage from `severity-bucketer.test.mjs`	`test.mjs:84`	code-review	Keep as-is — harmless, adds fixture self-documentation
`DEDUPE_ORDERING_DIFF` and `REFERENCE_FINDING` are structurally disconnected (no runtime path cross-validates them)	`test.mjs:44–72`	code-review	Optional: add clarifying comment; or keep as-is
Title-regex `/dedupe\|duplicate\|reset\|guard/i` is permissive (hardcoded constant makes it trivially pass)	`test.mjs:103`	code-review	Keep as-is — constant-scope makes it adequate
`"ADR-0002"` reference correct but unqualified in two-tier ADR system	`test.mjs:66`	comment-quality	Keep as-is — consistent with plugin-wide convention

✅ What's Good

Five focused assertions, each testing one distinct property — ordering invariant, bucket, validator acceptance, title constraint, body constraint.
Well-documented module header: fixture origin, false-negative context (pr-review v1.4.0), and exact code pattern all documented in-file.
CI-ready: 522/522 tests pass, typecheck clean, Biome clean, no guarded paths touched so verify:changelog passes without a version bump.
Clean-slate doctrine respected: zero imports from the deprecated pr-review v1 plugin.
Inline code diagram in DEDUPE_ORDERING_DIFF makes the bug ordering immediately graspable.
Load-bearing invariant surfaced in JSDoc: "If the thresholds or validator shape change in a way that breaks this finding, the regression fixture will fail."

⚠️ Partial Coverage Note

3 of 5 review agents did not produce artifacts for this run (error-handling, test-coverage, docs-impact). Given the PR scope (single test-only file, no production code changes), this is unlikely to surface additional findings — but full coverage was not achieved.

Reviewed by Archon comprehensive-pr-review workflow — 2/5 agents completed
Artifacts: /Users/oriol.torrent/.archon/workspaces/unic/unic-agents-plugins/artifacts/runs/65f708ccf46baa91eb0697ce06e42332/review/

…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.

orioltf · 2026-06-05T15:35:37Z

⚡ Self-Fix Report (Aggressive)

Status: COMPLETE
Pushed: ✅ Changes pushed to archon/task-feature-unic-pr-review-218-dedupe-ordering-regress
Commit: cf0a311
Philosophy: Fix everything unless clearly a new concern

Fixes Applied (1 total)

Severity	Count
🔴 CRITICAL	0
🟠 HIGH	0
🟡 MEDIUM	0
🟢 LOW	1

View all fixes

✅ REFERENCE_FINDING/DEDUPE_ORDERING_DIFF disconnection (test.mjs:63) — Added JSDoc note stating REFERENCE_FINDING is the expected output shape and is not runtime-derived from DEDUPE_ORDERING_DIFF; clarifies that the fixture validates the validator, not the detector.

Tests Added

(none) — 522/522 existing tests pass

Skipped (3)

Finding	Reason
Test 2 duplicates `bucketBySeverity(70)` coverage	Reviewer explicitly recommended keep as-is; harmless, adds fixture self-documentation
Title-regex `/dedupe\|duplicate\|reset\|guard/i` is permissive	Reviewer explicitly recommended keep as-is; hardcoded constant makes assertion adequate
`"ADR-0002"` reference is unqualified	Reviewer explicitly recommended keep as-is; consistent with plugin-wide convention

Suggested Follow-up Issues

(none)

Validation

✅ Type check | ✅ Lint | ✅ Tests (522 passed)

Self-fix by Archon · aggressive mode · fixes pushed to archon/task-feature-unic-pr-review-218-dedupe-ordering-regress

…n-null assertion Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot

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.mjs to 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) parseFinding accepts the reference finding and derives severity: 'minor'.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+	suggestion:
+		'Move the reset into the cleanup function after `cancelled = true`, so it only fires on unmount—not on every render cycle.',


+const DEDUPE_ORDERING_DIFF = `\
+  useEffect(() => {
+     let cancelled = false
+    lastFiredPathRef.current = null
+
+     router.events.on('routeChangeComplete', (url) => {
+       if (cancelled) return
+       if (url === lastFiredPathRef.current) return
+       lastFiredPathRef.current = url
+       trackPageView(url)
+     })


orioltf and others added 2 commits June 5, 2026 17:23

style(unic-pr-review): fix trailing commas in assert messages (biome …

d477dd0

…format) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

simplify(unic-pr-review): remove redundant optional chaining after no…

e9faf5f

…n-null assertion Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

orioltf mentioned this pull request Jun 5, 2026

tech-debt(unic-pr-review): regression fixture for dedupe-ordering catch (PR #5612) #218

Open

3 tasks

orioltf requested a review from Copilot June 5, 2026 16:03

Copilot started reviewing on behalf of orioltf June 5, 2026 16:03 View session

Copilot AI reviewed Jun 5, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tech-debt(unic-pr-review): regression fixture for dedupe-ordering catch (PR #5612)#221

tech-debt(unic-pr-review): regression fixture for dedupe-ordering catch (PR #5612)#221
orioltf wants to merge 4 commits into
developfrom
archon/task-feature-unic-pr-review-218-dedupe-ordering-regress

orioltf commented Jun 5, 2026

Uh oh!

orioltf commented Jun 5, 2026

Uh oh!

orioltf commented Jun 5, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		suggestion:
		'Move the reset into the cleanup function after `cancelled = true`, so it only fires on unmount—not on every render cycle.',

Conversation

orioltf commented Jun 5, 2026

Summary

Why

Test plan

Uh oh!

orioltf commented Jun 5, 2026

🔍 Comprehensive PR Review

Summary

🟢 Low Issues (For Consideration)

✅ What's Good

⚠️ Partial Coverage Note

Uh oh!

orioltf commented Jun 5, 2026

⚡ Self-Fix Report (Aggressive)

Fixes Applied (1 total)

Tests Added

Skipped (3)

Suggested Follow-up Issues

Validation

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants