perf(webapp): cache deployment logs across navigations - #4775
Conversation
Switching between deployments in the dashboard re-read the whole build log stream from the start every time. Logs are now kept in a small in-memory, per-tab cache keyed by deployment, seeded instantly on revisit, and the stream is resumed from the next unread record instead of record zero. A deployment whose stream has emitted its finalized event and reached a terminal status is served from the cache without opening a stream at all. The cache is bounded to 20 deployments and 20k log lines total, evicting least recently viewed deployments first. Incoming records are also batched into one state update per tick instead of one per line.
|
|
Preview Deployment
|
WalkthroughAdds a bounded Merge Risk: 🔵 Low · up to The cache change can leave in-progress logs stale after an idle session ends, delay reuse of finalized logs by up to 60 seconds, and allow some legacy log levels to distort warning or error counts. These are bounded freshness and display-correctness issues, so the PR is mergeable with explicit owner follow-up rather than blocked. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Description checkExplanation The description explains the problem, implementation, cache limits, resumption behavior, and batching. It does not follow the repository template because it omits the issue reference, checklist, testing steps, changelog section, and screenshots section. ✨ 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 |
…timeouts An in-progress deployment that produced no log output for 60 seconds ended the read session and stopped streaming until the next status change. The session is now reopened from the next unread record until the deployment is finalized.
Observability mapAs of 19/100 over 449 measured of 467 entry points (base 19, no change) What this PR changed
FIX FIRST
AUDIT 3 of 50 sensitive mutations record an actor. 47 without one. What the score is made ofThe score and findings here are report-only and never gate the merge. Separately, a required test suite keeps this tool's symbol and route lists in sync with the code they name, and can fail a pull request that renames or removes a symbol they reference, or that adds the first route with a segment they anticipate. Each failure names the list to edit. The rules and their reasons: internal-packages/observability-map/README.md. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/webapp/app/hooks/useDeploymentLogs.ts (1)
97-139: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd crumb markers to the streaming path.
Add
//@Crumbsmarkers or a `// `#region` `@crumbsblock around session creation, resume sequence updates, finalization, and error exits. Keep the markers on the branch untilagentcrumbs stripruns before merge.As per coding guidelines: “Add crumbs as you write code — not just when debugging.”
Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: bbe77c1b-cb40-4cad-915d-161e72adb3c7
📒 Files selected for processing (1)
apps/webapp/app/hooks/useDeploymentLogs.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review.
📜 Review details
⏰ Context from checks skipped due to timeout. (33)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (4, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (17, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (20, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (18, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (22, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (15, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (6, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (21, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (3, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (16, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (23, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (24, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (19, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (10, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (2, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (1, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (5, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (14, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (11, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (12, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (9, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (7, 24)
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (2, 2)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (13, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (8, 24)
- GitHub Check: obsmap / 🧪 Unit Tests: Observability Map
- GitHub Check: typecheck / typecheck
- GitHub Check: fk-cascade-guard / fk-cascade-guard
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (1, 2)
- GitHub Check: runops-guard / runops-guard
- GitHub Check: code-quality / code-quality
- GitHub Check: report
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (9)
Never use `request.signal` to detect client disconnects. Use `getRequestAbortSignal()` from `app/services/httpAsyncStorage.server.ts`, which is wired to Express response close events.
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Files:
apps/webapp/app/hooks/useDeploymentLogs.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/hooks/useDeploymentLogs.ts
**Prefer static imports over dynamic imports.** Only use dynamic `import()` when:
📄 CodeRabbit inference engine (AGENTS.md)
Files:
apps/webapp/app/hooks/useDeploymentLogs.ts
Add crumbs as you write code — not just when debugging. Mark lines with
📄 CodeRabbit inference engine (AGENTS.md)
Files:
apps/webapp/app/hooks/useDeploymentLogs.ts
Use zod for validation in packages/core and apps/webapp
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/webapp/app/hooks/useDeploymentLogs.ts
Access environment variables through the `env` export of `env.server.ts` instead of directly accessing `process.env`
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
apps/webapp/app/hooks/useDeploymentLogs.ts
Use function declarations instead of default exports
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/webapp/app/hooks/useDeploymentLogs.ts
Use types over interfaces for TypeScript
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/webapp/app/hooks/useDeploymentLogs.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
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
Files:
apps/webapp/app/hooks/useDeploymentLogs.ts
Switching between deployments in the dashboard re-fetched the whole build log stream from record zero and re-rendered the list line by line every time. Logs are now cached per deployment for the lifetime of the tab: revisiting a deployment shows its logs immediately, and the stream is resumed from the next unread record rather than restarted. Finished deployments whose stream has been read through the
finalizedevent are served entirely from the cache.Changes
The stream/cache logic moved out of the route into a
useDeploymentLogshook. On each deployment switch it seeds state from the cache, resumes the S2 read session atnextSeqNum, and writes back on cleanup or natural session end. Completion is derived from the stream's ownfinalizedevent (plus a terminal deployment status), not from the session closing, so a session cut short by token expiry or a proxy cannot pin a truncated log in the cache.Memory is bounded by a small LRU (
deploymentLogsCache): at most 20 deployments and 20,000 log lines in total, least recently viewed evicted first. The most recently viewed deployment is always kept, so a single very large log can temporarily exceed the line budget on its own. Records are batched into one state update per tick instead of one per line.