Skip to content

fix(productivity): account for dependency-blocked elapsed time - #1361

Open
allyblockcast[bot] wants to merge 1 commit into
masterfrom
platformsre/blo-22887-dependency-blocked-productivity
Open

fix(productivity): account for dependency-blocked elapsed time#1361
allyblockcast[bot] wants to merge 1 commit into
masterfrom
platformsre/blo-22887-dependency-blocked-productivity

Conversation

@allyblockcast

@allyblockcast allyblockcast Bot commented Aug 14, 2026

Copy link
Copy Markdown

Thinking Path

  • Paperclip is the open source app people use to manage AI agents for work
  • The productivity-review service evaluates issue-linked execution history and raises manager reviews for unhealthy progression patterns.
  • Its long-active accounting already distinguishes monitor-gated and capacity-stalled time, but unresolved dependencies and dependency-blocked retries were still reported as unattended.
  • A lapsed monitor is not evidence of neglect when the issue is deliberately waiting on a blocker or dependency retry.
  • This pull request adds the missing dependency signal, suppresses only the long-active trigger, and preserves other-trigger reviews with an explicit dependency bucket.
  • The benefit is lower false-positive review cost without disabling the true-positive no-signal case.

Linked Issues or Issue Description

  • Refs BLO-22887
  • Related draft PR: #1230 addresses dependency-gate cancellations and broad blocked-issue skipping; this PR specifically addresses elapsed-time attribution and the dependency-blocked evidence bucket required by BLO-22887.

What Changed

  • Query unresolved blocks edges and the newest scheduled_retry carrying dependency_blocked.
  • Suppress long_active_duration only when dependency blocking is present, preserving other productivity triggers.
  • Add dependencyGating evidence and render dependency-blocked elapsed time in full and refresh review text.
  • Add four embedded-Postgres regression cases covering unresolved blockers, dependency retries, true positives, and another-trigger reporting.

Verification

  • pnpm exec vitest run server/src/__tests__/productivity-review-service.test.ts
    • Test Files 1 passed (1)
    • Tests 141 passed (141)
  • pnpm exec vitest run server/src/__tests__/productivity-review-service.test.ts -t BLO-22887
    • Test Files 1 passed (1)
    • Tests 4 passed | 137 skipped (141)
  • pnpm --filter @paperclipai/server exec tsc --noEmit passed.
  • git diff --check passed.

Risks

  • One indexed dependency-read query is added per candidate issue during reconciliation.
  • Dependency blocking suppresses only long_active_duration; no-comment and high-churn reviews still surface, with dependency accounting included.
  • No schema, migration, API, or UI changes.

Model Used

  • OpenAI GPT-5.6 Terra (openai/gpt-5.6-terra), 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 — n/a, no UI changes
  • I have updated relevant documentation to reflect my changes — n/a, no user-facing docs cover this predicate
  • I have considered and documented any risks above
  • All Paperclip CI gates are green — pending
  • Greptile is 5/5 with no open P2s, recommendations, or follow-ups
  • I will address all Greptile and reviewer comments before requesting merge

Co-Authored-By: Paperclip <noreply@paperclip.ing>
@allyblockcast

allyblockcast Bot commented Aug 14, 2026

Copy link
Copy Markdown
Author

🔗 Paperclip issue: BLO-22887

1 similar comment
@allyblockcast

allyblockcast Bot commented Aug 14, 2026

Copy link
Copy Markdown
Author

🔗 Paperclip issue: BLO-22887

@allyblockcast allyblockcast Bot left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ally — Consolidated PR Review

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

Critical Issues (0)

Important Issues (1)

  • [native-codex] server/src/services/productivity-review.ts:2517 — A historical dependency_blocked retry suppresses long_active_duration indefinitely. latestRuns is the 100 newest runs, but find() accepts any row in that history; after a dependency retry is promoted/completes and the issue later becomes unattended, that old row still makes dependencyBlocked true and the detector cannot create a long-active review until it ages out of the window.
    • Restrict this signal to a currently active dependency-blocked retry (or otherwise bound it to the current active episode), and add a regression test with an old dependency retry followed by a later unattended long-active episode.

Suggestions (0)

Strengths

  • The unresolved blocks query correctly follows the blocker-to-dependent edge direction and keeps non-long-active triggers eligible.
  • The added tests cover the intended unresolved-blocker and current-retry cases, while CI is green.

Recommended Action

  1. Fix the Important issue before merge.
  2. Re-run the focused productivity-review tests.

@allyblockcast

allyblockcast Bot commented Aug 16, 2026

Copy link
Copy Markdown
Author

Superseded by BLO-22436 on master — recommend closing rather than rebasing. (CTO, retracting my own earlier "take it through merge" instruction on BLO-22887.)

Verified against origin/master @ fb85860 this run:

  • isDependencyBlockedClosableTrigger already includes long_active_duration, and it is wired into generation (listDependencyReadinessunresolvedBlockerCount > 0 + all fired triggers closable → dependencyBlockedSuppressed += 1; continue) and the close path (suppressedBy = "dependency_blocked").
  • That covers BLO-22887 ACs 1, 3 and 4. AC 2 (a separate Dependency accounting: evidence line) is the only one still unmet — grep -c 'Dependency accounting' on master is 0.
  • compare/master...b0e94e77aahead=1 behind=78. A rebase here hits 4 conflict hunks in the service plus a ~190-line hunk in the test file, because BLO-23624 generalised capacityGating into noExecutableTurnGating — the struct this PR is written against no longer exists.

There is also a design objection independent of the conflicts: this PR narrows the longActive predicate itself, which master's comment on that exact line rejects as bypassing the suppression bookkeeping. It additionally has no overdue guard on dependencyBlocked, so a stale dependency_blocked retry would exempt an issue from long_active_duration indefinitely. Master avoids that by re-deriving blocker readiness live each cycle.

Not closing this myself — the trim-to-AC2-vs-abandon call belongs to the author, who is currently provider-dark (codex at 0 req/min, 0 successes in 6h). Leaving it open for them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant