Skip to content

Commit c9f2389

Browse files
fix(OUT-3686): switch ORDERBY back to Id ASC for full stability
CreateTime DESC closed the common race (concurrent updates) but left a narrower one open (concurrent CREATE prepending to the front of the cursor). Id ASC closes both: Id is monotonic and immutable, so any customer added during the walk lands at the END of the cursor where we'll still encounter it. Cost: newly-created customers land on the last page instead of page 1, so drift recovery for a fresh customer in a 10k realm walks all pages (~5s) instead of hitting on page 1 (~500ms). The perf cost is bounded and only fires on local-DB-miss paths — full stability is the priority. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 1033870 commit c9f2389

2 files changed

Lines changed: 16 additions & 18 deletions

File tree

src/utils/intuitAPI.ts

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -307,19 +307,17 @@ export default class IntuitAPI {
307307
// CompanyName comparison uses the same `(value || undefined)` normalisation
308308
// as the post-filter in customer.service.ts so the two layers cannot disagree.
309309
//
310-
// ORDERBY MetaData.CreateTime DESC pins the cursor to an immutable key while
311-
// putting the newest customers on page 1. QBO's default ordering
312-
// (LastUpdatedTime DESC) is unstable — an update mid-walk shifts the row and
313-
// can push an unscanned customer past our STARTPOSITION cursor. CreateTime is
314-
// set on insert and never changes, so concurrent updates do not move
315-
// anything. Drift recovery overwhelmingly targets recently-created customers
316-
// (new to us → no local mapping), so DESC by creation time also keeps the
317-
// common case fast (page-1 hit, ~500ms vs full-realm walk).
310+
// ORDERBY Id ASC pins the cursor to a stable, append-only key. QBO's default
311+
// ordering is MetaData.LastUpdatedTime DESC — under that ordering, a customer
312+
// updated mid-walk shifts to the front and can push an unscanned row past
313+
// our STARTPOSITION cursor (false negative). Id is monotonic and immutable,
314+
// so concurrent updates do not move rows and any customer created during the
315+
// walk lands at the end of the cursor where we'll still encounter it.
318316
//
319-
// Tradeoff: a customer CREATED in QBO during the walk would prepend to the
320-
// front and slip past our cursor. This is an accepted miss class — same
321-
// shape as the parallel-webhook TOCTOU we already accept elsewhere in this
322-
// codebase, and bounded to a single ~5s window per drift-recovery call.
317+
// Tradeoff vs. CreateTime DESC: newly-created customers land on the LAST
318+
// page rather than page 1, so drift recovery for a fresh customer in a 10k
319+
// realm walks all pages (~5s) instead of hitting on page 1 (~500ms). The
320+
// perf cost is bounded and acceptable; full stability is the priority.
323321
async _getCustomerByEmail(
324322
email: string,
325323
sanitizedCompanyName: string | undefined,
@@ -336,7 +334,7 @@ export default class IntuitAPI {
336334
let startPosition = 1
337335

338336
while (true) {
339-
const customerQuery = `SELECT Id, SyncToken, Active, CompanyName, PrimaryEmailAddr FROM Customer WHERE Active IN (true, false) ORDERBY MetaData.CreateTime DESC STARTPOSITION ${startPosition} MAXRESULTS ${pageSize}`
337+
const customerQuery = `SELECT Id, SyncToken, Active, CompanyName, PrimaryEmailAddr FROM Customer WHERE Active IN (true, false) ORDERBY Id ASC STARTPOSITION ${startPosition} MAXRESULTS ${pageSize}`
340338
const qbCustomers = await this.customQuery(customerQuery)
341339
const customers = qbCustomers?.Customer ?? []
342340
if (customers.length === 0) return

test/unit/utils/intuitAPI.test.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -356,20 +356,20 @@ describe('IntuitAPI#getCustomerByEmail', () => {
356356
expect(query).toMatch(/Active IN \(true, false\)/)
357357
})
358358

359-
it('emits ORDERBY MetaData.CreateTime DESC so the cursor is stable across pages', async () => {
359+
it('emits ORDERBY Id ASC so the cursor is stable across pages', async () => {
360360
// Without an explicit order, QBO sorts by MetaData.LastUpdatedTime DESC.
361361
// A customer updated between page fetches shifts under that ordering and
362-
// can be skipped near a STARTPOSITION boundary. CreateTime is immutable,
363-
// so concurrent updates do not move rows; DESC also puts the newest
364-
// customers on page 1, which is where drift recovery typically targets.
362+
// can be skipped near a STARTPOSITION boundary. Id is monotonic and
363+
// immutable, so concurrent updates do not move rows and concurrent
364+
// creates always land at the end of the cursor.
365365
const { api, customQuery } = makeApi([
366366
{ Customer: [row('1', 'alice@example.com')] },
367367
])
368368

369369
await api.getCustomerByEmail('alice@example.com', undefined)
370370

371371
const query = customQuery.mock.calls[0][0] as string
372-
expect(query).toMatch(/ORDERBY MetaData\.CreateTime DESC/)
372+
expect(query).toMatch(/ORDERBY Id ASC/)
373373
})
374374

375375
// Company-aware matching: a Copilot client can be enrolled in multiple

0 commit comments

Comments
 (0)