Include links to jobs in product details #1384
Conversation
WalkthroughAdds centralized BullMQ enums, a new Prisma QueueRecords model plus migration, threads an optional transition id through the workflow/state-machine and job executors, records job→transition entries when jobs wait/run, surfaces records to super-admin SSE, and renders job-record links in product UI. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant SM as StateMachine
participant WF as Workflow
participant Bull as BullMQ
participant Worker as BullWorker
participant DB as Database
participant UI as Admin UI
Note over SM,WF: resolve transition then enqueue
SM->>WF: currentProductTransition(productId, initialState)
WF->>DB: query ProductTransitions (prefer DateTransition=null, include QueueRecords)
DB-->>WF: transitionId
SM->>Bull: enqueue Job(payload + transitionId)
Bull->>Worker: deliver job
Worker->>DB: create QueueRecords (ProductTransitionId, Queue, JobType, JobId)
DB-->>Worker: persisted record
Worker->>Bull: set span attribute / progress
Note over UI,DB: admin reads records
UI->>DB: load transitions + QueueRecords (if super-admin)
DB-->>UI: transitions with QueueRecords
UI->>UI: render links to admin queues
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
Co-Authored-By: Aidan Jones <[email protected]>
Still need to hook in transitionId... Co-Authored-By: Aidan Jones <[email protected]>
Co-Authored-By: Aidan Jones <[email protected]>
Co-Authored-By: Aidan Jones <[email protected]>
Co-Authored-By: Aidan Jones <[email protected]>
Co-Authored-By: Aidan Jones <[email protected]>
Co-Authored-By: Aidan Jones <[email protected]>
Co-Authored-By: Aidan Jones <[email protected]>
Co-Authored-By: Aidan Jones <[email protected]>
Co-Authored-By: Aidan Jones <[email protected]>
Co-Authored-By: Aidan Jones <[email protected]>
FyreByrd
force-pushed
the
feature/track-jobs
branch
from
7c40fe3 to
b979742
Compare
Co-Authored-By: Aidan Jones <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/lib/server/bullmq/BullWorker.ts (1)
31-52: LGTM! Robust queue record logging with idempotency.The guard checks and
skipDuplicates: truecorrectly handle job retries, preventing duplicate QueueRecords. The assertion on Line 37 is safe given the guard on Line 31.Minor note: The
.replace(/#/g, '%23')on Line 49 appears redundant sinceencodeURIComponentalready encodes#as%23. Consider simplifying:- `${encodeURIComponent(r.Queue)}/${encodeURIComponent(r.JobId).replace(/#/g, '%23')}` + `${encodeURIComponent(r.Queue)}/${encodeURIComponent(r.JobId)}`
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
src/lib/bullmq.ts(1 hunks)src/lib/locales/en-US.json(1 hunks)src/lib/locales/es-419.json(1 hunks)src/lib/locales/fr-FR.json(1 hunks)src/lib/prisma/migrations/16_track_jobs/migration.sql(1 hunks)src/lib/prisma/schema.prisma(1 hunks)src/lib/products/components/ProductDetails.svelte(4 hunks)src/lib/projects/sse.ts(2 hunks)src/lib/server/bullmq/BullWorker.ts(2 hunks)src/lib/server/bullmq/types.ts(10 hunks)src/lib/server/job-executors/build.ts(10 hunks)src/lib/server/job-executors/polling.ts(2 hunks)src/lib/server/job-executors/product.ts(8 hunks)src/lib/server/job-executors/publish.ts(11 hunks)src/lib/server/job-executors/userTasks.ts(1 hunks)src/lib/server/workflow/index.ts(4 hunks)src/lib/server/workflow/state-machine.ts(10 hunks)src/routes/(authenticated)/admin/settings/build-engines/+page.svelte(2 hunks)src/routes/(authenticated)/projects/[id=idNumber]/ProductCard.svelte(1 hunks)src/routes/(authenticated)/workflow-instances/[product_id]/+page.server.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: FyreByrd
Repo: sillsdev/appbuilder-portal PR: 1115
File: portal/node-server/BullWorker.ts:74-83
Timestamp: 2025-05-12T13:16:39.942Z
Learning: In BullMQ, using a fixed `jobId` provides idempotency for jobs. Without a `jobId`, each application restart will add a new job with a unique ID to the queue, causing startup jobs to accumulate. However, completed jobs are not automatically reprocessed - they remain in the completed state.
Learnt from: FyreByrd
Repo: sillsdev/appbuilder-portal PR: 1115
File: portal/node-server/BullWorker.ts:74-83
Timestamp: 2025-05-12T13:16:39.942Z
Learning: In BullMQ, using a fixed `jobId` provides idempotency for jobs. Without a `jobId`, each application restart will add a new job with a unique ID to the queue, causing startup jobs to accumulate over time. Completed jobs are not automatically reprocessed regardless of whether a `jobId` is specified.
📚 Learning: 2025-05-12T13:19:42.118Z
Learnt from: FyreByrd
Repo: sillsdev/appbuilder-portal PR: 1115
File: portal/common/bullmq/types.ts:205-220
Timestamp: 2025-05-12T13:19:42.118Z
Learning: In the AppBuilder portal, `System_CheckEngineStatuses` and `System_RefreshLangTags` job types are designed to run both periodically (as recurring jobs) and immediately upon startup, which is why they're included in both the `RecurringJob` and `StartupJob` type definitions.
Applied to files:
src/lib/bullmq.tssrc/lib/server/bullmq/types.ts
📚 Learning: 2025-09-12T14:57:08.647Z
Learnt from: FyreByrd
Repo: sillsdev/appbuilder-portal PR: 1283
File: src/routes/(authenticated)/tasks/[product_id]/+page.server.ts:252-264
Timestamp: 2025-09-12T14:57:08.647Z
Learning: In src/routes/(authenticated)/tasks/[product_id]/+page.server.ts, FyreByrd prefers to use transition.target[0] prediction mechanism for determining post-send state rather than calling flow.state() after flow.send() due to race condition concerns. The prediction approach is sufficient for redirect decisions between staying on task page vs going to projects page.
Applied to files:
src/lib/server/job-executors/product.tssrc/lib/server/workflow/state-machine.tssrc/lib/server/job-executors/userTasks.tssrc/lib/server/workflow/index.tssrc/routes/(authenticated)/workflow-instances/[product_id]/+page.server.tssrc/lib/products/components/ProductDetails.sveltesrc/routes/(authenticated)/projects/[id=idNumber]/ProductCard.sveltesrc/lib/projects/sse.tssrc/lib/server/job-executors/build.ts
📚 Learning: 2025-09-04T14:26:59.326Z
Learnt from: FyreByrd
Repo: sillsdev/appbuilder-portal PR: 1227
File: src/lib/server/job-executors/product.ts:247-250
Timestamp: 2025-09-04T14:26:59.326Z
Learning: In src/lib/server/job-executors/product.ts, the createLocal function's catch block returns false instead of rethrowing errors. This was implemented intentionally to fix another issue, so any changes to this error handling should be carefully evaluated for downstream impacts.
Applied to files:
src/lib/server/job-executors/product.tssrc/lib/server/job-executors/userTasks.tssrc/lib/server/bullmq/types.ts
📚 Learning: 2025-09-04T16:23:55.891Z
Learnt from: FyreByrd
Repo: sillsdev/appbuilder-portal PR: 1227
File: src/lib/server/job-executors/product.ts:247-250
Timestamp: 2025-09-04T16:23:55.891Z
Learning: In src/lib/server/job-executors/product.ts, createLocal’s catch should log the error via job.log with relevant context (projectId/productDefinitionId/storeId) and still return false to preserve the intentional “no-retry” behavior.
Applied to files:
src/lib/server/job-executors/product.tssrc/lib/server/bullmq/BullWorker.tssrc/lib/server/job-executors/build.tssrc/lib/server/bullmq/types.tssrc/lib/server/job-executors/publish.ts
📚 Learning: 2025-09-12T14:07:02.200Z
Learnt from: FyreByrd
Repo: sillsdev/appbuilder-portal PR: 1283
File: src/routes/(authenticated)/tasks/[product_id]/+page.server.ts:285-303
Timestamp: 2025-09-12T14:07:02.200Z
Learning: In src/routes/(authenticated)/tasks/[product_id]/+page.server.ts, FyreByrd prefers to optimize filterAvailableActions by creating Sets at the caller level rather than inside the function, so the function would take Set<number> arguments instead of arrays for better performance and separation of concerns.
Applied to files:
src/lib/server/workflow/state-machine.tssrc/lib/server/job-executors/userTasks.tssrc/routes/(authenticated)/workflow-instances/[product_id]/+page.server.tssrc/routes/(authenticated)/projects/[id=idNumber]/ProductCard.svelte
📚 Learning: 2025-09-12T14:02:04.558Z
Learnt from: FyreByrd
Repo: sillsdev/appbuilder-portal PR: 1283
File: src/lib/server/workflow/index.ts:52-74
Timestamp: 2025-09-12T14:02:04.558Z
Learning: In the appbuilder-portal codebase, when a Product exists, it always has an associated Project relationship. The Project._count fields (Authors, Reviewers) are safe to access directly when the Product query returns a result.
Applied to files:
src/lib/server/workflow/index.ts
📚 Learning: 2025-09-17T20:58:49.908Z
Learnt from: FyreByrd
Repo: sillsdev/appbuilder-portal PR: 1283
File: src/routes/(authenticated)/tasks/[product_id]/+page.svelte:48-71
Timestamp: 2025-09-17T20:58:49.908Z
Learning: In the codebase, ProductId fields are UUID strings, not numbers. In UserTasks model: `ProductId String db.Uuid` and in Products model: `Id String id db.Uuid`. Route parameters like page.params.product_id are also strings, so strict equality comparisons between ProductId and route parameters work correctly without type conversion.
Applied to files:
src/lib/prisma/schema.prisma
📚 Learning: 2025-08-04T18:34:58.818Z
Learnt from: 7dev7urandom
Repo: sillsdev/appbuilder-portal PR: 1186
File: src/hooks.server.ts:34-41
Timestamp: 2025-08-04T18:34:58.818Z
Learning: In the AppBuilder Portal codebase, BullMQ workers are only initialized when not building (`if (!building)` in BullWorker constructor), and the graceful shutdown handler in hooks.server.ts also only runs when not building. Therefore, the worker.worker property is guaranteed to be defined in the shutdown context, making optional chaining defensive rather than necessary.
Applied to files:
src/lib/server/bullmq/BullWorker.ts
📚 Learning: 2025-09-10T15:11:55.861Z
Learnt from: FyreByrd
Repo: sillsdev/appbuilder-portal PR: 1275
File: src/routes/(authenticated)/projects/[id=idNumber]/+page.svelte:221-221
Timestamp: 2025-09-10T15:11:55.861Z
Learning: In Svelte 5, component props are declared using interface Props and destructured from $props(), not with the export let syntax used in Svelte 4. For example, instead of "export let canEdit: boolean = false;", Svelte 5 uses interface Props { canEdit: boolean; } and let { canEdit }: Props = $props();
Applied to files:
src/routes/(authenticated)/projects/[id=idNumber]/ProductCard.svelte
📚 Learning: 2025-09-17T21:07:18.139Z
Learnt from: FyreByrd
Repo: sillsdev/appbuilder-portal PR: 1275
File: src/lib/projects/index.ts:183-201
Timestamp: 2025-09-17T21:07:18.139Z
Learning: In the AppBuilder portal project, org admins must be in the project group to claim/take ownership of projects. This is enforced both in client-side logic (canClaimProject function) and server-side validation.
Applied to files:
src/lib/projects/sse.ts
🧬 Code graph analysis (7)
src/lib/server/workflow/state-machine.ts (1)
src/lib/server/bullmq/queues.ts (1)
getQueues(150-155)
src/lib/server/workflow/index.ts (2)
src/lib/server/database/index.ts (1)
DatabaseWrites(68-71)src/lib/server/database/prisma.ts (1)
DatabaseReads(22-22)
src/lib/server/bullmq/BullWorker.ts (1)
src/lib/server/database/index.ts (1)
DatabaseWrites(68-71)
src/lib/projects/sse.ts (1)
src/lib/utils/roles.ts (1)
isSuperAdmin(33-35)
src/lib/server/job-executors/build.ts (2)
src/lib/server/job-executors/publish.ts (1)
product(10-161)src/lib/server/build-engine-api/types.ts (1)
BuildResponse(163-171)
src/lib/server/bullmq/types.ts (1)
src/lib/server/build-engine-api/types.ts (1)
Channels(177-177)
src/lib/server/job-executors/publish.ts (2)
src/lib/server/job-executors/build.ts (1)
product(11-134)src/lib/server/build-engine-api/types.ts (1)
ReleaseResponse(186-195)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: checks / build-and-test
- GitHub Check: check / build-and-test
🔇 Additional comments (17)
src/lib/locales/es-419.json (1)
227-227: LGTM - Spanish translation is correct.The Spanish translation "Registros de trabajo" for "Job Records" is accurate and appropriately placed.
src/lib/locales/en-US.json (1)
227-227: LGTM - English translation is clear.The "Job Records" label is clear and follows the existing naming conventions.
src/lib/bullmq.ts (1)
1-59: Well-structured enum definitions for BullMQ job metadata.The three enums provide clear type-safe identifiers for queues, job types, and schedulers. The JobType categories are well-organized, making the codebase more maintainable.
src/lib/server/job-executors/polling.ts (2)
44-53: Transitions metadata correctly propagated through build workflow.The transitions data is appropriately passed to the Build PostProcess job, maintaining the audit trail through the workflow pipeline.
100-109: Transitions metadata correctly propagated through publish workflow.Consistent with the build workflow, transitions data is passed to the Publish PostProcess job for tracking purposes.
src/lib/server/job-executors/userTasks.ts (1)
38-51: Queue records protection added to cleanup logic.The additional
QueueRecords: { none: {} }condition ensures that ProductTransitions with associated job metadata are preserved during PreExecuteEntries cleanup. This prevents accidental deletion of transitions that have recorded job execution history.src/routes/(authenticated)/workflow-instances/[product_id]/+page.server.ts (1)
91-97: Queue records data added to transitions query.Including QueueRecords with specific fields (Queue, JobId, JobType) enables the UI to display job metadata for each transition, supporting the debugging use case outlined in issue #1375.
src/routes/(authenticated)/admin/settings/build-engines/+page.svelte (2)
3-3: Appropriate use of new QueueName enum.Importing QueueName from the new centralized enums file promotes consistency across the codebase.
45-53: Improved UX with actionable link in empty state.The empty state now provides a helpful link to the "Check System Statuses" recurring job, making it easier for admins to diagnose why build engine information is missing. The use of QueueName.System_Recurring ensures consistency with queue definitions.
src/lib/projects/sse.ts (1)
162-170: LGTM! Secure admin-only access control.The conditional inclusion of QueueRecords based on
isSuperAdmincorrectly restricts queue metadata visibility to super admins only. This aligns with the PR objective of providing admin-level job search/tracking capabilities.src/lib/prisma/migrations/16_track_jobs/migration.sql (1)
1-15: LGTM! Well-designed schema with appropriate constraints.The composite primary key on
(Queue, JobId)correctly enforces uniqueness per queue while allowing the same JobId across different queues. The CASCADE delete ensures cleanup when transitions are removed, and the index onProductTransitionIdsupports efficient queries.src/lib/server/job-executors/product.ts (2)
50-51: LGTM! Consistent transitions propagation.All notification paths in the
createfunction correctly propagatejob.data.transitionsto their respective notification calls, ensuring traceability from jobs to notifications.Also applies to: 75-76, 84-85, 100-101
262-337: LGTM! Consistent notification helper signatures.All notification helpers use optional
transitions?: number[]parameters and consistently include them in email job payloads, maintaining backward compatibility while enabling job tracking.src/lib/server/workflow/state-machine.ts (1)
302-311: LGTM! Consistent async transition tracking across workflow states.The async entry/action pattern correctly:
- Awaits
Workflow.currentProductTransitionto capture the transition ID at state entry time- Transforms null results to undefined (safe for optional field)
- Passes transitions to job payloads for downstream tracking
The xstate v5 model properly handles async actions in entry arrays, and the workflow correctly remains in each state until jobs complete and trigger the next transition.
Also applies to: 504-527, 664-693, 784-792, 800-820, 1054-1063
src/lib/server/job-executors/publish.ts (1)
94-95: LGTM! Consistent transitions propagation in publish workflow.All notification and polling paths correctly propagate
transitionsthrough the publish executor, maintaining consistency with the patterns established inbuild.tsandproduct.ts.Also applies to: 102-103, 142-143, 209-210, 239-245, 273-384
src/lib/server/job-executors/build.ts (1)
71-72: LGTM! Consistent transitions propagation in build workflow.The build executor correctly propagates
transitionsthrough all notification and polling paths, maintaining the established pattern across all executors.Also applies to: 79-80, 115-116, 278-279, 283-289, 304-413
src/routes/(authenticated)/projects/[id=idNumber]/ProductCard.svelte (1)
16-18: LGTM! Good refactoring to shared type.Importing the
Transitiontype fromProductDetails.svelteensures type consistency across components and establishes a single source of truth for the Transition shape.
Co-Authored-By: Aidan Jones <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/products/components/ProductDetails.svelte (1)
167-202: Consider adding QueueRecords to mobile view.The desktop table (lines 106-166) includes the QueueRecords section for admin debugging, but the mobile table completely omits this functionality. Admin users on mobile devices won't be able to access job records for debugging purposes.
Since this feature aids debugging (linking to
/admin/jobs/...), consider adding a similar collapsible section to the mobile table layout, perhaps after line 194 or within the existing transition row structure.
♻️ Duplicate comments (1)
src/lib/products/components/ProductDetails.svelte (1)
8-26: Missing Prisma import in module script.This issue was already flagged in a previous review. The
Prismatype is used but not imported in the module script, causing a type-checking failure.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/lib/products/components/ProductDetails.svelte(4 hunks)src/lib/server/bullmq/BullWorker.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/server/bullmq/BullWorker.ts
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: 7dev7urandom
Repo: sillsdev/appbuilder-portal PR: 1186
File: src/hooks.server.ts:34-41
Timestamp: 2025-08-04T18:34:58.818Z
Learning: In the AppBuilder Portal codebase, BullMQ workers are only initialized when not building (`if (!building)` in BullWorker constructor), and the graceful shutdown handler in hooks.server.ts also only runs when not building. Therefore, the worker.worker property is guaranteed to be defined in the shutdown context, making optional chaining defensive rather than necessary.
Learnt from: FyreByrd
Repo: sillsdev/appbuilder-portal PR: 1115
File: portal/node-server/BullWorker.ts:74-83
Timestamp: 2025-05-12T13:16:39.942Z
Learning: In BullMQ, using a fixed `jobId` provides idempotency for jobs. Without a `jobId`, each application restart will add a new job with a unique ID to the queue, causing startup jobs to accumulate. However, completed jobs are not automatically reprocessed - they remain in the completed state.
Learnt from: FyreByrd
Repo: sillsdev/appbuilder-portal PR: 1115
File: portal/node-server/BullWorker.ts:74-83
Timestamp: 2025-05-12T13:16:39.942Z
Learning: In BullMQ, using a fixed `jobId` provides idempotency for jobs. Without a `jobId`, each application restart will add a new job with a unique ID to the queue, causing startup jobs to accumulate over time. Completed jobs are not automatically reprocessed regardless of whether a `jobId` is specified.
📚 Learning: 2025-09-12T14:57:08.647Z
Learnt from: FyreByrd
Repo: sillsdev/appbuilder-portal PR: 1283
File: src/routes/(authenticated)/tasks/[product_id]/+page.server.ts:252-264
Timestamp: 2025-09-12T14:57:08.647Z
Learning: In src/routes/(authenticated)/tasks/[product_id]/+page.server.ts, FyreByrd prefers to use transition.target[0] prediction mechanism for determining post-send state rather than calling flow.state() after flow.send() due to race condition concerns. The prediction approach is sufficient for redirect decisions between staying on task page vs going to projects page.
Applied to files:
src/lib/products/components/ProductDetails.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: checks / build-and-test
- GitHub Check: check / build-and-test
🔇 Additional comments (2)
src/lib/products/components/ProductDetails.svelte (2)
44-44: Good refactoring to use the exported Transition type.This improves type reusability and allows other components to import and use the same type definition.
145-145: URL encoding correctly implemented.The code properly uses
encodeURIComponentto encode the job ID, which correctly handles all reserved characters including/,?, and#. This resolves the concern from the previous review.
Co-Authored-By: Aidan Jones <[email protected]>
Co-Authored-By: Aidan Jones <[email protected]>
Co-Authored-By: Aidan Jones <[email protected]>
Co-Authored-By: Aidan Jones <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/lib/server/bullmq/BullWorker.ts (1)
48-48: Remove unnecessaryencodeURIComponentin span attribute.The Queue and JobId values are being encoded with
encodeURIComponent, but span attributes are not URLs and don't require encoding. Queue names and JobIds are already safe strings from the system.span.setAttribute( 'job.records', - records.map((r) => `${encodeURIComponent(r.Queue)}/${encodeURIComponent(r.JobId)}`) + records.map((r) => `${r.Queue}/${r.JobId}`) );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/lib/server/bullmq/BullWorker.ts(3 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: 7dev7urandom
Repo: sillsdev/appbuilder-portal PR: 1186
File: src/hooks.server.ts:34-41
Timestamp: 2025-08-04T18:34:58.818Z
Learning: In the AppBuilder Portal codebase, BullMQ workers are only initialized when not building (`if (!building)` in BullWorker constructor), and the graceful shutdown handler in hooks.server.ts also only runs when not building. Therefore, the worker.worker property is guaranteed to be defined in the shutdown context, making optional chaining defensive rather than necessary.
Learnt from: FyreByrd
Repo: sillsdev/appbuilder-portal PR: 1115
File: portal/node-server/BullWorker.ts:74-83
Timestamp: 2025-05-12T13:16:39.942Z
Learning: In BullMQ, using a fixed `jobId` provides idempotency for jobs. Without a `jobId`, each application restart will add a new job with a unique ID to the queue, causing startup jobs to accumulate. However, completed jobs are not automatically reprocessed - they remain in the completed state.
📚 Learning: 2025-08-04T18:34:58.818Z
Learnt from: 7dev7urandom
Repo: sillsdev/appbuilder-portal PR: 1186
File: src/hooks.server.ts:34-41
Timestamp: 2025-08-04T18:34:58.818Z
Learning: In the AppBuilder Portal codebase, BullMQ workers are only initialized when not building (`if (!building)` in BullWorker constructor), and the graceful shutdown handler in hooks.server.ts also only runs when not building. Therefore, the worker.worker property is guaranteed to be defined in the shutdown context, making optional chaining defensive rather than necessary.
Applied to files:
src/lib/server/bullmq/BullWorker.ts
📚 Learning: 2025-09-04T14:26:59.326Z
Learnt from: FyreByrd
Repo: sillsdev/appbuilder-portal PR: 1227
File: src/lib/server/job-executors/product.ts:247-250
Timestamp: 2025-09-04T14:26:59.326Z
Learning: In src/lib/server/job-executors/product.ts, the createLocal function's catch block returns false instead of rethrowing errors. This was implemented intentionally to fix another issue, so any changes to this error handling should be carefully evaluated for downstream impacts.
Applied to files:
src/lib/server/bullmq/BullWorker.ts
📚 Learning: 2025-09-04T16:23:55.891Z
Learnt from: FyreByrd
Repo: sillsdev/appbuilder-portal PR: 1227
File: src/lib/server/job-executors/product.ts:247-250
Timestamp: 2025-09-04T16:23:55.891Z
Learning: In src/lib/server/job-executors/product.ts, createLocal’s catch should log the error via job.log with relevant context (projectId/productDefinitionId/storeId) and still return false to preserve the intentional “no-retry” behavior.
Applied to files:
src/lib/server/bullmq/BullWorker.ts
🧬 Code graph analysis (1)
src/lib/server/bullmq/BullWorker.ts (1)
src/lib/server/database/index.ts (1)
DatabaseWrites(68-71)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: checks / build-and-test
- GitHub Check: check / build-and-test
🔇 Additional comments (2)
src/lib/server/bullmq/BullWorker.ts (2)
32-50: Verify database write failure handling strategy.If
DatabaseWrites.queueRecords.createManyAndReturnfails (e.g., database unavailable), the entire job will fail and throw. This couples job execution success to metadata tracking success.Please confirm this is the intended behavior. If metadata tracking should be non-blocking, consider wrapping the database operation in a try-catch that logs errors but allows job execution to proceed.
53-60: Good addition of explicit error rethrowing.The error is properly recorded in the span, logged to console, and then rethrown. This ensures BullMQ can handle job failures correctly (retries, dead letter queue, etc.) while maintaining observability.
Co-Authored-By: Aidan Jones <[email protected]>
Co-Authored-By: Aidan Jones <[email protected]>
Co-Authored-By: Aidan Jones <[email protected]>
Co-Authored-By: Aidan Jones <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/server/job-executors/product.ts (1)
324-337: Restore BuildEngine URL derivation
BuildEngine.Requests.getURLandTokenreturns an object; concatenating it directly yields"[object Object]/job-admin", so every failure email/link will break. Pull out the.urlbefore appending the path.- const endpoint = await BuildEngine.Requests.getURLandToken(organizationId); - const buildEngineUrl = endpoint + '/job-admin'; + const { url } = await BuildEngine.Requests.getURLandToken(organizationId); + const buildEngineUrl = url + '/job-admin';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/lib/server/bullmq/BullWorker.ts(2 hunks)src/lib/server/bullmq/queues.ts(2 hunks)src/lib/server/bullmq/types.ts(10 hunks)src/lib/server/job-executors/build.ts(10 hunks)src/lib/server/job-executors/polling.ts(2 hunks)src/lib/server/job-executors/product.ts(8 hunks)src/lib/server/job-executors/publish.ts(11 hunks)src/lib/server/workflow/index.ts(6 hunks)src/lib/server/workflow/state-machine.ts(10 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/server/job-executors/build.ts
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: FyreByrd
Repo: sillsdev/appbuilder-portal PR: 1115
File: portal/node-server/BullWorker.ts:74-83
Timestamp: 2025-05-12T13:16:39.942Z
Learning: In BullMQ, using a fixed `jobId` provides idempotency for jobs. Without a `jobId`, each application restart will add a new job with a unique ID to the queue, causing startup jobs to accumulate. However, completed jobs are not automatically reprocessed - they remain in the completed state.
Learnt from: 7dev7urandom
Repo: sillsdev/appbuilder-portal PR: 1186
File: src/hooks.server.ts:34-41
Timestamp: 2025-08-04T18:34:58.818Z
Learning: In the AppBuilder Portal codebase, BullMQ workers are only initialized when not building (`if (!building)` in BullWorker constructor), and the graceful shutdown handler in hooks.server.ts also only runs when not building. Therefore, the worker.worker property is guaranteed to be defined in the shutdown context, making optional chaining defensive rather than necessary.
Learnt from: FyreByrd
Repo: sillsdev/appbuilder-portal PR: 1115
File: portal/node-server/BullWorker.ts:74-83
Timestamp: 2025-05-12T13:16:39.942Z
Learning: In BullMQ, using a fixed `jobId` provides idempotency for jobs. Without a `jobId`, each application restart will add a new job with a unique ID to the queue, causing startup jobs to accumulate over time. Completed jobs are not automatically reprocessed regardless of whether a `jobId` is specified.
📚 Learning: 2025-09-12T14:57:08.647Z
Learnt from: FyreByrd
Repo: sillsdev/appbuilder-portal PR: 1283
File: src/routes/(authenticated)/tasks/[product_id]/+page.server.ts:252-264
Timestamp: 2025-09-12T14:57:08.647Z
Learning: In src/routes/(authenticated)/tasks/[product_id]/+page.server.ts, FyreByrd prefers to use transition.target[0] prediction mechanism for determining post-send state rather than calling flow.state() after flow.send() due to race condition concerns. The prediction approach is sufficient for redirect decisions between staying on task page vs going to projects page.
Applied to files:
src/lib/server/workflow/state-machine.tssrc/lib/server/job-executors/product.tssrc/lib/server/job-executors/publish.tssrc/lib/server/workflow/index.ts
📚 Learning: 2025-09-12T14:07:02.200Z
Learnt from: FyreByrd
Repo: sillsdev/appbuilder-portal PR: 1283
File: src/routes/(authenticated)/tasks/[product_id]/+page.server.ts:285-303
Timestamp: 2025-09-12T14:07:02.200Z
Learning: In src/routes/(authenticated)/tasks/[product_id]/+page.server.ts, FyreByrd prefers to optimize filterAvailableActions by creating Sets at the caller level rather than inside the function, so the function would take Set<number> arguments instead of arrays for better performance and separation of concerns.
Applied to files:
src/lib/server/workflow/state-machine.ts
📚 Learning: 2025-08-04T18:34:58.818Z
Learnt from: 7dev7urandom
Repo: sillsdev/appbuilder-portal PR: 1186
File: src/hooks.server.ts:34-41
Timestamp: 2025-08-04T18:34:58.818Z
Learning: In the AppBuilder Portal codebase, BullMQ workers are only initialized when not building (`if (!building)` in BullWorker constructor), and the graceful shutdown handler in hooks.server.ts also only runs when not building. Therefore, the worker.worker property is guaranteed to be defined in the shutdown context, making optional chaining defensive rather than necessary.
Applied to files:
src/lib/server/bullmq/queues.tssrc/lib/server/bullmq/BullWorker.ts
📚 Learning: 2025-09-04T14:26:59.326Z
Learnt from: FyreByrd
Repo: sillsdev/appbuilder-portal PR: 1227
File: src/lib/server/job-executors/product.ts:247-250
Timestamp: 2025-09-04T14:26:59.326Z
Learning: In src/lib/server/job-executors/product.ts, the createLocal function's catch block returns false instead of rethrowing errors. This was implemented intentionally to fix another issue, so any changes to this error handling should be carefully evaluated for downstream impacts.
Applied to files:
src/lib/server/bullmq/queues.tssrc/lib/server/bullmq/BullWorker.tssrc/lib/server/bullmq/types.ts
📚 Learning: 2025-09-04T16:23:55.891Z
Learnt from: FyreByrd
Repo: sillsdev/appbuilder-portal PR: 1227
File: src/lib/server/job-executors/product.ts:247-250
Timestamp: 2025-09-04T16:23:55.891Z
Learning: In src/lib/server/job-executors/product.ts, createLocal’s catch should log the error via job.log with relevant context (projectId/productDefinitionId/storeId) and still return false to preserve the intentional “no-retry” behavior.
Applied to files:
src/lib/server/bullmq/queues.tssrc/lib/server/job-executors/publish.tssrc/lib/server/bullmq/types.ts
📚 Learning: 2025-05-12T13:19:42.118Z
Learnt from: FyreByrd
Repo: sillsdev/appbuilder-portal PR: 1115
File: portal/common/bullmq/types.ts:205-220
Timestamp: 2025-05-12T13:19:42.118Z
Learning: In the AppBuilder portal, `System_CheckEngineStatuses` and `System_RefreshLangTags` job types are designed to run both periodically (as recurring jobs) and immediately upon startup, which is why they're included in both the `RecurringJob` and `StartupJob` type definitions.
Applied to files:
src/lib/server/bullmq/types.ts
📚 Learning: 2025-09-12T14:02:04.558Z
Learnt from: FyreByrd
Repo: sillsdev/appbuilder-portal PR: 1283
File: src/lib/server/workflow/index.ts:52-74
Timestamp: 2025-09-12T14:02:04.558Z
Learning: In the appbuilder-portal codebase, when a Product exists, it always has an associated Project relationship. The Project._count fields (Authors, Reviewers) are safe to access directly when the Product query returns a result.
Applied to files:
src/lib/server/workflow/index.ts
🧬 Code graph analysis (6)
src/lib/server/workflow/state-machine.ts (2)
src/lib/server/bullmq/queues.ts (1)
getQueues(199-204)src/lib/workflowTypes.ts (1)
isDeprecated(78-80)
src/lib/server/bullmq/queues.ts (4)
src/lib/server/bullmq/types.ts (2)
Job(247-247)BaseJob(22-26)src/lib/server/database/prisma.ts (1)
DatabaseReads(22-22)src/lib/server/workflow/index.ts (1)
Workflow(36-637)src/lib/server/database/index.ts (1)
DatabaseWrites(68-71)
src/lib/server/bullmq/BullWorker.ts (1)
src/lib/otel/index.ts (1)
error(46-53)
src/lib/server/job-executors/publish.ts (2)
src/lib/server/job-executors/build.ts (1)
product(11-134)src/lib/server/build-engine-api/types.ts (1)
ReleaseResponse(186-195)
src/lib/server/bullmq/types.ts (1)
src/lib/server/build-engine-api/types.ts (1)
Channels(177-177)
src/lib/server/workflow/index.ts (2)
src/lib/server/database/index.ts (1)
DatabaseWrites(68-71)src/lib/server/database/prisma.ts (1)
DatabaseReads(22-22)
🔇 Additional comments (2)
src/lib/server/workflow/index.ts (1)
519-546: Nice encapsulation for transition lookup.
Pulling the “active or latest” transition intoWorkflow.currentProductTransitionkeeps the queue/record plumbing centralized and cuts down on duplication across the new BullMQ call sites.src/lib/server/job-executors/publish.ts (1)
239-383: Threading transition IDs into notifications pays off.
Plumbing the optional transition through every publish notification lets BullBoard/job details link straight back to the workflow record without extra lookups—great for the debugging experience the PR is aiming for.
Co-Authored-By: Aidan Jones <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/lib/server/bullmq/queues.ts (1)
125-138: Fallback recovery logic adds complexity and could mask issues.The fallback recovery mechanism attempts to find the current product transition when the original transition ID is stale or invalid. While this adds resilience, consider:
- Masking issues: This could hide bugs where incorrect transition IDs are being passed through the workflow
- Context accuracy:
Workflow.currentProductTransitionreturns the most recent transition, which may not match the job's actual context if transitions have progressed- Debugging trade-off: For debugging purposes, having some record is better than none, but the recovered transition might not accurately reflect what was intended
This approach seems reasonable for the PR's debugging objectives, but document this behavior and consider logging warnings when fallbacks occur to help identify underlying issues.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/lib/server/bullmq/queues.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: FyreByrd
Repo: sillsdev/appbuilder-portal PR: 1115
File: portal/node-server/BullWorker.ts:74-83
Timestamp: 2025-05-12T13:16:39.942Z
Learning: In BullMQ, using a fixed `jobId` provides idempotency for jobs. Without a `jobId`, each application restart will add a new job with a unique ID to the queue, causing startup jobs to accumulate. However, completed jobs are not automatically reprocessed - they remain in the completed state.
Learnt from: 7dev7urandom
Repo: sillsdev/appbuilder-portal PR: 1186
File: src/hooks.server.ts:34-41
Timestamp: 2025-08-04T18:34:58.818Z
Learning: In the AppBuilder Portal codebase, BullMQ workers are only initialized when not building (`if (!building)` in BullWorker constructor), and the graceful shutdown handler in hooks.server.ts also only runs when not building. Therefore, the worker.worker property is guaranteed to be defined in the shutdown context, making optional chaining defensive rather than necessary.
Learnt from: FyreByrd
Repo: sillsdev/appbuilder-portal PR: 1115
File: portal/node-server/BullWorker.ts:74-83
Timestamp: 2025-05-12T13:16:39.942Z
Learning: In BullMQ, using a fixed `jobId` provides idempotency for jobs. Without a `jobId`, each application restart will add a new job with a unique ID to the queue, causing startup jobs to accumulate over time. Completed jobs are not automatically reprocessed regardless of whether a `jobId` is specified.
📚 Learning: 2025-09-04T14:26:59.326Z
Learnt from: FyreByrd
Repo: sillsdev/appbuilder-portal PR: 1227
File: src/lib/server/job-executors/product.ts:247-250
Timestamp: 2025-09-04T14:26:59.326Z
Learning: In src/lib/server/job-executors/product.ts, the createLocal function's catch block returns false instead of rethrowing errors. This was implemented intentionally to fix another issue, so any changes to this error handling should be carefully evaluated for downstream impacts.
Applied to files:
src/lib/server/bullmq/queues.ts
📚 Learning: 2025-09-04T16:23:55.891Z
Learnt from: FyreByrd
Repo: sillsdev/appbuilder-portal PR: 1227
File: src/lib/server/job-executors/product.ts:247-250
Timestamp: 2025-09-04T16:23:55.891Z
Learning: In src/lib/server/job-executors/product.ts, createLocal’s catch should log the error via job.log with relevant context (projectId/productDefinitionId/storeId) and still return false to preserve the intentional “no-retry” behavior.
Applied to files:
src/lib/server/bullmq/queues.ts
📚 Learning: 2025-08-04T18:34:58.818Z
Learnt from: 7dev7urandom
Repo: sillsdev/appbuilder-portal PR: 1186
File: src/hooks.server.ts:34-41
Timestamp: 2025-08-04T18:34:58.818Z
Learning: In the AppBuilder Portal codebase, BullMQ workers are only initialized when not building (`if (!building)` in BullWorker constructor), and the graceful shutdown handler in hooks.server.ts also only runs when not building. Therefore, the worker.worker property is guaranteed to be defined in the shutdown context, making optional chaining defensive rather than necessary.
Applied to files:
src/lib/server/bullmq/queues.ts
📚 Learning: 2025-09-12T14:57:08.647Z
Learnt from: FyreByrd
Repo: sillsdev/appbuilder-portal PR: 1283
File: src/routes/(authenticated)/tasks/[product_id]/+page.server.ts:252-264
Timestamp: 2025-09-12T14:57:08.647Z
Learning: In src/routes/(authenticated)/tasks/[product_id]/+page.server.ts, FyreByrd prefers to use transition.target[0] prediction mechanism for determining post-send state rather than calling flow.state() after flow.send() due to race condition concerns. The prediction approach is sufficient for redirect decisions between staying on task page vs going to projects page.
Applied to files:
src/lib/server/bullmq/queues.ts
🧬 Code graph analysis (1)
src/lib/server/bullmq/queues.ts (1)
src/lib/server/bullmq/types.ts (2)
Job(247-247)BaseJob(22-26)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: checks / build-and-test
- GitHub Check: checks / typecheck-lint
- GitHub Check: check / build-and-test
🔇 Additional comments (5)
src/lib/server/bullmq/queues.ts (5)
1-21: LGTM: Imports support the new job record tracking.The new imports (
prismaInternal,Workflow, andBaseJob) are necessary for the job record creation functionality and are used appropriately throughout the file.
120-148: Transaction client usage is now correct.The previous critical issue regarding transaction client usage has been resolved. The code now correctly uses the
txparameter throughout the transaction callback (lines 121, 140) instead of the root Prisma clients.
117-154: Verify async event handler behavior is intentional.The
createJobRecordfunction is registered as an event handler for thewaitingevent but is not awaited. This means:
- Job processing begins immediately without waiting for the record to be created
- If record creation fails or is slow, the job may complete before a record exists
- This is likely intentional to avoid blocking job processing, but could lead to missing records in edge cases
Please confirm this fire-and-forget behavior aligns with your debugging requirements.
136-136: Consider implications of mutating job data in Redis.The call to
job.updateData()modifies the job's persisted data in Redis, replacing the stale transition ID with the recovered one. This means:
- Future reads or retries will see the corrected transition ID
- The original (incorrect) transition ID is lost
- If multiple workers process the same job concurrently, there could be race conditions
While this ensures consistency going forward, consider whether you need to preserve the original transition ID for debugging purposes (e.g., logging it before the update).
162-182: Event handler registrations align with PR objectives.The
createJobRecordhandler is correctly registered on the relevant queues (Builds, Products, Publishing, Polling, Emails) and intentionally excluded from others (SystemRecurring, SystemStartup, Projects, UserTasks, SvelteSSE), matching the PR's stated tracking scope.
Closes #1375
Tracked:
Not Tracked:
Summary by CodeRabbit
New Features
Localization
Chores