You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
PR #5098 Review: feat(frontend): feature flag compute
Overview
This PR introduces a new compute feature flag that gates the Rivet Compute (managed pool) UI — namespace deployments, logs, actor deployment-logs tab, and the Rivet provider option in onboarding/Add Provider. The flag is opt-in even on cloud (not default-on), set per-environment via VITE_FEATURE_FLAGS. It also includes documentation improvements to rivet-compute.mdx.
Issues
1. Rule-of-Hooks violation in layout.tsx (biome-ignore suppression)
functionDeploymentsLink(){if(!features.compute){returnnull;}// biome-ignore lint/correctness/useHookAtTopLevel: guarded by build constantconstprovider=useCloudNamespaceDataProvider();// biome-ignore lint/correctness/useHookAtTopLevel: guarded by build constantconst{ data }=useSuspenseQuery(...)}
The biome-ignore comments correctly suppress the lint rule, and the rationale (guarded by build constant) is sound if features.compute is truly a compile-time constant. However, the adjacent comment text explains what the suppression is for but not why it's safe — it's safe because features.compute is evaluated at module load time from VITE_FEATURE_FLAGS and never changes at runtime. The existing pattern in actors-actor-details.tsx already uses this same convention, so it's consistent, but it requires care if this pattern spreads. Worth confirming nothing can make features.compute dynamic at runtime.
2. Unexplained field rename in upsert-deployment-frame.tsx
This changes two separate fields (minCount + maxCount) to one (maxConcurrentActors), and drops the numeric separator convention (100_000 → 10000). The PR description doesn't explain this change. It looks like an API schema update but is buried in a feature-flag PR without context. If this matches a backend API change it should be mentioned. If minCount: 0 is being silently dropped, confirm the default is handled server-side.
3. PR description is unfilled boilerplate
The PR description is the default template with every checkbox unchecked and no summary. For a multi-file change affecting routing, feature flags, onboarding flow, and docs, a brief description of what changed and how it was tested would be helpful.
Minor Notes
Feature flag definition (features.ts): compute: isEnabled("compute") && platform correctly enforces the platform prerequisite as documented.
Query enabled guards (actors-grid.tsx): Adding enabled: features.compute to managed pool queries is the right pattern — no unnecessary API calls when the feature is off.
Route beforeLoad redirects (deployments.tsx, logs.tsx): Clean guard using throw redirect() consistent with the CLAUDE.md routing conventions.
Provider filter (getting-started.tsx): features.compute || option.name !== 'rivet' logic is correct. The col-span-2 py-5 layout class is also properly gated, so layout won't break when the Rivet card is absent.
Default provider useEffect (getting-started.tsx ~line 494): The early return if (!features.compute) return; prevents auto-selecting "rivet" as default provider when compute is off. Confirm this is the intended UX — with no default, the user must manually pick a provider.
Docs fix (rivet-compute.mdx): Correcting "Rivet Cloud" → "Rivet Compute" and updating the code example from startRunner() to serve()/handler() is a good standalone improvement.
Flag docs (.claude/reference/feature-flags.md): The note that compute is opt-in even on cloud is clear and follows the doc style.
Summary
The feature flag implementation is clean and consistent with the project's established patterns. The main things to address: (1) explain the upsert-deployment-frame.tsx API field rename, and (2) confirm the no-default-provider UX when compute is off is intentional. The biome-ignore hook suppression is acceptable given the build-constant rationale but should stay limited to this pattern.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Please include a summary of the changes and the related issue. Please also include relevant motivation and context.
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
Checklist: