[NikhilChowdhury27]

Closes #1012

Summary

If you hit "Refresh" on a bookmark a few times quickly, each click queues
a separate crawl job — so the same page gets crawled 3, 5, or 10 times
doing the exact same work. This wastes resources since crawling is the
most expensive operation (browser launch, navigation, screenshots, archival).

The fix is straightforward: pass idempotencyKey: crawl:${bookmarkId} when
enqueuing crawl jobs. Both queue backends (liteque and restate) already
support this — if a job with the same key is already queued, the duplicate
gets silently dropped. Same pattern that triggerSearchReindex already
uses with index:${bookmarkId}.

What changed

Added idempotencyKey to all 4 places that enqueue crawl jobs:

• packages/trpc/routers/bookmarks.ts — user "Refresh" button
• packages/trpc/routers/admin.ts — admin bulk recrawl + single recrawl
• packages/trpc/lib/ruleEngine.ts — rule engine "download full page archive"
• packages/trpc/lib/__tests__/ruleEngine.test.ts — updated test expectation

Testing

• All 389 trpc tests pass
• Typecheck, lint, format all pass
• Verified with direct liteque test: 10 concurrent enqueues for the same
  bookmark → 1 pending job, 9 dropped

[coderabbitai]

Walkthrough

Added idempotency key support to crawler queue enqueues: enqueue calls now compute and pass an idempotencyKey using buildCrawlIdempotencyKey(payload). Tests updated to assert the new option. A new exported helper buildCrawlIdempotencyKey was added to packages/shared-server/src/queues.ts. No other control flow changed.

Changes

Cohort / File(s)	Summary
Rule Engine & Test packages/trpc/lib/ruleEngine.ts, packages/trpc/lib/__tests__/ruleEngine.test.ts	downloadFullPageArchive now constructs a payload and enqueues it with idempotencyKey: buildCrawlIdempotencyKey(payload); test updated to assert the payload and idempotencyKey alongside existing groupId/priority.
Admin Router packages/trpc/routers/admin.ts	recrawlLinks and adminRecrawlBookmark now build explicit payload objects and pass idempotencyKey: buildCrawlIdempotencyKey(payload) to LowPriorityCrawlerQueue.enqueue while keeping existing enqueue options.
Bookmarks Router packages/trpc/routers/bookmarks.ts	recrawlBookmark now creates a payload and enqueues with idempotencyKey: buildCrawlIdempotencyKey(payload) instead of inlining the payload; groupId/priority unchanged.
Shared queues helper packages/shared-server/src/queues.ts	Added exported buildCrawlIdempotencyKey(payload: ZCrawlLinkRequest): string which generates a stable idempotency key by JSON-stringifying the payload with sorted keys and prefixing with crawl:.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix: deduplicate crawl jobs using idempotency key' directly and clearly summarizes the main change: adding idempotency keys to prevent duplicate crawl job queuing.
Description check	✅ Passed	The description thoroughly explains the problem (duplicate crawl jobs wasting resources), the solution (idempotency keys), implementation details, and testing verification, all directly related to the changeset.
Linked Issues check	✅ Passed	The PR fully addresses issue #1012 by implementing idempotency key deduplication across all four crawl enqueue sites (bookmarks, admin, ruleEngine, tests) to prevent duplicate job queuing.
Out of Scope Changes check	✅ Passed	All changes are scoped to the idempotency key feature: adding the helper function, updating enqueue calls, and updating related tests. No unrelated modifications detected.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[greptile-apps]

Greptile Summary

Deduplicates recrawl jobs by adding payload-derived idempotency keys to all four LowPriorityCrawlerQueue.enqueue call sites. The key function, buildCrawlIdempotencyKey, canonicalises the payload via JSON.stringify with a sorted-keys array replacer, ensuring jobs with different flags (e.g. archiveFullPage: true) get distinct keys and are not incorrectly suppressed.

Confidence Score: 5/5

Safe to merge — clean, targeted fix with no blocking issues.

All findings are P2 or lower. The key generation logic is correct: JSON.stringify with a sorted-keys array replacer produces canonical output regardless of insertion order, and distinct flag combinations yield distinct keys. The previous concern about archive jobs sharing keys with plain recrawls is fully resolved. Tests pass and cover the new expectation.

No files require special attention.

Important Files Changed

Filename	Overview
packages/shared-server/src/queues.ts	Adds buildCrawlIdempotencyKey — serializes sorted payload keys via JSON.stringify's array replacer, producing a canonical key that distinguishes jobs with different flags.
packages/trpc/routers/bookmarks.ts	Adds idempotency key to recrawlBookmark mutation on LowPriorityCrawlerQueue; payload shape is correct and key generation is canonical.
packages/trpc/routers/admin.ts	Adds idempotency keys to both bulk recrawl and single admin recrawl enqueues; both correctly derive the key from the actual payload object.
packages/trpc/lib/ruleEngine.ts	Adds idempotency key to downloadFullPageArchive action; the payload includes archiveFullPage: true and runInference: false, so its key is distinct from plain recrawl keys — prior concern resolved.
packages/trpc/lib/tests/ruleEngine.test.ts	Test correctly validates the new idempotency key expectation using buildCrawlIdempotencyKey(expectedPayload), ensuring the key generation function is tested in integration.

_{Reviews (2): Last reviewed commit: "refactor: extract buildCrawlIdempotencyK..." | Re-trigger Greptile}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)

packages/trpc/routers/bookmarks.ts (1)

635-645: ⚠️ Potential issue | 🟠 Major

Current key dedupes non-equivalent recrawls (archiveFullPage / storePdf)

At Line 644, two recrawl requests for the same bookmark but different options are treated as duplicates. That can silently drop the more specific request (e.g., full-page archive/PDF).

Suggested fix

       await LowPriorityCrawlerQueue.enqueue(
         {
           bookmarkId: input.bookmarkId,
           archiveFullPage: input.archiveFullPage,
           storePdf: input.storePdf,
         },
         {
           groupId: ctx.user.id,
           priority: QueuePriority.Low,
-          idempotencyKey: `crawl:${input.bookmarkId}`,
+          idempotencyKey:
+            `crawl:${input.bookmarkId}:intent:userRecrawl:archive:${input.archiveFullPage ? 1 : 0}:pdf:${input.storePdf ? 1 : 0}`,
         },
       );

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/trpc/routers/bookmarks.ts` around lines 635 - 645, The idempotency
key used in LowPriorityCrawlerQueue.enqueue (`idempotencyKey:
\`crawl:${input.bookmarkId}\``) collapses distinct recrawl requests that differ
by archiveFullPage/storePdf; change the idempotency key to include those options
so each unique option set is treated separately (e.g., incorporate
input.archiveFullPage and input.storePdf values or a deterministic serialization
of the options into the idempotencyKey) in the call to
LowPriorityCrawlerQueue.enqueue.

packages/trpc/routers/admin.ts (1)

231-239: ⚠️ Potential issue | 🟠 Major

Bookmark-only idempotency keys collapse distinct admin recrawl intents

At Line 238 and Line 681, keys are scoped only by bookmark ID. In recrawlLinks, runInference is part of behavior but not part of the key, so different requested behaviors can be silently dropped as “duplicates.”

Suggested fix

         bookmarkIds.map((b) =>
           LowPriorityCrawlerQueue.enqueue(
             {
               bookmarkId: b.id,
               runInference: input.runInference,
             },
             {
               priority: QueuePriority.Low,
-              idempotencyKey: `crawl:${b.id}`,
+              idempotencyKey: `crawl:${b.id}:intent:adminBulkRecrawl:infer:${input.runInference ? 1 : 0}`,
             },
           ),
         ),
       );

       await LowPriorityCrawlerQueue.enqueue(
         {
           bookmarkId: input.bookmarkId,
         },
         {
           priority: QueuePriority.Low,
           groupId: "admin",
-          idempotencyKey: `crawl:${input.bookmarkId}`,
+          idempotencyKey: `crawl:${input.bookmarkId}:intent:adminRecrawl`,
         },
       );

Also applies to: 674-682

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/trpc/routers/admin.ts` around lines 231 - 239, The idempotency key
used in LowPriorityCrawlerQueue.enqueue (currently `crawl:${b.id}` in
recrawlLinks) only scopes by bookmarkId and causes different admin recrawl
intents (e.g., differing runInference values) to collapse; update the
idempotencyKey to include the runInference flag (or other distinguishing input
properties) so each distinct intent is unique (for example incorporate
input.runInference into the key generation used in
LowPriorityCrawlerQueue.enqueue and any other enqueue sites such as the other
occurrence around lines 674-682).

packages/trpc/lib/ruleEngine.ts (1)

290-301: ⚠️ Potential issue | 🟠 Major

Idempotency key is too coarse and can suppress full-page archive jobs

At Line 300, using only crawl:${this.bookmark.id} makes this specialized action collide with other crawl intents for the same bookmark. If a normal recrawl is already queued, this downloadFullPageArchive job can be dropped, so archive generation may never happen.

Suggested fix

         await LowPriorityCrawlerQueue.enqueue(
           {
             bookmarkId: this.bookmark.id,
             archiveFullPage: true,
             runInference: false,
           },
           {
             groupId: this.bookmark.userId,
             priority: QueuePriority.Low,
-            idempotencyKey: `crawl:${this.bookmark.id}`,
+            idempotencyKey: `crawl:${this.bookmark.id}:intent:downloadFullPageArchive`,
           },
         );

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/trpc/lib/ruleEngine.ts` around lines 290 - 301, The idempotency key
used when enqueuing the downloadFullPageArchive job is too generic
("crawl:${this.bookmark.id}") which causes LowPriorityCrawlerQueue.enqueue to
deduplicate this specialized full-page archive task against other crawl jobs;
update the idempotencyKey to include a specific marker for full-page archive
(e.g., include "downloadFullPageArchive" or "archiveFullPage=true") so the
enqueue call in the downloadFullPageArchive case produces a unique key and will
not be suppressed by other crawl intents for the same bookmark (modify the
idempotencyKey passed to LowPriorityCrawlerQueue.enqueue in the
downloadFullPageArchive branch accordingly).

🧹 Nitpick comments (1)

packages/trpc/lib/__tests__/ruleEngine.test.ts (1)

602-617: Add a regression test for option-sensitive idempotency behavior

This expectation is fine, but there’s no test guarding that different crawl intents/options for the same bookmark do not dedupe together. Please add coverage for at least one pair of different option sets.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/trpc/lib/__tests__/ruleEngine.test.ts` around lines 602 - 617, Add a
regression test to ensure option-sensitive idempotency for crawl enqueues: call
engine.executeAction (or trigger the same logic using RuleEngineAction) twice
for the same bookmarkId with two different option sets (e.g., archiveFullPage:
true vs archiveFullPage: false or runInference: true vs false) and assert that
LowPriorityCrawlerQueue.enqueue is invoked for both calls with distinct
idempotency keys (do not let both use the same `crawl:${bookmarkId}` key) —
update or add an it(...) near the existing "downloadFullPageArchive" test to
verify two separate enqueue calls or different idempotencyKey values.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@packages/trpc/lib/ruleEngine.ts`:
- Around line 290-301: The idempotency key used when enqueuing the
downloadFullPageArchive job is too generic ("crawl:${this.bookmark.id}") which
causes LowPriorityCrawlerQueue.enqueue to deduplicate this specialized full-page
archive task against other crawl jobs; update the idempotencyKey to include a
specific marker for full-page archive (e.g., include "downloadFullPageArchive"
or "archiveFullPage=true") so the enqueue call in the downloadFullPageArchive
case produces a unique key and will not be suppressed by other crawl intents for
the same bookmark (modify the idempotencyKey passed to
LowPriorityCrawlerQueue.enqueue in the downloadFullPageArchive branch
accordingly).

In `@packages/trpc/routers/admin.ts`:
- Around line 231-239: The idempotency key used in
LowPriorityCrawlerQueue.enqueue (currently `crawl:${b.id}` in recrawlLinks) only
scopes by bookmarkId and causes different admin recrawl intents (e.g., differing
runInference values) to collapse; update the idempotencyKey to include the
runInference flag (or other distinguishing input properties) so each distinct
intent is unique (for example incorporate input.runInference into the key
generation used in LowPriorityCrawlerQueue.enqueue and any other enqueue sites
such as the other occurrence around lines 674-682).

In `@packages/trpc/routers/bookmarks.ts`:
- Around line 635-645: The idempotency key used in
LowPriorityCrawlerQueue.enqueue (`idempotencyKey:
\`crawl:${input.bookmarkId}\``) collapses distinct recrawl requests that differ
by archiveFullPage/storePdf; change the idempotency key to include those options
so each unique option set is treated separately (e.g., incorporate
input.archiveFullPage and input.storePdf values or a deterministic serialization
of the options into the idempotencyKey) in the call to
LowPriorityCrawlerQueue.enqueue.

---

Nitpick comments:
In `@packages/trpc/lib/__tests__/ruleEngine.test.ts`:
- Around line 602-617: Add a regression test to ensure option-sensitive
idempotency for crawl enqueues: call engine.executeAction (or trigger the same
logic using RuleEngineAction) twice for the same bookmarkId with two different
option sets (e.g., archiveFullPage: true vs archiveFullPage: false or
runInference: true vs false) and assert that LowPriorityCrawlerQueue.enqueue is
invoked for both calls with distinct idempotency keys (do not let both use the
same `crawl:${bookmarkId}` key) — update or add an it(...) near the existing
"downloadFullPageArchive" test to verify two separate enqueue calls or different
idempotencyKey values.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b6ec43f8-e53f-4773-ace8-6ab7af399929

📥 Commits

Reviewing files that changed from the base of the PR and between 07ecdcd and e822614.

📒 Files selected for processing (4)

• packages/trpc/lib/__tests__/ruleEngine.test.ts
• packages/trpc/lib/ruleEngine.ts
• packages/trpc/routers/admin.ts
• packages/trpc/routers/bookmarks.ts

[MohamedBassem]

This key also need to take into account the flags of the recrawlCall

[NikhilChowdhury27]

Makes sense — should I include all the flags in the key like
crawl:${bookmarkId}:${archiveFullPage}:${storePdf} so each unique crawl configuration gets its own dedup? Or
would you prefer a simpler approach like only differentiating when archiveFullPage or storePdf is true?

[MohamedBassem]

Maybe we can stable JSON serialize the input as the safest most readable way?

[NikhilChowdhury27]

that's actually better stable JSON serialize is cleaner and future-proof. I'll update bookmarks.ts to use this
approach.
Quick question: should I do the same for the admin bulk recrawl? It has a similar pattern with a variant
flag:

 // admin.ts line 231-238                                                                                    
     LowPriorityCrawlerQueue.enqueue(                              
      { bookmarkId: b.id, runInference: input.runInference },                                                   
      { priority: QueuePriority.Low, idempotencyKey: `crawl:${b.id}` }                                          
 )            

Currently runInference isn't part of the key either. Want me to apply the same stable-serialize approach
here too?

[MohamedBassem]

yeah, let's do that

[NikhilChowdhury27]

done, pushed

[MohamedBassem]

can you extract this into a shared util beside the queue definition called (buildCrawlIdempotencyKey) and use it in all the callsites for consistency?

[NikhilChowdhury27]

@MohamedBassem happy to extract the shared util. Did a full audit of all *CrawlerQueue.enqueue call-sites first — wanted to flag a few things before I touch code:

Call-sites that can use buildCrawlIdempotencyKey:

1. bookmarks.ts:640 (user recrawl) — already JSON format ✅
2. admin.ts:235 (bulk recrawl) — already JSON format ✅
3. admin.ts:672 (admin single recrawl) — currently crawl:${bookmarkId}, legacy format
4. ruleEngine.ts:291 (archive action) — currently crawl:archive:${bookmarkId}, different prefix

Call-site that cannot use it:

• bookmarks.ts:338 (fresh bookmark create) — no idempotency key today, and I don't think we want one here. Each new bookmark has a freshly-generated bookmarkId, so dedup is implicit. Adding a key would block legitimate retries
  if the initial enqueue fails. Leaving this untouched unless you disagree.

Behavior changes I want to confirm before unifying:

1. Site 3 (admin.ts:672): key changes from crawl:abc123 → crawl:{"bookmarkId":"abc123"}. In-memory queue state only, so impact is basically zero, but wanted to flag it. OK to switch?
2. Site 4 (ruleEngine.ts): loses the archive: prefix and joins the generic crawl: namespace. As far as I can tell this is actually correct — a user-triggered recrawl with {archiveFullPage: true, storePdf: false} and a
   rule-engine archive with {archiveFullPage: true, runInference: false} still serialize to different JSON, so they won't collide. And two rapid rule-engine fires would still dedupe (same as today). Net: same or better. OK to
   drop the prefix?

Once you confirm (or correct) the above I'll push the refactor — util next to the queue definition in packages/shared-server/src/queues.ts, imports updated in all 4 call-sites + the rule engine test.

[NikhilChowdhury27]

Done — extracted buildCrawlIdempotencyKey into packages/shared-server/src/queues.ts right beside the queue definition, and switched all 4 callsites to use it: bookmarks.ts recrawl, admin.ts bulk recrawl, admin.ts single recrawl, and ruleEngine.ts archive action. Test updated to call the helper itself so it's not coupled to the serialization format.

Two behaviour notes worth flagging:

• admin.ts single recrawl key changes from crawl:${bookmarkId} to the JSON form. Since idempotency lives in-memory queue state, the deploy-window impact is effectively zero — old-format jobs drain within seconds.
• ruleEngine.ts archive loses its archive: prefix and joins the generic crawl: namespace. Verified this is still correct: archive payload is {bookmarkId, archiveFullPage:true, runInference:false} and bookmarks recrawl payload is {bookmarkId, archiveFullPage:true, storePdf:false} — different key-sets, so they serialize to different JSON and won't falsely dedup.

Full trpc suite passing (389/389) plus 16 manual scenarios covering cross-callsite equivalence / isolation / key-ordering stability / future-field forward compat. Happy to split behaviour changes into a separate commit if you prefer.

[coderabbitai]

🧹 Nitpick comments (1)

packages/shared-server/src/queues.ts (1)

115-117: Normalize payload before serialization to avoid dedupe misses on equivalent requests.

Using raw z.input can generate different keys for equivalent payloads (e.g., omitted vs explicit defaulted fields), which can reduce dedupe effectiveness.

Suggested patch

 export function buildCrawlIdempotencyKey(payload: ZCrawlLinkRequest): string {
-  return `crawl:${JSON.stringify(payload, Object.keys(payload).sort())}`;
+  const normalized = zCrawlLinkRequestSchema.parse(payload);
+  const stable = Object.fromEntries(
+    Object.keys(normalized)
+      .sort()
+      .map((k) => [k, normalized[k as keyof typeof normalized]]),
+  );
+  return `crawl:${JSON.stringify(stable)}`;
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/shared-server/src/queues.ts` around lines 115 - 117, The idempotency
key uses the raw payload which can differ for semantically equivalent requests;
update buildCrawlIdempotencyKey to first normalize the payload using the
ZCrawlLinkRequest schema (e.g., call ZCrawlLinkRequest.parse or safeParse to
apply defaults and coerce types) and then stringify it deterministically (e.g.,
JSON.stringify over Object.keys(sorted) or a stable serializer) so equivalent
requests produce the same key; reference the buildCrawlIdempotencyKey function
and ZCrawlLinkRequest schema when making the change.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/shared-server/src/queues.ts`:
- Around line 115-117: The idempotency key uses the raw payload which can differ
for semantically equivalent requests; update buildCrawlIdempotencyKey to first
normalize the payload using the ZCrawlLinkRequest schema (e.g., call
ZCrawlLinkRequest.parse or safeParse to apply defaults and coerce types) and
then stringify it deterministically (e.g., JSON.stringify over
Object.keys(sorted) or a stable serializer) so equivalent requests produce the
same key; reference the buildCrawlIdempotencyKey function and ZCrawlLinkRequest
schema when making the change.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4b4b3a17-6b3a-43b4-b55b-c81c420ad017

📥 Commits

Reviewing files that changed from the base of the PR and between b0d96aa and 0cd9e40.

📒 Files selected for processing (5)

• packages/shared-server/src/queues.ts
• packages/trpc/lib/__tests__/ruleEngine.test.ts
• packages/trpc/lib/ruleEngine.ts
• packages/trpc/routers/admin.ts
• packages/trpc/routers/bookmarks.ts

✅ Files skipped from review due to trivial changes (1)

• packages/trpc/lib/ruleEngine.ts

🚧 Files skipped from review as they are similar to previous changes (2)

• packages/trpc/routers/bookmarks.ts
• packages/trpc/routers/admin.ts

[MohamedBassem]

Thank you. The changes looks good to me. Will run CI, and another round of AI review and then merge if all good

[MohamedBassem]

@greptile review
@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0cd9e40ac0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Normalize crawl payload before computing idempotency key

buildCrawlIdempotencyKey hashes the raw enqueue payload, so semantically equivalent crawl requests can get different keys and bypass deduplication. For example, bookmarks.recrawlBookmark enqueues { bookmarkId, archiveFullPage: false, storePdf: false } while adminRecrawlBookmark enqueues { bookmarkId }; in crawlerWorker.runCrawler those missing flags are defaulted/treated equivalently, so both jobs do the same work but won't dedupe because their serialized payload differs. The same mismatch exists for omitted runInference vs runInference: true.

Useful? React with 👍 / 👎.

[MohamedBassem]

Thank you!