fix(productivity-review): derive active duration from run execution, not checkout (BLO-22016)

[allyblockcast]

Thinking Path

• Paperclip is the open source app people use to manage AI agents for work.
• Productivity review watches in-progress issue work and opens review issues when an agent appears stalled or wasteful.
• long_active_duration was measuring from issue checkout / execution lock timestamps, not from actual run execution.
• A checked-out issue can sit queued behind scheduler or provider capacity for hours before any model or adapter work starts.
• That queue time was counted as active work, creating false positive productivity reviews.
• This pull request anchors active duration to started heartbeat runs instead, while preserving master’s non-live execution holder clamps.
• The benefit is that queued-never-executed work no longer looks like an assignee stall, but real long-running execution still trips the review.

Linked Issues or Issue Description

• Fixes: BLO-22016
• Related production case: BLO-18846 / run 9e49405e queued for roughly 17.75h with no tokens executed before the false-positive long_active_duration review.
• Duplicate search: searched GitHub PRs for BLO-22016; only this PR matched.

What Changed

• server/src/services/productivity-review.ts now derives the active episode start from the earliest heartbeatRuns.startedAt since the current checkout.
• Queued or scheduled runs with startedAt: null, and checked-out issues with no run rows, produce no active duration instead of accumulating checkout wall time.
• The rebase keeps master’s BLO-19848 non-live execution holder logic, so terminal, parked, silent, and resumed holder intervals are still excluded from active duration.
• server/src/__tests__/productivity-review-service.test.ts seeds explicit active runs for aged fixtures by default, with activeRun: false for queued/no-run cases.

Verification

• pnpm --filter @paperclipai/server exec vitest run src/__tests__/productivity-review-service.test.ts completed locally; embedded Postgres tests are skipped in this worktree environment.
• pnpm --filter @paperclipai/server typecheck passed.
• git diff --check origin/master...HEAD passed.

Risks

Low-to-medium. This narrows long_active_duration to execution time instead of checkout time. The main risk is undercounting an unusual in-progress issue that has no started heartbeat run, but that is the safer direction for a detector whose failure mode was firing on work that never executed. Real long-running started runs and master’s non-live holder clamps remain covered.

Model Used

• Ally/Paperclip GitHub App generated the original change; exact model ID is not recorded in the PR metadata. Codex GPT-5 resolved the rebase against current master, updated the PR body, and ran local verification.

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
• I have updated relevant documentation to reflect my changes
• I have considered and documented any risks above
• All Paperclip CI gates are green
• Greptile is 5/5 with no open P2s, recommendations, or follow-ups
• I will address all Greptile and reviewer comments before requesting merge

[allyblockcast]

🔗 Paperclip issue: BLO-18846
🔗 Paperclip issue: BLO-21825
🔗 Paperclip issue: BLO-22016

[allyblockcast]

🔗 Paperclip issue: BLO-18846
🔗 Paperclip issue: BLO-21825
🔗 Paperclip issue: BLO-22016

[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: ## Verification
• 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

[kkroo]

Reviewed the rebase and merged behavior. active duration now starts from started heartbeat runs since checkout, while the existing BLO-19848 non-live holder and resumed-park clamps remain in place. New tests cover queued-never-started, zero-run, and started-run control cases. Local server typecheck passes; embedded Postgres productivity tests are skipped in this local worktree and left to CI.

[allyblockcast]

Ally — Consolidated PR Review

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

Important Issues (1)

• [code / gstack / native-codex] server/src/services/productivity-review.ts:458 — activeExecutionEpisodeStart derives the episode boundary from latestRuns, but that collection is ordered newest-first and capped at 100 rows. Once an issue has more than 100 linked runs after checkout, the actual first started run can fall outside the sample, shifting the boundary forward or returning null if the retained rows have not started. That makes long_active_duration depend on the streak-sampling window rather than the real execution episode. Query min(heartbeat_runs.started_at) independently with the same company, agent, issue, and checkout-time scope, while retaining the bounded sample for streak evidence.

Strengths

• The queued-never-started and zero-run cases directly cover the reported false-positive shape.
• Pre-checkout runs are excluded, and the existing non-live holder clamps remain composed with the new start boundary.
• The SQL remains parameterized and scoped by company, agent, and issue.

Recommended Action

1. Derive the execution episode start independently of MAX_RUNS_FOR_STREAK and add a regression with more than 100 post-checkout runs.
2. Re-run the targeted embedded-Postgres suite and full CI.

This PR is authored by app/allyblockcast, so the Ally GitHub App cannot approve it. After the finding is resolved, this exact head must be reopened under an independent author before an App approval can satisfy review/ally-complete.