feat: page the enrichment queue (CMP-92) - #160
Conversation
There was a problem hiding this comment.
1 issue found across 10 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/api/test/enrichment-queue.spec.ts">
<violation number="1" location="apps/api/test/enrichment-queue.spec.ts:101">
P2: These three pagination tests don't actually exercise the limit. The `beforeAll` fixture inserts exactly one due task and one scheduled task, so `queue(1)`, `queue(10_000)` and `queue(0)` each return at most one row regardless of the service's `take` logic. Every assertion (`rows.length/scheduled.length` <= 1 or <= ENRICHMENT_PAGE_MAX) trivially passes even if `enrichment.queue` dropped its limit entirely, and the "nonsense limit as one row, never as none" test's `<= 1` assertion also passes if the implementation returns zero rows. Seed more than ENRICHMENT_PAGE_MAX due (and scheduled) rows so the cap and the "one row never none" behavior are actually verified against `total`/`scheduledTotal` rather than tautologically.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| expect(row?.line).toBe("Waiting"); | ||
| }); | ||
|
|
||
| it("hands back no more rows than the caller asked for", async () => { |
There was a problem hiding this comment.
P2: These three pagination tests don't actually exercise the limit. The beforeAll fixture inserts exactly one due task and one scheduled task, so queue(1), queue(10_000) and queue(0) each return at most one row regardless of the service's take logic. Every assertion (rows.length/scheduled.length <= 1 or <= ENRICHMENT_PAGE_MAX) trivially passes even if enrichment.queue dropped its limit entirely, and the "nonsense limit as one row, never as none" test's <= 1 assertion also passes if the implementation returns zero rows. Seed more than ENRICHMENT_PAGE_MAX due (and scheduled) rows so the cap and the "one row never none" behavior are actually verified against total/scheduledTotal rather than tautologically.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/test/enrichment-queue.spec.ts, line 101:
<comment>These three pagination tests don't actually exercise the limit. The `beforeAll` fixture inserts exactly one due task and one scheduled task, so `queue(1)`, `queue(10_000)` and `queue(0)` each return at most one row regardless of the service's `take` logic. Every assertion (`rows.length/scheduled.length` <= 1 or <= ENRICHMENT_PAGE_MAX) trivially passes even if `enrichment.queue` dropped its limit entirely, and the "nonsense limit as one row, never as none" test's `<= 1` assertion also passes if the implementation returns zero rows. Seed more than ENRICHMENT_PAGE_MAX due (and scheduled) rows so the cap and the "one row never none" behavior are actually verified against `total`/`scheduledTotal` rather than tautologically.</comment>
<file context>
@@ -96,4 +97,25 @@ describe("what the enrichment widget reads", () => {
expect(row?.line).toBe("Waiting");
});
+
+ it("hands back no more rows than the caller asked for", async () => {
+ const queue = await enrichment.queue(1);
+
</file context>
Follows #159 — three findings from that review, plus the two limits it shipped with.
The queue endpoint took no input and always read 20 rows, while the footer counted every due task. Past 20 the widget promised records it could not open. It now takes a
limit. The footer offersShow allfor rows already fetched, andLoad morefor rows the server held back.ENRICHMENT_PAGEandENRICHMENT_PAGE_MAXlive in@crm/validation/enrichment-queue, so the widget and the service cannot disagree about the maximum.The footer decision moved out of the component into
queueFooter, covered for every combination of total, fetched, shown and limit. That is where the miscount was.The PR title job died with a permissions error on a fork pull request, taking the whole check down with it. It now reports the suggested title and lets the linter run.
Verified in a browser against 32 due rows: 27 more → Show all → 12 more → Load more → footer gone, every record reachable.
🤖 Generated with Claude Code
Summary by cubic
Pages the enrichment queue so the widget can reach all due tasks. Previously the API always returned 20 rows while the footer counted all due tasks; now the API accepts a limit, and the footer offers Show all for fetched rows or Load more to request more up to the maximum.
enrichment.queuenow takes{ limit }validated by@crm/validation/enrichment-queue; the service clamps withpageSize, defaults toENRICHMENT_PAGE, and applies the limit to both due and scheduled lists. Constants live in@crm/validation/enrichment-queueto keep client and server in sync.queueFooterwith tests. UseskeepPreviousDatato hold fetched rows while increasing the limit. Shows “Show all” when more rows are already fetched and “Load more” untilENRICHMENT_PAGE_MAX; stops offering beyond the max. Resets the limit on popover close and passes{ limit }to the query.Migration/Review
trpc.enrichment.queueshould pass{ limit }(or{}); omittinglimituses the server default (20). Check any external consumers that relied on a no-input signature.Written for commit 2632b3a. Summary will update on new commits.