mirror of
https://github.com/alkimake/paperclip.git
synced 2026-06-15 10:30:37 +09:00
[codex] Bound productivity review recovery loops (#4948)
## Thinking Path > - Paperclip orchestrates AI agents for zero-human companies. > - The heartbeat/productivity review subsystem detects when assigned work is likely stuck or churning. > - Productivity reviews are useful, but repeated reconciliation can create noisy refresh comments or repeated review issues around the same source issue. > - That makes manager follow-up harder because the signal can get buried under duplicate review activity. > - This pull request bounds productivity review refreshes and creation loops while preserving the existing escalation path. > - The benefit is a quieter recovery loop that still surfaces stuck or high-churn work for manager attention. ## What Changed - Added refresh throttling for open productivity review issues, including a one-hour default interval and a maximum of three refresh comments per open review. - Added a rolling 24-hour creation cap so completed/closed reviews cannot immediately recreate review issues indefinitely for the same source issue. - Excluded cancelled productivity reviews from the creation cap so manager cancellations do not silently suppress future legitimate reviews. - Preserved productivity review timestamps in deterministic test paths and added targeted coverage for immediate refresh suppression, refresh caps, creation caps, and cancelled-review exclusion. ## Verification - `pnpm run preflight:workspace-links && pnpm exec vitest run server/src/__tests__/productivity-review-service.test.ts` - `pnpm exec vitest run server/src/__tests__/productivity-review-service.test.ts` - Greptile Review: 5/5 on commit `bcf25832d0ffae25890b2ee7eed112d1c2d114fe` with review threads resolved. - GitHub PR checks passed on the latest head: `policy`, `verify`, `e2e`, `Greptile Review`, and `security/snyk (cryppadotta)`. - Verified the branch is rebased onto `public-gh/master` with no conflicts. - Verified the diff does not include `pnpm-lock.yaml`, database schema changes, or migrations. ## Risks - Low-to-medium risk: this changes automation cadence for productivity reviews. A truly stuck issue may receive fewer repeated refresh comments, but the original review issue remains open and assigned for manager action. - No migration risk: this is server logic and tests only. > Checked [`ROADMAP.md`](ROADMAP.md) for overlapping planned core work; this is a targeted recovery-loop fix and does not add a new roadmap feature. ## Model Used - OpenAI Codex coding agent, GPT-5 model family, tool-using software engineering mode. Exact context window is not exposed in this runtime. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] If this change affects the UI, I have included before/after screenshots (not applicable; server-only change) - [x] I have updated relevant documentation to reflect my changes (not applicable; no user-facing docs or commands changed) - [x] I have considered and documented any risks above - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Dotta
GitHub
parent
d2dd759caa
commit
42a299fb9d
2 changed files with 266 additions and 11 deletions
|
|
@ -16,7 +16,10 @@ import {
|
|||
} from "./helpers/embedded-postgres.js";
|
||||
import { MAX_ISSUE_REQUEST_DEPTH } from "@paperclipai/shared";
|
||||
import {
|
||||
DEFAULT_PRODUCTIVITY_REVIEW_MAX_REFRESH_COMMENTS,
|
||||
DEFAULT_PRODUCTIVITY_REVIEW_NO_COMMENT_STREAK_RUNS,
|
||||
DEFAULT_PRODUCTIVITY_REVIEW_REFRESH_INTERVAL_MS,
|
||||
PRODUCTIVITY_REVIEW_REFRESH_COMMENT_PREFIX,
|
||||
PRODUCTIVITY_REVIEW_ORIGIN_KIND,
|
||||
productivityReviewService,
|
||||
} from "../services/productivity-review.ts";
|
||||
|
|
@ -165,7 +168,18 @@ describeEmbeddedPostgres("productivity review service", () => {
|
|||
.orderBy(issues.createdAt);
|
||||
}
|
||||
|
||||
it("creates exactly one manager-assigned review for a no-comment run streak and refreshes it idempotently", async () => {
|
||||
async function listRefreshComments(reviewIssueId: string) {
|
||||
return db
|
||||
.select()
|
||||
.from(issueComments)
|
||||
.where(and(
|
||||
eq(issueComments.issueId, reviewIssueId),
|
||||
sql`${issueComments.body} like ${`${PRODUCTIVITY_REVIEW_REFRESH_COMMENT_PREFIX}%`}`,
|
||||
))
|
||||
.orderBy(issueComments.createdAt);
|
||||
}
|
||||
|
||||
it("creates exactly one manager-assigned review for a no-comment run streak and rate-limits immediate refresh", async () => {
|
||||
const now = new Date("2026-04-28T12:00:00.000Z");
|
||||
const seeded = await seedAssignedIssue();
|
||||
await insertRuns({
|
||||
|
|
@ -181,7 +195,8 @@ describeEmbeddedPostgres("productivity review service", () => {
|
|||
const second = await service.reconcileProductivityReviews({ now, companyId: seeded.companyId });
|
||||
|
||||
expect(first.created).toBe(1);
|
||||
expect(second.updated).toBe(1);
|
||||
expect(second.updated).toBe(0);
|
||||
expect(second.existing).toBe(1);
|
||||
const reviews = await listProductivityReviews(seeded.companyId);
|
||||
expect(reviews).toHaveLength(1);
|
||||
expect(reviews[0]?.parentId).toBe(seeded.issueId);
|
||||
|
|
@ -191,11 +206,134 @@ describeEmbeddedPostgres("productivity review service", () => {
|
|||
expect(reviews[0]?.description).toContain("Primary trigger: `no_comment_streak`");
|
||||
expect(reviews[0]?.description).toContain("No-comment completed-run streak: 10");
|
||||
|
||||
const comments = await db
|
||||
.select()
|
||||
.from(issueComments)
|
||||
.where(eq(issueComments.issueId, reviews[0]!.id));
|
||||
expect(comments.some((comment) => comment.body.includes("Productivity review evidence refreshed"))).toBe(true);
|
||||
expect(await listRefreshComments(reviews[0]!.id)).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("refreshes open productivity reviews only once per interval and caps refresh comments", async () => {
|
||||
const now = new Date("2026-04-28T12:00:00.000Z");
|
||||
const seeded = await seedAssignedIssue();
|
||||
await insertRuns({
|
||||
companyId: seeded.companyId,
|
||||
agentId: seeded.coderId,
|
||||
issueId: seeded.issueId,
|
||||
count: DEFAULT_PRODUCTIVITY_REVIEW_NO_COMMENT_STREAK_RUNS,
|
||||
now,
|
||||
});
|
||||
|
||||
const service = productivityReviewService(db);
|
||||
await service.reconcileProductivityReviews({ now, companyId: seeded.companyId });
|
||||
const [review] = await listProductivityReviews(seeded.companyId);
|
||||
|
||||
const firstRefreshAt = new Date(now.getTime() + DEFAULT_PRODUCTIVITY_REVIEW_REFRESH_INTERVAL_MS);
|
||||
const firstRefresh = await service.reconcileProductivityReviews({
|
||||
now: firstRefreshAt,
|
||||
companyId: seeded.companyId,
|
||||
});
|
||||
const tooSoonRefresh = await service.reconcileProductivityReviews({
|
||||
now: new Date(firstRefreshAt.getTime() + 30 * 60 * 1000),
|
||||
companyId: seeded.companyId,
|
||||
});
|
||||
await service.reconcileProductivityReviews({
|
||||
now: new Date(firstRefreshAt.getTime() + DEFAULT_PRODUCTIVITY_REVIEW_REFRESH_INTERVAL_MS),
|
||||
companyId: seeded.companyId,
|
||||
});
|
||||
await service.reconcileProductivityReviews({
|
||||
now: new Date(firstRefreshAt.getTime() + 2 * DEFAULT_PRODUCTIVITY_REVIEW_REFRESH_INTERVAL_MS),
|
||||
companyId: seeded.companyId,
|
||||
});
|
||||
const cappedRefresh = await service.reconcileProductivityReviews({
|
||||
now: new Date(firstRefreshAt.getTime() + 3 * DEFAULT_PRODUCTIVITY_REVIEW_REFRESH_INTERVAL_MS),
|
||||
companyId: seeded.companyId,
|
||||
});
|
||||
|
||||
expect(firstRefresh.updated).toBe(1);
|
||||
expect(tooSoonRefresh.updated).toBe(0);
|
||||
expect(tooSoonRefresh.existing).toBe(1);
|
||||
expect(cappedRefresh.updated).toBe(0);
|
||||
expect(cappedRefresh.existing).toBe(1);
|
||||
expect(await listRefreshComments(review!.id)).toHaveLength(DEFAULT_PRODUCTIVITY_REVIEW_MAX_REFRESH_COMMENTS);
|
||||
});
|
||||
|
||||
it("caps productivity review creation per source issue in the rolling creation window", async () => {
|
||||
const now = new Date("2026-04-28T12:00:00.000Z");
|
||||
const seeded = await seedAssignedIssue();
|
||||
await insertRuns({
|
||||
companyId: seeded.companyId,
|
||||
agentId: seeded.coderId,
|
||||
issueId: seeded.issueId,
|
||||
count: DEFAULT_PRODUCTIVITY_REVIEW_NO_COMMENT_STREAK_RUNS,
|
||||
now,
|
||||
});
|
||||
await db.insert(issues).values(
|
||||
[8, 9, 10].map((hoursAgo, index) => {
|
||||
const createdAt = new Date(now.getTime() - hoursAgo * 60 * 60 * 1000);
|
||||
return {
|
||||
id: randomUUID(),
|
||||
companyId: seeded.companyId,
|
||||
title: `Completed productivity review ${index + 1}`,
|
||||
status: "done",
|
||||
priority: "high",
|
||||
originKind: PRODUCTIVITY_REVIEW_ORIGIN_KIND,
|
||||
originId: seeded.issueId,
|
||||
originFingerprint: `productivity-review:${seeded.issueId}`,
|
||||
parentId: seeded.issueId,
|
||||
issueNumber: index + 2,
|
||||
identifier: `${seeded.issuePrefix}-${index + 2}`,
|
||||
createdAt,
|
||||
updatedAt: createdAt,
|
||||
};
|
||||
}),
|
||||
);
|
||||
|
||||
const result = await productivityReviewService(db).reconcileProductivityReviews({
|
||||
now,
|
||||
companyId: seeded.companyId,
|
||||
});
|
||||
|
||||
expect(result.created).toBe(0);
|
||||
expect(result.creationCapped).toBe(1);
|
||||
expect(await listProductivityReviews(seeded.companyId)).toHaveLength(3);
|
||||
});
|
||||
|
||||
it("does not count cancelled productivity reviews toward the creation cap", async () => {
|
||||
const now = new Date("2026-04-28T12:00:00.000Z");
|
||||
const seeded = await seedAssignedIssue();
|
||||
await insertRuns({
|
||||
companyId: seeded.companyId,
|
||||
agentId: seeded.coderId,
|
||||
issueId: seeded.issueId,
|
||||
count: DEFAULT_PRODUCTIVITY_REVIEW_NO_COMMENT_STREAK_RUNS,
|
||||
now,
|
||||
});
|
||||
await db.insert(issues).values(
|
||||
[8, 9, 10].map((hoursAgo, index) => {
|
||||
const createdAt = new Date(now.getTime() - hoursAgo * 60 * 60 * 1000);
|
||||
return {
|
||||
id: randomUUID(),
|
||||
companyId: seeded.companyId,
|
||||
title: `Cancelled productivity review ${index + 1}`,
|
||||
status: "cancelled",
|
||||
priority: "high",
|
||||
originKind: PRODUCTIVITY_REVIEW_ORIGIN_KIND,
|
||||
originId: seeded.issueId,
|
||||
originFingerprint: `productivity-review:${seeded.issueId}`,
|
||||
parentId: seeded.issueId,
|
||||
issueNumber: index + 2,
|
||||
identifier: `${seeded.issuePrefix}-${index + 2}`,
|
||||
createdAt,
|
||||
updatedAt: createdAt,
|
||||
};
|
||||
}),
|
||||
);
|
||||
|
||||
const result = await productivityReviewService(db).reconcileProductivityReviews({
|
||||
now,
|
||||
companyId: seeded.companyId,
|
||||
});
|
||||
|
||||
expect(result.created).toBe(1);
|
||||
expect(result.creationCapped).toBe(0);
|
||||
expect(await listProductivityReviews(seeded.companyId)).toHaveLength(4);
|
||||
});
|
||||
|
||||
it("creates a long-active review without enabling a continuation hold", async () => {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue