feat(webapp): reload LLM pricing registry on Redis pub/sub#3534
feat(webapp): reload LLM pricing registry on Redis pub/sub#3534
Conversation
Subscribe to LLM_PRICING_RELOAD_CHANNEL on the worker Redis. Any process that publishes on the channel triggers an immediate reload of the in-memory model registry. The 5-minute periodic reload stays as a backstop. Lets pricing and model changes propagate to the live registry within seconds instead of up to 5 minutes.
|
WalkthroughThis change introduces a Redis pub/sub-based reload mechanism for the LLM pricing registry. Previously, pricing updates relied on a 5-minute polling interval. Now, when the billing worker publishes a message to the LLM_PRICING_RELOAD_CHANNEL, the registry reloads immediately. The periodic interval remains as a backstop. An environment variable defines the Redis channel name, and signal handlers properly clean up the Redis subscriber on shutdown. A changelog entry documents the faster update propagation. Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Coalesce reload calls from the pub/sub subscriber so a burst of publishes only triggers one reload. The first publish in a window schedules a reload at T+LLM_PRICING_RELOAD_DEBOUNCE_MS (default 1s); subsequent publishes during that window are no-ops because the trailing reload will pick up everything when it queries the DB. Bounds reload rate to at most 1 per debounce window regardless of publisher chattiness, so a runaway upstream publisher can't fan out into a flood of full-table-scan reloads across every webapp pod.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Subscribing every replica to the reload channel — admin dashboards, workers, anything that imports the registry — fans out a full-table reload across processes that don't actually need real-time pricing freshness. The 5-minute interval is enough for those. Add LLM_PRICING_RELOAD_PUBSUB_ENABLED (default true). Set false on non-OTel services in multi-service deployments so only the span-ingesting processes subscribe and reload on publish. Default-true preserves current behavior for single-service self-hosted deployments without any env tuning.
Most processes that import the registry (dashboard, workers, single- service self-hosted webapp) don't actually need real-time pricing freshness — the existing 5-minute interval is fine. Only OTel-ingesting services where pricing directly affects span cost enrichment need to subscribe. Default off, opt-in on the span-ingesting services in multi-service deployments. Self-hosters running a single webapp can flip it on if they want sub-second freshness, but the default keeps reload load off processes that don't benefit from it.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/webapp/app/env.server.ts`:
- Line 1428: LLM_PRICING_RELOAD_DEBOUNCE_MS currently allows negative numbers
which can make setTimeout in llmPricingRegistry.server.ts run immediately;
update the env schema for LLM_PRICING_RELOAD_DEBOUNCE_MS to enforce a
non-negative lower bound (e.g., use z.coerce.number().int().min(0).default(1000)
or equivalent) so values below 0 are rejected or coerced, and optionally add a
defensive clamp (Math.max(0, debounceMs)) at the point where the value is passed
into setTimeout in llmPricingRegistry.server.ts to guarantee a non-negative
delay.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 987cd6b7-4011-41a2-852a-0ada99fed9bb
📒 Files selected for processing (2)
apps/webapp/app/env.server.tsapps/webapp/app/v3/llmPricingRegistry.server.ts
✅ Files skipped from review due to trivial changes (1)
- apps/webapp/app/v3/llmPricingRegistry.server.ts
📜 Review details
⏰ 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). (27)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / e2e-webapp / 🧪 E2E Tests: Webapp
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: sdk-compat / Bun Runtime
- GitHub Check: sdk-compat / Node.js 22.12 (ubuntu-latest)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: sdk-compat / Cloudflare Workers
- GitHub Check: sdk-compat / Node.js 20.20 (ubuntu-latest)
- GitHub Check: sdk-compat / Deno Runtime
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
Files:
apps/webapp/app/env.server.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/env.server.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/env.server.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries
Files:
apps/webapp/app/env.server.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: Access environment variables through theenvexport ofenv.server.tsinstead of directly accessingprocess.env
Use subpath exports from@trigger.dev/corepackage instead of importing from the root@trigger.dev/corepathUse named constants for sentinel/placeholder values (e.g.
const UNSET_VALUE = '__unset__') instead of raw string literals scattered across comparisons
Files:
apps/webapp/app/env.server.ts
apps/webapp/**/*.server.ts
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
apps/webapp/**/*.server.ts: Never userequest.signalfor detecting client disconnects. UsegetRequestAbortSignal()fromapp/services/httpAsyncStorage.server.tsinstead, which is wired directly to Expressres.on('close')and fires reliably
Access environment variables viaenvexport fromapp/env.server.ts. Never useprocess.envdirectly
Always usefindFirstinstead offindUniquein Prisma queries.findUniquehas an implicit DataLoader that batches concurrent calls and has active bugs even in Prisma 6.x (uppercase UUIDs returning null, composite key SQL correctness issues, 5-10x worse performance).findFirstis never batched and avoids this entire class of issues
Files:
apps/webapp/app/env.server.ts
{apps,internal-packages}/**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
pnpm run typecheckto verify changes in apps and internal packages (apps/*,internal-packages/*) instead ofbuild, which proves almost nothing about correctness
Files:
apps/webapp/app/env.server.ts
{package.json,**/*.{ts,tsx,js}}
📄 CodeRabbit inference engine (CLAUDE.md)
Pin Zod to version 3.25.76 exactly across the entire monorepo - never use a different version or version range
Files:
apps/webapp/app/env.server.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js}: Import from@trigger.dev/coreusing subpaths only, never the root export
Always import tasks from@trigger.dev/sdk, never from@trigger.dev/sdk/v3or deprecatedclient.defineJob
Add crumbs to code using//@Crumbscomments or `// `#region` `@crumbsblocks for debug tracing during development
Files:
apps/webapp/app/env.server.ts
**/*.{ts,tsx,js,jsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Code formatting is enforced using Prettier. Run
pnpm run formatbefore committing
Files:
apps/webapp/app/env.server.ts
🧠 Learnings (3)
📚 Learning: 2026-03-22T13:26:12.060Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3244
File: apps/webapp/app/components/code/TextEditor.tsx:81-86
Timestamp: 2026-03-22T13:26:12.060Z
Learning: In the triggerdotdev/trigger.dev codebase, do not flag `navigator.clipboard.writeText(...)` calls for `missing-await`/`unhandled-promise` issues. These clipboard writes are intentionally invoked without `await` and without `catch` handlers across the project; keep that behavior consistent when reviewing TypeScript/TSX files (e.g., usages like in `apps/webapp/app/components/code/TextEditor.tsx`).
Applied to files:
apps/webapp/app/env.server.ts
📚 Learning: 2026-03-22T19:24:14.403Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3187
File: apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.ts:200-204
Timestamp: 2026-03-22T19:24:14.403Z
Learning: In the triggerdotdev/trigger.dev codebase, webhook URLs are not expected to contain embedded credentials/secrets (e.g., fields like `ProjectAlertWebhookProperties` should only hold credential-free webhook endpoints). During code review, if you see logging or inclusion of raw webhook URLs in error messages, do not automatically treat it as a credential-leak/secrets-in-logs issue by default—first verify the URL does not contain embedded credentials (for example, no username/password in the URL, no obvious secret/token query params or fragments). If the URL is credential-free per this project’s conventions, allow the logging.
Applied to files:
apps/webapp/app/env.server.ts
📚 Learning: 2026-05-05T09:38:02.512Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3523
File: apps/webapp/app/routes/api.v3.batches.ts:178-181
Timestamp: 2026-05-05T09:38:02.512Z
Learning: When reviewing code that catches `ServiceValidationError` in `*.server.ts` files, do not blindly forward `error.status` to HTTP responses, because SVEs may be thrown with non-default statuses (e.g., 400/500) and forwarding them can cause client-visible behavioral regressions (e.g., surfacing 500s to clients). Prefer a safe default response status of `error.status ?? 422`, but only after confirming via the reachable call graph that the caught `ServiceValidationError` instances are expected to carry those non-default statuses; otherwise, normalize to `422` to avoid unexpected client-visible 5xx behavior.
Applied to files:
apps/webapp/app/env.server.ts
🔇 Additional comments (2)
apps/webapp/app/env.server.ts (2)
1427-1427:LLM_PRICING_RELOAD_CHANNELstill allows empty input.This is the same unresolved concern previously raised:
z.string()accepts"", which can silently break pub/sub routing when misconfigured.
1429-1434: Good addition: clear rollout guidance and safe default for pub/sub subscription.The inline docs explain the multi-service behavior well, and defaulting
LLM_PRICING_RELOAD_PUBSUB_ENABLEDto true preserves self-hosted/single-service expectations.
| LLM_COST_TRACKING_ENABLED: BoolEnv.default(true), | ||
| LLM_PRICING_RELOAD_INTERVAL_MS: z.coerce.number().int().default(5 * 60 * 1000), // 5 minutes | ||
| LLM_PRICING_RELOAD_CHANNEL: z.string().default("llm-registry:reload"), | ||
| LLM_PRICING_RELOAD_DEBOUNCE_MS: z.coerce.number().int().default(1000), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/webapp/app/env.server.ts | sed -n '1420,1440p'Repository: triggerdotdev/trigger.dev
Length of output: 1638
🏁 Script executed:
# Search for usages of LLM_PRICING_RELOAD_DEBOUNCE_MS
rg "LLM_PRICING_RELOAD_DEBOUNCE_MS" --type ts --type tsxRepository: triggerdotdev/trigger.dev
Length of output: 97
🏁 Script executed:
# Check other debounce or timeout env variables in the same file to see if they have bounds
rg "DEBOUNCE|TIMEOUT|INTERVAL" apps/webapp/app/env.server.ts -A 1 -B 1Repository: triggerdotdev/trigger.dev
Length of output: 15620
🏁 Script executed:
# Search for usages of LLM_PRICING_RELOAD_DEBOUNCE_MS
rg "LLM_PRICING_RELOAD_DEBOUNCE_MS" -A 3 -B 3Repository: triggerdotdev/trigger.dev
Length of output: 1880
🏁 Script executed:
# Check the context where this debounce variable is used - look for debounce function calls
rg "debounce" --type ts -B 2 -A 2 | head -100Repository: triggerdotdev/trigger.dev
Length of output: 7594
🏁 Script executed:
# Look at how debounceMs is used in the llmPricingRegistry.server.ts file
cat -n apps/webapp/app/v3/llmPricingRegistry.server.ts | head -150Repository: triggerdotdev/trigger.dev
Length of output: 5508
🏁 Script executed:
# Get the full context of the scheduleReload function and how debounceMs is used
rg "scheduleReload|debounceMs|pendingReloadTimer" apps/webapp/app/v3/llmPricingRegistry.server.ts -B 2 -A 5Repository: triggerdotdev/trigger.dev
Length of output: 1367
Add a lower bound to debounce milliseconds to prevent misconfiguration.
At line 1428, LLM_PRICING_RELOAD_DEBOUNCE_MS accepts negative values which would cause setTimeout() to execute immediately (line 81 of llmPricingRegistry.server.ts) rather than applying the intended debounce delay.
Suggested patch
- LLM_PRICING_RELOAD_DEBOUNCE_MS: z.coerce.number().int().default(1000),
+ LLM_PRICING_RELOAD_DEBOUNCE_MS: z.coerce.number().int().min(0).default(1000),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| LLM_PRICING_RELOAD_DEBOUNCE_MS: z.coerce.number().int().default(1000), | |
| LLM_PRICING_RELOAD_DEBOUNCE_MS: z.coerce.number().int().min(0).default(1000), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/webapp/app/env.server.ts` at line 1428, LLM_PRICING_RELOAD_DEBOUNCE_MS
currently allows negative numbers which can make setTimeout in
llmPricingRegistry.server.ts run immediately; update the env schema for
LLM_PRICING_RELOAD_DEBOUNCE_MS to enforce a non-negative lower bound (e.g., use
z.coerce.number().int().min(0).default(1000) or equivalent) so values below 0
are rejected or coerced, and optionally add a defensive clamp (Math.max(0,
debounceMs)) at the point where the value is passed into setTimeout in
llmPricingRegistry.server.ts to guarantee a non-negative delay.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/webapp/app/v3/llmPricingRegistry.server.ts (2)
89-98: 💤 Low valueConsider extracting the duplicate shutdown body into a shared handler
The SIGTERM and SIGINT handlers (lines 89–93 and 94–98) are identical. A shared function also prevents
subscriber.quit()from being called twice if both signals arrive.♻️ Proposed refactor
+ function handleShutdown() { + clearInterval(interval); + if (pendingReloadTimer) clearTimeout(pendingReloadTimer); + void subscriber.quit().catch(() => {}); + } - signalsEmitter.on("SIGTERM", () => { - clearInterval(interval); - if (pendingReloadTimer) clearTimeout(pendingReloadTimer); - void subscriber.quit().catch(() => {}); - }); - signalsEmitter.on("SIGINT", () => { - clearInterval(interval); - if (pendingReloadTimer) clearTimeout(pendingReloadTimer); - void subscriber.quit().catch(() => {}); - }); + signalsEmitter.on("SIGTERM", handleShutdown); + signalsEmitter.on("SIGINT", handleShutdown);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/webapp/app/v3/llmPricingRegistry.server.ts` around lines 89 - 98, The SIGTERM and SIGINT listeners contain identical shutdown logic; extract that logic into a single handler (e.g., handleShutdown) that clears interval, clears pendingReloadTimer if set, and calls subscriber.quit(), and then have both signalsEmitter.on("SIGTERM", handleShutdown) and signalsEmitter.on("SIGINT", handleShutdown) (or use signalsEmitter.once to avoid double invocation); ensure the handler is idempotent by using a local boolean (e.g., isShuttingDown) so subscriber.quit() is not called twice.
46-54: ⚡ Quick winRemove unnecessary
keyPrefixfrom subscriber-only Redis clientThe
keyPrefixoption has no effect onSUBSCRIBEcommands in ioredis (confirmed by ioredis issues#1910and#306). Since this client only uses pub/sub (never key-value operations), the option is misleading and should be removed. This pattern is not seen elsewhere in subscriber-only clients throughout the codebase.♻️ Proposed fix
const subscriber = createRedisClient("llm-pricing:subscriber", { - keyPrefix: "llm-pricing:subscriber:", host: env.COMMON_WORKER_REDIS_HOST, port: env.COMMON_WORKER_REDIS_PORT, username: env.COMMON_WORKER_REDIS_USERNAME, password: env.COMMON_WORKER_REDIS_PASSWORD, tlsDisabled: env.COMMON_WORKER_REDIS_TLS_DISABLED === "true", clusterMode: env.COMMON_WORKER_REDIS_CLUSTER_MODE_ENABLED === "1", });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/webapp/app/v3/llmPricingRegistry.server.ts` around lines 46 - 54, The subscriber Redis client is created with a keyPrefix even though it only uses pub/sub, which is ineffective for SUBSCRIBE commands; update the createRedisClient call that assigns to subscriber to remove the keyPrefix option so the config passed to subscriber (variable name subscriber in llmPricingRegistry.server.ts) does not include keyPrefix while preserving all other options (host, port, username, password, tlsDisabled, clusterMode).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/webapp/app/v3/llmPricingRegistry.server.ts`:
- Around line 89-98: The SIGTERM and SIGINT listeners contain identical shutdown
logic; extract that logic into a single handler (e.g., handleShutdown) that
clears interval, clears pendingReloadTimer if set, and calls subscriber.quit(),
and then have both signalsEmitter.on("SIGTERM", handleShutdown) and
signalsEmitter.on("SIGINT", handleShutdown) (or use signalsEmitter.once to avoid
double invocation); ensure the handler is idempotent by using a local boolean
(e.g., isShuttingDown) so subscriber.quit() is not called twice.
- Around line 46-54: The subscriber Redis client is created with a keyPrefix
even though it only uses pub/sub, which is ineffective for SUBSCRIBE commands;
update the createRedisClient call that assigns to subscriber to remove the
keyPrefix option so the config passed to subscriber (variable name subscriber in
llmPricingRegistry.server.ts) does not include keyPrefix while preserving all
other options (host, port, username, password, tlsDisabled, clusterMode).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: ff2956cf-c19a-4197-aa0b-77ccac7fa470
📒 Files selected for processing (2)
apps/webapp/app/env.server.tsapps/webapp/app/v3/llmPricingRegistry.server.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/webapp/app/env.server.ts
📜 Review details
⏰ 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). (29)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / e2e-webapp / 🧪 E2E Tests: Webapp
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: typecheck / typecheck
- GitHub Check: sdk-compat / Node.js 20.20 (ubuntu-latest)
- GitHub Check: sdk-compat / Bun Runtime
- GitHub Check: sdk-compat / Cloudflare Workers
- GitHub Check: sdk-compat / Node.js 22.12 (ubuntu-latest)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: sdk-compat / Deno Runtime
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
Files:
apps/webapp/app/v3/llmPricingRegistry.server.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/v3/llmPricingRegistry.server.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/v3/llmPricingRegistry.server.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries
Files:
apps/webapp/app/v3/llmPricingRegistry.server.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: Access environment variables through theenvexport ofenv.server.tsinstead of directly accessingprocess.env
Use subpath exports from@trigger.dev/corepackage instead of importing from the root@trigger.dev/corepathUse named constants for sentinel/placeholder values (e.g.
const UNSET_VALUE = '__unset__') instead of raw string literals scattered across comparisons
Files:
apps/webapp/app/v3/llmPricingRegistry.server.ts
apps/webapp/**/*.server.ts
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
apps/webapp/**/*.server.ts: Never userequest.signalfor detecting client disconnects. UsegetRequestAbortSignal()fromapp/services/httpAsyncStorage.server.tsinstead, which is wired directly to Expressres.on('close')and fires reliably
Access environment variables viaenvexport fromapp/env.server.ts. Never useprocess.envdirectly
Always usefindFirstinstead offindUniquein Prisma queries.findUniquehas an implicit DataLoader that batches concurrent calls and has active bugs even in Prisma 6.x (uppercase UUIDs returning null, composite key SQL correctness issues, 5-10x worse performance).findFirstis never batched and avoids this entire class of issues
Files:
apps/webapp/app/v3/llmPricingRegistry.server.ts
{apps,internal-packages}/**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
pnpm run typecheckto verify changes in apps and internal packages (apps/*,internal-packages/*) instead ofbuild, which proves almost nothing about correctness
Files:
apps/webapp/app/v3/llmPricingRegistry.server.ts
{package.json,**/*.{ts,tsx,js}}
📄 CodeRabbit inference engine (CLAUDE.md)
Pin Zod to version 3.25.76 exactly across the entire monorepo - never use a different version or version range
Files:
apps/webapp/app/v3/llmPricingRegistry.server.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js}: Import from@trigger.dev/coreusing subpaths only, never the root export
Always import tasks from@trigger.dev/sdk, never from@trigger.dev/sdk/v3or deprecatedclient.defineJob
Add crumbs to code using//@Crumbscomments or `// `#region` `@crumbsblocks for debug tracing during development
Files:
apps/webapp/app/v3/llmPricingRegistry.server.ts
**/*.{ts,tsx,js,jsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Code formatting is enforced using Prettier. Run
pnpm run formatbefore committing
Files:
apps/webapp/app/v3/llmPricingRegistry.server.ts
🧠 Learnings (4)
📚 Learning: 2026-03-22T13:26:12.060Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3244
File: apps/webapp/app/components/code/TextEditor.tsx:81-86
Timestamp: 2026-03-22T13:26:12.060Z
Learning: In the triggerdotdev/trigger.dev codebase, do not flag `navigator.clipboard.writeText(...)` calls for `missing-await`/`unhandled-promise` issues. These clipboard writes are intentionally invoked without `await` and without `catch` handlers across the project; keep that behavior consistent when reviewing TypeScript/TSX files (e.g., usages like in `apps/webapp/app/components/code/TextEditor.tsx`).
Applied to files:
apps/webapp/app/v3/llmPricingRegistry.server.ts
📚 Learning: 2026-03-22T19:24:14.403Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3187
File: apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.ts:200-204
Timestamp: 2026-03-22T19:24:14.403Z
Learning: In the triggerdotdev/trigger.dev codebase, webhook URLs are not expected to contain embedded credentials/secrets (e.g., fields like `ProjectAlertWebhookProperties` should only hold credential-free webhook endpoints). During code review, if you see logging or inclusion of raw webhook URLs in error messages, do not automatically treat it as a credential-leak/secrets-in-logs issue by default—first verify the URL does not contain embedded credentials (for example, no username/password in the URL, no obvious secret/token query params or fragments). If the URL is credential-free per this project’s conventions, allow the logging.
Applied to files:
apps/webapp/app/v3/llmPricingRegistry.server.ts
📚 Learning: 2026-03-29T19:16:28.864Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 3291
File: apps/webapp/app/v3/featureFlags.ts:53-65
Timestamp: 2026-03-29T19:16:28.864Z
Learning: When reviewing TypeScript code that uses Zod v3, treat `z.coerce.*()` schemas as their direct Zod type (e.g., `z.coerce.boolean()` returns a `ZodBoolean` with `_def.typeName === "ZodBoolean"`) rather than a `ZodEffects`. Only `.preprocess()`, `.refine()`/`.superRefine()`, and `.transform()` are expected to wrap schemas in `ZodEffects`. Therefore, in reviewers’ logic like `getFlagControlType`, do not flag/unblock failures that require unwrapping `ZodEffects` when the input schema is a `z.coerce.*` schema.
Applied to files:
apps/webapp/app/v3/llmPricingRegistry.server.ts
📚 Learning: 2026-05-05T09:38:02.512Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3523
File: apps/webapp/app/routes/api.v3.batches.ts:178-181
Timestamp: 2026-05-05T09:38:02.512Z
Learning: When reviewing code that catches `ServiceValidationError` in `*.server.ts` files, do not blindly forward `error.status` to HTTP responses, because SVEs may be thrown with non-default statuses (e.g., 400/500) and forwarding them can cause client-visible behavioral regressions (e.g., surfacing 500s to clients). Prefer a safe default response status of `error.status ?? 422`, but only after confirming via the reachable call graph that the caught `ServiceValidationError` instances are expected to carry those non-default statuses; otherwise, normalize to `422` to avoid unexpected client-visible 5xx behavior.
Applied to files:
apps/webapp/app/v3/llmPricingRegistry.server.ts
🔇 Additional comments (2)
apps/webapp/app/v3/llmPricingRegistry.server.ts (2)
63-87: Debounce + message filter LGTMThe debounce guard on line 73 correctly coalesces concurrent publishes into at most one reload per window. Nulling
pendingReloadTimerbefore callingregistry.reload()(line 75) ensures bursts that arrive during an in-flight reload schedule exactly one trailing reload — as intended. The channel filter on line 85 is also correct since, as confirmed, ioredis does not applykeyPrefixtoSUBSCRIBEcommands and reports the raw channel name in themessageevent.
52-53: No issue found—these variables are correctly typed as strings inenv.server.ts.
COMMON_WORKER_REDIS_TLS_DISABLEDandCOMMON_WORKER_REDIS_CLUSTER_MODE_ENABLEDare defined asz.string()(not coerced to boolean), so the string comparisons at lines 52–53 are correct.> Likely an incorrect or invalid review comment.
2 participants
Summary
Adds a Redis pub/sub reload path to the webapp's in-memory LLM pricing registry. When enabled on a process, the registry reloads from the database whenever a publish lands on the configured channel — instead of waiting for the existing 5-minute interval. Lets pricing/model changes propagate to cost enrichment within seconds.
Subscription is off by default and opt-in per process. Only OTel-ingesting services need real-time freshness; dashboard and worker services run fine on the periodic interval and shouldn't pile onto each publish with a full-table reload.
Design
When
LLM_PRICING_RELOAD_PUBSUB_ENABLED=true, subscribes viacreateRedisClientagainstCOMMON_WORKER_REDIS_*and listens onLLM_PRICING_RELOAD_CHANNEL(defaultllm-registry:reload). The 5-minute periodic reload stays as a backstop, and a SIGTERM/SIGINT handler closes the subscription cleanly.The publisher side lives outside this PR — any process running in the same Redis namespace can trigger a reload by
PUBLISH llm-registry:reload <anything>. Includes a.server-changes/note for the changelog.Debounced reload
Bursts of publishes are coalesced. The first publish schedules a reload at T+
LLM_PRICING_RELOAD_DEBOUNCE_MS(default 1s); subsequent publishes during that window are no-ops because the trailing reload picks up everything when it queries the DB. Bounds reload rate to at most 1 per debounce window regardless of publisher chattiness, so a runaway upstream publisher can't fan out into a flood of full-table-scan reloads.Test plan
LLM_PRICING_RELOAD_PUBSUB_ENABLED=false(default):redis-cli PUBSUB NUMSUB llm-registry:reloadreturns0while the webapp is uptrue: returns>= 1redis-cli PUBLISH llm-registry:reload testreturns1(one subscriber received) on a subscribed processLlmModelrow externally, publish on the channel, observe the registry's match() picks up the change without waiting for the 5-min tick