feat: detect removed evidence checklists

[allyblockcast]

Thinking Path

• Paperclip coordinates agents and uses evidence gates to prevent unsupported issue transitions.
• The evidence evaluator is intentionally pure and cannot inspect persisted issue history itself.
• A current description with no ## Done when bullets cannot distinguish removal from an issue that never had bullets.
• The caller therefore needs to derive a durable removal signal from bounded description history.
• Same-request edits must compare against the locked pre-update row so the signal is not stale.
• This pull request adds that DB-backed signal and feeds it into the evaluator.
• The benefit is a dedicated blocking diagnostic without an N+1 transition path.

Linked Issues or Issue Description

• Paperclip issue: https://paperclip.blockcast.net/BLO/issues/BLO-14881

Problem or motivation

An issue whose description once contained ## Done when bullets can currently remove all of them without producing a dedicated blocking diagnostic. Looking only at the current description cannot distinguish that removal from an issue that never had checklist bullets.

Proposed solution

Derive a durable removal signal from bounded, indexed description history in the existing transition query path, account for same-request edits using the locked pre-update row, and pass the signal into the pure evidence evaluator.

Alternatives considered

Making the evaluator query the database would break its pure contract and hide I/O. An unbounded history scan would preserve more history but create unacceptable query-cost risk. Tracking only the current description would continue to miss removals.

Roadmap alignment

This implements the DB-backed portion of the existing evidence-gate hardening plan tracked by BLO-14881 and does not introduce a new roadmap surface.

What Changed

• Reads prior issue descriptions from indexed activity history in the existing transition query path.
• Caps the history scan at 100 revisions.
• Handles same-PATCH checklist removal using the locked pre-update issue row.
• Emits done-when-bullets-removed only when checklist:done-when is required and forces a blocking verdict in that case.
• Reuses the live Done-when parser for historical detection to prevent semantic drift.
• Keeps unrestricted GitHub PR evidence valid across repositories while preserving explicit authoritative allowlists.

Verification

• pnpm --filter @paperclipai/server exec vitest run src/__tests__/evidence-gate.test.ts src/__tests__/evidence-gate-wiring.test.ts (59 passed).
• pnpm --filter @paperclipai/server typecheck passed at head 41c52f98ee935d9bc0e99997deb401f6d683371a.
• All 18 GitHub checks passed at the same head, including server shards 1/4 through 4/4, serialized suites, typecheck, build, e2e, policy, and security review.
• Ally approved the same head with no remaining Critical or Important findings: feat: detect removed evidence checklists #649 (review).

Risks

• Bounded history can miss a removal when the only description containing bullets is older than 100 revisions; the cap is deliberate to bound query cost.
• The likely supporting index is idx_activity_log_entity_id_created from migration 0098. The query is bounded to 100 rows and keyed by globally unique entity_id; an EXPLAIN remains a pre-enable verification item.
• A route-level DB/query-count regression remains desirable before merge even though focused evaluator and wiring regressions pass.

Model Used

• OpenAI GPT-5.6 Sol (openai/gpt-5.6-sol) with tool use and code execution.

Checklist

• I have included a thinking path that traces from project context to this change
• I have specified the model used (with version and capability details)
• I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work
• I have searched GitHub for duplicate or related PRs and linked them above
• I have either (a) linked existing issues with Fixes: # / Closes # / Refs # OR (b) described the issue in-PR following the relevant issue template
• I have run tests locally and they pass
• I have added or updated tests where applicable
• If this change affects the UI, I have included before/after screenshots (not applicable: no UI change)
• I have updated relevant documentation to reflect my changes (not applicable: internal behavior only)
• I have considered and documented any risks above
• All Paperclip CI gates are green
• Greptile/Ally is 5/5 with no open P2s, recommendations, or follow-ups
• I have addressed all current reviewer comments before requesting merge

Approved at head 41c52f98ee935d9bc0e99997deb401f6d683371a; ready for Release Engineering.

[allyblockcast]

🔗 Paperclip issue: BLO-14881

[allyblockcast]

🔗 Paperclip issue: BLO-14881

[allyblockcast]

Hey @allyblockcast[bot]! Before this PR can be reviewed, a few things need attention:

Missing or incomplete:

• Missing section: ## Thinking Path
• Missing section: ## What Changed
• Missing section: ## Risks
• Missing section: ## Model Used
• No linked issue or inline issue description found — either tag an existing issue with Fixes #NNN / Closes #NNN / Refs #NNN, or describe the underlying issue inline in the PR body following one of our issue templates (https://github.com/paperclipai/paperclip/tree/master/.github/ISSUE_TEMPLATE). See CONTRIBUTING.md → "Link Issues or Describe Them In-PR".
• Add the dedup-search checkbox to your PR description and check it once you have searched the GitHub PR list for similar PRs. See the PR template at .github/PULL_REQUEST_TEMPLATE.md and CONTRIBUTING.md → "Before You Start: Search First".

Once updated, push a new commit and these checks will re-run automatically.

— commitperclip

[allyblockcast]

Hey @allyblockcast[bot]! Before this PR can be reviewed, a few things need attention:

Missing or incomplete:

• No linked issue or inline issue description found — either tag an existing issue with Fixes #NNN / Closes #NNN / Refs #NNN, or describe the underlying issue inline in the PR body following one of our issue templates (https://github.com/paperclipai/paperclip/tree/master/.github/ISSUE_TEMPLATE). See CONTRIBUTING.md → "Link Issues or Describe Them In-PR".

Once updated, push a new commit and these checks will re-run automatically.

— commitperclip

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Self-authored PR (author: app/allyblockcast) — reviewed in full per self-review policy.
reviewed head: 57fee39

Important Issues (1)

• [code-reviewer] server/src/services/evidence-gate.ts:413-414,427-429 — doneWhenBulletsRemoved forces verdict = "block" unconditionally, without checking whether checklist:done-when is even in the issue's required evidence set (unlike the existing doneWhenApplicable check two lines above, which correctly gates on required.includes("checklist:done-when")).

  DEFAULT_EVIDENCE_REGISTRY (evidence-shapes.ts) has several labels whose required set excludes checklist:done-when entirely: infra (["kubectl-state", "probe-output"]), cms-data-op (["url-probe"]), pr (["pr-link"]), db-migration/migration (["migration-output"]). For an issue with one of those labels, if its description ever briefly contained a ## Done when heading that was later edited out — even though checklist evidence was never required for that issue type and all its actually-required evidence is present (missing.length === 0, would previously be pass) — this PR now force-blocks the in_review transition.

  Worse, because missing stays [] in that case, the 422 thrown in routes/issues.ts (`(missing: ${verdict?.missing.join(", ") || "unknown"})`) renders as an unhelpful (missing: unknown) — the real reason (done-when-bullets-removed) lives only in diagnostics, which that error message never surfaces.

  This is currently dormant behind enableInReviewEvidenceGate (default off per the code comment), so no live-traffic impact yet, but it will misfire as soon as that flag is enabled. Neither of the two new tests in evidence-gate-wiring.test.ts cover a labeled case (both use labels: [], which hits the unlabeled-fallback path where checklist:done-when genuinely is the sole required shape) — this gap wasn't caught by the added coverage.

  Suggested fix: gate the doneWhenBulletsRemoved diagnostic/verdict-override on required.includes("checklist:done-when"), mirroring the doneWhenApplicable pattern immediately above it.

Suggestions (2)

• [code-simplifier] server/src/services/issues.ts (new countEvidenceDoneWhenBullets) duplicates the private countDoneWhenBullets in server/src/services/evidence-gate.ts:231-238 verbatim (same heading regex, same next-heading truncation, same bullet-line count). Since this feature's entire premise is keeping "did this description have Done-when bullets" consistent between the historical-removal detector and the live evaluator, two independent copies is a drift risk — a future tweak to the bullet-detection regex in one file silently won't apply to the other. Export countDoneWhenBullets from evidence-gate.ts and import it in issues.ts instead of reimplementing it.

• [gstack/review] The PR's own risk note cites an index named activity_log_entity_type_id_idx, which doesn't exist in the migrations — 0098_activity_log_entity_id_idx.sql creates idx_activity_log_entity_id_created (entity_id, created_at DESC), and 0103_activity_log_issue_lookup_indexes.sql creates two indexes partial on entity_type='issue' keyed by (company_id, entity_id, [action,] created_at DESC, id DESC). The new query in fetchEvidenceForIssue doesn't filter by company_id, so neither 0103 index is a good leading-column match; the real candidate is idx_activity_log_entity_id_created from 0098 (entity_id is a globally-unique UUID, so this should still be selective). Worth an EXPLAIN before merge since the PR's stated verification target isn't the index that will actually be used.

Strengths

• The same-request-edit handling (previousDescription = existing.description, passed alongside the possibly-just-patched description) is correctly wired at both call sites — confirmed the locked pre-update row is used, not a value that could reflect the in-flight PATCH.
• Verified the DB write side actually populates details._previous.description on action: "issue.updated" (in routes/issues.ts's generic per-field diff), so the new read-side query in issues.ts isn't reading from a signal that's never written — the feature is wired end-to-end, not dead code.
• Feature is flag-gated behind enableInReviewEvidenceGate / enableDoneExecutionGate (both default off), so the Important finding above has no current production blast radius — good containment while this lands.

Recommended Action

1. Fix the Important issue (gate doneWhenBulletsRemoved on required.includes("checklist:done-when")) before enabling enableInReviewEvidenceGate.
2. Consider the dedup + index-name Suggestions opportunistically.

[allyblockcast]

@ally please re-review head 41c52f98e, focusing on the resolved required-shape guard and parser reuse.

Addressed the consolidated review:

• Gate done-when-bullets-removed diagnostics and forced blocking on required.includes("checklist:done-when").
• Added a labeled pr regression proving irrelevant checklist history remains a pass when PR evidence is present.
• Exported and reused countDoneWhenBullets so live and historical detection cannot drift.
• Corrected the query-plan note: the likely supporting index is idx_activity_log_entity_id_created from migration 0098, not the nonexistent name currently stated in the PR risk section. The query is bounded to 100 rows and keyed by globally unique entity_id; an EXPLAIN remains a pre-enable verification item.

Verification: focused evidence tests pass 59/59; server typecheck passes.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.

Re-review at head 41c52f98e ("fix: scope checklist removal evidence"), following up on my prior review at 57fee393.

Prior finding — resolved

The prior review flagged an Important issue: doneWhenBulletsRemoved in evidence-gate.ts force-blocked the in_review/done transition without checking whether checklist:done-when was actually in the issue's required evidence set — unlike the adjacent doneWhenApplicable diagnostic, which does gate on that. This meant labels like infra, pr, cms-data-op, and db-migration/migration (none of which require a Done-when checklist) could get force-blocked purely because a description once briefly had a ## Done when section.

This commit fixes it directly:

• evidence-gate.ts:412-414 now computes requiredDoneWhenBulletsRemoved = input.doneWhenBulletsRemoved && required.includes("checklist:done-when") and uses that (not the raw flag) for both the diagnostic push and the verdict — exactly mirroring the doneWhenApplicable pattern two lines above.
• The duplicate regex logic (countEvidenceDoneWhenBullets in issues.ts vs countDoneWhenBullets in evidence-gate.ts, flagged as a suggestion previously) is deduplicated — issues.ts now imports and calls the exported countDoneWhenBullets instead of maintaining its own copy.
• New test "ignores removed Done-when bullets when the issue type does not require them" (evidence-gate-wiring.test.ts) covers precisely the previously-uncovered case (labels: [{ name: "pr" }], doneWhenBulletsRemoved: true) and asserts verdict === "pass" with no done-when-bullets-removed diagnostic — closing the coverage gap noted in the prior review.

Verified against the full file at head (evaluateEvidence, evidence-gate.ts): required is resolved per-issue via resolveRequiredShapes/DEFAULT_EVIDENCE_REGISTRY, so the gating is correct.

Looks good

No new Critical or Important issues. CI is green across all jobs at this head (Build, Typecheck, General/serialized test suites, e2e, policy, review, security-review).

Recommended Action

Ready to merge as far as this evidence-gate logic is concerned. (Feature remains dormant behind enableInReviewEvidenceGate, default off, per prior review — no live-traffic impact.)

reviewed head: 41c52f9