Cache pricing + custom model entry - #736
Merged
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe changes add cache pricing and custom model handling to AI provider configuration, preserve cache-rate fields through API mapping, enable regular onboarding for self-hosted deployments with end-to-end coverage, and correct access-log and table sort displays. ChangesAI provider cache pricing
Self-hosted onboarding
Access log and table display corrections
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ModelRowEditor
participant AIProviderModal
participant AIProvidersProvider
participant ProviderAPI
ModelRowEditor->>AIProviderModal: submit model and cache pricing values
AIProviderModal->>AIProvidersProvider: send sanitized model list
AIProvidersProvider->>ProviderAPI: serialize optional cache-rate fields
ProviderAPI-->>AIProvidersProvider: return provider model data
AIProvidersProvider-->>AIProviderModal: map cache-rate fields for editing
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
e2e/tests/onboarding-form.spec.ts (1)
162-169: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftUse the shared dashboard fixture for authentication.
The new specs create an owner context from
owner.jsonand authenticate vialoginToApp(page, "owner"). The E2E guidelines requiredashboardAsOwnerordashboardAsUser; move the edition/localStorage setup into a fixture-compatible helper instead of maintaining a separate authentication path.As per coding guidelines, onboarding specs should use the shared dashboard authentication fixtures.
Also applies to: 181-189
🤖 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 `@e2e/tests/onboarding-form.spec.ts` around lines 162 - 169, Update the onboarding tests around openOnboarding to use the shared dashboardAsOwner or dashboardAsUser fixture for authentication instead of creating an owner context from owner.json and calling loginToApp. Move the edition and localStorage setup into a fixture-compatible helper, while preserving the existing onboardingFlowPending and intent-step assertions.Source: Coding guidelines
🤖 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 `@e2e/tests/onboarding-form.spec.ts`:
- Around line 108-112: Update openOnboarding around mockAccounts and loginToApp
to wait for the matching /api/accounts response before returning. Register
page.waitForResponse for the accounts request before invoking loginToApp, await
it after login completes, and preserve the existing mocked delay and onboarding
expectations.
- Around line 173-175: Update the intent-step assertion in the onboarding test
to use page.getByTestId("regular-onboarding") instead of the text-based
getByText selector, while preserving the existing visibility expectation.
In `@src/modules/agent-network/AIProviderModal.tsx`:
- Around line 402-404: Update the submittedModels sanitization in
AIProviderModal to deduplicate models by their trimmed id, preserving only the
first row for each non-empty id. Keep trimming and blank-id filtering, and
ensure duplicate custom entries cannot be included in the save payload.
- Around line 1478-1482: Update the onChange handler in AIProviderModal to
validate the trimmed non-empty input before calling priceFromInput. Treat
non-numeric or otherwise invalid values as undefined rather than parsing them to
0, while preserving the existing undefined behavior for empty input and normal
parsing for valid prices.
- Around line 1551-1554: Update the customMode state synchronization near its
initializer so that when the catalog finishes loading and row.id is non-empty
but absent from catalogModels, customMode is set to true. Preserve user-driven
custom-mode changes and avoid resetting the state on unrelated catalog or row
updates; use the existing hasCatalog, row.id, and catalogModels values.
---
Nitpick comments:
In `@e2e/tests/onboarding-form.spec.ts`:
- Around line 162-169: Update the onboarding tests around openOnboarding to use
the shared dashboardAsOwner or dashboardAsUser fixture for authentication
instead of creating an owner context from owner.json and calling loginToApp.
Move the edition and localStorage setup into a fixture-compatible helper, while
preserving the existing onboardingFlowPending and intent-step assertions.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 19b7acb5-b6aa-434b-ab44-92d3be57589e
📒 Files selected for processing (6)
e2e/tests/onboarding-form.spec.tssrc/modules/agent-network/AIProviderModal.tsxsrc/modules/agent-network/AIProvidersProvider.tsxsrc/modules/agent-network/data/mockData.tssrc/modules/agent-network/useProviderCatalog.tssrc/modules/onboarding/OnboardingProvider.tsx
Comment on lines
+173
to
+175
| // The signup survey relies on a JWT domain claim self-hosted IdPs don't | ||
| // emit, so the flow skips it and opens on the intent step. | ||
| await expect(page.getByText("Get started with NetBird")).toBeVisible(); |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- target spec around lines 150-190 ---\n'
sed -n '140,190p' e2e/tests/onboarding-form.spec.ts | nl -ba -v140
printf '\n--- relevant selector occurrences in target spec ---\n'
rg -n 'getByText\(|getByTestId\(|Get started with NetBird|intent step|intent' e2e/tests/onboarding-form.spec.ts || true
printf '\n--- getStarted text occurrences in repo ---\n'
rg -n '"?Get started with NetBird"?|Get started with NetBird' . --glob '!node_modules' --glob '!dist' --glob '!build' || trueRepository: netbirdio/dashboard
Length of output: 248
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- target spec around lines 150-190 ---'
sed -n '140,190p' e2e/tests/onboarding-form.spec.ts | nl -ba -v140
printf '%s\n' '\n--- relevant selector occurrences in target spec ---'
rg -n 'getByText\(|getByTestId\(|Get started with NetBird|intent step|intent' e2e/tests/onboarding-form.spec.ts || true
printf '%s\n' '\n--- Get started with NetBird occurrences in repo ---'
rg -n 'Get started with NetBird' . --glob '!node_modules' --glob '!dist' --glob '!build' || trueRepository: netbirdio/dashboard
Length of output: 238
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- target spec around lines 150-190 ---'
awk '{printf "%5d\t%s\n", NR, $0}' e2e/tests/onboarding-form.spec.ts | sed -n '140,190p'
printf '%s\n' '--- relevant selector occurrences in target spec ---'
rg -n 'getByText\(|getByTestId\(|Get started with NetBird|intent step|intent' e2e/tests/onboarding-form.spec.ts || true
printf '%s\n' '--- Get started with NetBird occurrences in repo ---'
rg -n 'Get started with NetBird' . --glob '!node_modules' --glob '!dist' --glob '!build' || trueRepository: netbirdio/dashboard
Length of output: 3835
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- OnboardingIntent.tsx relevant section ---'
awk '{printf "%5d\t%s\n", NR, $0}' src/modules/onboarding/OnboardingIntent.tsx | sed -n '1,140p'
printf '%s\n' '--- current e2e data-testid usage for onboarding/OnboardingIntent ---'
rg -n 'data-testid|byTestId|OnboardingIntent|Get started with NetBird' src/modules/onboarding e2e/tests/onboarding-form.spec.ts | head -n 200Repository: netbirdio/dashboard
Length of output: 7474
Use a data-testid selector for the intent-step assertion.
OnboardingIntent.tsx exposes the root as data-testid="regular-onboarding", so replace the brittle getByText("Get started with NetBird") assertion with page.getByTestId("regular-onboarding").
🤖 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 `@e2e/tests/onboarding-form.spec.ts` around lines 173 - 175, Update the
intent-step assertion in the onboarding test to use
page.getByTestId("regular-onboarding") instead of the text-based getByText
selector, while preserving the existing visibility expectation.
Source: Coding guidelines
Contributor
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 `@src/modules/agent-network/AgentAccessLogTable.tsx`:
- Around line 1048-1056: Update TokensCell in
src/modules/agent-network/AgentAccessLogTable.tsx#L1048-L1056 to include input,
output, cache-read, and cache-write buckets when deciding whether to render
EmptyRow; update the session total at
src/modules/agent-network/AgentAccessLogTable.tsx#L1427-L1438 to sum the same
four buckets so cache-only requests and displayed totals match TokenBreakdown.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1f5eb446-587c-4e39-84ac-6b66e2ea250e
📒 Files selected for processing (1)
src/modules/agent-network/AgentAccessLogTable.tsx
TokensCell and the session per-request total now sum cache read/write alongside input/output, matching TokenBreakdown so cache-only requests display and totals line up.
ModelRowEditor owns row-local state (custom mode, cache disclosure); keying by array index leaked it to a neighbour on remove/reorder. Give each row a client-only _key, stripped from the API payload by toAPIModels.
mlsmaycon
approved these changes
Aug 1, 2026
1 task
ocoj
pushed a commit
to ocoj/dashboard
that referenced
this pull request
Aug 16, 2026
2 tasks
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Issue ticket number and link
Documentation
Select exactly one:
Docs PR URL (required if "docs added" is checked)
Paste the PR link from https://github.com/netbirdio/docs here:
https://github.com/netbirdio/docs/pull/__
E2E tests
Optional: override the image tags used by the Playwright e2e workflow.
Defaults to
mainwhen omitted.management-cloud-tag: main
reverse-proxy-tag: main
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by CodeRabbit