Repository navigation
feat(devpass): default service tier setting - #3071
Conversation
Add a DevPass dashboard setting to choose the default service tier for dev-plan routing. Defaults to standard; can be set to flex to save plan credits. The gateway applies the org-level flex default only when the request doesn't specify service_tier itself and only for provider mappings that support flex with an upstream-eligible credential, so unsupported models stay on standard processing instead of failing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HfQCYRgCLi3GvW5ymvTkDE
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Caution Review failedFailed to post review comments. We encountered an issue with GitHub. Use ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout. (16)
🧰 Additional context used📓 Path-based instructions (3)**/*.{ts,tsx,js,jsx,json,md,yml,yaml}📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/db/migrations/**📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*📄 CodeRabbit inference engine (AGENTS.md)
Files:
WalkthroughAdds a persisted DevPlan service-tier setting, exposes it through APIs and the dashboard, applies eligible DevPlan-flex defaults in chat routing, and updates service-tier metadata plus gateway test coverage. ChangesDevPlan service tier
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Dashboard
participant DevPlanAPI
participant Database
participant ChatClient
participant Gateway
participant ProviderCredentials
Dashboard->>DevPlanAPI: update devPlanServiceTier
DevPlanAPI->>Database: persist organization tier
ChatClient->>Gateway: submit chat request
Gateway->>Database: load organization DevPlan tier
Gateway->>ProviderCredentials: check flex routing eligibility
ProviderCredentials-->>Gateway: return credential eligibility
Gateway-->>ChatClient: return response with requested and used service tiers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
apps/code/src/app/dashboard/components/DevPlanSettings.tsx (1)
134-142: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueInvalidate the query cache to keep global state in sync.
Consider calling
await invalidateStatus()here after the successful mutation, similar to how it is done inhandleRetentionToggle. This ensures that any other components relying onuseDevPlanStatusreflect the updated service tier immediately without waiting for a refetch (e.g., on window focus).♻️ Proposed refactor
try { await updateSettingsMutation.mutateAsync({ body: { devPlanServiceTier: tier }, }); + await invalidateStatus(); toast.success( tier === "flex" ? "Requests default to flex processing"🤖 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/code/src/app/dashboard/components/DevPlanSettings.tsx` around lines 134 - 142, After the successful updateSettingsMutation.mutateAsync call in the tier update handler, await invalidateStatus() before displaying the success toast, matching the existing handleRetentionToggle flow so useDevPlanStatus consumers receive the updated service tier immediately.apps/gateway/src/chat/chat.ts (2)
2017-2019: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winReuse
serviceTierOrgKeysto avoid redundant cache/database calls.If
serviceTierOrgKeysis already declared in the outer scope at this point (as seen utilized later down inenforceServiceTierKeyEligibility), consider populating and reusing it here to save an unnecessaryfindActiveProviderKeyscache/DB call.♻️ Proposed refactor
- const orgKeysForDefaultTier = await findActiveProviderKeys( - project.organizationId, - ); + if (serviceTierOrgKeys === undefined) { + serviceTierOrgKeys = await findActiveProviderKeys(project.organizationId); + }(and update the reference inside
providerHasEligibleTierCredentialto useserviceTierOrgKeys!instead oforgKeysForDefaultTier)🤖 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/gateway/src/chat/chat.ts` around lines 2017 - 2019, Reuse the outer-scope serviceTierOrgKeys when determining default-tier provider keys instead of calling findActiveProviderKeys again, populating it only when needed and preserving the existing lookup result. Update providerHasEligibleTierCredential to reference serviceTierOrgKeys! rather than orgKeysForDefaultTier, while keeping enforceServiceTierKeyEligibility aligned with the shared value.
2000-2002: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSimplify boolean expression and remove redundant optional chaining.
Since
organizationis already checked and guaranteed to be non-null above (it throws a 500 otherwise), the optional chaining?.is unnecessary. Additionally, the explicitBoolean()wrapper can be safely removed, as the expression inherently evaluates to a boolean.♻️ Proposed refactor
- const isDevPlan = Boolean( - organization?.kind === "devpass" && organization.devPlan !== "none", - ); + const isDevPlan = + organization.kind === "devpass" && organization.devPlan !== "none";🤖 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/gateway/src/chat/chat.ts` around lines 2000 - 2002, Update the isDevPlan assignment to remove the redundant Boolean wrapper and optional chaining, using the already validated organization object directly in the boolean comparison. Preserve the existing devpass and devPlan !== "none" conditions.
🤖 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/gateway/src/chat/tools/transform-response-to-openai.ts`:
- Around line 61-63: Update the conditional spread in the response
transformation to check requestedServiceTier and usedServiceTier directly
against undefined rather than relying on truthiness, so an explicitly assigned
null usedServiceTier still emits used_service_tier: null.
---
Nitpick comments:
In `@apps/code/src/app/dashboard/components/DevPlanSettings.tsx`:
- Around line 134-142: After the successful updateSettingsMutation.mutateAsync
call in the tier update handler, await invalidateStatus() before displaying the
success toast, matching the existing handleRetentionToggle flow so
useDevPlanStatus consumers receive the updated service tier immediately.
In `@apps/gateway/src/chat/chat.ts`:
- Around line 2017-2019: Reuse the outer-scope serviceTierOrgKeys when
determining default-tier provider keys instead of calling findActiveProviderKeys
again, populating it only when needed and preserving the existing lookup result.
Update providerHasEligibleTierCredential to reference serviceTierOrgKeys! rather
than orgKeysForDefaultTier, while keeping enforceServiceTierKeyEligibility
aligned with the shared value.
- Around line 2000-2002: Update the isDevPlan assignment to remove the redundant
Boolean wrapper and optional chaining, using the already validated organization
object directly in the boolean comparison. Preserve the existing devpass and
devPlan !== "none" conditions.
🪄 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: 2a5a7584-b287-42b7-940b-77a79112c207
📒 Files selected for processing (13)
apps/api/src/routes/dev-plans.tsapps/api/src/routes/organization.tsapps/code/src/app/dashboard/(main)/settings/page.tsxapps/code/src/app/dashboard/components/DevPlanSettings.tsxapps/gateway/src/api.spec.tsapps/gateway/src/chat/chat.tsapps/gateway/src/chat/tools/transform-response-to-openai.tsapps/gateway/src/lib/rate-limit.spec.tsapps/gateway/src/test-utils/gateway-api-test-harness.tspackages/db/migrations/1784117115_lonely_the_liberteens.sqlpackages/db/migrations/meta/1784117115_snapshot.jsonpackages/db/migrations/meta/_journal.jsonpackages/db/src/schema.ts
| ...(extras.requestedServiceTier || extras.usedServiceTier | ||
| ? { used_service_tier: extras.usedServiceTier ?? null } | ||
| : {}), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Check against undefined to safely include null values.
If the intent is to surface used_service_tier: null when an organization-default tier is downgraded by the provider, this condition will fail to do so. Since null is falsy, extras.requestedServiceTier || extras.usedServiceTier evaluates to false when requestedServiceTier is undefined and usedServiceTier is null, causing the field to be omitted rather than emitted as null.
To safely include explicitly assigned null values, compare directly against undefined.
♻️ Proposed fix
- ...(extras.requestedServiceTier || extras.usedServiceTier
+ ...(extras.requestedServiceTier !== undefined || extras.usedServiceTier !== undefined
? { used_service_tier: extras.usedServiceTier ?? null }
: {}),📝 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.
| ...(extras.requestedServiceTier || extras.usedServiceTier | |
| ? { used_service_tier: extras.usedServiceTier ?? null } | |
| : {}), | |
| ...(extras.requestedServiceTier !== undefined || extras.usedServiceTier !== undefined | |
| ? { used_service_tier: extras.usedServiceTier ?? null } | |
| : {}), |
🤖 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/gateway/src/chat/tools/transform-response-to-openai.ts` around lines 61
- 63, Update the conditional spread in the response transformation to check
requestedServiceTier and usedServiceTier directly against undefined rather than
relying on truthiness, so an explicitly assigned null usedServiceTier still
emits used_service_tier: null.
End-user wallets are always backed by regular PAYG/credits orgs, never dev-plan orgs, so the withWalletCredits substitution has no bearing on the dev-plan default service tier and stays where credit gating needs it. Document the invariant in AGENTS.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HfQCYRgCLi3GvW5ymvTkDE
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HfQCYRgCLi3GvW5ymvTkDE
…e-tier-setting-c1yn5q
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HfQCYRgCLi3GvW5ymvTkDE
Summary
Adds a Default service tier setting to the DevPass dashboard so users can opt their dev-plan routing into cheaper flex processing to save on plan credits. Defaults to standard; flex is optional.
Dashboard (apps/code)
PATCH /dev-plans/settingswith optimistic state + rollback on failure, linking to the service-tiers docs page.API (apps/api)
devPlanServiceTier("default" | "flex") added toGET /dev-plans/statusandPATCH /dev-plans/settings(with audit logging of changes), and to the organization response schema.DB (packages/db)
organization.devPlanServiceTiercolumn (text enum, not null, default"default") + generated migration.Gateway (apps/gateway)
service_tieritself, the gateway appliesservice_tier: "flex"before service-tier provider filtering. The org fetch (andisDevPlan) was moved ahead of that filtering block to make the setting available there; the end-user-wallet credits substitution (withWalletCredits) stays at its original site, since orgs backing end-user wallets are always regular PAYG/credits orgs — never dev-plan orgs — so it cannot affect the dev-plan default (invariant documented in AGENTS.md).service_tieron the request always wins over the org default.used_service_tieris now surfaced in response metadata whenever a premium tier was actually served, even without a client-requested tier, so default-applied flex stays visible. Billing already keys off the served tier, so the flex multiplier applies automatically.Tests
usedServiceTier: "flex",requestedServiceTier: null), explicitservice_tieroverrides the default, and the default is skipped for models without flex support.setDevPlannow acceptsserviceTier; existing mock org literals updated for the new column.🤖 Generated with Claude Code
https://claude.ai/code/session_01HfQCYRgCLi3GvW5ymvTkDE
Summary by CodeRabbit