perf(convex): bound unbounded owner-wide collects on hot paths

2026-07-11 14:41:31 -04:00
parent e4a7f96633
commit 9067d16734
3 changed files with 300 additions and 10 deletions
@@ -0,0 +1,273 @@
+import { convexTest } from 'convex-test';
+import { describe, expect, test } from 'vitest';
+
+import type { Id } from '../../convex/_generated/dataModel.js';
+import { api } from '../../convex/_generated/api.js';
+import schema from '../../convex/schema';
+
+const modules = import.meta.glob('../../convex/**/*.*s');
+
+const createUser = async (t: ReturnType<typeof convexTest>, email: string) =>
+  (await t.mutation(async (ctx) => {
+    return await ctx.db.insert('users', { email, name: email });
+  })) as Id<'users'>;
+
+const authed = (t: ReturnType<typeof convexTest>, userId: string) =>
+  t.withIdentity({
+    subject: `${userId}|session`,
+    issuer: 'https://convex.test',
+  });
+
+const seedSpoon = async (
+  t: ReturnType<typeof convexTest>,
+  ownerId: Id<'users'>,
+) =>
+  (await t.mutation(async (ctx) => {
+    const now = Date.now();
+    return await ctx.db.insert('spoons', {
+      ownerId,
+      name: 'Hot Path Spoon',
+      provider: 'github',
+      upstreamOwner: 'upstream',
+      upstreamRepo: 'repo',
+      upstreamDefaultBranch: 'main',
+      upstreamUrl: 'https://github.com/upstream/repo',
+      forkOwner: 'me',
+      forkRepo: 'repo-fork',
+      forkUrl: 'https://github.com/me/repo-fork',
+      visibility: 'public',
+      maintenanceMode: 'watch',
+      syncCadence: 'daily',
+      productionRefStrategy: 'default_branch',
+      status: 'active',
+      createdAt: now,
+      updatedAt: now,
+    });
+  })) as Id<'spoons'>;
+
+type CommitSeed = {
+  side: 'upstream' | 'fork';
+  committedAt: number;
+  sha: string;
+};
+
+const seedCommits = async (
+  t: ReturnType<typeof convexTest>,
+  spoonId: Id<'spoons'>,
+  ownerId: Id<'users'>,
+  commits: CommitSeed[],
+) =>
+  await t.mutation(async (ctx) => {
+    const now = Date.now();
+    for (const commit of commits) {
+      await ctx.db.insert('spoonCommits', {
+        spoonId,
+        ownerId,
+        sha: commit.sha,
+        side: commit.side,
+        message: `commit ${commit.sha}`,
+        committedAt: commit.committedAt,
+        createdAt: now,
+        updatedAt: now,
+      });
+    }
+  });
+
+describe('spoonCommits.listForSpoon (no side) bounded + merged', () => {
+  test('returns the most-recent-N across both sides, committedAt desc', async () => {
+    const t = convexTest(schema, modules);
+    const ownerId = await createUser(t, 'owner@example.com');
+    const spoonId = await seedSpoon(t, ownerId);
+
+    // Insert so that DB creation order != committedAt order, forcing the
+    // implementation to sort by committedAt (not fall back on index order).
+    // committedAt values interleave the two sides so a correct top-4 must
+    // include both sides — a naive single-side take(4) would be wrong.
+    await seedCommits(t, spoonId, ownerId, [
+      { side: 'upstream', committedAt: 500, sha: 'u500' },
+      { side: 'upstream', committedAt: 300, sha: 'u300' },
+      { side: 'upstream', committedAt: 100, sha: 'u100' },
+      { side: 'fork', committedAt: 600, sha: 'f600' },
+      { side: 'fork', committedAt: 400, sha: 'f400' },
+      { side: 'fork', committedAt: 200, sha: 'f200' },
+    ]);
+
+    const result = await authed(t, ownerId).query(
+      api.spoonCommits.listForSpoon,
+      { spoonId, limit: 4 },
+    );
+
+    expect(result).toHaveLength(4);
+    // Top-4 by committedAt desc across both sides.
+    expect(result.map((c) => c.sha)).toEqual(['f600', 'u500', 'f400', 'u300']);
+    // Both sides represented in the result.
+    expect(result.some((c) => c.side === 'upstream')).toBe(true);
+    expect(result.some((c) => c.side === 'fork')).toBe(true);
+    // Strictly descending committedAt.
+    const times = result.map((c) => c.committedAt ?? 0);
+    expect(times).toEqual([...times].sort((a, b) => b - a));
+  });
+
+  test('returns all rows when under the limit', async () => {
+    const t = convexTest(schema, modules);
+    const ownerId = await createUser(t, 'owner@example.com');
+    const spoonId = await seedSpoon(t, ownerId);
+
+    await seedCommits(t, spoonId, ownerId, [
+      { side: 'upstream', committedAt: 10, sha: 'u10' },
+      { side: 'fork', committedAt: 20, sha: 'f20' },
+    ]);
+
+    const result = await authed(t, ownerId).query(
+      api.spoonCommits.listForSpoon,
+      { spoonId, limit: 100 },
+    );
+
+    expect(result.map((c) => c.sha)).toEqual(['f20', 'u10']);
+  });
+
+  test('only returns commits for the requested spoon', async () => {
+    const t = convexTest(schema, modules);
+    const ownerId = await createUser(t, 'owner@example.com');
+    const spoonA = await seedSpoon(t, ownerId);
+    const spoonB = await seedSpoon(t, ownerId);
+
+    await seedCommits(t, spoonA, ownerId, [
+      { side: 'upstream', committedAt: 50, sha: 'a-up' },
+    ]);
+    await seedCommits(t, spoonB, ownerId, [
+      { side: 'fork', committedAt: 999, sha: 'b-fork' },
+    ]);
+
+    const result = await authed(t, ownerId).query(
+      api.spoonCommits.listForSpoon,
+      { spoonId: spoonA, limit: 100 },
+    );
+
+    expect(result.map((c) => c.sha)).toEqual(['a-up']);
+  });
+});
+
+const seedJobs = async (
+  t: ReturnType<typeof convexTest>,
+  spoonId: Id<'spoons'>,
+  ownerId: Id<'users'>,
+  count: number,
+  status:
+    | 'failed'
+    | 'cancelled'
+    | 'timed_out'
+    | 'running'
+    | 'queued' = 'failed',
+  updatedAt = 1_000,
+) =>
+  await t.mutation(async (ctx) => {
+    const now = Date.now();
+    for (let i = 0; i < count; i += 1) {
+      const requestId = await ctx.db.insert('agentRequests', {
+        spoonId,
+        ownerId,
+        prompt: 'work',
+        status: 'running',
+        createdAt: now,
+        updatedAt: now,
+      });
+      await ctx.db.insert('agentJobs', {
+        spoonId,
+        ownerId,
+        agentRequestId: requestId,
+        status,
+        prompt: 'work',
+        runtime: 'opencode',
+        baseBranch: 'main',
+        workBranch: `spoon/work-${i}`,
+        forkOwner: 'me',
+        forkRepo: 'repo-fork',
+        forkUrl: 'https://github.com/me/repo-fork',
+        upstreamOwner: 'upstream',
+        upstreamRepo: 'repo',
+        selectedSecretIds: [],
+        model: 'openai/gpt-5.1-codex',
+        reasoningEffort: 'medium',
+        createdAt: now,
+        updatedAt,
+      });
+    }
+  });
+
+describe('agentJobs.countOldWorkspaces bounded scan', () => {
+  test('caps the count at the 500-row scan bound', async () => {
+    const t = convexTest(schema, modules);
+    const ownerId = await createUser(t, 'owner@example.com');
+    const spoonId = await seedSpoon(t, ownerId);
+    // Seed more deletable jobs than the bound.
+    await seedJobs(t, spoonId, ownerId, 505, 'failed');
+
+    const count = await authed(t, ownerId).query(
+      api.agentJobs.countOldWorkspaces,
+      {},
+    );
+
+    // Bounded scan means the reported count is capped at 500 ("up to 500").
+    expect(count).toBe(500);
+  });
+
+  test('returns the exact count when under the bound', async () => {
+    const t = convexTest(schema, modules);
+    const ownerId = await createUser(t, 'owner@example.com');
+    const spoonId = await seedSpoon(t, ownerId);
+    await seedJobs(t, spoonId, ownerId, 3, 'failed');
+    // Non-deletable jobs must not be counted.
+    await seedJobs(t, spoonId, ownerId, 4, 'running');
+
+    const count = await authed(t, ownerId).query(
+      api.agentJobs.countOldWorkspaces,
+      {},
+    );
+
+    expect(count).toBe(3);
+  });
+});
+
+describe('agentJobs.deleteOldWorkspaces bounded', () => {
+  test('deletes at most the per-call max, leaving the rest', async () => {
+    const t = convexTest(schema, modules);
+    const ownerId = await createUser(t, 'owner@example.com');
+    const spoonId = await seedSpoon(t, ownerId);
+    await seedJobs(t, spoonId, ownerId, 120, 'failed');
+
+    const result = await authed(t, ownerId).mutation(
+      api.agentJobs.deleteOldWorkspaces,
+      { limit: 100 },
+    );
+
+    expect(result.deleted).toBe(100);
+
+    const remaining = await authed(t, ownerId).query(
+      api.agentJobs.countOldWorkspaces,
+      {},
+    );
+    expect(remaining).toBe(20);
+  });
+
+  test('deletes only deletable jobs and leaves active ones', async () => {
+    const t = convexTest(schema, modules);
+    const ownerId = await createUser(t, 'owner@example.com');
+    const spoonId = await seedSpoon(t, ownerId);
+    await seedJobs(t, spoonId, ownerId, 5, 'failed');
+    await seedJobs(t, spoonId, ownerId, 3, 'running');
+
+    const result = await authed(t, ownerId).mutation(
+      api.agentJobs.deleteOldWorkspaces,
+      { limit: 50 },
+    );
+
+    expect(result.deleted).toBe(5);
+
+    const stillActive = await authed(t, ownerId).query(
+      api.agentJobs.countOldWorkspaces,
+      {},
+    );
+    expect(stillActive).toBe(0);
+  });
+});