Skip to content

refactor: extract provider config accordion into reusable ProviderConfigCard component - #5637

Merged
akshaydeo merged 2 commits into
devfrom
07-29-feat_switch_vk_to_use_the_new_provider_config
Jul 29, 2026
Merged

akshaydeo merged 2 commits into
devfrom
07-29-feat_switch_vk_to_use_the_new_provider_config

Conversation

@BearTS

@BearTS BearTS commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Extracts the inline provider configuration UI from virtualKeySheet.tsx into a reusable ProviderConfigCard component. The previous implementation embedded ~340 lines of accordion-based provider config rendering directly in the sheet, making it difficult to maintain and reuse. This refactor delegates that responsibility to a dedicated component with a clean value/onChange interface.

Changes

  • Replaced the inline Accordion-based provider config rendering with a ProviderConfigCard component that accepts a structured value prop and an onChange callback
  • Removed the handleUpdateProviderConfig helper function, as field-level updates are now handled inside ProviderConfigCard via the unified onChange interface
  • Removed the local VirtualKeyType type definition and associated react-select component imports (components, MultiValueProps, OptionProps) that were only used in the inline key selector
  • Removed imports for Accordion, AsyncMultiSelect, ModelMultiselect, cn, and ModelPlaceholders that are no longer needed in this file

Type of change

  • Bug fix
  • Feature
  • Refactor
  • Documentation
  • Chore/CI

Affected areas

  • Core (Go)
  • Transports (HTTP)
  • Providers/Integrations
  • Plugins
  • UI (React)
  • Docs

How to test

cd ui
pnpm i || npm i
pnpm build || npm run build
  1. Navigate to the Virtual Keys section in the workspace.
  2. Open an existing virtual key that has one or more provider configurations.
  3. Verify that each provider config renders correctly with its icon, label, allowed/blocked models, allowed keys, budget, and rate limit fields.
  4. Add a new provider config and confirm all fields are editable and saved correctly.
  5. Remove a provider config and confirm it is removed from the list.

Screenshots/Recordings

Before: Provider configs rendered as accordion items inline within virtualKeySheet.tsx.

After: Provider configs rendered via ProviderConfigCard with identical visual behavior and a cleaner component boundary.

Breaking changes

  • No

Related issues

Security considerations

None. This is a pure UI refactor with no changes to data handling, authentication, or secrets management.

Checklist

  • I read docs/contributing/README.md and followed the guidelines
  • I added/updated tests where appropriate
  • I updated documentation where needed
  • I verified builds succeed (Go and UI)
  • I verified the CI pipeline passes locally if applicable

@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

VirtualKeySheet replaces the accordion-based provider editor with ProviderConfigCard, mapping provider values and card edits through the existing provider configuration form state.

Changes

Provider configuration UI

Layer / File(s) Summary
ProviderConfigCard integration
ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx
Removes obsolete editor dependencies, types, and update logic, then renders provider configurations through ProviderConfigCard with mapped keys, budgets, rate limits, and form-state updates.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

  • maximhq/bifrost#5636: Introduces the related ProviderConfigCard and budget UI APIs used by this migration.

Suggested reviewers: akshaydeo, impoiler, pratham-mishra04

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title accurately summarizes the main change: extracting provider config accordion UI into a reusable component.
Description check ✅ Passed The PR description matches the template well and includes summary, changes, type, affected areas, testing, screenshots, breaking changes, security, and checklist.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 07-29-feat_switch_vk_to_use_the_new_provider_config
⚔️ Resolve merge conflicts
  • Resolve merge conflict in branch 07-29-feat_switch_vk_to_use_the_new_provider_config

Comment @coderabbitai help to get the list of available commands.

BearTS commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

@BearTS BearTS changed the title feat: switch vk to use the new provider config refactor: extract provider config accordion into reusable ProviderConfigCard component Jul 28, 2026
@BearTS
BearTS marked this pull request as ready for review July 28, 2026 21:21
@coderabbitai
coderabbitai Bot requested review from akshaydeo and impoiler July 28, 2026 21:22

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx (1)

1114-1126: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Write back through a functional read of form state rather than the watched snapshot.

onChange closes over providerConfigs (from form.watch) and config. Two cards updating within the same render pass (or an update racing a handleRemoveProvider) would overwrite each other, since the whole array is replaced from a stale snapshot. Reading the current value at write time removes the hazard.

♻️ Proposed refactor
 														onChange={(next) => {
-															const updated = [...providerConfigs];
-															updated[index] = {
-																...config,
+															const current = form.getValues("providerConfigs") || [];
+															const updated = [...current];
+															updated[index] = {
+																...current[index],
 																allowed_models: next.allowedModels,
 																blacklisted_models: next.blacklistedModels,
 																weight: next.weight ?? undefined,
 																key_ids: next.keyIds,
 																budgets: next.budgets.map((l) => ({ id: l.id, max_limit: l.max_limit, reset_duration: l.reset_duration })),
 																rate_limit: next.rateLimit ?? undefined,
 															};
 															form.setValue("providerConfigs", updated, { shouldDirty: true });
 														}}
🤖 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 `@ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx` around lines 1114 -
1126, Update the onChange handler for the provider configuration card to read
the current providerConfigs value from form state at write time, rather than
using the watched providerConfigs snapshot or closed-over config. Apply the
mapped update against that current array and pass the resulting array to
form.setValue, preserving the existing field transformations and dirty-state
behavior.
🤖 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 `@ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx`:
- Around line 1096-1099: In the provider configuration mapping that renders
ProviderConfigCard, replace the array index key with the stable unique
config.provider value. Keep the existing index prop unchanged if it is used for
ordering or updates, and only change the React key.

---

Nitpick comments:
In `@ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx`:
- Around line 1114-1126: Update the onChange handler for the provider
configuration card to read the current providerConfigs value from form state at
write time, rather than using the watched providerConfigs snapshot or
closed-over config. Apply the mapped update against that current array and pass
the resulting array to form.setValue, preserving the existing field
transformations and dirty-state behavior.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 0eba1a98-95de-4f67-a977-92bdae44e928

📥 Commits

Reviewing files that changed from the base of the PR and between d5e0d2e and 3982603.

📒 Files selected for processing (1)
  • ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx

Comment thread ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx
@BearTS
BearTS force-pushed the 07-29-feat_switch_vk_to_use_the_new_provider_config branch from 3982603 to 1941e05 Compare July 28, 2026 21:41
@BearTS
BearTS force-pushed the 07-29-feat_provider_config_card branch from d5e0d2e to 00febca Compare July 28, 2026 21:41
@coderabbitai
coderabbitai Bot requested a review from Pratham-Mishra04 July 28, 2026 21:42

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx (1)

1114-1126: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider hoisting the card onChange into a stable handleUpdateProviderConfig(index, next) helper.

Inlining rebuilds the closure per render over providerConfigs/config; a small named handler keeps the mapping logic testable and mirrors the existing handleUpdateMCPConfig pattern in this file.

🤖 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 `@ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx` around lines 1114 -
1126, Extract the inline provider card onChange logic into a named
handleUpdateProviderConfig(index, next) helper, following the existing
handleUpdateMCPConfig pattern. Keep the same providerConfigs update mapping and
form.setValue behavior, then pass the helper to the card instead of rebuilding
the mapping closure inline.
🤖 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.

Nitpick comments:
In `@ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx`:
- Around line 1114-1126: Extract the inline provider card onChange logic into a
named handleUpdateProviderConfig(index, next) helper, following the existing
handleUpdateMCPConfig pattern. Keep the same providerConfigs update mapping and
form.setValue behavior, then pass the helper to the card instead of rebuilding
the mapping closure inline.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: f324527d-3e10-44c0-9846-c41336966d43

📥 Commits

Reviewing files that changed from the base of the PR and between 3982603 and 1941e05.

📒 Files selected for processing (1)
  • ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx

@BearTS
BearTS force-pushed the 07-29-feat_switch_vk_to_use_the_new_provider_config branch from 1941e05 to de59e2c Compare July 28, 2026 21:46
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jul 28, 2026

akshaydeo commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Merge activity

  • Jul 29, 7:04 AM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Jul 29, 7:05 AM UTC: @akshaydeo merged this pull request with Graphite.

@akshaydeo
akshaydeo changed the base branch from 07-29-feat_provider_config_card to graphite-base/5637 July 29, 2026 07:05
@akshaydeo
akshaydeo changed the base branch from graphite-base/5637 to dev July 29, 2026 07:05
@akshaydeo
akshaydeo dismissed coderabbitai[bot]’s stale review July 29, 2026 07:05

The base branch was changed.

@akshaydeo
akshaydeo merged commit 537fd5b into dev Jul 29, 2026
9 checks passed
@akshaydeo
akshaydeo deleted the 07-29-feat_switch_vk_to_use_the_new_provider_config branch July 29, 2026 07:05
akhsaul pushed a commit to akhsaul/bifrost that referenced this pull request Aug 27, 2026
…nfigCard` component (maximhq#5637)

## Summary

Extracts the inline provider configuration UI from `virtualKeySheet.tsx` into a reusable `ProviderConfigCard` component. The previous implementation embedded ~340 lines of accordion-based provider config rendering directly in the sheet, making it difficult to maintain and reuse. This refactor delegates that responsibility to a dedicated component with a clean value/onChange interface.

## Changes

- Replaced the inline `Accordion`-based provider config rendering with a `ProviderConfigCard` component that accepts a structured `value` prop and an `onChange` callback
- Removed the `handleUpdateProviderConfig` helper function, as field-level updates are now handled inside `ProviderConfigCard` via the unified `onChange` interface
- Removed the local `VirtualKeyType` type definition and associated `react-select` component imports (`components`, `MultiValueProps`, `OptionProps`) that were only used in the inline key selector
- Removed imports for `Accordion`, `AsyncMultiSelect`, `ModelMultiselect`, `cn`, and `ModelPlaceholders` that are no longer needed in this file

## Type of change

- [ ] Bug fix
- [ ] Feature
- [x] Refactor
- [ ] Documentation
- [ ] Chore/CI

## Affected areas

- [ ] Core (Go)
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [x] UI (React)
- [ ] Docs

## How to test

```sh
cd ui
pnpm i || npm i
pnpm build || npm run build
```

1. Navigate to the Virtual Keys section in the workspace.
2. Open an existing virtual key that has one or more provider configurations.
3. Verify that each provider config renders correctly with its icon, label, allowed/blocked models, allowed keys, budget, and rate limit fields.
4. Add a new provider config and confirm all fields are editable and saved correctly.
5. Remove a provider config and confirm it is removed from the list.

## Screenshots/Recordings

Before: Provider configs rendered as accordion items inline within `virtualKeySheet.tsx`.

After: Provider configs rendered via `ProviderConfigCard` with identical visual behavior and a cleaner component boundary.

## Breaking changes

- [x] No

## Related issues

## Security considerations

None. This is a pure UI refactor with no changes to data handling, authentication, or secrets management.

## Checklist

- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [ ] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [ ] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
occcat pushed a commit to occcat/bifrost that referenced this pull request Sep 2, 2026
…nfigCard` component (maximhq#5637)

## Summary

Extracts the inline provider configuration UI from `virtualKeySheet.tsx` into a reusable `ProviderConfigCard` component. The previous implementation embedded ~340 lines of accordion-based provider config rendering directly in the sheet, making it difficult to maintain and reuse. This refactor delegates that responsibility to a dedicated component with a clean value/onChange interface.

## Changes

- Replaced the inline `Accordion`-based provider config rendering with a `ProviderConfigCard` component that accepts a structured `value` prop and an `onChange` callback
- Removed the `handleUpdateProviderConfig` helper function, as field-level updates are now handled inside `ProviderConfigCard` via the unified `onChange` interface
- Removed the local `VirtualKeyType` type definition and associated `react-select` component imports (`components`, `MultiValueProps`, `OptionProps`) that were only used in the inline key selector
- Removed imports for `Accordion`, `AsyncMultiSelect`, `ModelMultiselect`, `cn`, and `ModelPlaceholders` that are no longer needed in this file

## Type of change

- [ ] Bug fix
- [ ] Feature
- [x] Refactor
- [ ] Documentation
- [ ] Chore/CI

## Affected areas

- [ ] Core (Go)
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [x] UI (React)
- [ ] Docs

## How to test

```sh
cd ui
pnpm i || npm i
pnpm build || npm run build
```

1. Navigate to the Virtual Keys section in the workspace.
2. Open an existing virtual key that has one or more provider configurations.
3. Verify that each provider config renders correctly with its icon, label, allowed/blocked models, allowed keys, budget, and rate limit fields.
4. Add a new provider config and confirm all fields are editable and saved correctly.
5. Remove a provider config and confirm it is removed from the list.

## Screenshots/Recordings

Before: Provider configs rendered as accordion items inline within `virtualKeySheet.tsx`.

After: Provider configs rendered via `ProviderConfigCard` with identical visual behavior and a cleaner component boundary.

## Breaking changes

- [x] No

## Related issues

## Security considerations

None. This is a pure UI refactor with no changes to data handling, authentication, or secrets management.

## Checklist

- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [ ] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [ ] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants