fix(productivity-review): make three evidence labels true of what they count (BLO-29535)

[allyblockcast]

Thinking Path

• Paperclip is the open source app people use to manage AI agents for work
• One of its governance surfaces is the productivity review detector (server/src/services/productivity-review.ts), which samples an issue's heartbeat runs and files a manager-facing review when an agent looks stalled
• That review's whole value is an evidence block a human can read and act on correctly — the numbers are only useful if their labels describe the population actually counted
• BLO-26165 / fix(productivity-review): key no_comment_streak exclusion on invocation, not comment policy (BLO-26165) #1414 existed because a label asserted something the code did not do. Ally's review of that PR (head a38c12fe2, 0 Critical / 0 Important) found three more instances of the same class, one order of magnitude smaller
• They were deliberately not blocked on at the time: fix(productivity-review): exclude never-invoked runs from no_comment_streak (BLO-26165) #1342's defective exclusion was live in production, and each edit would have restarted a full CI cycle while that blind spot stood
• This pull request fixes all three now that the file is otherwise quiet, plus a fourth instance at the second render site
• The benefit is that a manager summing or skimming the evidence block reaches the right conclusion instead of a plausible wrong one

Linked Issues or Issue Description

• Fixes: BLO-29535
• Refs: BLO-26165 (parent) / fix(productivity-review): key no_comment_streak exclusion on invocation, not comment policy (BLO-26165) #1414 (merged as e33e40c5f) — this is the ride-along polish carried out of that review
• Dedup search: no open or merged PR touches these labels. Nearest neighbours are fix(productivity-review): key no_comment_streak exclusion on invocation, not comment policy (BLO-26165) #1414 (parent, merged) and the earlier productivity-review streak work in fix(productivity-review): exclude never-executed runs from no_comment_streak (BLO-21769) #1041 / fix(productivity-review): generalize capacity bucket to noExecutableTurnMs (BLO-23624) #1268 / fix(productivity-review): stop misattributing ccrotate_capacity scheduled_retry stalls to the assignee #1188, none of which overlap.

What Changed

Strings and comments only. No predicate, count, or trigger-selection logic is edited — isNeverInvokedRun, isNeverExecutedRun, collectEvidence's filters, and choosePrimaryTrigger are untouched.

• :3624 — commentExemptExecutedRunCount no longer claims to be the streak. The label said "counted toward the streak", but the count is taken over all of noCommentEligibleRuns (:3058) while noCommentStreak is only the prefix before the first commented run (:3061-3065). With 15 eligible exempt runs and a comment on the 4th, the block renders No-comment streak: 3 beside …counted toward the streak: 15. Both numbers were correct; the label invited reading the larger one as the streak length. Now "not excluded from the streak walk".
• :3352-3353 — the no_comment_streak trigger reason no longer says "additional" bare. Per isNeverInvokedRun's own note (:1494-1496), that population is mostly a subset of isNeverExecutedRun, so those runs are usually already inside nonExecutingRunCount. The evidence block measures the overlap via nonExecutingAlsoNeverInvokedCount, but this prose reason carries no such field — "additional" was the one place a reader summing the two counts could still double-count. The reason now states the overlap and that the counts do not sum.
• :3623 — never-invoked label now reads `logBytes` null or 0, matching the predicate (run.logBytes ?? 0) === 0. Worth noting the struct doc comment at :295 already said "null-or-zero" — only the rendered label had drifted from its own documentation.
• :3743 — same fix at the second render site (beyond the three). The notification comment rendered - Comment-policy-exempt runs that DID execute (counted) directly beneath - No-comment streak: N, where a bare "(counted)" reads as counted into that streak. Its own adjacent comment (:3744-3748) says that line exists so the notification "tells the same story as the description it summarises" — fixing only the description would have re-split exactly the pair that comment was written to keep together. No test pins this line, so it would have drifted silently.
• Six test assertions in productivity-review-service.test.ts pin these labels verbatim and were updated with them.

Verification

pnpm --filter @paperclipai/server exec vitest run \
  src/__tests__/productivity-review-service.test.ts

  Test Files  1 passed (1)
       Tests  155 passed (155)

• pnpm --filter @paperclipai/server typecheck — clean.
• Repo-wide grep confirms no other file (docs, snapshots, other suites) still carries the old strings; the only surviving hit is an explanatory comment quoting the old wording deliberately.
• No UI change, so no screenshots.

⚠️ Note for reviewers — the verifying signal on the linked issue is itself a false green. It prescribes pnpm --filter @paperclip/server vitest run …, which prints No projects matched the filters and exits 0, having run zero tests. The workspace scope is @paperclipai/server. That is the same trap the issue's own AC warns about for --reporter=basic, one line above. Use the command block above and confirm a non-zero test count.

Risks

Low risk. The diff is confined to string literals and comments in one service file plus the matching test assertions.

• No behavioural change: no predicate, count, filter, or trigger-selection path is edited, so which reviews fire and what they count is bit-for-bit unchanged. Only the words around the numbers move.
• The one non-obvious hazard is under-changing rather than over-changing: these labels are asserted verbatim by six tests, so a missed call site fails the suite rather than drifting silently. The :3743 site is the exception — it is unpinned, which is exactly why it is included here rather than left for later.
• No migration, no API surface, no config, no UI.

Model Used

Claude Opus (Anthropic), model id claude-opus-5[1m] — 1M context window, extended thinking enabled, with tool use / code execution. Run under the Paperclip heartbeat harness as agent PlatformSREEngineer.

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 — six existing assertions updated; the change is strings-only, so no new test cases are warranted
• If this change affects the UI, I have included before/after screenshots — N/A, no UI change
• I have updated relevant documentation to reflect my changes — the labels are the documentation here; in-code comments explaining each rewording are included
• I have considered and documented any risks above
• All Paperclip CI gates are green — in flight at time of writing; auto-merge is armed so this lands only on green
• Greptile is 5/5 with no open P2s, recommendations, or follow-ups — not yet run
• I will address all Greptile and reviewer comments before requesting merge

[allyblockcast]

🔗 Paperclip issue: BLO-29535
🔗 Paperclip issue: BLO-26165

[allyblockcast]

🔗 Paperclip issue: BLO-29535
🔗 Paperclip issue: BLO-26165

[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
• 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]

Ally — Consolidated PR Review

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

Critical Issues (0)

Important Issues (0)

Suggestions (0)

Strengths

• The three evidence labels now describe the populations they actually count, including null-or-zero logBytes handling and overlap with non-executing runs.
• The notification comment uses the same clarified terminology as the manager-facing description.
• The regression assertions cover each changed label at the exact rendered output sites.

Recommended Action

1. No Critical or Important issues found. The App-authored PR requires a formal COMMENTED review rather than an approval.