Review brief

A data-retention worker is adding bulk deletion for one tenant's expired audit rows. The table changes as the worker deletes, IDs are stable and ascending, and the storage API supports both offset and cursor listing for comparison.

Behavioral contract

List expired rows in ascending id order using afterId cursor pagination; each next cursor is the final listed id, so deleting earlier rows cannot skip later ones.
Every delete repeats both tenantId and row id in its predicate to protect against stale or incorrectly scoped list results.
Run at most 10 delete calls concurrently, continue attempting the listed batch after individual failures, and stop fetching new batches after a batch has any failure.
If any delete fails, reject with the failed row IDs and count only confirmed deletions. Otherwise return the exact confirmed deletion count.

Review instructions

1Trace the collection after the first full page is deleted and inspect failure accounting.
2Anchor each finding to the narrowest useful changed line and explain the data impact.
3Recommend a direction rather than writing replacement code.
4Prioritize no more than four findings.

Learning objectives

• Choose pagination stable under deletion
• Preserve tenant scope at mutation time
• Bound concurrency and report partial failure honestly

src/expired-row-store.tsTypeScript

@@ -0,0 +1,24 @@

1+export interface ExpiredRow {

2+ id: string;

3+ tenantId: string;

4+}

6+export interface ExpiredRowStore {

7+ listExpired(options: {

8+ tenantId: string;

9+ before: Date;

10+ limit: number;

11+ offset?: number;

12+ afterId?: string;

13+ }): Promise<readonly ExpiredRow[]>;

14+

15+ deleteExpired(options: {

16+ tenantId?: string;

17+ id: string;

18+ before: Date;

19+ }): Promise<void>;

20+}

21+

22+export interface DeleteSummary {

23+ deleted: number;

24+}

Reference solution

The findings below are ordered by priority.

Findings

1. Offset pagination skips rows after each successful page

Location: src/delete-expired-rows.ts:10-18 and src/delete-expired-rows.ts:33-34

Deleting the first 100 rows shifts the next records to offset zero, but the worker advances to offset 100. It skips the next 100 matching rows and can report completion while expired data remains.

Request pages with afterId, advancing the cursor to the final listed id regardless of deletions.

2. Deletes omit the tenant predicate

Location: src/delete-expired-rows.ts:25-30

Although listing is scoped, each mutation sends only a row id and cutoff. A stale, colliding, or incorrectly scoped list result can delete a row outside the requested tenant, contrary to the defense required at the mutation boundary.

Pass tenantId with every delete and require the store to include it in the delete predicate.

3. A full page launches 100 deletes at once

Location: src/delete-expired-rows.ts:24-31

Promise.allSettled receives the entire 100-row page, exceeding the maximum concurrency of 10. The worker can overload the database precisely during a retention sweep.

Process each page through a bounded worker pool of 10 while still waiting for every started delete in the page.

4. Failed deletes are counted as successes and processing continues

Location: src/delete-expired-rows.ts:24-39

The settled results are discarded, deleted increases by the whole page, and the loop fetches another page. Callers receive a successful inflated count with no failed IDs, and continued mutation makes targeted retry harder.

Count only fulfilled deletes, collect rejected row IDs, finish the already-listed batch, then reject before fetching another batch.

Reasonable non-findings

A page size of 100 is compatible with a separate concurrency limit of 10.
Promise.allSettled is useful for finishing a listed batch; the defect is ignoring its results and launching it without a limiter.
Breaking on an empty page is correct when cursor pagination is used.