perf(webapp): bound missing LLM model queries - #4877
Conversation
|
WalkthroughThe missing-model service now validates lookback hours from 1 to 720, caches ClickHouse query promises, evicts failed promises, and logs query activity. Queries use shared settings and Merge Risk: ⚪ Minimal · up to The PR bounds and caches the admin diagnostics queries without introducing a demonstrated production-impacting defect. No actionable merge-blocking risk remains beyond normal review and checks. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 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 |
223bf38 to
c175655
Compare
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
c175655 to
3c5f83f
Compare
@trigger.dev/build
trigger.dev
@trigger.dev/core
@trigger.dev/python
@trigger.dev/react-hooks
@trigger.dev/redis-worker
@trigger.dev/rsc
@trigger.dev/schema-to-json
@trigger.dev/sdk
commit: |
e8c2412 to
fa495e7
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Team
Run ID: a7aa1296-6061-455e-b1b2-75a174937b6f
📒 Files selected for processing (2)
apps/webapp/app/services/admin/missingLlmModels.server.tsapps/webapp/app/services/clickhouse/clickhouseFactory.server.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.
📜 Review details
⏰ Context from checks skipped due to timeout. (34)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (21, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (23, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (22, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (24, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (16, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (19, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (13, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (20, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (15, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (11, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (18, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (5, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (2, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (3, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (17, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (9, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (12, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (4, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (6, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (14, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (10, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (8, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (1, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (7, 24)
- GitHub Check: obsmap / 🧪 Unit Tests: Observability Map
- GitHub Check: fk-cascade-guard / fk-cascade-guard
- GitHub Check: typecheck / typecheck
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (1, 2)
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (2, 2)
- GitHub Check: runops-guard / runops-guard
- GitHub Check: code-quality / code-quality
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: audit
- GitHub Check: audit
🧰 Additional context used
📓 Path-based instructions (9)
Never use `request.signal` to detect client disconnects.
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Files:
apps/webapp/app/services/clickhouse/clickhouseFactory.server.tsapps/webapp/app/services/admin/missingLlmModels.server.ts
For dashboard changes, visually verify the running Remix app with Chrome DevTools MCP, using snapshots, screenshots, interaction, and console-message checks as appropriate.
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Files:
apps/webapp/app/services/clickhouse/clickhouseFactory.server.tsapps/webapp/app/services/admin/missingLlmModels.server.ts
**Prefer static imports over dynamic imports.**
📄 CodeRabbit inference engine (AGENTS.md)
Files:
apps/webapp/app/services/clickhouse/clickhouseFactory.server.tsapps/webapp/app/services/admin/missingLlmModels.server.ts
Add crumbs as you write code — not just when debugging.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
apps/webapp/app/services/clickhouse/clickhouseFactory.server.tsapps/webapp/app/services/admin/missingLlmModels.server.ts
Use zod for validation in packages/core and apps/webapp
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/webapp/app/services/clickhouse/clickhouseFactory.server.tsapps/webapp/app/services/admin/missingLlmModels.server.ts
Access environment variables through the `env` export of `env.server.ts` instead of directly accessing `process.env` Use subpath exports from `@trigger.dev/core` package instead of importing from the root `@trigger.dev/core` path
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
apps/webapp/app/services/clickhouse/clickhouseFactory.server.tsapps/webapp/app/services/admin/missingLlmModels.server.ts
Use function declarations instead of default exports
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/webapp/app/services/clickhouse/clickhouseFactory.server.tsapps/webapp/app/services/admin/missingLlmModels.server.ts
Use types over interfaces for TypeScript Avoid using enums; prefer string unions or const objects instead
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/webapp/app/services/clickhouse/clickhouseFactory.server.tsapps/webapp/app/services/admin/missingLlmModels.server.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 metr...
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
Files:
apps/webapp/app/services/clickhouse/clickhouseFactory.server.tsapps/webapp/app/services/admin/missingLlmModels.server.ts
🧠 Learnings (3)
📚 Learning: 2026-06-04T18:16:35.386Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 3836
File: apps/supervisor/src/backpressure/backpressureMonitor.ts:3-5
Timestamp: 2026-06-04T18:16:35.386Z
Learning: When reviewing TypeScript in this repo, apply the rule “prefer type aliases over interfaces” only to data/object shapes and union/intersection type modeling. If an interface is being used as a behavioral contract for collaborators to implement (e.g., method-shape interfaces that define required behavior, such as `BackpressureLogger` / `BackpressureSignalSource` in `apps/supervisor/src/backpressure/backpressureMonitor.ts`), keep it as an `interface` and do not flag it as a type-alias-vs-interface violation.
Applied to files:
apps/webapp/app/services/admin/missingLlmModels.server.ts
📚 Learning: 2026-08-20T20:04:24.997Z
Learnt from: D-K-P
Repo: triggerdotdev/trigger.dev PR: 4743
File: apps/webapp/app/services/platformNotifications.server.ts:650-773
Timestamp: 2026-08-20T20:04:24.997Z
Learning: In this repository, `crumbs` markers and `#region crumbs` blocks are temporary development instrumentation that is removed before merge. Do not flag their presence or absence during code review in TypeScript files.
Applied to files:
apps/webapp/app/services/admin/missingLlmModels.server.ts
📚 Learning: 2026-08-21T14:06:29.516Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 4741
File: apps/webapp/app/components/integrations/VercelBuildSettings.tsx:22-30
Timestamp: 2026-08-21T14:06:29.516Z
Learning: In triggerdotdev/trigger.dev webapp TypeScript and TSX files, treat `// crumbs`, `#region crumbs`, and agentcrumbs imports as temporary runtime-debugging instrumentation. Do not require contributors to add these markers or flag existing markers during review; they are used only when runtime tracing is needed during development and are removed before merge.
Applied to files:
apps/webapp/app/services/admin/missingLlmModels.server.ts
🔇 Additional comments (2)
apps/webapp/app/services/clickhouse/clickhouseFactory.server.ts (1)
100-100: LGTM!Also applies to: 114-114, 126-129
apps/webapp/app/services/admin/missingLlmModels.server.ts (1)
90-92: 🎯 Functional CorrectnessNo change needed:
prewhere()emits all three predicates joined withAND.
| function validateLookbackHours(lookbackHours = DEFAULT_LOOKBACK_HOURS): number { | ||
| if (!Number.isInteger(lookbackHours) || lookbackHours < 1 || lookbackHours > MAX_LOOKBACK_HOURS) { | ||
| throw new RangeError(`lookbackHours must be between 1 and ${MAX_LOOKBACK_HOURS}`); | ||
| } | ||
|
|
||
| return lookbackHours; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Confirm the webapp's installed Zod version and local schema conventions.
fd -a '^package\.json$' apps/webapp | xargs -r rg -n '"zod"'
rg -n -C 2 --glob '*.{ts,tsx}' 'from "zod"|z\.(number|object)' apps/webapp | head -n 160Repository: triggerdotdev/trigger.dev
Length of output: 14176
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- repository conventions and learnings ---'
find /tmp/coderabbit-repo-knowledge/triggerdotdev-trigger-dev-0bdd0019 \
-type f \( -path '*/apps/webapp/*' -o -path '*/learnings/*' \) -name '*.md' -print \
-exec sh -c 'echo "--- $1"; cat "$1"' _ {} \;
printf '%s\n' '--- target file ---'
cat -n apps/webapp/app/services/admin/missingLlmModels.server.ts | sed -n '1,215p'Repository: triggerdotdev/trigger.dev
Length of output: 49878
Use Zod for lookbackHours validation.
Both getMissingLlmModels and getMissingModelSamples call validateLookbackHours. Replace the hand-written checks with a Zod schema that preserves the default and 1–720 range.
Source: Coding guidelines
2 participants
Summary
Protect missing LLM model diagnostics from expensive event scans by filtering narrow columns before parsing attributes, enforcing per-query resource limits, and reusing recent aggregate results.
Design
Both queries apply their time, kind, and status predicates in
PREWHEREbefore readingattributes_text. Lookbacks are constrained to the existing supported range, while execution time, memory, and thread use are bounded per query.Aggregate results are cached for one minute with a small entry limit. Concurrent requests share the same in-flight query, and failed queries are removed from the cache immediately.
This PR builds on #4866, which builds on #4860.