mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-31 22:24:16 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - GitHub review tasks can retain a passive wait after a review is published. > - The wait check reads current requester, task, agent, repository, and publication bindings. > - It also transfers the complete assessment for one commit comparison. > - Large review summaries and findings add repeated database transfer. > - This pull request selects only the typed reviewed commit value. > - The wait keeps its existing checks and transfers less data. ## Linked Issues or Issue Description **What existing behavior does this improve?** The inherited GitHub review-wait check in the server. **Current behavior** The joined query selects the complete assessment JSON. The caller uses only `assessment.reviewedCommit`. **Proposed behavior** Select `assessment->'reviewedCommit'`. Keep its JSON type and the strict comparison with the head SHA. Keep all joins, filters, ordering, and publication checks. **Reason and benefit** The synthetic PostgreSQL 17 fixture reduces one result DataRow from 6,874 bytes to 423 bytes (93.85%). This measurement excludes response metadata, TLS, and TCP. It does not predict a monthly bill or deployed savings. **Breaking changes** None. No public API, stored data, schema, or execution policy changes. **Additional context** Related work: #17562 narrowed a different external-review candidate query. This PR changes the inherited published-review lookup. The duplicate search found no matching open change. ## What Changed - Select the commit as JSON, rather than text, to preserve strict comparison for legacy numeric, object, array, missing, and null values. - Add real PostgreSQL regressions for narrow responses, unchanged stored assessments, revocation, tenant and agent bindings, and a newer queued head. - Add a synthetic benchmark that captures the real baseline and treatment SQL, compares outputs, and measures raw DataRow bytes. - Document the read contract and benchmark command. ## Verification - Baseline: `663fe667d66d3863d0919a8724f1d99500feef0b`. - `vitest run server/src/__tests__/github-review-wait.test.ts`: 14 tests passed on disposable PostgreSQL 17.11. - Restoring the baseline query makes the response-size regression fail: 8,823 column-value bytes exceed the 1,000-byte bound. Restoring treatment passes. - Benchmark: `pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-github-review-wait.ts --baseline-ref=663fe667d66d3863d0919a8724f1d99500feef0b`. - The benchmark compares every unchanged SQL column and full service output. It also compares 15 JSON cases and live connection revocation. - Two warmups and seven alternating samples had overlapping latency ranges. No latency improvement is claimed. - `pnpm -r typecheck` and `pnpm build` passed locally. - Twenty existing published-review consumer regressions passed on PostgreSQL 17.11. These cover continued review, a newer head, publication state, and a connection-disable race at final commit. - Final-head focused tests passed again after the benchmark-only review fix. - Final benchmark treatment: `d94410a6e6bb83e727afdf09cff450a91a89edd6`, with an empty source diff. DataRow bytes remained 6,874 to 423. Warm medians were 3.11 ms before and 3.43 ms after, with overlapping ranges. This proves byte reduction, not a latency improvement. - A provenance check with one staged and one unstaged source edit confirmed that the benchmark records both from the documented server working directory. Those temporary edits were removed. - All final-head CI checks passed for `d94410a6e6bb83e727afdf09cff450a91a89edd6`. Greptile scored this exact commit 5/5. There are zero unresolved review threads or recommendations. The independent Paperclip review also scored this commit 5/5. - The first local embedded PostgreSQL attempt failed before assertions because the host exhausted System V semaphores. A direct `initdb` reproduced `could not create semaphores: No space left on device`. The passing focused proof used an owned disposable Docker PostgreSQL 17 container. `pnpm test:run` was not completed locally because of that host limit. The full final-head CI suite passed. - The final CI aggregate briefly reported `in_progress` after all of its steps and the workflow had completed successfully. One rerun of only that aggregate produced a terminal success. The test matrix was not rerun. - The initial frozen install hit the existing upstream patch/lock configuration mismatch. A cached non-frozen local install succeeded. The generated lock change is excluded from this PR. ## Risks The selected commit must retain its JSON type. Using `->>` could turn numeric `1` into text and accept a malformed assessment against head `1`. The regression covers this case. The complete assessment remains available for actual review and publication. Existing fresh authority checks remain in place. No migration is required. ## Model Used OpenAI GPT-6, accessed through Codex. The exact runtime model ID and context-window size were not exposed in this session. Used repository inspection, code execution, and PostgreSQL validation. ## 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 searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
75 lines
5.4 KiB
TypeScript
75 lines
5.4 KiB
TypeScript
import { createHash, randomUUID } from "node:crypto";
|
|
import { and, eq } from "drizzle-orm";
|
|
import { agents, chatActions, chatConversations, chatEndpointResources, chatEndpoints,
|
|
chatGitHubReviews, chatMessageLinks, companies, heartbeatRuns, issueComments, issues,
|
|
runIdentityContexts, toolApplications, toolConnections, type Db } from "@paperclipai/db";
|
|
import { defaultGitHubReviewPolicy } from "@paperclipai/shared";
|
|
|
|
export async function seedGitHubReviewWait(db: Db) {
|
|
const companyId = randomUUID(), agentId = randomUUID(), issueId = randomUUID(), runId = randomUUID();
|
|
const reviewRunId = randomUUID(), reviewId = randomUUID(), endpointId = randomUUID(), resourceId = randomUUID();
|
|
const conversationId = randomUUID(), commentId = randomUUID(), activeIdentityContextId = randomUUID();
|
|
const applicationId = randomUUID(), connectionId = randomUUID(), publicationId = randomUUID();
|
|
const headSha = "1".repeat(40), repository = "synthetic/repository", repositoryId = "42", pullNumber = 7;
|
|
await db.insert(companies).values({ id: companyId, name: "Synthetic review", issuePrefix: companyId });
|
|
await db.insert(agents).values({ id: agentId, companyId, name: "Reviewer", adapterType: "process" });
|
|
await db.insert(issues).values({ id: issueId, companyId, title: "Review", assigneeAgentId: agentId });
|
|
await db.insert(heartbeatRuns).values([
|
|
{ id: runId, companyId, agentId, nativeIssueId: issueId, responsibleUserId: "operator", activeIdentityContextId },
|
|
{ id: reviewRunId, companyId, agentId, nativeIssueId: issueId, status: "succeeded" },
|
|
]);
|
|
await db.insert(issueComments).values({ id: commentId, companyId, issueId, authorUserId: "operator", body: "Review" });
|
|
await db.insert(runIdentityContexts).values({ id: activeIdentityContextId, companyId, runId, revision: 1,
|
|
responsibleUserId: "operator", messageId: commentId, cause: "test", correlationId: "test", status: "accepted" });
|
|
await db.insert(toolApplications).values({ id: applicationId, companyId, name: "GitHub", type: "chat" });
|
|
await db.insert(toolConnections).values({ id: connectionId, companyId, applicationId, name: "GitHub",
|
|
uid: connectionId, transport: "chat_sdk", connectionPurpose: "channel", enabled: true, status: "active" });
|
|
await db.insert(chatEndpoints).values({ id: endpointId, companyId, connectionId, provider: "github",
|
|
publicId: endpointId, assignedAgentId: agentId });
|
|
await db.insert(chatEndpointResources).values({ id: resourceId, companyId, endpointId, type: "repository",
|
|
providerResourceId: repository, label: repository, metadata: { providerRepositoryId: repositoryId } });
|
|
await db.insert(chatConversations).values({ id: conversationId, companyId, endpointId, resourceId, issueId,
|
|
externalConversationId: repository, externalThreadId: `github:${repository}:${pullNumber}`, externalLabel: "Review" });
|
|
await db.insert(chatMessageLinks).values({ companyId, endpointId, conversationId, commentId,
|
|
providerMessageId: commentId, direction: "inbound" });
|
|
await db.insert(chatGitHubReviews).values({ id: reviewId, companyId, endpointId, issueId, runId: reviewRunId,
|
|
repositoryId, repository, pullNumber, headSha, deliveryId: reviewId, configurationRevision: 1,
|
|
state: "completed", policySnapshot: defaultGitHubReviewPolicy(),
|
|
event: { event: "mention", deliveryId: reviewId, repositoryId, repository, pullNumber, title: "Review", body: "",
|
|
baseSha: "0".repeat(40), headSha, baseBranch: "main", author: { id: "1", login: "author", isBot: false },
|
|
sender: { id: "1", login: "author" }, draft: false, labels: [] },
|
|
assessment: { reviewedCommit: headSha, score: 5, complete: true, summary: "Reviewed", findings: [] } });
|
|
await db.insert(chatActions).values({ id: publicationId, companyId, endpointId, kind: "github_review_publication",
|
|
providerActionId: `github_publication:${issueId}:assessment:${createHash("sha256").update(JSON.stringify(reviewId)).digest("hex")}`,
|
|
status: "processed" });
|
|
const [run] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, runId));
|
|
return { run: run!, companyId, agentId, issueId, runId, reviewRunId, reviewId, endpointId, resourceId,
|
|
conversationId, commentId, activeIdentityContextId, connectionId, publicationId, headSha };
|
|
}
|
|
|
|
export function setGitHubReviewAssessment(db: Db, reviewId: string, assessment: unknown) {
|
|
return db.$client.unsafe("UPDATE chat_github_reviews SET assessment = $1::jsonb WHERE id = $2::uuid",
|
|
[JSON.stringify(assessment), reviewId]);
|
|
}
|
|
|
|
export type CapturedReviewWaitQuery = { sql: string; params: unknown[] };
|
|
export function captureReviewWaitQueries(db: Db, queries: CapturedReviewWaitQuery[]): Db {
|
|
return new Proxy(db, { get(target, key) {
|
|
if (key !== "select") return Reflect.get(target, key);
|
|
return (...fields: Parameters<Db["select"]>) => {
|
|
const builder = target.select(...fields), from = builder.from.bind(builder);
|
|
builder.from = ((...args: Parameters<typeof from>) => {
|
|
const selection = from(...args), execute = selection.execute.bind(selection);
|
|
selection.execute = async () => { queries.push(selection.toSQL()); return execute(); };
|
|
return selection;
|
|
}) as typeof builder.from;
|
|
return builder;
|
|
};
|
|
}});
|
|
}
|
|
|
|
export async function revokeReviewWaitConnection(db: Db, fixture: Awaited<ReturnType<typeof seedGitHubReviewWait>>) {
|
|
await db.update(toolConnections).set({ enabled: false }).where(and(
|
|
eq(toolConnections.id, fixture.connectionId), eq(toolConnections.companyId, fixture.companyId)));
|
|
}
|