diff --git a/packages/backend/convex/schema.ts b/packages/backend/convex/schema.ts index 1e23bbc..3e2b375 100644 --- a/packages/backend/convex/schema.ts +++ b/packages/backend/convex/schema.ts @@ -186,6 +186,7 @@ const applicationTables = { updatedAt: v.number(), }) .index('by_spoon_side', ['spoonId', 'side']) + .index('by_spoon_side_committed', ['spoonId', 'side', 'committedAt']) .index('by_owner', ['ownerId']) .index('by_sha', ['spoonId', 'sha']) .index('by_committed', ['spoonId', 'committedAt']), diff --git a/packages/backend/convex/spoonCommits.ts b/packages/backend/convex/spoonCommits.ts index af4d656..8be1986 100644 --- a/packages/backend/convex/spoonCommits.ts +++ b/packages/backend/convex/spoonCommits.ts @@ -17,7 +17,7 @@ export const listForSpoon = query({ if (side) { return await ctx.db .query('spoonCommits') - .withIndex('by_spoon_side', (q) => + .withIndex('by_spoon_side_committed', (q) => q.eq('spoonId', spoonId).eq('side', side), ) .order('desc') @@ -27,14 +27,14 @@ export const listForSpoon = query({ const [upstream, fork] = await Promise.all([ ctx.db .query('spoonCommits') - .withIndex('by_spoon_side', (q) => + .withIndex('by_spoon_side_committed', (q) => q.eq('spoonId', spoonId).eq('side', 'upstream'), ) .order('desc') .take(take), ctx.db .query('spoonCommits') - .withIndex('by_spoon_side', (q) => + .withIndex('by_spoon_side_committed', (q) => q.eq('spoonId', spoonId).eq('side', 'fork'), ) .order('desc') @@ -56,7 +56,7 @@ export const listInternal = internalQuery({ handler: async (ctx, { spoonId, ownerId, side, limit }) => { const rows = await ctx.db .query('spoonCommits') - .withIndex('by_spoon_side', (q) => + .withIndex('by_spoon_side_committed', (q) => q.eq('spoonId', spoonId).eq('side', side), ) .order('desc') diff --git a/packages/backend/tests/unit/hot-paths.test.ts b/packages/backend/tests/unit/hot-paths.test.ts index 01c689d..b80c4bb 100644 --- a/packages/backend/tests/unit/hot-paths.test.ts +++ b/packages/backend/tests/unit/hot-paths.test.ts @@ -108,6 +108,33 @@ describe('spoonCommits.listForSpoon (no side) bounded + merged', () => { expect(times).toEqual([...times].sort((a, b) => b - a)); }); + test('keeps the true newest-N per side when a side exceeds the limit', async () => { + const t = convexTest(schema, modules); + const ownerId = await createUser(t, 'owner@example.com'); + const spoonId = await seedSpoon(t, ownerId); + + // One side holds MORE commits than the limit, inserted in NON-monotonic + // committedAt order (so DB creation order != committedAt order). A query + // that bounds by insertion/_creationTime order would drop the true newest + // commit (500). Only ordering by committedAt keeps the real top-3. + await seedCommits(t, spoonId, ownerId, [ + { side: 'upstream', committedAt: 500, sha: 'u500' }, + { side: 'upstream', committedAt: 100, sha: 'u100' }, + { side: 'upstream', committedAt: 200, sha: 'u200' }, + { side: 'upstream', committedAt: 300, sha: 'u300' }, + { side: 'upstream', committedAt: 400, sha: 'u400' }, + ]); + + const result = await authed(t, ownerId).query( + api.spoonCommits.listForSpoon, + { spoonId, limit: 3 }, + ); + + // True top-3 by committedAt desc. The buggy index (by_spoon_side, ordered + // by _creationTime) would instead return the last-3-inserted [400,300,200]. + expect(result.map((c) => c.sha)).toEqual(['u500', 'u400', 'u300']); + }); + test('returns all rows when under the limit', async () => { const t = convexTest(schema, modules); const ownerId = await createUser(t, 'owner@example.com'); @@ -148,6 +175,31 @@ describe('spoonCommits.listForSpoon (no side) bounded + merged', () => { }); }); +describe('spoonCommits.listForSpoon (single side) bounded', () => { + test('keeps the true newest-N when the side exceeds the limit', async () => { + const t = convexTest(schema, modules); + const ownerId = await createUser(t, 'owner@example.com'); + const spoonId = await seedSpoon(t, ownerId); + + // Non-monotonic insertion order so _creationTime order != committedAt. + await seedCommits(t, spoonId, ownerId, [ + { side: 'fork', committedAt: 500, sha: 'f500' }, + { side: 'fork', committedAt: 100, sha: 'f100' }, + { side: 'fork', committedAt: 200, sha: 'f200' }, + { side: 'fork', committedAt: 300, sha: 'f300' }, + { side: 'fork', committedAt: 400, sha: 'f400' }, + ]); + + const result = await authed(t, ownerId).query( + api.spoonCommits.listForSpoon, + { spoonId, side: 'fork', limit: 3 }, + ); + + // True top-3 by committedAt desc, not the last-3-inserted [400,300,200]. + expect(result.map((c) => c.sha)).toEqual(['f500', 'f400', 'f300']); + }); +}); + const seedJobs = async ( t: ReturnType, spoonId: Id<'spoons'>,