minor ux uplift, restart required status storage #1176
Conversation
📝 WalkthroughSummary by CodeRabbitRelease NotesNew Features
UI/UX Improvements
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughAdds a restart-required configuration entity and APIs; backend tracks and persists restart-triggering config changes and clears the flag on startup; frontend displays a non-dismissible warning promo card and includes type/UI updates to surface the restart reason. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Client (Browser)
participant HTTP as HTTP Handler
participant Store as ConfigStore
participant DB as Database
participant UI as Frontend UI
rect `#E8F0FF`
Note over Browser,HTTP: Config update triggers restart detection
Browser->>HTTP: PATCH /config (changes)
HTTP->>HTTP: Determine changed settings\n(accumulate restartReasons)
alt restartReasons non-empty
HTTP->>Store: SetRestartRequiredConfig(ctx, {Required:true, Reason: "..."} )
Store->>DB: persist `restart_required` key (JSON)
DB-->>Store: OK
end
HTTP-->>Browser: 200 OK
end
rect `#F2FFF0`
Note over Browser,HTTP: Client reads config including restart state
Browser->>HTTP: GET /config
HTTP->>Store: GetRestartRequiredConfig(ctx)
Store->>DB: read `restart_required` key
DB-->>Store: RestartRequiredConfig JSON
Store-->>HTTP: RestartRequiredConfig
HTTP-->>Browser: 200 + restart_required
Browser->>UI: render
UI->>UI: Sidebar shows non-dismissible warning card when restart_required.required == true
end
rect `#FFF6E8`
Note over HTTP,Store: Server startup clears flag
HTTP->>Store: ClearRestartRequiredConfig(ctx)
Store->>DB: write cleared `restart_required` (Required:false)
DB-->>Store: OK
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
🧪 Test Suite AvailableThis PR can be tested by a repository admin. |
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
akshaydeo
force-pushed
the
12-26-minor_ux_uplifg_restart_required_status_storage
branch
from
4f5736c to
bce4df2
Compare
akshaydeo
changed the base branch from
graphite-base/1176
to
12-24-fix_content_block_type_input_file_to_file
akshaydeo
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
ui/app/workspace/config/views/securityView.tsx (1)
241-241: Verify styling consistency with other toggle controls.This control now has different styling compared to other toggles in the same view (e.g., lines 265, 282, 300 which retain
rounded-lg border p-4). Since this is a sub-setting within the auth config section, the flatter visual hierarchy might be intentional, but please confirm this styling change aligns with the design intent.framework/configstore/rdb.go (1)
1983-1989: Consider using the struct instead of hardcoded JSON for consistency.While the hardcoded JSON string works, using the
RestartRequiredConfigstruct would be more maintainable if the struct evolves.🔎 Suggested refactor
// ClearRestartRequiredConfig clears the restart required configuration in the database. func (s *RDBConfigStore) ClearRestartRequiredConfig(ctx context.Context) error { - return s.db.WithContext(ctx).Save(&tables.TableGovernanceConfig{ - Key: tables.ConfigRestartRequiredKey, - Value: `{"required":false,"reason":""}`, - }).Error + return s.SetRestartRequiredConfig(ctx, &tables.RestartRequiredConfig{ + Required: false, + Reason: "", + }) }transports/bifrost-http/handlers/config.go (1)
600-607: Proxy config update always sets restart flag regardless of actual changes.Unlike
updateConfigwhich tracks specific changes, this handler sets the restart flag on every proxy update, even if the configuration didn't actually change (e.g., user submits the same values). Consider comparing with the existing config to avoid false positives.🔎 Suggested approach
+ // Check if proxy config actually changed before setting restart flag + existingConfig, _ := h.store.ConfigStore.GetProxyConfig(ctx) + proxyChanged := existingConfig == nil || + existingConfig.Enabled != payload.Enabled || + existingConfig.URL != payload.URL || + existingConfig.Type != payload.Type || + existingConfig.Username != payload.Username || + existingConfig.NoProxy != payload.NoProxy || + existingConfig.Timeout != payload.Timeout || + existingConfig.SkipTLSVerify != payload.SkipTLSVerify + // Note: Password comparison is tricky due to encryption, so any non-redacted password counts as change + if payload.Password != "<redacted>" && payload.Password != "" { + proxyChanged = true + } + // Set restart required flag for proxy config changes - if err := h.store.ConfigStore.SetRestartRequiredConfig(ctx, &configstoreTables.RestartRequiredConfig{ - Required: true, - Reason: "Proxy configuration has been updated. A restart is required for all changes to take full effect.", - }); err != nil { - logger.Warn(fmt.Sprintf("failed to set restart required config: %v", err)) + if proxyChanged { + if err := h.store.ConfigStore.SetRestartRequiredConfig(ctx, &configstoreTables.RestartRequiredConfig{ + Required: true, + Reason: "Proxy configuration has been updated. A restart is required for all changes to take full effect.", + }); err != nil { + logger.Warn(fmt.Sprintf("failed to set restart required config: %v", err)) + } }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
framework/configstore/rdb.goframework/configstore/store.goframework/configstore/tables/config.gotransports/bifrost-http/handlers/config.gotransports/bifrost-http/lib/config.goui/app/workspace/config/views/pricingConfigView.tsxui/app/workspace/config/views/securityView.tsxui/app/workspace/logs/views/columns.tsxui/components/sidebar.tsxui/components/ui/promoCardStack.tsxui/components/ui/sheet.tsxui/components/ui/toggle.tsxui/lib/constants/logs.tsui/lib/types/config.ts
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
always check the stack if there is one for the current PR. do not give localized reviews for the PR, always see all changes in the light of the whole stack of PRs (if there is a stack, if there is no stack you can continue to make localized suggestions/reviews)
Files:
framework/configstore/store.goui/lib/constants/logs.tsui/app/workspace/logs/views/columns.tsxui/lib/types/config.tsui/app/workspace/config/views/pricingConfigView.tsxui/components/ui/toggle.tsxui/components/sidebar.tsxui/app/workspace/config/views/securityView.tsxframework/configstore/tables/config.gotransports/bifrost-http/handlers/config.gotransports/bifrost-http/lib/config.goframework/configstore/rdb.goui/components/ui/sheet.tsxui/components/ui/promoCardStack.tsx
🧠 Learnings (2)
📚 Learning: 2025-12-09T17:07:42.007Z
Learnt from: qwerty-dvorak
Repo: maximhq/bifrost PR: 1006
File: core/schemas/account.go:9-18
Timestamp: 2025-12-09T17:07:42.007Z
Learning: In core/schemas/account.go, the HuggingFaceKeyConfig field within the Key struct is currently unused and reserved for future Hugging Face inference endpoint deployments. Do not flag this field as missing from OpenAPI documentation or require its presence in the API spec until the feature is actively implemented and used. When the feature is added, update the OpenAPI docs accordingly; otherwise, treat this field as non-breaking and not part of the current API surface.
Applied to files:
framework/configstore/store.goframework/configstore/tables/config.gotransports/bifrost-http/handlers/config.gotransports/bifrost-http/lib/config.goframework/configstore/rdb.go
📚 Learning: 2025-12-12T08:25:02.629Z
Learnt from: Pratham-Mishra04
Repo: maximhq/bifrost PR: 1000
File: transports/bifrost-http/integrations/router.go:709-712
Timestamp: 2025-12-12T08:25:02.629Z
Learning: In transports/bifrost-http/**/*.go, update streaming response handling to align with OpenAI Responses API: use typed SSE events such as response.created, response.output_text.delta, response.done, etc., and do not rely on the legacy data: [DONE] termination marker. Note that data: [DONE] is only used by the older Chat Completions and Text Completions streaming APIs. Ensure parsers, writers, and tests distinguish SSE events from the [DONE] sentinel and handle each event type accordingly for correct stream termination and progress updates.
Applied to files:
transports/bifrost-http/handlers/config.gotransports/bifrost-http/lib/config.go
🧬 Code graph analysis (5)
framework/configstore/store.go (2)
framework/configstore/tables/config.go (1)
RestartRequiredConfig(16-19)ui/lib/types/config.ts (1)
RestartRequiredConfig(303-306)
ui/app/workspace/logs/views/columns.tsx (2)
ui/lib/constants/logs.ts (2)
Status(173-173)StatusBarColors(84-89)ui/components/ui/button.tsx (1)
Button(70-70)
ui/lib/types/config.ts (1)
framework/configstore/tables/config.go (2)
RestartRequiredConfig(16-19)GlobalProxyConfig(24-37)
ui/components/ui/toggle.tsx (1)
ui/lib/utils.ts (1)
cn(4-6)
framework/configstore/rdb.go (2)
framework/configstore/tables/config.go (4)
RestartRequiredConfig(16-19)TableGovernanceConfig(40-43)TableGovernanceConfig(46-46)ConfigRestartRequiredKey(11-11)ui/lib/types/config.ts (1)
RestartRequiredConfig(303-306)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
🔇 Additional comments (24)
ui/components/ui/toggle.tsx (2)
6-6: LGTM! Good component flexibility enhancement.The addition of the optional
classNameprop and its integration using thecnutility follows best practices for component composition. The implementation correctly:
- Preserves existing styling logic (including the disabled state conditional)
- Allows external className customization without breaking internal styles
- Uses the
cnutility to properly merge classes viatwMergeandclsxAlso applies to: 15-15, 19-22
3-3: No issues found. The importimport { cn } from "./utils";correctly resolves toui/components/ui/utils.ts, which exports thecnutility function. The relative path is valid and will not cause module resolution errors.Likely an incorrect or invalid review comment.
ui/app/workspace/config/views/pricingConfigView.tsx (1)
92-92: LGTM - Good UX improvement.The conditional button text provides clear visual feedback to users during the force sync operation, matching the pattern already used for the Save button.
ui/components/ui/sheet.tsx (2)
94-126: LGTM - Clean API enhancement for SheetHeader.The addition of the
showCloseButtonprop provides better control over close button visibility while maintaining backward compatibility (defaults totrue). The layout refinements and hover styling improvements enhance the component's visual polish.
75-79: No positioning changes detected in SheetContent.The
top-2 right-0 bottom-2andtop-2 bottom-2 left-0positioning classes remain intact in lines 75 and 78-79. The SheetHeader changes (adding theshowCloseButtonprop withtruedefault) are backward compatible—all existing sheet implementations continue to work without modification.Likely an incorrect or invalid review comment.
ui/lib/constants/logs.ts (1)
84-89: LGTM - Well-structured constant addition.The
StatusBarColorsconstant follows the established pattern in this file and provides a clean separation between badge colors (StatusColors) and status bar colors. Theas constassertion ensures type safety.transports/bifrost-http/lib/config.go (2)
414-417: LGTM - Proper restart flag cleanup on server initialization.Clearing the restart-required flag on server startup is the correct behavior. The warning-level error logging is appropriate since this is a non-critical operation that shouldn't block server startup.
1529-1532: LGTM - Consistent restart flag cleanup for default config path.Good practice to clear the restart flag in both initialization paths (file-based and defaults). The error handling matches the approach in
initStoresFromFile.ui/components/sidebar.tsx (2)
631-644: LGTM - Well-implemented restart notification.The restart required promo card is properly configured:
- Non-dismissible for critical system notifications
- Warning variant provides appropriate visual emphasis
- Fallback message handles missing reason gracefully
- Positioned first to ensure visibility
The optional chaining on
coreConfig?.restart_required?.requiredsafely handles cases where the config isn't loaded yet.
668-668: LGTM - Proper memoization dependency.Adding
coreConfig?.restart_requiredto the dependency array ensures the promo cards update when the restart status changes.ui/lib/types/config.ts (2)
302-306: LGTM - Type definition matches backend structure.The
RestartRequiredConfiginterface correctly mirrors the Go backend struct fromframework/configstore/tables/config.go:
required: booleanmaps toRequired boolreason?: stringmaps toReason stringwithomitemptytag
314-314: LGTM - Optional field is appropriate.Making
restart_requiredoptional inBifrostConfigis correct since this field may not always be present in the configuration response.framework/configstore/tables/config.go (2)
11-11: LGTM - Constant follows established pattern.The
ConfigRestartRequiredKeyconstant follows the same naming convention as the existing config keys in this file.
14-19: LGTM - Clean struct definition with appropriate JSON tags.The
RestartRequiredConfigstruct is well-documented and correctly uses:
json:"required"for mandatory boolean fieldjson:"reason,omitempty"to omit empty reason strings from JSON outputThis aligns with the TypeScript type definition in
ui/lib/types/config.ts.ui/components/ui/promoCardStack.tsx (2)
23-45: LGTM! Sorting logic correctly prioritizes non-dismissible cards.The implementation properly places non-dismissible cards (like the restart warning) at the top of the stack. The duplicate sorting logic in both the initial state and the effect is acceptable for a component of this size.
97-107: Warning variant styling looks good.The amber-themed styling for the warning variant provides clear visual distinction and aligns with common UI patterns for cautionary notices.
framework/configstore/store.go (1)
126-130: Clean interface extension for restart-required config management.The three new methods follow the existing patterns in the interface and provide a complete API for managing the restart-required state.
ui/app/workspace/logs/views/columns.tsx (1)
50-57: Status column improvements look correct.The empty header with explicit size constraints (8px) appropriately creates a compact color-bar indicator for status. The
StatusBarColorsimport rename aligns with the constant definition inlogs.ts.framework/configstore/rdb.go (2)
1952-1969: GetRestartRequiredConfig implementation is correct.Properly handles the record-not-found case by returning
nil, niland correctly unmarshals the JSON value when present.
1971-1981: SetRestartRequiredConfig implementation follows existing patterns.The implementation is consistent with similar methods like
UpdateProxyConfig, using JSON marshaling and the governance_config table.transports/bifrost-http/handlers/config.go (4)
160-166: Good addition of restart_required config exposure in getConfig.The implementation correctly fetches and exposes the restart status with proper error handling (logs warning but doesn't fail the request).
220-266: Comprehensive restart reason tracking for client config changes.The implementation properly tracks which settings require a restart and provides clear, user-facing reason messages.
383-401: Auth change detection logic is thorough.The comparison correctly handles the initial case (no existing config) and detects changes to enabled state, username, inference auth setting, and password updates.
438-447: Restart flag is set with a consolidated, user-friendly message.The joined reasons provide clear feedback about which settings triggered the restart requirement.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
ui/components/ui/promoCardStack.tsx (2)
23-45: Consider extracting duplicate sorting logic.The same sorting logic for non-dismissible cards appears in both the
useStateinitializer (lines 23-30) and theuseEffect(lines 36-43). Consider extracting to a helper function.🔎 Proposed refactor
+const sortByDismissible = (cards: PromoCardItem[]) => + [...cards].sort((a, b) => { + const aDismissible = a.dismissible !== false; + const bDismissible = b.dismissible !== false; + if (aDismissible === bDismissible) return 0; + return aDismissible ? 1 : -1; + }); + export function PromoCardStack({ cards, className = "", onCardsEmpty }: PromoCardStackProps) { - const [items, setItems] = useState(() => { - // Sort so non-dismissible cards appear at the top - return [...cards].sort((a, b) => { - const aDismissible = a.dismissible !== false; - const bDismissible = b.dismissible !== false; - if (aDismissible === bDismissible) return 0; - return aDismissible ? 1 : -1; // Non-dismissible first - }); - }); + const [items, setItems] = useState(() => sortByDismissible(cards)); // ... useEffect(() => { - // Sort so non-dismissible cards appear at the top - const sortedCards = [...cards].sort((a, b) => { - const aDismissible = a.dismissible !== false; - const bDismissible = b.dismissible !== false; - if (aDismissible === bDismissible) return 0; - return aDismissible ? 1 : -1; // Non-dismissible first - }); - setItems(sortedCards); + setItems(sortByDismissible(cards)); }, [cards]);
109-109: Redundant ternary expression.The expression
typeof card.title === "string" ? card.title : card.titlealways evaluates tocard.title. The same issue exists on line 124 forcard.description.🔎 Proposed fix
-<div className="min-w-0 flex-1">{typeof card.title === "string" ? card.title : card.title}</div> +<div className="min-w-0 flex-1">{card.title}</div>And for line 124:
-{typeof card.description === "string" ? card.description : card.description} +{card.description}framework/configstore/rdb.go (1)
1983-1989: Consider using struct marshaling for consistency.
ClearRestartRequiredConfiguses a hardcoded JSON string whileSetRestartRequiredConfigmarshals a struct. While the hardcoded string is correct, using consistent marshaling would be slightly more maintainable if the struct definition changes.🔎 Proposed refactor for consistency
func (s *RDBConfigStore) ClearRestartRequiredConfig(ctx context.Context) error { + config := &tables.RestartRequiredConfig{Required: false, Reason: ""} + configJSON, err := json.Marshal(config) + if err != nil { + return fmt.Errorf("failed to marshal restart required config: %w", err) + } return s.db.WithContext(ctx).Save(&tables.TableGovernanceConfig{ Key: tables.ConfigRestartRequiredKey, - Value: `{"required":false,"reason":""}`, + Value: string(configJSON), }).Error }transports/bifrost-http/handlers/config.go (1)
600-607: Proxy config always triggers restart flag.This sets the restart-required flag on every proxy config update, even if the values haven't actually changed. Consider comparing with the existing config before setting the flag to avoid unnecessary restart notifications.
🔎 Proposed enhancement
+ // Check if proxy config actually changed before setting restart flag + existingProxyConfig, _ := h.store.ConfigStore.GetProxyConfig(ctx) + proxyChanged := existingProxyConfig == nil || + existingProxyConfig.Enabled != payload.Enabled || + existingProxyConfig.URL != payload.URL || + existingProxyConfig.Type != payload.Type || + existingProxyConfig.Username != payload.Username || + existingProxyConfig.NoProxy != payload.NoProxy || + existingProxyConfig.Timeout != payload.Timeout || + existingProxyConfig.SkipTLSVerify != payload.SkipTLSVerify + // Set restart required flag for proxy config changes + if proxyChanged { if err := h.store.ConfigStore.SetRestartRequiredConfig(ctx, &configstoreTables.RestartRequiredConfig{ Required: true, Reason: "Proxy configuration has been updated. A restart is required for all changes to take full effect.", }); err != nil { logger.Warn(fmt.Sprintf("failed to set restart required config: %v", err)) } + }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
framework/configstore/rdb.goframework/configstore/store.goframework/configstore/tables/config.gotransports/bifrost-http/handlers/config.gotransports/bifrost-http/lib/config.goui/app/workspace/config/views/pricingConfigView.tsxui/app/workspace/config/views/securityView.tsxui/app/workspace/logs/views/columns.tsxui/components/sidebar.tsxui/components/ui/promoCardStack.tsxui/components/ui/sheet.tsxui/components/ui/toggle.tsxui/lib/constants/logs.tsui/lib/types/config.ts
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
always check the stack if there is one for the current PR. do not give localized reviews for the PR, always see all changes in the light of the whole stack of PRs (if there is a stack, if there is no stack you can continue to make localized suggestions/reviews)
Files:
ui/lib/types/config.tsui/app/workspace/config/views/pricingConfigView.tsxframework/configstore/store.goframework/configstore/tables/config.goui/components/sidebar.tsxtransports/bifrost-http/handlers/config.gotransports/bifrost-http/lib/config.goui/app/workspace/config/views/securityView.tsxui/components/ui/promoCardStack.tsxui/components/ui/toggle.tsxui/components/ui/sheet.tsxframework/configstore/rdb.goui/lib/constants/logs.tsui/app/workspace/logs/views/columns.tsx
🧠 Learnings (2)
📚 Learning: 2025-12-09T17:07:42.007Z
Learnt from: qwerty-dvorak
Repo: maximhq/bifrost PR: 1006
File: core/schemas/account.go:9-18
Timestamp: 2025-12-09T17:07:42.007Z
Learning: In core/schemas/account.go, the HuggingFaceKeyConfig field within the Key struct is currently unused and reserved for future Hugging Face inference endpoint deployments. Do not flag this field as missing from OpenAPI documentation or require its presence in the API spec until the feature is actively implemented and used. When the feature is added, update the OpenAPI docs accordingly; otherwise, treat this field as non-breaking and not part of the current API surface.
Applied to files:
framework/configstore/store.goframework/configstore/tables/config.gotransports/bifrost-http/handlers/config.gotransports/bifrost-http/lib/config.goframework/configstore/rdb.go
📚 Learning: 2025-12-12T08:25:02.629Z
Learnt from: Pratham-Mishra04
Repo: maximhq/bifrost PR: 1000
File: transports/bifrost-http/integrations/router.go:709-712
Timestamp: 2025-12-12T08:25:02.629Z
Learning: In transports/bifrost-http/**/*.go, update streaming response handling to align with OpenAI Responses API: use typed SSE events such as response.created, response.output_text.delta, response.done, etc., and do not rely on the legacy data: [DONE] termination marker. Note that data: [DONE] is only used by the older Chat Completions and Text Completions streaming APIs. Ensure parsers, writers, and tests distinguish SSE events from the [DONE] sentinel and handle each event type accordingly for correct stream termination and progress updates.
Applied to files:
transports/bifrost-http/handlers/config.gotransports/bifrost-http/lib/config.go
🧬 Code graph analysis (7)
ui/lib/types/config.ts (2)
framework/configstore/tables/config.go (1)
RestartRequiredConfig(16-19)core/schemas/bifrost.go (1)
BifrostConfig(21-29)
framework/configstore/store.go (2)
framework/configstore/tables/config.go (1)
RestartRequiredConfig(16-19)ui/lib/types/config.ts (1)
RestartRequiredConfig(303-306)
transports/bifrost-http/handlers/config.go (4)
ui/lib/types/config.ts (2)
AuthConfig(261-266)RestartRequiredConfig(303-306)transports/bifrost-http/handlers/utils.go (1)
SendError(35-44)framework/encrypt/encrypt.go (1)
Hash(63-69)framework/configstore/tables/config.go (1)
RestartRequiredConfig(16-19)
ui/components/ui/promoCardStack.tsx (1)
ui/components/ui/card.tsx (1)
CardHeader(50-50)
ui/components/ui/toggle.tsx (1)
ui/lib/utils.ts (1)
cn(4-6)
framework/configstore/rdb.go (2)
framework/configstore/tables/config.go (4)
RestartRequiredConfig(16-19)TableGovernanceConfig(40-43)TableGovernanceConfig(46-46)ConfigRestartRequiredKey(11-11)ui/lib/types/config.ts (1)
RestartRequiredConfig(303-306)
ui/app/workspace/logs/views/columns.tsx (1)
ui/lib/constants/logs.ts (2)
Status(173-173)StatusBarColors(84-89)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (25)
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
🔇 Additional comments (19)
ui/components/ui/toggle.tsx (1)
3-3: LGTM! Clean implementation of className prop.The addition of an optional
classNameprop with proper class merging viacn()follows React best practices and maintains backward compatibility.Also applies to: 6-6, 15-15, 19-22
ui/app/workspace/config/views/pricingConfigView.tsx (1)
92-92: LGTM! Better UX terminology.Changing "Forcing..." to "Syncing..." is clearer and more user-friendly.
ui/lib/types/config.ts (1)
302-306: LGTM! Type definitions align with backend.The
RestartRequiredConfiginterface correctly mirrors the Go struct fromframework/configstore/tables/config.go, and the optionalrestart_requiredfield properly extendsBifrostConfig.Also applies to: 314-314
ui/components/sidebar.tsx (1)
631-644: LGTM! Restart notification properly integrated.The restart-required promo card is correctly:
- Prioritized first in the array
- Conditionally displayed based on
restart_required.required- Configured as non-dismissible with warning variant
- Tracked in the
useMemodependency array for reactive updatesAlso applies to: 668-668
transports/bifrost-http/lib/config.go (2)
414-417: LGTM! Restart flag cleared on startup.Clearing the restart-required flag after server initialization ensures the system starts with a clean state. The warning log for failures is appropriate.
1529-1532: LGTM! Consistent restart flag clearing in default load path.This correctly mirrors the clearing logic in the config-from-file path (lines 414-417), ensuring both startup scenarios begin with a clean restart state.
ui/lib/constants/logs.ts (1)
84-89: LGTM! Purpose-specific status colors added.The
StatusBarColorsconstant provides background-only colors for status indicators, complementing the existingStatusColorsthat include text colors.framework/configstore/tables/config.go (1)
11-11: LGTM! Clean restart configuration definition.The
ConfigRestartRequiredKeyconstant andRestartRequiredConfigstruct are well-defined, with appropriate JSON tags and theomitemptydirective on the optionalReasonfield.Also applies to: 14-19
ui/components/ui/promoCardStack.tsx (1)
97-107: Warning variant styling looks good.The amber color scheme for the warning variant is appropriate and provides clear visual distinction. The conditional styling using
cn()is clean and follows the existing patterns.framework/configstore/store.go (1)
126-130: Interface additions are well-designed.The new restart-required config methods follow the established patterns in the
ConfigStoreinterface. The method signatures are consistent with similar config CRUD operations (e.g.,GetProxyConfig/UpdateProxyConfig).framework/configstore/rdb.go (1)
1952-1969: Implementation follows established patterns.The
GetRestartRequiredConfigimplementation correctly handles the not-found case and empty value scenario, consistent withGetProxyConfig.transports/bifrost-http/handlers/config.go (3)
160-167: Restart config retrieval integrated correctly.The restart-required config is fetched and included in the response appropriately. Error handling logs a warning but doesn't fail the entire request, which is the right approach for this non-critical data.
220-265: Restart reason tracking is well-structured.The approach of collecting restart reasons into a slice and joining them for a descriptive message is clean and user-friendly. The tracking covers the appropriate configuration fields.
383-401: Auth change detection logic is reasonable.The auth change detection covers the key scenarios: enabled state, username, password changes (when not redacted), and inference auth flag. The logic correctly handles the case where no existing config exists.
ui/app/workspace/logs/views/columns.tsx (3)
48-58: Status column redesign looks good.The change from a Badge to a thin colored bar (
w-1) is a cleaner visual indicator. Thesize: 8andmaxSize: 8constraints appropriately minimize the column width for this indicator style.
1-11: Imports are correctly updated.The
StatusBarColorsimport replacesStatusColorsand aligns with the new status bar rendering. TheBadgeimport is still needed for the request_type column.
56-56: StatusBarColors correctly covers all status values.Verification confirms
StatusBarColorsincludes all four possibleStatusvalues (success,error,processing,cancelled) with no gaps or undefined styling risk.ui/components/ui/sheet.tsx (2)
75-75: LGTM! Minor className reordering.The className adjustments for the right and left sheet sides are purely organizational with no functional impact.
Also applies to: 79-79
94-126: Backward-compatible API; code structure mitigates original concerns.The
showCloseButtonprop is a clean addition using correct React 19 patterns. Verification across the codebase confirms:
Negative margin handling: SheetHeader is consistently used with SheetContent padding (p-8 = 32px). The
-ml-5expand button has adequate surrounding padding (~12px minimum) and poses no clipping risk in actual usage patterns.Accessibility maintained: No instances of
showCloseButton={false}exist in the codebase. All sheets use standard Radix UI dismissal mechanisms (ESC key, overlay click,onOpenChangecallbacks) that function independently of the close button state.The implementation is sound and follows established patterns in related components (dialog.tsx, command.tsx).
TejasGhatte
changed the base branch from
12-24-fix_content_block_type_input_file_to_file
to
graphite-base/1176
akshaydeo
force-pushed
the
graphite-base/1176
branch
from
0769d11 to
f8d8fa0
Compare
akshaydeo
force-pushed
the
12-26-minor_ux_uplifg_restart_required_status_storage
branch
from
bce4df2 to
6445a81
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
ui/app/workspace/config/views/securityView.tsx (1)
241-241: Visual inconsistency: missing border and padding.This section no longer has the
space-x-2 rounded-lg border p-4classes that adjacent sections retain (lines 265-279, 282-297), creating a visual inconsistency where the "Disable authentication on inference calls" control appears without the border/padding container.🔎 Restore consistent styling
-<div className="flex items-center justify-between"> +<div className="flex items-center justify-between space-x-2 rounded-lg border p-4">
🧹 Nitpick comments (4)
ui/components/sidebar.tsx (1)
631-644: Consider optional chaining for consistency and safety.The code correctly checks
coreConfig?.restart_required?.requiredbefore adding the card, but line 638 accessescoreConfig.restart_required.reasonwithout optional chaining. While the outer condition ensuresrestart_requiredexists, using optional chaining (coreConfig.restart_required?.reason) would be more defensive and consistent with the rest of the codebase.🔎 Suggested refinement
if (coreConfig?.restart_required?.required) { cards.push({ id: "restart-required", title: "Restart Required", description: ( <div className="text-xs text-amber-700 dark:text-amber-300/80"> - {coreConfig.restart_required.reason || "Configuration changes require a server restart to take effect."} + {coreConfig.restart_required?.reason || "Configuration changes require a server restart to take effect."} </div> ), dismissible: false, variant: "warning" as const, }); }framework/configstore/rdb.go (1)
1983-1989: Refactor to marshal struct instead of hardcoded JSON.The method uses a hardcoded JSON string
{"required":false,"reason":""}instead of marshaling a struct like other methods (SetRestartRequiredConfig,UpdateProxyConfig, etc.). This is inconsistent and more fragile.🔎 Proposed refactor
func (s *RDBConfigStore) ClearRestartRequiredConfig(ctx context.Context) error { + configJSON, err := json.Marshal(&tables.RestartRequiredConfig{ + Required: false, + Reason: "", + }) + if err != nil { + return fmt.Errorf("failed to marshal restart required config: %w", err) + } return s.db.WithContext(ctx).Save(&tables.TableGovernanceConfig{ Key: tables.ConfigRestartRequiredKey, - Value: `{"required":false,"reason":""}`, + Value: string(configJSON), }).Error }transports/bifrost-http/handlers/config.go (2)
220-263: Good approach to tracking restart-requiring changes.The restart reason tracking is well-implemented with clear, human-readable messages. Each configuration change that requires a restart appropriately appends to the
restartReasonsslice.Optional: For better maintainability, you could extract the reason strings as constants if they need to be referenced elsewhere or for i18n in the future:
const ( RestartReasonPrometheusLabels = "Prometheus labels" RestartReasonAllowedOrigins = "Allowed origins" // etc. )However, for this current scope, inline strings are perfectly acceptable.
438-447: Consider using error-level logging for restart flag failures.The restart flag setting logic is well-implemented, but the failure case at line 445 uses
logger.Warn. Since failing to set the restart flag means users won't be notified of the required restart (which could lead to unexpected behavior), consider usinglogger.Errorinstead to ensure this issue gets appropriate attention in logs.if err := h.store.ConfigStore.SetRestartRequiredConfig(ctx, &configstoreTables.RestartRequiredConfig{ Required: true, Reason: reason, }); err != nil { - logger.Warn(fmt.Sprintf("failed to set restart required config: %v", err)) + logger.Error(fmt.Sprintf("failed to set restart required config: %v", err)) }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
framework/configstore/rdb.goframework/configstore/store.goframework/configstore/tables/config.gotransports/bifrost-http/handlers/config.gotransports/bifrost-http/lib/config.goui/app/workspace/config/views/pricingConfigView.tsxui/app/workspace/config/views/securityView.tsxui/app/workspace/logs/views/columns.tsxui/components/sidebar.tsxui/components/ui/promoCardStack.tsxui/components/ui/sheet.tsxui/components/ui/toggle.tsxui/lib/constants/logs.tsui/lib/types/config.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- ui/app/workspace/config/views/pricingConfigView.tsx
- ui/components/ui/sheet.tsx
- ui/lib/types/config.ts
- ui/components/ui/promoCardStack.tsx
- ui/components/ui/toggle.tsx
- transports/bifrost-http/lib/config.go
- ui/lib/constants/logs.ts
- ui/app/workspace/logs/views/columns.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
always check the stack if there is one for the current PR. do not give localized reviews for the PR, always see all changes in the light of the whole stack of PRs (if there is a stack, if there is no stack you can continue to make localized suggestions/reviews)
Files:
framework/configstore/tables/config.goui/components/sidebar.tsxtransports/bifrost-http/handlers/config.goframework/configstore/store.goframework/configstore/rdb.goui/app/workspace/config/views/securityView.tsx
🧠 Learnings (2)
📚 Learning: 2025-12-09T17:07:42.007Z
Learnt from: qwerty-dvorak
Repo: maximhq/bifrost PR: 1006
File: core/schemas/account.go:9-18
Timestamp: 2025-12-09T17:07:42.007Z
Learning: In core/schemas/account.go, the HuggingFaceKeyConfig field within the Key struct is currently unused and reserved for future Hugging Face inference endpoint deployments. Do not flag this field as missing from OpenAPI documentation or require its presence in the API spec until the feature is actively implemented and used. When the feature is added, update the OpenAPI docs accordingly; otherwise, treat this field as non-breaking and not part of the current API surface.
Applied to files:
framework/configstore/tables/config.gotransports/bifrost-http/handlers/config.goframework/configstore/store.goframework/configstore/rdb.go
📚 Learning: 2025-12-12T08:25:02.629Z
Learnt from: Pratham-Mishra04
Repo: maximhq/bifrost PR: 1000
File: transports/bifrost-http/integrations/router.go:709-712
Timestamp: 2025-12-12T08:25:02.629Z
Learning: In transports/bifrost-http/**/*.go, update streaming response handling to align with OpenAI Responses API: use typed SSE events such as response.created, response.output_text.delta, response.done, etc., and do not rely on the legacy data: [DONE] termination marker. Note that data: [DONE] is only used by the older Chat Completions and Text Completions streaming APIs. Ensure parsers, writers, and tests distinguish SSE events from the [DONE] sentinel and handle each event type accordingly for correct stream termination and progress updates.
Applied to files:
transports/bifrost-http/handlers/config.go
🧬 Code graph analysis (3)
transports/bifrost-http/handlers/config.go (5)
framework/configstore/store.go (1)
ConfigStore(17-157)ui/lib/types/config.ts (1)
RestartRequiredConfig(303-306)transports/bifrost-http/handlers/utils.go (1)
SendError(35-44)framework/encrypt/encrypt.go (1)
Hash(63-69)framework/configstore/tables/config.go (1)
RestartRequiredConfig(16-19)
framework/configstore/store.go (2)
framework/configstore/tables/config.go (1)
RestartRequiredConfig(16-19)ui/lib/types/config.ts (1)
RestartRequiredConfig(303-306)
framework/configstore/rdb.go (2)
framework/configstore/tables/config.go (4)
RestartRequiredConfig(16-19)TableGovernanceConfig(40-43)TableGovernanceConfig(46-46)ConfigRestartRequiredKey(11-11)ui/lib/types/config.ts (1)
RestartRequiredConfig(303-306)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
🔇 Additional comments (10)
ui/components/sidebar.tsx (1)
668-668: LGTM! Proper dependency tracking.Correctly added
coreConfig?.restart_requiredto the useMemo dependency array, ensuring the promo cards list recomputes whenever the restart status changes.framework/configstore/tables/config.go (1)
11-19: LGTM! Clean and well-designed types.The
ConfigRestartRequiredKeyconstant andRestartRequiredConfigstruct are well-defined with appropriate JSON tags and clear documentation. Theomitemptytag on theReasonfield is correct for optional usage.framework/configstore/store.go (1)
126-129: LGTM! Interface design follows existing patterns.The three new methods for restart-required configuration management are well-designed and consistent with the existing ConfigStore interface patterns. The signatures properly use context, pointer types, and error returns.
framework/configstore/rdb.go (3)
1952-1969: LGTM! Implementation follows established patterns.The
GetRestartRequiredConfigmethod correctly follows the same implementation pattern as other config retrieval methods (e.g.,GetProxyConfig), with proper error handling and safe nil returns for missing or empty configurations.
1971-1981: LGTM! Proper implementation.The
SetRestartRequiredConfigmethod correctly marshals the configuration to JSON and saves it using the established pattern. Error handling is appropriate.
1952-1989: No duplicate method declarations exist in the file. Each method (GetRestartRequiredConfig,SetRestartRequiredConfig,ClearRestartRequiredConfig) appears only once. The AI summary's claim about duplicate signatures can be disregarded.Likely an incorrect or invalid review comment.
transports/bifrost-http/handlers/config.go (4)
160-166: LGTM! Consistent with existing config retrieval.The restart configuration retrieval follows the same pattern as the auth and proxy config fetching above, with appropriate error handling and conditional inclusion in the response.
383-401: Auth change detection logic is sound.The authentication configuration change detection properly handles various cases:
- New auth enablement (nil config)
- Field changes (username, enabled state, inference auth setting)
- Password changes (excluding redacted values)
The password change detection at line 395 correctly excludes both redacted and empty passwords, which aligns with the validation at lines 404-407 that rejects empty passwords when auth is enabled.
403-427: LGTM! Robust password handling.The password handling logic properly manages three cases:
- Validation when auth is enabled but no existing config
- Preservation of existing password when redacted
- Hashing of new passwords with proper error handling
The defensive check at lines 411-413 prevents accepting a redacted password when no existing password is available.
600-606: LGTM! Consistent with updateConfig pattern.The proxy configuration restart flag setting follows the same pattern as in
updateConfig, appropriately notifying users that a restart is required after proxy changes.Optional: Same consideration as the previous comment regarding error logging level applies here (line 605).
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
ui/components/ui/toggle.tsx (1)
19-22: Consider extracting the conditional from the template literal.The conditional cursor logic is embedded in a template literal inside
cn(). For cleaner code that better leveragesclsx's conditional handling, consider separating it:🔎 Suggested refactor
- className={cn( - `dark:bg-input/30 flex w-full items-center justify-between gap-2 py-2 text-sm select-none ${disabled ? "cursor-default" : "cursor-pointer"}`, - className, - )} + className={cn( + "dark:bg-input/30 flex w-full items-center justify-between gap-2 py-2 text-sm select-none", + disabled ? "cursor-default" : "cursor-pointer", + className, + )}ui/app/workspace/logs/views/columns.tsx (1)
69-69: Consider retaining monospace font for timestamp readability.Removing
font-monofrom the timestamp may reduce readability and alignment consistency, as monospace fonts help ensure uniform character width for date-time values. Consider whether this change improves the overall UX or if reverting tofont-mono text-xswould be preferable.transports/bifrost-http/lib/config.go (1)
408-418: Startup clearing of restart-required flag looks right; minor DRY opportunityCalling
ClearRestartRequiredConfigimmediately after initializing the config store (both file-backed and default paths) matches the intended “clear on server startup” behavior, and treating failures as warnings is reasonable.You do duplicate the same clear-and-warn pattern in two places; if this grows (e.g., more init paths), consider a small helper (e.g.,
clearRestartRequiredIfSupported(ctx, store)) to centralize this behavior and keep future changes consistent.Also applies to: 1525-1533
framework/configstore/rdb.go (1)
1952-1989: Restart-required config CRUD is consistent; consider using struct-based clearThe new
GetRestartRequiredConfig,SetRestartRequiredConfig, andClearRestartRequiredConfigfollow existingTableGovernanceConfigpatterns and look correct.For
ClearRestartRequiredConfig, you currently write a hard‑coded JSON string. Using the same struct+json.Marshalpath asSetRestartRequiredConfig(e.g., delegating toSetRestartRequiredConfig(ctx, &tables.RestartRequiredConfig{Required: false})) would:
- Avoid divergence from the Go/TS types if fields change.
- Respect the
omitemptytag onReasonautomatically.Not critical, but it would tighten type safety and reduce duplication.
transports/bifrost-http/handlers/config.go (3)
160-166: Wiring restart_required into GET /api/config is correct; consider only exposing when requiredPulling
restartConfigfrom the store and returning it asrestart_requiredmatches the new type and backend API.If the UI only cares when a restart is actually needed, you may want to only include this field (or set it to
null) whenrestartConfig.Requiredistrueto avoid consumers accidentally keying off mere presence rather than the boolean. This is a minor shape/semantics polish, not a blocker.
220-266: Client/auth restart tracking is reasonable; a few polish opportunitiesThe
restartReasonsaggregation for client config (Prometheus labels, allowed origins, pool size, logging, governance, max body size) plus theauthChangeddetection is a solid way to centralize “restart-required” conditions and surface a single, user-friendly reason string.Two small suggestions:
- You might want to deduplicate reasons (or use constants) if this list grows, to avoid accidental duplicates or typos across future call sites.
- The generated sentence (
"<reasons> settings have been updated...") is slightly awkward in the single-reason case; if UX copy matters, branching onlen(restartReasons)to choose singular/plural text would read more naturally.Behavior-wise this looks correct and low-risk.
Also applies to: 371-447
600-606: Proxy updates always flag restart-required—confirm this matches operational intentUnconditionally setting
Required: trueinSetRestartRequiredConfigafter successfully updating and reloading the proxy config is clear and simple. It will keep the restart banner up until the next server restart, even though you already apply the new config viaReloadProxyConfig.If that’s intentional (e.g., to signal that long-lived processes or other nodes still need a restart), this is fine. If proxy changes are fully hot‑reloadable, you may want to gate this behind a comparison with the previous config or skip marking restart-required for pure proxy tweaks.
ui/components/ui/promoCardStack.tsx (1)
8-14: Non-dismissible-first sorting and warning variant look good; can DRY the sorterThe changes to:
- Add
variant?: "default" | "warning"onPromoCardItemand apply warning styles to theCard/CardHeader, and- Sort so
dismissible === falsecards are always stacked on top,are both sensible and match the intended UX.
You do repeat the same sort comparator in both the
useStateinitializer and theuseEffectwatchingcards. Extracting a small helper (e.g.,const sortCards = (cards: PromoCardItem[]) => [...cards].sort(...)) would reduce duplication and keep behavior in sync if the sort rules evolve.Also applies to: 23-31, 36-45, 99-107
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
framework/configstore/rdb.goframework/configstore/store.goframework/configstore/tables/config.gotransports/bifrost-http/handlers/config.gotransports/bifrost-http/lib/config.goui/app/workspace/config/views/pricingConfigView.tsxui/app/workspace/config/views/securityView.tsxui/app/workspace/logs/views/columns.tsxui/components/sidebar.tsxui/components/ui/promoCardStack.tsxui/components/ui/sheet.tsxui/components/ui/toggle.tsxui/lib/constants/logs.tsui/lib/types/config.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- framework/configstore/tables/config.go
- ui/components/ui/sheet.tsx
- ui/lib/constants/logs.ts
- ui/app/workspace/config/views/pricingConfigView.tsx
- ui/app/workspace/config/views/securityView.tsx
- framework/configstore/store.go
- ui/components/sidebar.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
always check the stack if there is one for the current PR. do not give localized reviews for the PR, always see all changes in the light of the whole stack of PRs (if there is a stack, if there is no stack you can continue to make localized suggestions/reviews)
Files:
transports/bifrost-http/handlers/config.goui/components/ui/promoCardStack.tsxui/lib/types/config.tsui/components/ui/toggle.tsxtransports/bifrost-http/lib/config.goui/app/workspace/logs/views/columns.tsxframework/configstore/rdb.go
🧠 Learnings (2)
📚 Learning: 2025-12-09T17:07:42.007Z
Learnt from: qwerty-dvorak
Repo: maximhq/bifrost PR: 1006
File: core/schemas/account.go:9-18
Timestamp: 2025-12-09T17:07:42.007Z
Learning: In core/schemas/account.go, the HuggingFaceKeyConfig field within the Key struct is currently unused and reserved for future Hugging Face inference endpoint deployments. Do not flag this field as missing from OpenAPI documentation or require its presence in the API spec until the feature is actively implemented and used. When the feature is added, update the OpenAPI docs accordingly; otherwise, treat this field as non-breaking and not part of the current API surface.
Applied to files:
transports/bifrost-http/handlers/config.gotransports/bifrost-http/lib/config.goframework/configstore/rdb.go
📚 Learning: 2025-12-12T08:25:02.629Z
Learnt from: Pratham-Mishra04
Repo: maximhq/bifrost PR: 1000
File: transports/bifrost-http/integrations/router.go:709-712
Timestamp: 2025-12-12T08:25:02.629Z
Learning: In transports/bifrost-http/**/*.go, update streaming response handling to align with OpenAI Responses API: use typed SSE events such as response.created, response.output_text.delta, response.done, etc., and do not rely on the legacy data: [DONE] termination marker. Note that data: [DONE] is only used by the older Chat Completions and Text Completions streaming APIs. Ensure parsers, writers, and tests distinguish SSE events from the [DONE] sentinel and handle each event type accordingly for correct stream termination and progress updates.
Applied to files:
transports/bifrost-http/handlers/config.gotransports/bifrost-http/lib/config.go
🧬 Code graph analysis (5)
ui/components/ui/promoCardStack.tsx (1)
ui/components/ui/card.tsx (1)
CardHeader(50-50)
ui/lib/types/config.ts (1)
framework/configstore/tables/config.go (1)
RestartRequiredConfig(16-19)
ui/components/ui/toggle.tsx (1)
ui/lib/utils.ts (1)
cn(4-6)
ui/app/workspace/logs/views/columns.tsx (1)
ui/lib/constants/logs.ts (2)
Status(173-173)StatusBarColors(84-89)
framework/configstore/rdb.go (2)
framework/configstore/tables/config.go (4)
RestartRequiredConfig(16-19)TableGovernanceConfig(40-43)TableGovernanceConfig(46-46)ConfigRestartRequiredKey(11-11)ui/lib/types/config.ts (1)
RestartRequiredConfig(303-306)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
🔇 Additional comments (2)
ui/components/ui/toggle.tsx (1)
3-3: LGTM! Improved component flexibility.The addition of the optional
classNameprop and thecnutility import enables flexible external styling customization, which is a good practice for reusable UI components.Also applies to: 6-6, 15-15
ui/lib/types/config.ts (1)
302-315: RestartRequiredConfig type matches backend shapeThe
RestartRequiredConfiginterface and optionalrestart_requiredfield onBifrostConfigalign with the Go struct (required/reason) and the handler response keys, so UI consumers can rely on strong typing here. No changes needed.
Summary
Adds a restart required notification system to inform users when configuration changes require a server restart to take full effect.
Changes
RestartRequiredConfigdata structure to track when a restart is neededType of change
Affected areas
How to test
Screenshots/Recordings
N/A
Breaking changes
Related issues
N/A
Security considerations
No security implications as this is a UI notification feature.
Checklist
docs/contributing/README.mdand followed the guidelines