mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-28 20:21:50 +01:00
perf: use indexed latest-run probes for task interruption lists (#17969)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Task lists show the latest run's interruption state for each visible task. > - The old lookup applies a coalesced text expression to company run history. > - That lookup cannot use the separate binding indexes for a small task page. > - This pull request selects the newest run through those existing indexes. > - The benefit is less database work with the same interruption result. ## Linked Issues or Issue Description **What existing behavior does this improve?** The latest-run lookup used by task-list interruption enrichment. **Current behavior** The query scans company history for a coalesced native, first-class, or legacy task ID. A local two-task fixture inspects 1,506 runs to select six candidates. A recent hosted sample recorded about 110 ms per latest-run lookup. This is one part of the list handler, not its whole latency. **Proposed behavior** Probe each existing binding index for the newest candidate. Pick the newest candidate across the three binding branches. Read the same evidence fields in the same SQL statement. Preserve native precedence, first-class precedence, legacy text matching, company scope, and ordering ties. **Reason and benefit** The representative local 100-task page improves from 17.17 ms to 3.00 ms (5.72×). A modern 100-task page with 40 runs per task improves from 26.78 ms to 3.82 ms (7.02×). All positional SQL fields, row order, and full service results match. **Breaking changes** None. Stored history, API fields, visibility filtering, execution authority, and external-conversation state are unchanged. Related behavior: #16965, #17509, #17552. The duplicate search found no open PR for this query rewrite. ROADMAP.md has no duplicate planned feature. ## What Changed - Use three disjoint indexed candidate branches with a lateral newest-run selection. - Guard UUID parameters to preserve the old text matching for noncanonical input. - Keep final evidence projection in the same database snapshot. - Add real PostgreSQL work and contract regressions, a pinned-source benchmark, and a verification note. - Require the plan regression to find real run-table nodes. Record the deep-native-history follow-up without adding indexes or DDL. ## Verification - Baseline red: the real two-task lookup inspects 1,506 run rows and fails the work bound. The unchanged semantic oracle passes. - Treatment green: both PostgreSQL query regressions pass. They cover conflicting bindings, malformed JSON, empty values, noncanonical IDs, duplicates, foreign companies, missing runs, and timestamp ties across branches. - Seven paired PostgreSQL 17.11 cases pass complete positional SQL and full service equality. Each arm has two warmups and five alternating measured samples. Fixture payload sizes are random input bytes before Base64 encoding: 24 KiB becomes 32 KiB of stored text per noise field. - Representative 100 × 12 history: 17.17 ms (16.79–17.48) → 3.00 ms (2.64–4.28). Shared buffer hits: 30,033 → 3,400. - Modern 100 × 40 history: 26.78 ms (25.79–27.40) → 3.82 ms (3.47–4.78). - The benchmark command, all cases, plans, fixture limits, and feedback-loop timing are described in [the verification note](doc/plans/2026-10-27-task-interruption-query-performance.md). - Reproduce with `pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-task-interruption.ts --baseline-ref=09ad9adce2cf4742944353a3707a2f908e9bb35e`. - `pnpm -r typecheck` and `pnpm build` passed at `20d46e3a16d10cf3c5399fc1211b17db6a4afdc6`. The current head has the identical production runtime; it adds one test assertion and documentation. - The focused query, real project-access repair/retry, and presentation suites pass: 19 tests across three files. The earlier plugin-sdk loading failure is resolved by normal build setup. - `pnpm test:run` was attempted, then stopped after repeated native embedded PostgreSQL bootstrap failures. The full local suite is incomplete. All 16 affected real-DB executor cases pass when rerun with disposable PostgreSQL 17 in Docker. - Linux CI passes the full server suites on this head. One unchanged Runner close fixture failed on the first CI attempt; its source tree is identical to the passing upstream #17439 baseline. One targeted job retry passed on the original head. Fresh checks run again for the test/documentation review fix. A separate macOS diagnostic fails the unchanged close variants too; that close issue is not diagnosed or counted as a local pass here. - The original head passed 55 checks with one neutral result and two expected Storybook skips. Greptile gave 5/5. The current head adds one nonempty-plan assertion and documents the deep-history follow-up. Its focused PostgreSQL suite passes 2/2. Final head `c1db106d7ab56b530af82d2bb0556feb0eda290b` is mergeable with all CI checks complete and passing, Greptile 5/5, and no unresolved review findings; the documented per-task history limitation remains an explicit follow-up. ## Risks - More index probes add about 0.43 ms to the one-run tiny control. The maximum 1,000-task page with one modern run per task is 9.99 → 14.11 ms, a 4.12 ms regression. Dense history pages save substantially more in the local fixture. - Each branch can still walk one task's own history. The first-class and legacy probes can discard native-bound rows after index lookup. The plan note specifies a 10,000-run follow-up, migration-free alternatives, and a measured index-build budget if indexes prove necessary. This PR does not make work independent of history depth. - Local SQL results do not prove deployed HTTP gains. The affected query averaged about 110 ms in the hosted sample; its critical-path contribution can be smaller because other enrichment reads run concurrently. - The main issue-list SQL and external-conversation gates remain separate work. This PR has no migration or startup job. ## Model Used OpenAI Codex, GPT-6 family, with code execution and repository tool use. The runtime does not expose the exact model ID or context window size. ## 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>
This commit is contained in:
1 parent
356c7de9b7
commit
f69dfc0e23
5 files changed
+362
-5
No files matched your search
@@ -0,0 +1,80 @@
|
||||
import assert from "node:assert/strict";
|
||||
import { createHash } from "node:crypto";
|
||||
import { execFileSync } from "node:child_process";
|
||||
import { promises as fs } from "node:fs";
|
||||
import path from "node:path";
|
||||
import { fileURLToPath, pathToFileURL } from "node:url";
|
||||
import { createDb, startEmbeddedPostgresTestDatabase, type Db } from "../packages/db/src/index.js";
|
||||
import { taskInterruptionPresentations } from "../server/src/services/task-interruption-presentation.js";
|
||||
import { captureInterruptionQuery, seedInterruptionFixture, type CapturedQuery } from "../server/src/__tests__/helpers/task-interruption-fixture.js";
|
||||
|
||||
const startedAt = performance.now();
|
||||
const baselineRef = process.argv.find(arg => arg.startsWith("--baseline-ref="))?.slice(15);
|
||||
assert.ok(baselineRef, "Pass an explicit pre-fix --baseline-ref");
|
||||
const baselineRevision = execFileSync("git", ["rev-parse", "--verify", `${baselineRef}^{commit}`], { encoding: "utf8" }).trim();
|
||||
const modulePath = "server/src/services/task-interruption-presentation.ts";
|
||||
const changedRuntime = execFileSync("git", ["diff", "--name-only", baselineRevision, "--", "server/src/services", "packages/db/src", "packages/shared/src", "pnpm-lock.yaml"], { encoding: "utf8" }).trim().split("\n").filter(Boolean);
|
||||
assert.deepEqual(changedRuntime.filter(file => file !== modulePath), [], "Changed baseline dependency requires explicit isolation");
|
||||
const keepAlive = setInterval(() => {}, 1000);
|
||||
let database: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>> | undefined;
|
||||
let db: Db | undefined, baselineDir: string | undefined;
|
||||
try {
|
||||
const oldSource = execFileSync("git", ["show", `${baselineRevision}:${modulePath}`], { encoding: "utf8" });
|
||||
const moduleUrl = new URL(`../${modulePath}`, import.meta.url);
|
||||
const source = oldSource.replace(/from "(\.{1,2}\/[^\"]+)"/g, (_match, specifier: string) =>
|
||||
`from ${JSON.stringify(fileURLToPath(new URL(specifier, moduleUrl)))}`);
|
||||
baselineDir = await fs.mkdtemp(path.join(fileURLToPath(new URL("../server/scripts/", import.meta.url)), ".interruption-baseline-"));
|
||||
const baselineFile = path.join(baselineDir, "task-interruption.ts");
|
||||
await fs.writeFile(baselineFile, source);
|
||||
const baseline = await import(pathToFileURL(baselineFile).href) as typeof import("../server/src/services/task-interruption-presentation.js");
|
||||
database = await startEmbeddedPostgresTestDatabase("interruption-benchmark-");
|
||||
db = createDb(database.connectionString);
|
||||
const settings = await db.$client.unsafe("select version(), current_setting('shared_buffers') shared_buffers, current_setting('work_mem') work_mem, current_setting('jit') jit");
|
||||
const setupMs = performance.now() - startedAt;
|
||||
const results = [];
|
||||
for (const fixture of [
|
||||
{ selected: 1, history: 1, unrelated: 0, payloadBytes: 128 },
|
||||
{ selected: 100, history: 1, unrelated: 1200, payloadBytes: 24 * 1024 },
|
||||
{ selected: 100, history: 12, unrelated: 1200, payloadBytes: 24 * 1024 },
|
||||
{ selected: 1, history: 120, unrelated: 1200, payloadBytes: 24 * 1024 },
|
||||
{ selected: 100, history: 40, unrelated: 1200, payloadBytes: 12 * 1024 },
|
||||
{ selected: 100, history: 40, unrelated: 1200, payloadBytes: 12 * 1024, preserveLegacyBindings: false },
|
||||
{ selected: 1000, history: 1, unrelated: 1200, payloadBytes: 12 * 1024, preserveLegacyBindings: false },
|
||||
]) {
|
||||
await db.$client.begin(async client => {
|
||||
await client.unsafe("set local client_min_messages = warning");
|
||||
await client.unsafe("truncate heartbeat_runs cascade");
|
||||
});
|
||||
const seedStart = performance.now();
|
||||
const f = await seedInterruptionFixture(db, fixture);
|
||||
const seedMs = performance.now() - seedStart;
|
||||
const before = await captureInterruptionQuery(db, baseline.taskInterruptionPresentations, f.companyId, f.input);
|
||||
const after = await captureInterruptionQuery(db, taskInterruptionPresentations, f.companyId, f.input);
|
||||
const execute = async (query: CapturedQuery) => db!.$client.unsafe(query.sql, query.params as never[]).values();
|
||||
const beforeRows = await execute(before), afterRows = await execute(after);
|
||||
assert.deepEqual(afterRows, beforeRows, "Every positional SQL field and row order must match");
|
||||
assert.deepEqual(await taskInterruptionPresentations(db, f.companyId, f.input),
|
||||
await baseline.taskInterruptionPresentations(db, f.companyId, f.input), "Full service output must match");
|
||||
const samples = { baseline: [] as number[], treatment: [] as number[] };
|
||||
for (let round = 0; round < 7; round++) {
|
||||
for (const arm of round % 2 ? ["treatment", "baseline"] as const : ["baseline", "treatment"] as const) {
|
||||
const start = performance.now();
|
||||
await execute(arm === "baseline" ? before : after);
|
||||
if (round >= 2) samples[arm].push(performance.now() - start);
|
||||
}
|
||||
}
|
||||
const explain = async (query: CapturedQuery) => (await db!.$client.unsafe(`explain (analyze, buffers, format json) ${query.sql}`, query.params as never[]))[0]!["QUERY PLAN"];
|
||||
const summarize = (values: number[]) => { const sorted = [...values].sort((a, b) => a - b); return { medianMs: sorted[2], minMs: sorted[0], maxMs: sorted[4], samplesMs: values }; };
|
||||
const baselineTiming = summarize(samples.baseline), treatmentTiming = summarize(samples.treatment);
|
||||
results.push({ fixture, seedMs, returnedRows: afterRows.length, positionalEquality: true, serviceEquality: true,
|
||||
baseline: { ...baselineTiming, query: before, plan: await explain(before) },
|
||||
treatment: { ...treatmentTiming, query: after, plan: await explain(after) },
|
||||
speedup: baselineTiming.medianMs! / treatmentTiming.medianMs! });
|
||||
}
|
||||
console.log(JSON.stringify({ baselineRevision, treatmentRevision: execFileSync("git", ["rev-parse", "HEAD"], { encoding: "utf8" }).trim(),
|
||||
treatmentSourceSha256: createHash("sha256").update(await fs.readFile(fileURLToPath(new URL(`../${modulePath}`, import.meta.url)))).digest("hex"),
|
||||
changedRuntime, settings, node: process.version, setupMs, totalMs: performance.now() - startedAt, warmupsPerArm: 2, measuredSamplesPerArm: 5, results }, null, 2));
|
||||
} finally {
|
||||
try { await db?.$client.end({ timeout: 0 }); await database?.cleanup(); if (baselineDir) await fs.rm(baselineDir, { recursive: true, force: true }); }
|
||||
finally { clearInterval(keepAlive); }
|
||||
}
|
||||
Reference in new issue
Block a user