-
Notifications
You must be signed in to change notification settings - Fork 684
more config updates (secrets, waveai, ai:provider) #2631
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…u, hook up modes context menu to config, some utilities
WalkthroughIntroduces Wave AI Modes configuration (types, schemas, defaults) with a visual editor placeholder and full secrets management UI. Backend AI handling moves from BaseURL to Endpoint, adds provider-aware defaults and provider-gated Wave headers, and updates AI option structs. Adds SSH password secret support, config watcher update callbacks, telemetry panic guards, new UI components (SecretsContent, WaveAIVisualContent, VersionBadge), utilities (sortByDisplayOrder, getFilteredAIModeConfigs), and multiple schema/type edits across frontend and pkg/wconfig. Documentation and default config files for Wave AI and secrets were added or updated. Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Areas requiring extra attention:
Potential quick checks:
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/app/view/waveconfig/waveconfig-model.ts (1)
54-62: Error message is inconsistent with the updated pattern.The
keyPatternnow includes dots and hyphens (/^[a-zA-Z0-9_@.-]+$/), but the error message only mentions "@ and hyphens", omitting dots:if (!keyPattern.test(key)) { return { - error: `Invalid key "${key}": keys must only contain letters, numbers, underscores, @ and hyphens`, + error: `Invalid key "${key}": keys must only contain letters, numbers, underscores, @, dots, and hyphens`, }; }
🧹 Nitpick comments (19)
frontend/app/modals/userinputmodal.tsx (3)
66-76: AlignhandleKeyDownreturn type with all code paths
handleKeyDownis annotated to returnboolean, but the Escape branch (return;) and the default path don’t return a boolean. Depending on your TS settings this can cause type errors or subtle inconsistencies. Consider either:
- Returning an explicit boolean in all branches (e.g.,
return falsewhen not handled), or- Relaxing the return type to
boolean | voidif that’s whatkeydownWrapperexpects.
92-101: Consider making the input full‑widthThe new input style is clean, but without an explicit width the browser default may render a relatively narrow field. To keep it aligned with the modal body width, consider adding
w-fullto the class list:- className="resize-none bg-panel rounded-md border border-border py-1.5 pl-4 min-h-[30px] text-inherit cursor-text focus:outline-accent" + className="w-full resize-none bg-panel rounded-md border border-border py-1.5 pl-4 min-h-[30px] text-inherit cursor-text focus:outline-accent"
104-121: Checkbox layout/styling LGTM, but watch theuseMemodepsThe new flex/gap layout and
accent-accentcheckbox styling look good and keep label/box nicely aligned.One minor thing:
optionalCheckbox’suseMemohas an empty dependency array but readsuserInputRequest.checkboxmsganduserInputRequest.requestid. If this component is ever reused for a differentuserInputRequestwithout unmounting, the checkbox label/id won’t update. If that’s a possible future scenario, consider either droppinguseMemohere or adding the relevant fields to its dependency list.pkg/remote/sshclient.go (1)
625-642: Secret-backed SSH password retrieval looks good; confirm fallback and BatchMode semanticsThe
SshPasswordSecretNamehandling is wired sensibly: you resolve the secret once, fail fast on lookup errors or missing entries, and log only the secret name (not its value), then inject the password into the existingssh.PasswordCallbackpath.Two behavioral points worth explicitly confirming:
No interactive fallback when a secret is configured
IfSshPasswordSecretNameis set but the secret is missing or retrieval fails,createClientConfigreturns an error and you never fall back to prompting the user. This is stricter than the previous behavior and seems intentional; just make sure this matches UX expectations for misconfigured secrets.BatchMode disables secret-backed password auth
Because the"password"auth method is gated bySshPasswordAuthentication && !SshBatchMode, setting BatchMode will also prevent using a secret-backed password (even though it’s non-interactive). If you ever want “non-interactive password from secret, but still BatchMode=yes from ssh_config”, this might need a follow-up knob or a slight tweak.If both are intentional, the implementation is solid as-is.
cmd/server/main-server.go (2)
92-106: Panic guarding long‑running goroutines is a solid robustness improvementWrapping
stdinReadWatch,telemetryLoop,startupActivityUpdate, and theGetSystemSummarygoroutine withpanichandler.PanicHandlerensures unexpected panics in background tasks are logged and reported without crashing the whole server. The surrounding logic and shutdown paths remain unchanged.Also applies to: 114-127, 276-287, 529-534
129-163: Telemetry config watcher correctly reacts to TelemetryEnabled toggles
setupTelemetryConfigHandlersubscribes to full config updates and callssendNoTelemetryUpdatewheneverSettings.TelemetryEnabledflips, with appropriate nil‑watcher guards and a bounded 5s context for the outbound call. This should keep the cloud side in sync with user opt‑out state.If you ever need stronger guarantees that the remote “no telemetry” flag matches local config even after a failed call or an out‑of‑band config edit while the server was down, consider an initial sync on startup (e.g., fire
sendNoTelemetryUpdateonce after loading the first full config).Also applies to: 513-526
pkg/aiusechat/aiutil/aiutil.go (1)
188-210: Reasoning‑model detection and helpers look correctRefactoring
IsOpenAIReasoningModelto useCheckModelPrefix/CheckModelSubPrefixsimplifies the checks and cleanly extends support togpt-5*andgpt-6*(including dotted numeric subversions). Bounds checks inCheckModelSubPrefixavoid panics on short model names.It would be good to add a small table‑driven test covering edge cases like
gpt-5,gpt-5.0,gpt-5.0-mini,gpt-5.turbo, etc., so future model naming changes don’t accidentally widen or narrow the reasoning set.pkg/wconfig/filewatcher.go (1)
119-135: Consider adding an unregister mechanism for handlers.The current implementation allows registering handlers but provides no way to unregister them. If handlers are dynamically registered throughout the application lifecycle, this could lead to memory leaks as the handler slice grows indefinitely.
If this is intentional (e.g., handlers are only registered during initialization), consider adding a comment to clarify the expected usage pattern. Otherwise, consider adding an
UnregisterUpdateHandlermethod:func (w *Watcher) UnregisterUpdateHandler(handler ConfigUpdateHandler) { w.mutex.Lock() defer w.mutex.Unlock() for i, h := range w.handlers { // Compare function pointers if &h == &handler { w.handlers = append(w.handlers[:i], w.handlers[i+1:]...) break } } }Note: Function pointer comparison in Go has limitations, so you might need a different identification mechanism (e.g., handler IDs).
frontend/app/aipanel/waveai-model.tsx (1)
80-81: Consider extracting default mode logic to reduce duplication.The logic for retrieving the default AI mode (
getSettingsKeyAtom("waveai:defaultmode") ?? "waveai@balanced") is duplicated on lines 80-81 and 370-371. Consider extracting this into a helper method or getter to ensure consistency and make future updates easier.private getDefaultAIMode(): string { return globalStore.get(getSettingsKeyAtom("waveai:defaultmode")) ?? "waveai@balanced"; }Then use it in both locations:
-const defaultMode = globalStore.get(getSettingsKeyAtom("waveai:defaultmode")) ?? "waveai@balanced"; +const defaultMode = this.getDefaultAIMode(); this.currentAIMode = jotai.atom(defaultMode);schema/settings.json (1)
71-73: Consider validating waveai:defaultmode with enum values.The
waveai:defaultmodefield currently accepts any string value. Consider adding an enum constraint to validate that only known AI mode identifiers are accepted, preventing configuration errors."waveai:defaultmode": { "type": "string", "enum": ["waveai@quick", "waveai@balanced", "waveai@deep"] }Note: If custom AI modes are allowed, this suggestion may not apply. In that case, consider pattern validation instead.
docs/src/components/versionbadge.tsx (1)
7-9: LGTM! Simple and well-typed component.The component is straightforward and correctly typed. Consider adding a
displayNamefor better debugging in React DevTools if desired.export function VersionBadge({ version }: VersionBadgeProps) { return <span className="version-badge">{version}</span>; } + +VersionBadge.displayName = "VersionBadge";frontend/app/view/waveconfig/waveconfig.tsx (1)
263-267: Consider simplifying the IIFE pattern for visual component rendering.The immediately-invoked function expression (IIFE) pattern works but adds unnecessary complexity. You can render the component directly.
- ) : selectedFile.visualComponent && (!selectedFile.hasJsonView || activeTab === "visual") ? ( - (() => { - const VisualComponent = selectedFile.visualComponent; - return <VisualComponent model={model} />; - })() + ) : selectedFile.visualComponent && (!selectedFile.hasJsonView || activeTab === "visual") ? ( + <selectedFile.visualComponent model={model} />aiprompts/aimodesconfig.md (1)
117-137: Consider adding language identifier to ASCII diagram code blocks.The static analysis tool flags these as missing language specifiers. While ASCII diagrams don't need syntax highlighting, you can use
textorplaintextto silence the warning.-``` +```text ┌─────────────────────────────────────────────────────────┐frontend/app/aipanel/aimode.tsx (1)
58-58: Fix inconsistent indentation.Line 58 uses tabs while the rest of the file uses spaces for indentation.
- return ( + return (frontend/app/view/waveconfig/secretscontent.tsx (3)
135-136: Duplicate regex definition - consider importing from model.The
secretNameRegexis defined locally here butSECRET_NAME_REGEXis also exported fromwaveconfig-model.ts. Use the shared constant to avoid potential drift:+import { SECRET_NAME_REGEX } from "@/app/view/waveconfig/waveconfig-model"; + const AddSecretForm = memo( ({ newSecretName, ... }: AddSecretFormProps) => { - const secretNameRegex = /^[A-Za-z][A-Za-z0-9_]*$/; - const isNameInvalid = newSecretName !== "" && !secretNameRegex.test(newSecretName); + const isNameInvalid = newSecretName !== "" && !SECRET_NAME_REGEX.test(newSecretName);
179-192: Submit button allows click when name is invalid.The button is only disabled when
isLoading, but not whenisNameInvalid. Users can submit invalid names which then fails validation inaddNewSecret(). Consider disabling the button proactively:<button className="px-4 py-2 bg-accent-600 hover:bg-accent-500 rounded cursor-pointer disabled:opacity-50 disabled:cursor-not-allowed flex items-center gap-2" onClick={onSubmit} - disabled={isLoading} + disabled={isLoading || isNameInvalid || newSecretName.trim() === ""} >
262-279: Consider separate loading states for Delete vs Save.Both Delete and Save buttons show spinners when
isLoadingis true, but only one operation runs at a time. The UX could be confusing if the user clicks Delete and both buttons appear to be processing. Consider tracking which operation is in progress or disabling the other button more explicitly.pkg/aiusechat/usechat-mode.go (1)
160-166: Consider precompiling the Azure resource name regex.
regexp.MatchStringcompiles the regex on every call. For a function that may be called frequently during config loading, precompile for efficiency:+var azureResourceNameRegex = regexp.MustCompile(`^[a-z0-9]([a-z0-9-]*[a-z0-9])?$`) + func isValidAzureResourceName(name string) bool { if name == "" || len(name) > 63 { return false } - matched, _ := regexp.MatchString(`^[a-z0-9]([a-z0-9-]*[a-z0-9])?$`, name) + matched := azureResourceNameRegex.MatchString(name) return matched }frontend/app/view/waveconfig/waveconfig-model.ts (1)
90-96: WaveAIVisualContent is commented out.This appears intentional (work in progress). Consider adding a TODO comment or removing the import if it's not yet ready.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (42)
aiprompts/aimodesconfig.md(1 hunks)cmd/server/main-server.go(5 hunks)docs/docs/ai-presets.mdx(1 hunks)docs/docs/connections.mdx(2 hunks)docs/docs/faq.mdx(0 hunks)docs/docs/secrets.mdx(1 hunks)docs/docs/waveai-modes.mdx(1 hunks)docs/src/components/versionbadge.css(1 hunks)docs/src/components/versionbadge.tsx(1 hunks)emain/emain-menu.ts(1 hunks)frontend/app/aipanel/ai-utils.ts(2 hunks)frontend/app/aipanel/aimessage.tsx(1 hunks)frontend/app/aipanel/aimode.tsx(6 hunks)frontend/app/aipanel/aipanel-contextmenu.ts(2 hunks)frontend/app/aipanel/aipanelmessages.tsx(1 hunks)frontend/app/aipanel/waveai-model.tsx(3 hunks)frontend/app/modals/conntypeahead.tsx(1 hunks)frontend/app/modals/userinputmodal.scss(0 hunks)frontend/app/modals/userinputmodal.tsx(4 hunks)frontend/app/view/waveconfig/secretscontent.tsx(1 hunks)frontend/app/view/waveconfig/waveaivisual.tsx(1 hunks)frontend/app/view/waveconfig/waveconfig-model.ts(11 hunks)frontend/app/view/waveconfig/waveconfig.tsx(4 hunks)frontend/app/workspace/widgets.tsx(2 hunks)frontend/types/custom.d.ts(2 hunks)frontend/types/gotypes.d.ts(3 hunks)frontend/util/util.ts(2 hunks)pkg/aiusechat/aiutil/aiutil.go(3 hunks)pkg/aiusechat/tools.go(1 hunks)pkg/aiusechat/uctypes/uctypes.go(2 hunks)pkg/aiusechat/usechat-mode.go(2 hunks)pkg/aiusechat/usechat.go(1 hunks)pkg/remote/sshclient.go(5 hunks)pkg/wconfig/defaultconfig/settings.json(1 hunks)pkg/wconfig/defaultconfig/waveai.json(2 hunks)pkg/wconfig/filewatcher.go(2 hunks)pkg/wconfig/metaconsts.go(1 hunks)pkg/wconfig/settingsconfig.go(4 hunks)schema/bgpresets.json(1 hunks)schema/connections.json(1 hunks)schema/settings.json(1 hunks)schema/waveai.json(3 hunks)
💤 Files with no reviewable changes (2)
- frontend/app/modals/userinputmodal.scss
- docs/docs/faq.mdx
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-14T06:30:54.763Z
Learnt from: sawka
Repo: wavetermdev/waveterm PR: 2430
File: frontend/app/aipanel/aimessage.tsx:137-144
Timestamp: 2025-10-14T06:30:54.763Z
Learning: In `frontend/app/aipanel/aimessage.tsx`, the `AIToolUseGroup` component splits file operation tool calls into separate batches (`fileOpsNeedApproval` and `fileOpsNoApproval`) based on their approval state before passing them to `AIToolUseBatch`. This ensures each batch has homogeneous approval states, making group-level approval handling valid.
Applied to files:
frontend/app/aipanel/aimessage.tsxfrontend/app/aipanel/ai-utils.ts
📚 Learning: 2025-10-15T03:21:02.229Z
Learnt from: sawka
Repo: wavetermdev/waveterm PR: 2433
File: pkg/aiusechat/tools_readfile.go:197-197
Timestamp: 2025-10-15T03:21:02.229Z
Learning: In Wave Terminal's AI tool definitions (pkg/aiusechat/tools_*.go), the Description field should not mention approval requirements even when ToolApproval returns ApprovalNeedsApproval. This prevents the LLM from asking users for approval before calling the tool, avoiding redundant double-approval prompts since the runtime will enforce approval anyway.
Applied to files:
pkg/aiusechat/tools.gopkg/aiusechat/aiutil/aiutil.gopkg/aiusechat/usechat-mode.gopkg/aiusechat/usechat.go
🧬 Code graph analysis (10)
pkg/wconfig/filewatcher.go (2)
pkg/wconfig/settingsconfig.go (1)
FullConfigType(286-297)pkg/panichandler/panichandler.go (1)
PanicHandler(25-43)
frontend/app/view/waveconfig/secretscontent.tsx (3)
tsunami/demo/tsunamiconfig/app.go (2)
ErrorDisplayProps(45-47)ErrorDisplay(225-242)frontend/util/util.ts (1)
cn(502-502)frontend/app/view/waveconfig/waveconfig-model.ts (1)
WaveConfigViewModel(133-561)
cmd/server/main-server.go (7)
pkg/panichandler/panichandler.go (1)
PanicHandler(25-43)pkg/wstore/wstore_dbops.go (1)
DBGetSingleton(102-105)pkg/waveobj/wtype.go (2)
Client(127-135)Client(137-139)pkg/wcloud/wcloud.go (1)
SendNoTelemetryUpdate(269-279)pkg/wconfig/filewatcher.go (1)
GetWatcher(36-60)pkg/wconfig/settingsconfig.go (1)
FullConfigType(286-297)pkg/wavebase/wavebase.go (1)
GetSystemSummary(376-383)
frontend/app/view/waveconfig/waveaivisual.tsx (1)
frontend/app/view/waveconfig/waveconfig-model.ts (1)
WaveConfigViewModel(133-561)
pkg/aiusechat/usechat-mode.go (4)
pkg/wconfig/settingsconfig.go (1)
AIModeConfigType(265-284)pkg/wconfig/filewatcher.go (1)
GetWatcher(36-60)pkg/aiusechat/uctypes/uctypes.go (10)
AIModeBalanced(162-162)AIProvider_Wave(28-28)DefaultAIEndpoint(13-13)WaveAIEndpointEnvName(16-16)AIProvider_OpenAI(31-31)AIProvider_OpenRouter(30-30)DefaultOpenRouterEndpoint(15-15)APIType_OpenAIChat(24-24)AIProvider_AzureLegacy(33-33)AIProvider_Azure(32-32)pkg/aiusechat/aiutil/aiutil.go (2)
CheckModelPrefix(199-201)CheckModelSubPrefix(203-210)
frontend/app/aipanel/aipanel-contextmenu.ts (4)
frontend/app/store/global.ts (1)
getSettingsKeyAtom(835-835)frontend/app/aipanel/ai-utils.ts (1)
getFilteredAIModeConfigs(546-574)frontend/app/store/wshclientapi.ts (1)
RpcApi(682-682)frontend/app/store/wshrpcutil.ts (1)
TabRpcClient(37-37)
pkg/aiusechat/usechat.go (1)
pkg/aiusechat/uctypes/uctypes.go (2)
ThinkingLevelMedium(156-156)AIOptsType(269-282)
pkg/remote/sshclient.go (2)
pkg/blocklogger/blocklogger.go (1)
Infof(78-84)pkg/secretstore/secretstore.go (1)
GetSecret(251-263)
pkg/wconfig/settingsconfig.go (1)
pkg/wcloud/wcloud.go (1)
APIVersion(34-34)
frontend/app/aipanel/waveai-model.tsx (1)
frontend/app/store/global.ts (2)
globalStore(839-839)getSettingsKeyAtom(835-835)
🪛 markdownlint-cli2 (0.18.1)
aiprompts/aimodesconfig.md
117-117: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
260-260: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
326-326: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
637-637: Multiple headings with the same content
(MD024, no-duplicate-heading)
647-647: Multiple headings with the same content
(MD024, no-duplicate-heading)
657-657: Multiple headings with the same content
(MD024, no-duplicate-heading)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Build for TestDriver.ai
🔇 Additional comments (69)
pkg/aiusechat/tools.go (1)
125-129: waveconfig block description is clear and consistentThe new
waveconfigcase cleanly mirrors existing patterns (e.g.,preview), handles missing/emptyfilemetadata gracefully, and produces concise, useful descriptions for tab state prompts. No changes needed.frontend/app/aipanel/aimessage.tsx (1)
226-226: LGTM! Responsive styling simplified.The removal of the breakpoint-specific max-width override simplifies the user message container's responsive behavior. This aligns well with the UI consolidation efforts mentioned in the PR objectives.
frontend/app/modals/conntypeahead.tsx (1)
277-278: The waveconfig view is properly registered and connections.json is explicitly supported as a configuration file with full editor support, documentation, and JSON schema validation. The hardcoded file reference follows the established pattern used throughout the codebase for all waveconfig-based config files (settings.json, widgets.json, waveai.json, etc.).Likely an incorrect or invalid review comment.
frontend/app/modals/userinputmodal.tsx (2)
80-85: Query text rendering change looks goodSwitching to plain
<Markdown>and<span>without the olduserinput-*classes is fine, and the new containerfont-mono text-primarywill control most of the appearance. Just confirm there are no regressions where prior SCSS targeted those specific classes (spacing/margins around the Markdown block).
148-162: Modal padding and content layout changes look consistentThe added
className="pt-6 pb-4 px-5"onModaland the new header/body wrappers (font-bold text-primarytitle,flex flex-col ... max-w-[500px] font-mono) give the dialog a clearer hierarchy and constrained width. This aligns with the Tailwind-style direction and should integrate well with other modals.pkg/remote/sshclient.go (3)
24-38: Secret store import is correctly scopedThe new
secretstoreimport is used only for SSH password retrieval increateClientConfigand doesn’t introduce unused dependencies here. No changes needed.
227-262: Password callback cleanly supports both secret-backed and interactive flowsThe updated
createPasswordCallbackPromptcorrectly short-circuits to the providedpasswordwhen non-nil and otherwise preserves the existing interactive prompt behavior and panic handling. This keeps the control flow simple and avoids unnecessary user interaction when a secret is configured.
986-1043: SshPasswordSecretName propagation in mergeKeywords matches existing override semanticsCopying
SshPasswordSecretNamefromnewKeywordsintooutKeywordswhen non-nil aligns with how other scalar fields (user, port, auth flags, etc.) are merged, so later sources correctly override earlier ones. This should integrate cleanly with the existing cascade logic.docs/docs/ai-presets.mdx (1)
4-8: Deprecation label in title is consistent with notice blockRenaming the page to “AI Presets (Deprecated)” matches the warning section and helps steer users toward Wave AI; no further changes needed here.
emain/emain-menu.ts (1)
396-416: Enabled state propagation for context menu items looks correctWiring
enabled: menuDef.enabledinto the template cleanly exposes per-item disable/enable behavior to the actual Electron menu without changing click semantics.schema/bgpresets.json (1)
2-45: BgPresetsType schema structure is soundThe schema correctly models a map of background presets keyed by name, with
additionalProperties: falseonBgPresetsTypeto prevent stray fields. This looks consistent with how other preset maps are modeled.frontend/app/aipanel/aipanelmessages.tsx (1)
58-66: AIMode dropdown layout change keeps scroll behavior intactMoving
AIModeDropdowninto normal flow (mb-2) and dropping therelativeclass from the scroll container still works withmessagesContainerRefandscrollToBottom; the whole messages region (including the dropdown) remains scrollable as expected.frontend/app/workspace/widgets.tsx (2)
259-269: Secrets entry now correctly targets waveconfig viewRouting “Secrets” to
view: "waveconfig"withfile: "secrets"matches the new consolidated config UI and should simplify secrets management flows, assuming waveconfig is wired for that file key.Please double‑check that the waveconfig model/view is expecting
file: "secrets"(and not e.g."secrets.json") so this opens the intended panel.
379-393: Editing widgets via waveconfig is a nice simplificationSwapping the widgets bar context menu to open
view: "waveconfig", file: "widgets.json"removes the need to construct filesystem paths in the client and aligns with the new config editor.Confirm that the waveconfig view exposes a JSON editor for
widgets.jsonunder this file key so users land directly in the right document.frontend/types/custom.d.ts (1)
133-155: Adding"header"to menu item type unions aligns with new grouped menusExtending both
ElectronContextMenuItem.typeandContextMenuItem.typeto include"header"lets you type the new section headers in context menus without casting, and keeps the API surface consistent.pkg/aiusechat/aiutil/aiutil.go (1)
258-287: Whitespace and parsing flow around SendToolProgress remain safeThe minor spacing adjustments around JSON parsing and progress event creation don’t alter control flow; the early returns on parse/descriptor errors still prevent bad progress payloads from being sent.
pkg/wconfig/filewatcher.go (1)
125-135: Handler execution is well-protected.The implementation correctly:
- Copies the handler slice before iteration to avoid holding the lock during callbacks
- Executes each handler in a separate goroutine
- Protects each handler with panic recovery
This ensures that a slow or panicking handler won't block the watcher or affect other handlers.
schema/connections.json (1)
78-80: LGTM!The new
ssh:passwordsecretnamefield is properly defined in the schema with the correct type. The placement is logical, grouping it near other SSH authentication fields.pkg/wconfig/defaultconfig/waveai.json (2)
1-40: Configuration structure is consistent.All three AI modes (quick, balanced, deep) now consistently include the
ai:providerfield set to "wave", which aligns with the PR's goal of standardizing provider configuration.
8-8: Provider field is properly validated in schema with documented enum values.The
ai:providerfield is defined inschema/waveai.jsonwith type"string"and explicit enum validation allowing: "wave", "google", "openrouter", "openai", "azure", "azure-legacy", and "custom". The addition of"ai:provider": "wave"to the default configuration is valid and aligns with the schema requirements.docs/docs/connections.mdx (2)
7-7: LGTM!The VersionBadge import is properly added to support the version indicator for the new SSH secret feature.
161-161: Documentation is clear and well-integrated.The new
ssh:passwordsecretnamefield is properly documented with:
- Clear description of its purpose
- Version badge indicating when it was introduced (v0.13)
- Cross-reference to the secrets documentation
The formatting and placement are consistent with other SSH configuration options.
docs/docs/secrets.mdx (2)
1-147: Comprehensive and well-structured documentation.This new documentation provides excellent coverage of the secrets management feature:
- Clear explanation of use cases and benefits
- Multiple access methods (UI and CLI)
- Practical examples for SSH integration
- Security considerations
- Platform-specific storage backend information
- Troubleshooting guidance
The documentation is well-organized and user-friendly.
61-69: Secret naming rules are clearly defined.The regex pattern
^[A-Za-z][A-Za-z0-9_]*$and its explanation with valid/invalid examples make it easy for users to understand the constraints.frontend/app/aipanel/waveai-model.tsx (2)
11-11: LGTM!The import of
getSettingsKeyAtomenables reading the default AI mode from global settings.
370-372: Default mode initialization is working correctly.The change to read the default AI mode from settings (with fallback to "waveai@balanced") ensures consistency with user preferences.
pkg/wconfig/defaultconfig/settings.json (1)
27-28: LGTM!The new Wave AI settings are properly configured with sensible defaults:
waveai:showcloudmodes: trueenables cloud mode visibility by defaultwaveai:defaultmode: "waveai@balanced"provides a reasonable default AI modeThese align with the broader Wave AI configuration system introduced in this PR.
schema/settings.json (1)
68-73: Schema additions are correct.The new Wave AI settings are properly defined in the schema with appropriate types. These enable user configuration of cloud mode visibility and default AI mode selection.
frontend/util/util.ts (1)
482-492: LGTM! Clean sorting utility.The implementation correctly handles two-level sorting with appropriate defaults for optional fields. The use of
localeCompareensures locale-aware alphabetic ordering.pkg/wconfig/metaconsts.go (1)
32-34: LGTM! Constants follow conventions.The new Wave AI configuration keys follow the established naming pattern and are appropriately positioned among related AI settings.
frontend/app/view/waveconfig/waveaivisual.tsx (1)
11-18: LGTM! Placeholder component ready for implementation.The component structure is sound with proper memoization and typing. The
modelprop is currently unused, which is expected for a placeholder component.docs/docs/waveai-modes.mdx (1)
1-326: Excellent comprehensive documentation.The documentation is well-organized with clear examples for multiple providers, security best practices, and troubleshooting guidance. The structure flows logically from overview to specific examples.
frontend/app/aipanel/aipanel-contextmenu.ts (2)
86-114: "Custom Modes" section properly handles non-Wave providers.The separation of Wave and custom provider modes with appropriate headers and conditional separators maintains good UX organization.
60-84: Data-driven menu construction improves maintainability.The refactor from hardcoded menu items to dynamically generated ones based on configuration is a good improvement. The premium gating and active mode tracking logic are preserved correctly.
However, verify that the context menu framework supports the
"header"type used for section labels in lines 62-65 and 88-91. If the header type is not supported, these lines should use a different approach (such as a disabled separator or visual divider).frontend/types/gotypes.d.ts (3)
16-36: LGTM! Type updates reflect expanded AI configuration.The transition of multiple AI mode fields from required to optional provides needed flexibility for different provider configurations. This aligns with the schema changes described in the PR.
612-612: SSH password secret support added.The new
ssh:passwordsecretnamefield enables secure storage of SSH passwords via the secret store, following the same pattern as API token secrets.
1070-1071: Wave AI settings extended appropriately.The new
waveai:showcloudmodesandwaveai:defaultmodesettings provide control over AI mode visibility and defaults, complementing the expanded mode configuration system.pkg/aiusechat/usechat.go (2)
94-97: Good practice to default thinking level.Defaulting to
ThinkingLevelMediumensures consistent behavior when the field is not explicitly set. The constant is correctly defined inuctypes.gowith the value"medium", and represents a balanced middle-ground option among the available levels (ThinkingLevelLow,ThinkingLevelMedium,ThinkingLevelHigh).
88-92: Endpoint logic and thinking level defaults are correct.The simplified endpoint configuration is properly implemented with direct use of
config.Endpointand an appropriate fallback error message referencing the correctai:endpointconfiguration key. The thinking level correctly defaults touctypes.ThinkingLevelMediumwhen not configured.frontend/app/view/waveconfig/waveconfig.tsx (2)
186-207: LGTM!The save button and unsaved changes indicator are correctly gated by
hasJsonView, ensuring they only appear for files that support JSON editing. The tooltip and disabled states are properly handled.
210-235: LGTM!The tab bar implementation for switching between Visual and Raw JSON views is clean. The conditional rendering (
visualComponent && hasJsonView) correctly ensures tabs only appear when both views are available.docs/src/components/versionbadge.css (1)
1-18: LGTM!The version badge styling is clean and well-structured. The use of CSS custom properties ensures theme consistency, and the dark mode override with
[data-theme="dark"]follows Docusaurus conventions.pkg/aiusechat/uctypes/uctypes.go (2)
14-16: LGTM!The new endpoint constants and environment variable name are properly defined and follow Go naming conventions. Centralizing these in the types package improves maintainability.
27-35: LGTM!The provider type constants are well-organized and cover all the expected AI providers. The naming convention (
AIProvider_*) is consistent and clear.frontend/app/aipanel/ai-utils.ts (2)
540-544: LGTM!The
FilteredAIModeConfigsinterface clearly defines the structure for the filtered results. The intersection type{ mode: string } & AIModeConfigTypeproperly combines the mode key with the config properties.
546-574: LGTM!The
getFilteredAIModeConfigsfunction implements sensible filtering logic:
- Hides quick mode in builder context with premium access
- Separates Wave cloud modes from custom provider modes
- Ensures users always have modes available by showing cloud modes when no custom models exist
The fallback behavior (
showCloudModes || !hasCustomModels) is a good UX decision.frontend/app/aipanel/aimode.tsx (3)
159-177: LGTM!The "Configure Modes" button is a valuable UX addition that allows users to quickly access the AI modes configuration. The implementation correctly uses
fireAndForgetfor the async block creation and properly closes the dropdown after navigation.
23-30: LGTM!The integration with
getFilteredAIModeConfigscleanly separates the filtering logic from the component. The derivedhasBothModeTypesflag is correctly used to conditionally render section headers.
42-51: Default mode is guaranteed to exist in aiModeConfigs.The backend provides default waveai configurations containing
waveai@balanced,waveai@quick, andwaveai@deep. The code already handles the edge case wherecurrentConfigmight be undefined (line 44 check), and falls back to displaying "? Unknown" if needed (line 53). No changes required.schema/waveai.json (5)
21-32: Provider enum is well-defined.The provider enumeration covers the major AI service providers appropriately. The structure is clean and consistent with the Go backend types.
33-40: API type enum with restricted values.Good use of an enum to constrain
ai:apitypeto valid values. This aligns with the backend logic inusechat-mode.go.
52-54: Verify migration path for renamed property.The summary indicates
ai:baseurlwas renamed toai:endpoint. Ensure existing user configurations usingai:baseurlare handled gracefully (via migration or fallback) to avoid breaking changes.
64-69: Azure-specific fields properly scoped.The addition of
ai:azureresourcenameandai:azuredeploymentsupports Azure OpenAI configurations cleanly separated from the generic endpoint.
90-92: Reduced required fields to justdisplay:name.This relaxation allows more flexible configurations where provider-specific defaults can fill in missing values. Verify this aligns with the
applyProviderDefaultslogic in the backend.frontend/app/view/waveconfig/secretscontent.tsx (3)
14-28: ErrorDisplay component is clean and reusable.Good use of a memoized component with variant support for error/warning states.
59-76: CLI info bubble is a nice UX touch.Providing CLI command hints helps users discover alternative workflows.
313-390: Main SecretsContent component is well-structured.The conditional rendering flow is logical: error → loading → adding → selected → empty → list. Good separation of concerns.
pkg/aiusechat/usechat-mode.go (4)
74-77: Verify the Azure API version is intentional.The hardcoded version
"2025-04-01-preview"is a preview version. Ensure this is the intended default and consider documenting or making it configurable if Azure's preview API changes.
42-51: Provider defaults logic is clean.Environment variable override for the Wave endpoint is a good pattern for development/testing flexibility.
115-122: API type selection based on model is a sensible default.The logic to default to
openai-responsesfor newer models andopenai-chatfor legacy models is appropriate.
16-40: Mode resolution with fallback to balanced mode.Good fallback behavior when no mode is specified, reading from config first then defaulting to
AIModeBalanced.frontend/app/view/waveconfig/waveconfig-model.ts (4)
32-32: SECRET_NAME_REGEX is properly exported.This allows sharing the validation pattern with UI components.
260-275: Secrets loading integrated cleanly into loadFile.Early return for secrets files with appropriate state initialization before calling
checkStorageBackendandrefreshSecrets.
510-547: addNewSecret has proper validation flow.Checks for empty name, regex validation, and duplicate detection before attempting to save. Good defensive coding.
386-400: This method correctly handles cross-platform scenarios.
GetSecretsLinuxStorageBackendCommandis intentionally Linux-specific because only Linux has multiple secret storage backend options (Secret Service, basic_text, unknown). On macOS and Windows, the backend safely returns an empty string (line 283-285 inpkg/secretstore/secretstore.go), and the frontend's condition check (if (backend === "basic_text" || backend === "unknown")) correctly ignores this, setting the error to null. macOS and Windows use single native backends (Keychain and Credential Manager respectively) that don't require this check. The implementation is correct across all platforms.pkg/wconfig/settingsconfig.go (4)
79-81: WaveAiShowCloudModes usesomitemptywhich is appropriate.Both new settings use
omitemptyso they won't clutter config files with false/empty defaults.
264-284: AIModeConfigType is well-structured.The struct properly mirrors the JSON schema with appropriate jsonschema tags for enum constraints. Fields are clearly organized by purpose (display, AI config, Azure-specific, Wave-specific).
326-326: SshPasswordSecretName enables secret-backed SSH authentication.This is a good security enhancement allowing passwords to be stored securely in the secrets manager rather than in plain config files.
35-35: Deprecation comment is helpful.The comment clarifies that
AiSettingsTypeis the old/deprecated structure whileAIModeConfigTypeis the new one.
| #### Phase 8: Raw JSON Editor | ||
| - "Edit Raw JSON" button in mode editor (all modes) | ||
| - Modal with Monaco editor for single mode | ||
| - JSON validation before save: | ||
| - Syntax check with error highlighting | ||
| - Required fields check (`display:name`, `ai:apitype`, `ai:model`) | ||
| - Enum validation (provider, apitype, thinkinglevel, capabilities) | ||
| - Display specific error messages per validation failure | ||
| - Parse validated JSON and update mode in main JSON | ||
| - Useful for edge cases (modes without provider) and power users | ||
|
|
||
| #### Phase 9: Drag & Drop Reordering | ||
| - Add drag handle icon to custom mode list items | ||
| - Implement drag & drop functionality: | ||
| - Visual feedback during drag (opacity, cursor) | ||
| - Drop target highlighting | ||
| - Smooth reordering animation | ||
| - On drop: | ||
| - Recalculate `display:order` for all affected modes | ||
| - Use spacing (0, 10, 20, 30...) for easy manual adjustment | ||
| - Update JSON with new order values | ||
| - Built-in modes always stay at top (negative order values) | ||
|
|
||
| #### Phase 10: Polish & UX Refinements | ||
| - Field validation with inline error messages | ||
| - Empty state when no mode selected | ||
| - Icon picker dropdown (Font Awesome icons) | ||
| - Capabilities checkboxes with descriptions | ||
| - Thinking level dropdown with explanations | ||
| - Help tooltips throughout | ||
| - Keyboard shortcuts (e.g., Ctrl/Cmd+E for raw JSON) | ||
| - Loading states for secret checks | ||
| - Smooth transitions and animations | ||
|
|
||
| #### Phase 8: Raw JSON Editor | ||
| - "Edit Raw JSON" button in mode editor | ||
| - Modal with Monaco editor for single mode | ||
| - JSON validation before save: | ||
| - Syntax check | ||
| - Required fields check | ||
| - Enum validation | ||
| - Display specific error messages | ||
| - Parse and update mode in main JSON | ||
|
|
||
| #### Phase 9: Drag & Drop Reordering | ||
| - Make mode list items draggable (custom modes only) | ||
| - Visual feedback during drag (drag handle icon) | ||
| - Drop target highlighting | ||
| - On drop: | ||
| - Calculate new `display:order` values | ||
| - Maintain spacing between modes | ||
| - Update all affected modes in JSON | ||
| - Preserve built-in modes at top | ||
|
|
||
| #### Phase 10: Polish & UX Refinements | ||
| - Field validation (required, format) | ||
| - Error messages inline | ||
| - Empty state when no mode selected | ||
| - Icon picker dropdown | ||
| - Capabilities checkboxes | ||
| - Thinking level dropdown | ||
| - Help tooltips throughout | ||
| - Keyboard shortcuts (e.g., Cmd+E for raw JSON) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove duplicate implementation phases.
Phases 8, 9, and 10 are duplicated in the document. Lines 603-634 contain the detailed versions, while lines 637-665 contain abbreviated duplicates. Remove the duplicate section to avoid confusion.
The following sections appear twice:
- "Phase 8: Raw JSON Editor" (lines 603 and 637)
- "Phase 9: Drag & Drop Reordering" (lines 614 and 647)
- "Phase 10: Polish & UX Refinements" (lines 626 and 657)
Remove lines 637-665 as they are redundant.
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
637-637: Multiple headings with the same content
(MD024, no-duplicate-heading)
647-647: Multiple headings with the same content
(MD024, no-duplicate-heading)
657-657: Multiple headings with the same content
(MD024, no-duplicate-heading)
🤖 Prompt for AI Agents
In aiprompts/aimodesconfig.md around lines 603-665, the Phase 8/9/10 sections
are duplicated: the detailed versions are at ~603-634 and an abbreviated repeat
at ~637-665; remove the redundant duplicate block (lines 637-665) so only the
detailed Phase 8, 9, and 10 descriptions remain, ensuring the surrounding
numbering/spacing stays consistent and no other content is altered.
| try { | ||
| await RpcApi.SetSecretsCommand(TabRpcClient, { [selectedSecret]: null }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# Search for SetSecretsCommand or SetSecrets handling in backend (Go code)
rg -n "SetSecrets" --type go -C 5Repository: wavetermdev/waveterm
Length of output: 4880
🏁 Script executed:
# Also search for RPC command definitions
rg -n "SetSecretsCommand" -C 5Repository: wavetermdev/waveterm
Length of output: 12945
🏁 Script executed:
# Look for any API documentation or comments about the secrets API
rg -n "secrets.*null|null.*secret|delete.*secret" --type-list | head -20
rg -n "delete.*secret" --type go --type ts -C 3Repository: wavetermdev/waveterm
Length of output: 2540
🏁 Script executed:
# Check the full context of SetSecretsCommand type definition in frontend
cat -n frontend/app/store/wshclientapi.ts | sed -n '525,535p'Repository: wavetermdev/waveterm
Length of output: 579
🏁 Script executed:
# Look for the actual RPC API definition that gets called
rg -n "SetSecretsCommand.*null" frontendRepository: wavetermdev/waveterm
Length of output: 343
Null-value deletion is the expected API contract, but the TypeScript type definition needs updating.
The backend implementation (pkg/wshrpc/wshserver/wshserver.go:1413) explicitly handles nil values by calling secretstore.DeleteSecret(), and the CLI tool uses the same pattern. However, the frontend type definition at frontend/app/store/wshclientapi.ts:531 incorrectly types the data parameter as {[key: string]: string} when it should be {[key: string]: string | null} to match the backend's map[string]*string and the actual usage at lines 486 and 179.
🤖 Prompt for AI Agents
In frontend/app/view/waveconfig/waveconfig-model.ts around lines 485-486, the
code passes null to RpcApi.SetSecretsCommand but the request type in
frontend/app/store/wshclientapi.ts (around line 531) types the data as {[key:
string]: string}; update that signature to {[key: string]: string | null} (and
any related interfaces/usages) so the TypeScript type matches the backend
map[string]*string contract and allows null for deletions, then fix any
downstream call sites and run TypeScript build/tests to ensure no further type
errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (1)
pkg/aiusechat/usechat-mode.go (1)
134-148: Duplicate "gpt-6" in newPrefixes slice.The prefix
"gpt-6"appears twice in the slice, which is redundant:- newPrefixes := []string{"gpt-6", "gpt-5", "gpt-4.1", "gpt-6", "o1", "o3"} + newPrefixes := []string{"gpt-6", "gpt-5", "gpt-4.1", "o1", "o3"}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
cmd/testai/main-testai.go(2 hunks)docs/docs/waveai-modes.mdx(1 hunks)frontend/types/gotypes.d.ts(3 hunks)pkg/aiusechat/anthropic/anthropic-convertmessage.go(1 hunks)pkg/aiusechat/openai/openai-backend.go(1 hunks)pkg/aiusechat/openai/openai-convertmessage.go(2 hunks)pkg/aiusechat/openaichat/openaichat-convertmessage.go(3 hunks)pkg/aiusechat/uctypes/uctypes.go(4 hunks)pkg/aiusechat/usechat-mode.go(2 hunks)pkg/aiusechat/usechat.go(2 hunks)pkg/wconfig/defaultconfig/waveai.json(1 hunks)pkg/wconfig/settingsconfig.go(4 hunks)schema/waveai.json(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/docs/waveai-modes.mdx
- schema/waveai.json
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-15T03:21:02.229Z
Learnt from: sawka
Repo: wavetermdev/waveterm PR: 2433
File: pkg/aiusechat/tools_readfile.go:197-197
Timestamp: 2025-10-15T03:21:02.229Z
Learning: In Wave Terminal's AI tool definitions (pkg/aiusechat/tools_*.go), the Description field should not mention approval requirements even when ToolApproval returns ApprovalNeedsApproval. This prevents the LLM from asking users for approval before calling the tool, avoiding redundant double-approval prompts since the runtime will enforce approval anyway.
Applied to files:
pkg/aiusechat/openaichat/openaichat-convertmessage.gopkg/aiusechat/usechat-mode.gopkg/aiusechat/usechat.gopkg/wconfig/defaultconfig/waveai.json
🧬 Code graph analysis (3)
pkg/aiusechat/openai/openai-convertmessage.go (2)
pkg/aiusechat/uctypes/uctypes.go (2)
AIProvider_Wave(28-28)APIType_OpenAIResponses(23-23)cmd/server/main-server.go (1)
WaveVersion(50-50)
pkg/aiusechat/openaichat/openaichat-convertmessage.go (1)
pkg/aiusechat/uctypes/uctypes.go (4)
AIProvider_AzureLegacy(33-33)AIProvider_Azure(32-32)AIProvider_Wave(28-28)APIType_OpenAIChat(24-24)
pkg/aiusechat/usechat.go (1)
pkg/aiusechat/uctypes/uctypes.go (2)
ThinkingLevelMedium(156-156)AIOptsType(270-284)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build for TestDriver.ai
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (15)
cmd/testai/main-testai.go (1)
169-169: Field rename fromBaseURLtoEndpointlooks correct.The test configuration correctly uses the new
Endpointfield with appropriate hardcoded URLs for OpenAI and OpenRouter APIs. This aligns with the broader refactoring in the PR.Also applies to: 219-219
pkg/aiusechat/usechat.go (2)
88-112: Endpoint and thinking level defaults look correct.The endpoint resolution now properly requires an explicit
config.Endpointvalue, and the error message clearly references the config key. The thinking level default toThinkingLevelMediumprovides a sensible fallback.
389-394: Premium request counting correctly scopes to Wave proxy premium models.The nested condition ensures
PremiumReqCountincrements only when bothIsWaveProxy()andIsPremiumModel()are true. TheIsPremiumModel()method onAIOptsTypedirectly returns theWaveAIPremiumfield, so the counting logic properly reflects the premium model configuration.pkg/wconfig/defaultconfig/waveai.json (1)
1-37: Provider-based configuration looks well-structured.The addition of
"ai:provider": "wave"to all modes is consistent with the new provider-based architecture. The removal of redundantwaveai:cloud(now implied by provider) and keepingwaveai:premiumonly on balanced/deep modes makes sense given quick uses the non-premiumgpt-5-minimodel.frontend/types/gotypes.d.ts (3)
16-35: AIModeConfigType updates align with backend schema changes.The new optional fields (
ai:provider,ai:endpoint,ai:azureresourcename,ai:azuredeployment) and the fields made optional correctly reflect the Go type changes for the provider-based configuration system.
611-611: SSH password secret support added correctly.The new
ssh:passwordsecretnamefield enables secure SSH password storage via the secrets system.
1069-1070: Wave AI settings additions look correct.The new
waveai:showcloudmodesandwaveai:defaultmodesettings properly expose UI configuration options for the AI mode system.pkg/aiusechat/usechat-mode.go (3)
16-24: Default mode resolution logic is clean.The fallback chain (requested mode → config default → balanced) provides sensible defaults while allowing user customization.
42-112: Provider defaults implementation is comprehensive.The
applyProviderDefaultsfunction properly handles all supported providers with appropriate endpoint construction, API type inference, and secret name defaults. The Azure endpoint construction correctly validates resource names before building URLs.
163-169: Azure resource name validation looks correct.The regex
^[a-z0-9]([a-z0-9-]*[a-z0-9])?$properly validates Azure resource naming conventions (lowercase alphanumeric, optional hyphens in middle, max 63 chars).pkg/aiusechat/uctypes/uctypes.go (1)
14-16: LGTM! Simplified provider detection logic.The migration from URL-based detection to explicit provider fields makes the code clearer and more maintainable. The simplified
IsWaveProxy()andIsPremiumModel()methods are easier to understand and less error-prone than parsing URLs or matching model strings.Also applies to: 27-35, 191-191, 271-271, 276-276, 283-283, 286-292
pkg/wconfig/settingsconfig.go (1)
35-53: LGTM! Well-structured configuration schema updates.The new configuration fields are properly typed with appropriate JSON schema enums and omitempty tags. The deprecation comment on
AiSettingsTypeclearly communicates the legacy nature of that structure. The addition ofSshPasswordSecretNamealigns well with the PR's secrets management objectives.Also applies to: 79-81, 264-283, 325-325
pkg/aiusechat/openai/openai-backend.go (1)
523-523: LGTM! Consistent with the Endpoint field migration.The error sanitization logic correctly uses the new
Endpointfield.pkg/aiusechat/openaichat/openaichat-convertmessage.go (1)
65-71: LGTM! Well-implemented provider-specific logic.The conditional model requirement for Azure-legacy and the provider-specific header logic are correctly implemented. Good practices:
- Error messages use consistent config key format (
ai:model,ai:endpoint)- Azure providers correctly use
api-keyheader instead ofAuthorization: Bearer- Wave-specific headers are properly gated to only be sent when using the Wave provider, which is good for security and correctness
Also applies to: 100-104, 130-130, 138-157
pkg/aiusechat/openai/openai-convertmessage.go (1)
294-305: LGTM! Proper Wave header gating.Wave-specific headers are correctly gated to only be sent when using the Wave provider. This prevents unnecessary header leakage to third-party providers.
| if opts.APIToken != "" { | ||
| req.Header.Set("Authorization", "Bearer "+opts.APIToken) | ||
| } | ||
| req.Header.Set("Authorization", "Bearer "+opts.APIToken) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Authorization header set even when token is empty.
The Authorization header is now always set, even if opts.APIToken is empty. This results in Authorization: Bearer with an empty token. While this might work for some providers, it could cause authentication errors with others.
Consider conditionally setting the header only when the token is non-empty:
-req.Header.Set("Authorization", "Bearer "+opts.APIToken)
+if opts.APIToken != "" {
+ req.Header.Set("Authorization", "Bearer "+opts.APIToken)
+}📝 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.
| req.Header.Set("Authorization", "Bearer "+opts.APIToken) | |
| if opts.APIToken != "" { | |
| req.Header.Set("Authorization", "Bearer "+opts.APIToken) | |
| } |
🤖 Prompt for AI Agents
In pkg/aiusechat/openai/openai-convertmessage.go around line 291, the
Authorization header is always set to "Bearer "+opts.APIToken even when
opts.APIToken is empty; change the code to only add the Authorization header
when opts.APIToken is a non-empty string (e.g., check if opts.APIToken != ""
before calling req.Header.Set) so requests without a token do not send an empty
"Bearer " value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (5)
frontend/app/aipanel/aimode.tsx (4)
16-31: Drop or useshouldShowCloudModesto avoid dead codeYou destructure
shouldShowCloudModesfromgetFilteredAIModeConfigsbut never use it. This will likely trip lint and makes the API look more important than it is here. Either:
- remove it from the destructuring, or
- actually gate some UI (e.g., the cloud section) on
shouldShowCloudModesif that flag is semantically meaningful.
42-50: Revisit non‑premium fallback when current/default mode is premiumRight now, if
aiMode || defaultModeresolves to a premium mode andhasPremiumis false, you always coercecurrentModeto"waveai@quick". Ifwaveai@quickitself is markedwaveai:premium, a non‑premium user could end up with a selected mode they can’t actually use until they manually pick another one.Consider instead:
- falling back to a known non‑premium mode (e.g.,
"waveai@balanced") when!hasPremium && currentConfig["waveai:premium"], or- using the new
defaultModeonly when it’s allowed for the current entitlement tier.This keeps the “current mode” always selectable for non‑premium users.
58-71: Minor: simplify staticclassNameon label
<span className={text-[11px]}>is now syntactically correct, but since the class is static you can simplify it to a plain string:- <span className={`text-[11px]`}> + <span className="text-[11px]">Pure readability / consistency nit.
159-177: EnsureBlockDefis in scope and consider closing dropdown before async workTwo small points in the “Configure Modes” handler:
BlockDeftype
const blockDef: BlockDef = { ... }is used without an import in this file.- If
BlockDefisn’t provided as a global ambient type, this will fail TS checking and should be imported from wherever it’s defined.- Please verify its scope and add the appropriate import if needed.
Close the dropdown immediately on click
- You currently call
setIsOpen(false)afterawait createBlock(...)inside the async closure. IfcreateBlockis slow or throws, the menu may stay open longer than necessary.- Consider closing first and then kicking off the async work:
- <button - onClick={() => { - fireAndForget(async () => { - const blockDef: BlockDef = { + <button + onClick={() => { + setIsOpen(false); + fireAndForget(async () => { + const blockDef: BlockDef = { meta: { view: "waveconfig", file: "waveai.json", }, }; - await createBlock(blockDef, false, true); - setIsOpen(false); + await createBlock(blockDef, false, true); }); }}That keeps the UI snappy and makes the block creation truly fire‑and‑forget from the user’s perspective.
frontend/app/view/waveconfig/secretscontent.tsx (1)
199-307: Textarea focus logic may re-focus on every render; consider gating itIn
SecretDetailView, the inline ref callback both storesmodel.secretValueRefand callsref.focus()whenever React invokes it. Because the callback is recreated each render, this can result in repeated focusing of the textarea, which may be slightly jarring if the component re-renders frequently.If you notice cursor jumps or unwanted focus changes, consider separating assignment from focusing and only calling
focus()when the detail view first opens (e.g., keyed onsecretNameor a dedicated “just opened” flag), rather than on every commit.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
frontend/app/aipanel/aimode.tsx(6 hunks)frontend/app/modals/userinputmodal.tsx(4 hunks)frontend/app/view/waveconfig/secretscontent.tsx(1 hunks)frontend/app/view/waveconfig/waveconfig-model.ts(11 hunks)pkg/aiusechat/usechat-mode.go(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/app/view/waveconfig/waveconfig-model.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-14T06:30:54.763Z
Learnt from: sawka
Repo: wavetermdev/waveterm PR: 2430
File: frontend/app/aipanel/aimessage.tsx:137-144
Timestamp: 2025-10-14T06:30:54.763Z
Learning: In `frontend/app/aipanel/aimessage.tsx`, the `AIToolUseGroup` component splits file operation tool calls into separate batches (`fileOpsNeedApproval` and `fileOpsNoApproval`) based on their approval state before passing them to `AIToolUseBatch`. This ensures each batch has homogeneous approval states, making group-level approval handling valid.
Applied to files:
frontend/app/aipanel/aimode.tsx
📚 Learning: 2025-10-15T03:21:02.229Z
Learnt from: sawka
Repo: wavetermdev/waveterm PR: 2433
File: pkg/aiusechat/tools_readfile.go:197-197
Timestamp: 2025-10-15T03:21:02.229Z
Learning: In Wave Terminal's AI tool definitions (pkg/aiusechat/tools_*.go), the Description field should not mention approval requirements even when ToolApproval returns ApprovalNeedsApproval. This prevents the LLM from asking users for approval before calling the tool, avoiding redundant double-approval prompts since the runtime will enforce approval anyway.
Applied to files:
pkg/aiusechat/usechat-mode.go
🧬 Code graph analysis (1)
frontend/app/view/waveconfig/secretscontent.tsx (1)
frontend/app/view/waveconfig/waveconfig-model.ts (2)
SecretNameRegex(32-32)WaveConfigViewModel(133-561)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
- GitHub Check: Build for TestDriver.ai
🔇 Additional comments (17)
frontend/app/modals/userinputmodal.tsx (4)
70-70: LGTM! Explicit return values improve clarity.The explicit
return trueafter handling Escape andreturn falsefor unhandled keys makes the event handling contract clear and follows best practices.Also applies to: 76-76
83-83: LGTM! Styling simplified.The removal of component-specific classes aligns with the Tailwind migration. The parent container styling (line 159) should handle the layout requirements.
Also applies to: 85-85
110-111: LGTM! Checkbox layout improved with Tailwind.The flexbox layout with proper spacing and cursor styling improves both visual consistency and user experience. The label-input association is correctly maintained.
Also applies to: 115-115, 118-118
151-151: LGTM! Modal structure cleaned up with Tailwind.The padding, spacing, and layout utilities create a well-structured modal with consistent spacing and appropriate width constraints.
Also applies to: 158-159
pkg/aiusechat/usechat-mode.go (6)
8-9: LGTM! Clean imports and Azure validation setup.The new imports support environment variable handling, Azure resource name validation, and model prefix checking. The Azure resource name regex correctly enforces the naming constraints.
Also applies to: 11-11, 16-16
20-26: LGTM! Clean default mode resolution.The fallback chain (requested → configured default → "balanced") is intuitive and aligns well with the new configuration system.
44-114: Well-structured provider defaults with good validation.The function cleanly handles provider-specific configurations with appropriate validation checks. The environment variable override for Wave (line 49-51) and the resource name validation for Azure endpoints (lines 80, 98) are solid defensive practices.
Note: Line 93 mentions the Azure API version "v1" is "purely informational for now" — verify whether this needs to be configurable or validated in the future.
116-134: LGTM! Intentional API type resolution strategy.The difference in default behavior is sound: OpenAI defaults unknown models to the newer responses API (progressive), while Azure defaults to the chat API (conservative). This accounts for Azure potentially lagging in API adoption.
136-170: LGTM! Solid model classification and validation logic.The model prefix checks correctly distinguish between legacy and new OpenAI models, with proper handling of empty strings and sub-versions (lines 146-148). The Azure resource name validation enforces correct length (≤63) and format constraints.
179-179: LGTM! Clean integration of provider defaults.Applying provider defaults before returning ensures consistent configuration across all AI modes.
frontend/app/aipanel/aimode.tsx (3)
4-8: Imports match downstream usageThe added imports (
createBlock,getSettingsKeyAtom,fireAndForget,getFilteredAIModeConfigs) are all used below and there are no obvious unused or missing symbols in this header block.
79-116: Wave AI Cloud group rendering looks consistentThe “Wave AI Cloud” header and
waveProviderConfigsmapping correctly:
- gate the header on
hasBothModeTypes,- adjust top padding for the first item when there’s no header,
- respect
hasPremiumwhen disabling premium modes,- and render optional
display:descriptionnicely with pre‑line whitespace.No functional issues stand out here.
117-158: Custom provider section logic and spacing are soundThe “Custom” header and
otherProviderConfigsmapping mirror the cloud section cleanly, including:
- first/last item padding,
- disabled state and premium labeling,
- selected checkmark, and
- optional description block.
The grouping and UX behavior look coherent.
frontend/app/view/waveconfig/secretscontent.tsx (4)
9-40: Error and loading components look clean and reusable
ErrorDisplayandLoadingSpinnerare small, memoized, and self-contained, with clear props and variants; nothing concerning here.
41-77: Empty/CLI helper states are well-structured
EmptyStateandCLIInfoBubbleprovide clear affordances and guidance, with good visual hierarchy and straightforward props; implementation looks solid.
115-198: Add secret form validation and disabling behavior look correctThe
AddSecretFormcorrectly mirrorsSecretNameRegex, surfaces a clear validation message, and ties button disabled states toisLoading, invalid names, and empty trimmed names. This should prevent most bad submissions at the UI level and aligns with the ViewModel validation.
312-389: Overall secrets flow wiring looks coherent; verify loading spinner condition matches intentThe main
SecretsContentcomponent cleanly orchestrates the modes (backend error, initial loading, add-new, detail, empty, list) using the ViewModel atoms, and centralizes generic vs storage-backend errors viaerrorMessageandstorageBackendError.One thing to double-check: the top-level loading spinner only shows when
isLoading && secretNames.length === 0 && !selectedSecret. That means you won’t see the “Loading secrets...” overlay when reloading while a list already exists; instead, you’ll rely on the more granular spinners in the detail/add views. If that’s the intended UX, this is fine; otherwise, you may want to loosen that condition.
| interface SecretListViewProps { | ||
| secretNames: string[]; | ||
| onSelectSecret: (name: string) => void; | ||
| onAddSecret: () => void; | ||
| } | ||
|
|
||
| const SecretListView = memo(({ secretNames, onSelectSecret, onAddSecret }: SecretListViewProps) => { | ||
| return ( | ||
| <div className="flex flex-col h-full w-full bg-gray-800/50 rounded-lg"> | ||
| <div className="flex flex-col divide-y divide-gray-700"> | ||
| {secretNames.map((name) => ( | ||
| <div | ||
| key={name} | ||
| className={cn("flex items-center gap-3 p-4 hover:bg-gray-700/50 cursor-pointer transition-colors")} | ||
| onClick={() => onSelectSecret(name)} | ||
| > | ||
| <i className="fa-sharp fa-solid fa-key text-accent-500" /> | ||
| <span className="flex-1 font-mono">{name}</span> | ||
| <i className="fa-sharp fa-solid fa-chevron-right text-gray-500 text-sm" /> | ||
| </div> | ||
| ))} | ||
| <div | ||
| className={cn( | ||
| "flex items-center justify-center gap-2 p-4 hover:bg-gray-700/50 cursor-pointer transition-colors border-t-2 border-gray-600" | ||
| )} | ||
| onClick={onAddSecret} | ||
| > | ||
| <i className="fa-sharp fa-solid fa-plus text-accent-500" /> | ||
| <span className="font-medium text-accent-500">Add New Secret</span> | ||
| </div> | ||
| </div> | ||
| <CLIInfoBubble /> | ||
| </div> | ||
| ); | ||
| }); | ||
| SecretListView.displayName = "SecretListView"; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make secret list rows keyboard-accessible (use <button> instead of clickable <div>s)
The secret rows and “Add New Secret” entry are clickable <div>s with onClick but no keyboard or focus handling. This prevents keyboard-only users and screen readers from reliably selecting secrets or adding a new one, which is an accessibility blocker.
Refactor these to actual <button> elements (or links) so they are tabbable and activatable via keyboard:
- {secretNames.map((name) => (
- <div
- key={name}
- className={cn("flex items-center gap-3 p-4 hover:bg-gray-700/50 cursor-pointer transition-colors")}
- onClick={() => onSelectSecret(name)}
- >
+ {secretNames.map((name) => (
+ <button
+ key={name}
+ type="button"
+ className={cn(
+ "flex w-full items-center gap-3 p-4 hover:bg-gray-700/50 cursor-pointer transition-colors text-left"
+ )}
+ onClick={() => onSelectSecret(name)}
+ >
<i className="fa-sharp fa-solid fa-key text-accent-500" />
<span className="flex-1 font-mono">{name}</span>
<i className="fa-sharp fa-solid fa-chevron-right text-gray-500 text-sm" />
- </div>
+ </button>
))}
- <div
+ <button
className={cn(
- "flex items-center justify-center gap-2 p-4 hover:bg-gray-700/50 cursor-pointer transition-colors border-t-2 border-gray-600"
+ "flex w-full items-center justify-center gap-2 p-4 hover:bg-gray-700/50 cursor-pointer transition-colors border-t-2 border-gray-600"
)}
+ type="button"
onClick={onAddSecret}
>
<i className="fa-sharp fa-solid fa-plus text-accent-500" />
<span className="font-medium text-accent-500">Add New Secret</span>
- </div>
+ </button>This preserves the visual design while fixing keyboard and a11y behavior.
📝 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.
| interface SecretListViewProps { | |
| secretNames: string[]; | |
| onSelectSecret: (name: string) => void; | |
| onAddSecret: () => void; | |
| } | |
| const SecretListView = memo(({ secretNames, onSelectSecret, onAddSecret }: SecretListViewProps) => { | |
| return ( | |
| <div className="flex flex-col h-full w-full bg-gray-800/50 rounded-lg"> | |
| <div className="flex flex-col divide-y divide-gray-700"> | |
| {secretNames.map((name) => ( | |
| <div | |
| key={name} | |
| className={cn("flex items-center gap-3 p-4 hover:bg-gray-700/50 cursor-pointer transition-colors")} | |
| onClick={() => onSelectSecret(name)} | |
| > | |
| <i className="fa-sharp fa-solid fa-key text-accent-500" /> | |
| <span className="flex-1 font-mono">{name}</span> | |
| <i className="fa-sharp fa-solid fa-chevron-right text-gray-500 text-sm" /> | |
| </div> | |
| ))} | |
| <div | |
| className={cn( | |
| "flex items-center justify-center gap-2 p-4 hover:bg-gray-700/50 cursor-pointer transition-colors border-t-2 border-gray-600" | |
| )} | |
| onClick={onAddSecret} | |
| > | |
| <i className="fa-sharp fa-solid fa-plus text-accent-500" /> | |
| <span className="font-medium text-accent-500">Add New Secret</span> | |
| </div> | |
| </div> | |
| <CLIInfoBubble /> | |
| </div> | |
| ); | |
| }); | |
| SecretListView.displayName = "SecretListView"; | |
| interface SecretListViewProps { | |
| secretNames: string[]; | |
| onSelectSecret: (name: string) => void; | |
| onAddSecret: () => void; | |
| } | |
| const SecretListView = memo(({ secretNames, onSelectSecret, onAddSecret }: SecretListViewProps) => { | |
| return ( | |
| <div className="flex flex-col h-full w-full bg-gray-800/50 rounded-lg"> | |
| <div className="flex flex-col divide-y divide-gray-700"> | |
| {secretNames.map((name) => ( | |
| <button | |
| key={name} | |
| type="button" | |
| className={cn( | |
| "flex w-full items-center gap-3 p-4 hover:bg-gray-700/50 cursor-pointer transition-colors text-left" | |
| )} | |
| onClick={() => onSelectSecret(name)} | |
| > | |
| <i className="fa-sharp fa-solid fa-key text-accent-500" /> | |
| <span className="flex-1 font-mono">{name}</span> | |
| <i className="fa-sharp fa-solid fa-chevron-right text-gray-500 text-sm" /> | |
| </button> | |
| ))} | |
| <button | |
| type="button" | |
| className={cn( | |
| "flex w-full items-center justify-center gap-2 p-4 hover:bg-gray-700/50 cursor-pointer transition-colors border-t-2 border-gray-600" | |
| )} | |
| onClick={onAddSecret} | |
| > | |
| <i className="fa-sharp fa-solid fa-plus text-accent-500" /> | |
| <span className="font-medium text-accent-500">Add New Secret</span> | |
| </button> | |
| </div> | |
| <CLIInfoBubble /> | |
| </div> | |
| ); | |
| }); | |
| SecretListView.displayName = "SecretListView"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/app/modals/userinputmodal.tsx (1)
122-122: Add missing dependencies to useMemo.The
optionalCheckboxuseMemo referencesuserInputRequest.checkboxmsg(lines 106, 118) anduserInputRequest.requestid(line 114) but has an empty dependency array. This is inconsistent with other hooks in the file (see lines 64, 78, 103) and could lead to stale values.- }, []); + }, [userInputRequest.checkboxmsg, userInputRequest.requestid]);
🧹 Nitpick comments (4)
frontend/app/modals/userinputmodal.tsx (1)
98-98: Remove unnecessaryresize-noneclass.The
resize-noneclass only applies to<textarea>elements. Since this is an<input>element, the class has no effect and can be removed.- className="resize-none bg-panel rounded-md border border-border py-1.5 pl-4 min-h-[30px] text-inherit cursor-text focus:ring-2 focus:ring-accent focus:outline-none" + className="bg-panel rounded-md border border-border py-1.5 pl-4 min-h-[30px] text-inherit cursor-text focus:ring-2 focus:ring-accent focus:outline-none"frontend/app/aipanel/aimode.tsx (3)
42-51: Consider making fallback mode selection more data‑drivenThe premium/builder fallbacks:
let currentMode = aiMode || defaultMode; // ... if (!hasPremium && currentConfig["waveai:premium"]) { currentMode = "waveai@quick"; } if (model.inBuilder && hasPremium && currentMode === "waveai@quick") { currentMode = "waveai@balanced"; }assume
"waveai@quick"and"waveai@balanced"always exist inaiModeConfigs. If those keys ever become optional or user‑configurable, this could degrade into the"? Unknown"display fallback.If you expect more config flexibility long‑term, consider deriving the non‑premium / builder‑preferred fallback from config metadata (e.g., a
waveai:defaultWhenNoPremiumflag or picking the first non‑premium cloud mode) instead of hard‑coding keys.
58-59: Dropdown rendering and premium gating look good; item markup could be dedupedThe dropdown UI behaves consistently:
- Top button uses
displayConfig["display:icon"] || "sparkles"anddisplayConfig["display:name"], giving sane fallbacks.waveProviderConfigsandotherProviderConfigsare rendered in clearly separated sections with the appropriate headers whenhasBothModeTypesis true.- Premium modes are both visually disabled and further guarded in
handleSelect, which is a nice defense‑in‑depth pattern.- Optional
display:descriptionrendering withwhiteSpace: "pre-line"is a good touch for multi‑line copy.The only notable nit is that the JSX for each mode item in the two
.mapcalls is almost identical, differing mainly in top/bottom padding logic:{waveProviderConfigs.map(... <button>...</button> )} ... {otherProviderConfigs.map(... <button>...</button> )}If this component continues to evolve, consider extracting a small
ModeItemcomponent (props:config,isSelected,isDisabled,isFirst,isLast,onSelect) to reduce duplication and keep styling changes in one place.Also applies to: 68-69, 78-158
159-177: VerifyBlockDeftyping and consider minor UX/cleanup tweaks for “Configure Modes”The “Configure Modes” button logic is clear and matches the config story:
fireAndForget(async () => { const blockDef: BlockDef = { meta: { view: "waveconfig", file: "waveai.json" }, }; await createBlock(blockDef, false, true); setIsOpen(false); });Two small follow‑ups to consider:
BlockDeftype availability
Make sureBlockDefis declared in a global.d.tsor imported locally; otherwise this will fail type‑checking and may need:import type { BlockDef } from "...";State update timing / UX
IfcreateBlockcan be slow or the dropdown can unmount (e.g., route change) while it runs, callingsetIsOpen(false)after theawaitcan lead to a “setState on unmounted component” warning and a slightly laggy close. Closing the dropdown immediately, then kicking off the async work, would avoid that:onClick={() => { setIsOpen(false); fireAndForget(async () => { const blockDef: BlockDef = { /* ... */ }; await createBlock(blockDef, false, true); }); }}Both are non‑blocking but worth a quick check.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/app/aipanel/aimode.tsx(6 hunks)frontend/app/modals/userinputmodal.tsx(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: sawka
Repo: wavetermdev/waveterm PR: 2433
File: pkg/aiusechat/tools_readfile.go:197-197
Timestamp: 2025-10-15T03:21:02.229Z
Learning: In Wave Terminal's AI tool definitions (pkg/aiusechat/tools_*.go), the Description field should not mention approval requirements even when ToolApproval returns ApprovalNeedsApproval. This prevents the LLM from asking users for approval before calling the tool, avoiding redundant double-approval prompts since the runtime will enforce approval anyway.
🧬 Code graph analysis (1)
frontend/app/modals/userinputmodal.tsx (2)
frontend/app/aipanel/waveai-model.tsx (1)
handleSubmit(388-453)frontend/app/element/markdown.tsx (1)
Markdown(510-510)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build for TestDriver.ai
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (5)
frontend/app/modals/userinputmodal.tsx (4)
70-76: LGTM!The explicit return statements improve code clarity and make the control flow more predictable for keyboard event handling.
83-85: LGTM!The removal of custom CSS classes in favor of default styling is consistent with the broader SCSS-to-Tailwind migration in this file.
110-120: LGTM!The flex-based layout and
cursor-pointerclasses provide good alignment and user experience for the checkbox and label.
151-159: LGTM!The modal structure with Tailwind utilities provides appropriate spacing and layout. The nested margin pattern (Modal
px-5+ contentmx-4) creates consistent visual hierarchy.frontend/app/aipanel/aimode.tsx (1)
4-5: Settings wiring and mode grouping viagetFilteredAIModeConfigslook solidUsing
waveai:showcloudmodes/waveai:defaultmodeatoms plusgetFilteredAIModeConfigs(aiModeConfigs, showCloudModes, model.inBuilder, hasPremium)cleanly centralizes filtering and grouping intowaveProviderConfigs/otherProviderConfigs, andhasBothModeTypeskeeps the layout logic straightforward. This looks correct and maintainable as-is.Also applies to: 8-8, 16-18, 23-31
No description provided.