Skip to content

unic-spec-review: gate dedup posts when the existing-comment set is truncated #238

@orioltf

Description

@orioltf

Parent

Epic: #200 — unic-spec-review plugin. Follow-up to #208 (S8 — dedup + cancellable Approval Loop), surfaced during review of PR #237.

Status: ready-for-agent. Decisions below are locked (grilled via grill-with-docs, 2026-06-09). Captured in ADR-0005 and the Complete comparison term in CONTEXT.md.

Problem

The dedup-matcher decision (post / skip / flag) is computed against whatever comment set the Approval Loop hands it, with no awareness of whether that set is complete. When the Confluence comment-list fetch hits its pagination cap (or partially fails to read), the comparison silently runs against a partial set, so a Finding that is a genuine duplicate of a comment living beyond the cap is decided post and shown a clean (no-badge) entry — with the same apparent authority as a Finding compared against every comment.

This is the precise failure mode the S8 design set out to prevent (#208), and it silently breaks ADR-0002's premise: similarity against all existing comments. Truncation quietly breaks the "all".

Evidence (current code, post-#237)

  • Truncation is detected and propagated but only as far as orchestrator prose:
    • scripts/atlassian-fetch.mjsfetchConfluenceComments sets truncated = true when MAX_PAGES = 50 pagination pages are exhausted; returns { comments, truncated }.
    • commands/review-spec.md Step 10a — records COMMENTS_TRUNCATED; Step 10c appends (comment list may be incomplete - deduplication is best-effort) to the summary line.
  • The deterministic layer drops the signal:
    • scripts/lib/dedup-matcher.mjs CLI main() reads commentsRaw?.comments ?? [] and never reads truncated; it prints a bare DedupResult[].
    • matchDedup has no completeness parameter; DedupResult (decision + nearDuplicates) carries no completeness signal.
    • Result: per-Finding badges and the post/skip/flag decision are identical whether the comment set was complete or truncated.
  • The read-error path is also advisory-only: Step 10a warns Near-duplicate detection may be incomplete when errors is non-empty, but nothing gates on it either.

Locked design (grilled 2026-06-09)

# Branch Resolution
1 Granularity Incompleteness is run-level (a property of the comment set, not any Finding). Rejects per-Finding postflag downgrade and a per-Finding post-uncertain state — both smear one run-level fact across Findings and overload the flag badge ("match found" vs "couldn't look").
2 Where it becomes structural The dedup-matcher CLI emits a run-level envelope { truncated, results } instead of a bare DedupResult[]. matchDedup is unchanged — still pure, still per-Finding. The completeness axis lives one level up, in the CLI and the command, where write-gating already lives.
3 Sources of incompleteness Converge. The command computes one flag: COMPARISON_INCOMPLETE = truncated OR (errors non-empty, excluding the hard auth-stop). Both mean the same thing to the reviewer; the cause is named in the warning text for diagnostics, but drives one gate.
4 Gate UX In an incomplete run: every clean post Finding renders a [?incomplete] badge instead of a blank one, and one run-level confirmation precedes the first clean-post write. skip/flag keep their existing per-Finding gates — a real match stays real regardless of truncation. One confirm, not confirm-each: the reviewer already exercised per-Finding judgement at selection.
5 MAX_PAGES Left at 50, not configurable. The gate is correct at any cap; raising the cap only moves the boundary, never closes it. Out of scope.

See ADR-0005 for the full rationale and rejected alternatives.

Acceptance criteria

  • The dedup-matcher CLI emits a run-level envelope { truncated, results } (reading truncated from the comments object); matchDedup's signature and purity are unchanged.
  • commands/review-spec.md Step 10b parses .results off the envelope; the failure-fallback path ("dedup-matcher failed - posting without deduplication") still treats every decision as post.
  • The command computes COMPARISON_INCOMPLETE = truncated OR (errors non-empty, excluding the auth-stop), converging the two existing advisory warnings into one gate.
  • In an incomplete run: each clean post Finding shows [?incomplete] in the 10c list; a single run-level confirmation precedes the first clean-post write in 10d; skip and flag keep their existing per-Finding gates. No Finding shows unqualified post authority when the comparison was partial.
  • The write-safety invariant is preserved (bare run read-only; --post cancellable at every step incl. post-none exit).
  • node:test unit tests cover the envelope with injected { comments, truncated: true } (no live services).
  • CHANGELOG.md bullets + patch bump; verify:changelog passes. No em dash in authored text (except the mandated CHANGELOG version header).

References

Metadata

Metadata

Assignees

No one assigned

    Labels

    app:unic-spec-reviewArea: apps/claude-code/unic-spec-reviewfeatureNew capabilityp3Low priorityready-for-agentFully specified, ready for an AFK agent

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions