mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-14 00:26:28 +01:00
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - External chat messages go into a durable delivery queue. Each
conversation drains in provider order.
> - Each drain step reads the head of one conversation. The head query
filters one endpoint's deliveries by state and thread.
> - On a hosted instance, this query ran about 10,000 times at about 300
ms each. The database was busy, and request reads waited behind
background work.
> - PostgreSQL read every processed delivery of the endpoint and its
`normalized_event` value to find a few open rows.
> - This pull request finds the endpoint's open delivery IDs first and
fetches only those rows.
> - The benefit is less background database load. The selected head and
the delivery order do not change.
## Linked Issues or Issue Description
**What existing behavior does this improve?**
The inbound conversation head query
(`earliestOpenConversationDelivery`).
**Subsystem affected**
Server chat inbound delivery queue.
**Current behavior**
The query has `endpoint_id = $1 and (state in ('received', 'retry',
'processing') or (state = 'processed' and <pending inbound wake>))`, a
thread filter on `normalized_event`, and a sort on `normalized_event`
keys. No index matches the `or`. PostgreSQL scans all deliveries of the
endpoint. For each row it reads `normalized_event` to test the thread.
It also reads all of `chat_actions` to build the pending-wake set. A
busy endpoint keeps all of its processed history, so the cost grows with
that history.
**Proposed behavior**
The query selects `id in (<open delivery IDs of the endpoint>)`. That
set is a `union all` of two small reads:
- received, retry and processing deliveries of the endpoint, through the
existing `chat_deliveries_work_idx`;
- processed deliveries that an issued or processing `inbound_wakeup`
action of the same endpoint (and company) still owns.
PostgreSQL fetches only those rows by primary key, then applies the same
reaction, thread and order rules.
**Reason and benefit**
The cost no longer grows with processed history. On a local fixture with
60,000 processed deliveries, the head query reads 1,309 shared buffers
instead of 262,630.
**Breaking changes**
None. There is no schema change, migration, API change, timer or poll
change.
Related work: #17971 changed the pending-wake predicate of the same
queue. #20400 added the index for the other chat delivery statement in
the same report (`select state ...
normalized_event->'githubAutomatic'->'context'->>'deliveryId'`). That
statement already uses the #20400 index on master, so this pull request
does not change it.
## What Changed
- `delivery-ledger.ts`: add `openConversationDeliveryIds(endpointId)`.
It returns the endpoint's open delivery IDs. It selects the same set as
the old state predicate for that endpoint.
- `conversation-queue.ts`: the conversation head query uses `id in
(openConversationDeliveryIds(endpointId))` in place of the state `or`.
The thread filter, reaction exclusion, order, limit and lifecycle
dependency read do not change.
- `inbound-queue-discovery.test.ts`: add a real PostgreSQL test. It
compares the new set with the old predicate on three endpoints. It
covers each state, issued and processing wakes, duplicate wakes, wakes
of non-processed deliveries, processed, other-kind and null-delivery
actions, an action on a different endpoint of the same company, and
reactions. It also checks that the real captured head query returns the
oracle's first row.
- `benchmark-inbound-queue-discovery.ts`: add
`--endpoint-history=<rows>` (a busy endpoint with 4 KiB payloads) and
`--only=<fixture>`. Add a work budget: the treatment head query must
visit at most 100 delivery rows on the history fixtures.
- `doc/DATABASE.md`: describe the head query and the benchmark command.
## Verification
Local SQL measurements. PostgreSQL 17.10 (Postgres.app, macOS arm64),
disposable database, synthetic data, default settings with 256 MB shared
buffers. The host was under heavy load from other work, so timings are
noisy. Shared buffers and visited rows are stable. Each arm has two
warmups and five alternating samples (median [min..max]). Baseline:
`origin/master` at `8d2bf78323`.
| Fixture | Query | Baseline | Treatment | Shared buffers | Delivery
rows visited | Output |
| --- | --- | ---: | ---: | ---: | ---: | --- |
| 60,000 processed, 4 KiB | head | 290.09 ms [220.21..554.19] | 7.34 ms
[5.85..7.68] | 262,630 → 1,309 | 60,002 → 4 | Equal |
| 20,000 processed, 4 KiB | head | 276.02 ms [211.77..322.12] | 34.61 ms
[4.45..194.15] | 67,618 → 486 | 20,002 → 4 | Equal |
| 1,500 processed, 32 KiB | head | 15.88 ms [13.66..21.79] | 2.29 ms
[1.64..7.75] | 10,602 → 80 | 1,502 → 4 | Equal |
| 20 processed | head | 1.49 ms [0.92..3.66] | 1.85 ms [1.01..1.97] | 2
→ 45 | 22 → 968 | Equal |
- The deadline and ready selectors do not change. Their buffers are
identical in both arms; their timings move only with host noise.
- The small control is neutral. On a near-empty table, PostgreSQL picks
a sequential scan for the open-state branch. This costs about 45
buffers.
- The 60,000-row run passes `--require-work-budget`. The baseline
exceeds the budget (`baselineExceedsWorkBudget: true`). This is the
red/green proof. There is no timing threshold in CI.
- On the treatment, the remaining head cost is the scan of
`chat_actions` for pending wakes of the endpoint. The baseline also
reads all of `chat_actions` for its hashed subplan.
- `pnpm exec vitest run src/__tests__/inbound-queue-discovery.test.ts`
(server): 6 tests pass, including the new equivalence test.
- `chat-channels.integration.test.ts` filtered with `-t
"reorder|orders|FIFO|conversation drain|lease renewal|receipt
order|edit|lifecycle"`: 148 assembled chat tests pass. They cover Slack,
Teams, Telegram, Discord and GitHub provider-order drains, head-of-line
barriers, lease loss, lifecycle edits and wake retries.
- `pnpm exec tsc --noEmit -p tsconfig.json` (server) passes.
Reproduce:
```sh
pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-inbound-queue-discovery.ts --baseline-ref=8d2bf78323 --endpoint-history=60000 --only=endpoint-history --require-work-budget
```
Set `PAPERCLIP_TEST_POSTGRES_ADMIN_URL` to use an existing disposable
PostgreSQL server. The benchmark uses synthetic data only.
## Risks
- Query plans depend on data and settings. These local numbers do not
prove hosted request latency. A follow-up statistics window after deploy
must confirm the gain.
- The pending-wake branch still scans the endpoint's `chat_actions`. A
partial index on pending `inbound_wakeup` actions would remove that
scan. I did not add it, to avoid a migration.
- Low behavior risk. The new set equals the old predicate for each
endpoint, and the order expression is unchanged. Tests compare both on
the same rows.
## Model Used
- Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use (shell,
file edit) in Claude Code. Extended reasoning was used.
## 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
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
157 lines
12 KiB
TypeScript
157 lines
12 KiB
TypeScript
import assert from "node:assert/strict";
|
|
import { execFileSync } from "node:child_process";
|
|
import { createHash } from "node:crypto";
|
|
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 { captureInboundQuery, runQuery, seedInboundDiscovery, type QueueFactory, type ServiceFactory } from "../server/src/__tests__/helpers/inbound-discovery-fixture.js";
|
|
|
|
const ref = process.argv.find(arg => arg.startsWith("--baseline-ref="))?.slice(15);
|
|
assert.ok(ref, "Pass an explicit pre-change --baseline-ref=<commit>");
|
|
// `--endpoint-history=<rows>` adds one endpoint with a long processed history
|
|
// and 4 KiB payloads, like a busy hosted bot. `--only=<name>` selects one fixture.
|
|
const endpointHistoryArg = process.argv.find(arg => arg.startsWith("--endpoint-history="))?.slice(19);
|
|
const endpointHistory = endpointHistoryArg === undefined ? 0 : Number(endpointHistoryArg);
|
|
assert.ok(endpointHistoryArg === undefined || (Number.isSafeInteger(endpointHistory) && endpointHistory > 0), "--endpoint-history must be a positive integer");
|
|
const only = process.argv.find(arg => arg.startsWith("--only="))?.slice(7) ?? (process.argv.includes("--retained-only") ? "retained-history" : undefined);
|
|
const fixtures = [{ name: "small", history: 20, payloadBytes: 0 }, { name: "retained-history", history: 1500, payloadBytes: 32 * 1024 },
|
|
...(endpointHistory > 0 ? [{ name: "endpoint-history", history: endpointHistory, payloadBytes: 4096 }] : [])];
|
|
const selectedFixtures = fixtures.filter(value => !only || value.name === only);
|
|
assert.ok(selectedFixtures.length > 0, `No fixture matches --only=${only}; endpoint-history also needs --endpoint-history=<rows>`);
|
|
const baselineRevision = execFileSync("git", ["rev-parse", "--verify", `${ref}^{commit}`], { encoding: "utf8" }).trim();
|
|
const root = fileURLToPath(new URL("../", import.meta.url));
|
|
const sources = ["server/src/services/chat-channels/inbound/delivery-ledger.ts", "server/src/services/chat-channels/inbound/conversation-queue.ts", "server/src/services/chat-channels.ts"];
|
|
const runtimeChanges = 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(runtimeChanges.filter(name => !sources.includes(name)), [], "Baseline must not import changed runtime helpers or dependencies");
|
|
const dir = await fs.mkdtemp(path.join(root, "server/scripts/.inbound-baseline-"));
|
|
let database: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>> | undefined;
|
|
let db: Db | undefined;
|
|
const keepAlive = setInterval(() => {}, 1000);
|
|
function nodes(plan: Record<string, unknown>): Record<string, unknown>[] {
|
|
return [plan, ...((plan.Plans ?? []) as Record<string, unknown>[]).flatMap(nodes)];
|
|
}
|
|
const started = performance.now();
|
|
try {
|
|
const targets = new Map(sources.map(source => [path.join(root, source).replace(/\.ts$/, ".js"), path.join(dir, path.basename(source))]));
|
|
for (const source of sources) {
|
|
const original = path.join(root, source);
|
|
const text = execFileSync("git", ["show", `${baselineRevision}:${source}`], { encoding: "utf8" });
|
|
const rewritten = text.replace(/from "(\.{1,2}\/[^\"]+)"/g, (_match, specifier: string) => {
|
|
const resolved = path.resolve(path.dirname(original), specifier);
|
|
return `from ${JSON.stringify(targets.get(resolved) ?? resolved)}`;
|
|
});
|
|
await fs.writeFile(path.join(dir, path.basename(source)), rewritten);
|
|
}
|
|
const oldQueue = await import(pathToFileURL(path.join(dir, "conversation-queue.ts")).href) as { createChatConversationDeliveryQueue: QueueFactory };
|
|
const oldService = await import(pathToFileURL(path.join(dir, "chat-channels.ts")).href) as { chatChannelService: ServiceFactory };
|
|
database = await startEmbeddedPostgresTestDatabase("inbound-discovery-benchmark-");
|
|
db = createDb(database.connectionString, { maxConnections: 1, idleTimeoutSeconds: 0 });
|
|
const [{ version }] = await db.$client.unsafe("select version()");
|
|
const settings = await db.$client.unsafe("select name, setting from pg_settings where name in ('shared_buffers', 'work_mem', 'jit', 'jit_above_cost', 'jit_inline_above_cost', 'random_page_cost', 'max_connections') order by name");
|
|
const results = [];
|
|
const malformedReadyControls = [];
|
|
let baselineExceedsWorkBudget = false;
|
|
const budgetFailures: string[] = [];
|
|
for (const fixture of selectedFixtures) {
|
|
const f = await seedInboundDiscovery(db, fixture.history, fixture.payloadBytes);
|
|
await db.$client.unsafe("begin");
|
|
if (process.argv.includes("--jit-off")) await db.$client.unsafe("set local jit = off");
|
|
try {
|
|
const pairs = [];
|
|
for (const kind of ["deadline", "head", "ready"] as const) {
|
|
const before = await captureInboundQuery(db, f, kind, oldQueue.createChatConversationDeliveryQueue, oldService.chatChannelService);
|
|
const after = await captureInboundQuery(db, f, kind);
|
|
pairs.push({ kind, before, after });
|
|
}
|
|
const connection = await db.$client.reserve();
|
|
const timedDb = new Proxy(db, { get(target, key) { return key === "$client" ? connection : Reflect.get(target, key); } });
|
|
try {
|
|
if (process.argv.includes("--wait-for-slot")) {
|
|
process.stderr.write(`READY ${fixture.name}: database seeded, all real queries captured; press Enter to time\n`);
|
|
await new Promise<void>(resolve => { process.stdin.once("data", () => { process.stdin.pause(); resolve(); }); process.stdin.resume(); });
|
|
}
|
|
for (const { kind, before, after } of pairs) {
|
|
const samples = { before: [] as number[], after: [] as number[] };
|
|
const expected = await runQuery(timedDb, before);
|
|
assert.deepEqual(await runQuery(timedDb, after), expected, `${fixture.name}/${kind}: complete positional output differs`);
|
|
for (let n = 0; n < 7; n++) {
|
|
for (const arm of n % 2 ? ["after", "before"] as const : ["before", "after"] as const) {
|
|
const start = performance.now();
|
|
await runQuery(timedDb, arm === "before" ? before : after);
|
|
if (n >= 2) samples[arm].push(performance.now() - start);
|
|
}
|
|
}
|
|
const plans = {} as Record<"before" | "after", unknown>;
|
|
const actionLoops = {} as Record<"before" | "after", number>;
|
|
for (const arm of ["before", "after"] as const) {
|
|
const query = arm === "before" ? before : after;
|
|
const [row] = await connection.unsafe(`EXPLAIN (ANALYZE, BUFFERS, FORMAT JSON) ${query.sql}`, query.params as never[]);
|
|
plans[arm] = row["QUERY PLAN"];
|
|
actionLoops[arm] = Math.max(0, ...nodes(row["QUERY PLAN"][0].Plan).filter(node => node["Relation Name"] === "chat_actions").map(node => Number(node["Actual Loops"])));
|
|
}
|
|
if (fixture.name === "retained-history") {
|
|
const oldPlan = (plans.before as { JIT?: unknown; Plan: Record<string, number> }[])[0]!;
|
|
const newPlan = (plans.after as { JIT?: unknown; Plan: Record<string, number> }[])[0]!;
|
|
const oldBuffers = Number(oldPlan.Plan["Shared Hit Blocks"] ?? 0) + Number(oldPlan.Plan["Shared Read Blocks"] ?? 0);
|
|
const newBuffers = Number(newPlan.Plan["Shared Hit Blocks"] ?? 0) + Number(newPlan.Plan["Shared Read Blocks"] ?? 0);
|
|
baselineExceedsWorkBudget ||= Boolean(oldPlan.JIT) || (kind === "deadline" && oldBuffers > 1000);
|
|
if (kind !== "ready" && newPlan.JIT) budgetFailures.push(`${kind}: unnecessary JIT on the retained-history fixture`);
|
|
if (kind === "deadline" && newBuffers > 1000) budgetFailures.push(`${kind}: ${newBuffers} shared buffer accesses exceeds fixture budget 1000`);
|
|
}
|
|
// The conversation head must start from open work. It must not visit
|
|
// the endpoint's processed delivery history to find the few open rows.
|
|
const deliveryRowsVisited = {} as Record<"before" | "after", number>;
|
|
for (const arm of ["before", "after"] as const) {
|
|
deliveryRowsVisited[arm] = nodes((plans[arm] as { Plan: Record<string, unknown> }[])[0]!.Plan)
|
|
.filter(node => node["Relation Name"] === "chat_deliveries")
|
|
.reduce((sum, node) => sum + (Number(node["Actual Rows"] ?? 0) + Number(node["Rows Removed by Filter"] ?? 0)) * Number(node["Actual Loops"] ?? 1), 0);
|
|
}
|
|
if (kind === "head" && fixture.name !== "small") {
|
|
baselineExceedsWorkBudget ||= deliveryRowsVisited.before > 100;
|
|
if (deliveryRowsVisited.after > 100) budgetFailures.push(`head: visits ${deliveryRowsVisited.after} delivery rows on the ${fixture.name} fixture; budget 100`);
|
|
}
|
|
const median = (values: number[]) => [...values].sort((a, b) => a - b)[Math.floor(values.length / 2)]!;
|
|
results.push({ fixture, kind, equalPositionalRows: expected.length, before, after, samples, beforeMedianMs: median(samples.before), afterMedianMs: median(samples.after), actionLoops, deliveryRowsVisited, plans });
|
|
process.stderr.write(`${fixture.name}/${kind}: ${median(samples.before).toFixed(2)} -> ${median(samples.after).toFixed(2)} ms; action scans ${actionLoops.before} -> ${actionLoops.after}; delivery rows visited ${deliveryRowsVisited.before} -> ${deliveryRowsVisited.after}\n`);
|
|
}
|
|
} finally { connection.release(); }
|
|
if (fixture.name === "small") {
|
|
const unrelated = await f.delivery({ state: "filtered" });
|
|
const other = await seedInboundDiscovery(db, 0);
|
|
for (const input of [
|
|
{ scope: "same-company-unrelated-delivery", companyId: f.companyId, endpointId: f.endpointId, deliveryId: unrelated.id },
|
|
{ scope: "other-company", companyId: other.companyId, endpointId: other.endpointId, deliveryId: other.pending.id },
|
|
{ scope: "null-delivery", companyId: f.companyId, endpointId: f.endpointId, deliveryId: null },
|
|
]) {
|
|
const action = await f.action(input.deliveryId, { companyId: input.companyId, endpointId: input.endpointId, result: { retryAt: "invalid timestamp" } });
|
|
const before = await captureInboundQuery(db, f, "ready", oldQueue.createChatConversationDeliveryQueue, oldService.chatChannelService, f.pending.id);
|
|
const after = await captureInboundQuery(db, f, "ready", undefined, undefined, f.pending.id);
|
|
const outcome = async (query: typeof before) => {
|
|
await db!.$client.unsafe("savepoint malformed_probe");
|
|
try { return { rows: await runQuery(db!, query) }; }
|
|
catch (error) { await db!.$client.unsafe("rollback to savepoint malformed_probe"); return { error: (error as { code?: string }).code }; }
|
|
finally { await db!.$client.unsafe("release savepoint malformed_probe"); }
|
|
};
|
|
const oldResult = await outcome(before), newResult = await outcome(after);
|
|
assert.deepEqual(newResult, oldResult, `${input.scope}: malformed ready selection outcome differs`);
|
|
malformedReadyControls.push({ scope: input.scope, before: oldResult, after: newResult });
|
|
await db.$client.unsafe("delete from chat_actions where id = $1::uuid", [action.id]);
|
|
}
|
|
}
|
|
} finally { await db.$client.unsafe("rollback"); }
|
|
await db.$client.unsafe("delete from chat_endpoints where id = $1::uuid", [f.endpointId]);
|
|
}
|
|
const runtimeSources = sources.map(source => ({ path: source,
|
|
baselineSha256: createHash("sha256").update(execFileSync("git", ["show", `${baselineRevision}:${source}`])).digest("hex"),
|
|
treatmentDiffSha256: createHash("sha256").update(execFileSync("git", ["diff", "--no-ext-diff", baselineRevision, "--", source])).digest("hex"),
|
|
}));
|
|
console.log(JSON.stringify({ baselineRevision, treatmentRevision: execFileSync("git", ["rev-parse", "HEAD"], { encoding: "utf8" }).trim(), runtimeSources,
|
|
version, settings, jitOffControl: process.argv.includes("--jit-off"), baselineExceedsWorkBudget, budgetFailures,
|
|
malformedReadyControls, warmups: 2, timedSamplesPerArm: 5, setupAndProofMs: performance.now() - started, results }, null, 2));
|
|
if (process.argv.includes("--require-work-budget")) assert.deepEqual(budgetFailures, [], "Fixture work budget failed");
|
|
} finally {
|
|
try { await db?.$client.end({ timeout: 0 }); await database?.cleanup(); await fs.rm(dir, { recursive: true, force: true }); }
|
|
finally { clearInterval(keepAlive); }
|
|
}
|