AI model compatibility, other misc fixes for release #2641
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis pull request refactors the AIModeDropdown into a compatibility-aware, sectioned menu (adds AIModeDropdownProps, AIModeMenuItem, ConfigSection, computeCompatibleSections, computeWaveCloudSections), moves premium gating and selection logic into handleSelect, and replaces an inline "Configure Modes" action with a handleConfigureClick that calls a new WaveAIModel.openWaveAIConfig. It adds Tooltip wrappers to aipanel input buttons, surfaces descriptions in the Wave Config sidebar and docs links as Tooltips, updates config metadata (names/descriptions) and the ConfigFile type, updates AIModeConfigType (adds ai:switchcompat, renames ai:apiversion → ai:azureapiversion), updates default waveai.json profiles to include ai:switchcompat, and edits docs to add inline example references. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
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.
Actionable comments posted: 2
🧹 Nitpick comments (7)
frontend/app/aipanel/aipanelinput.tsx (1)
149-177: Tooltip-wrapped attach/send buttons look good; consider minor UX/a11y tweaksThe tooltip integration and absolute positioning inside the relative container look correct, and existing click behavior (attach + submit) is preserved.
Two small follow-ups to consider:
- Add explicit accessible names to the icon-only buttons so screen readers don’t rely on tooltip behavior, e.g.:
aria-label="Attach files"on the paperclip buttonaria-label="Send message"on the send button- The send button is disabled when
status !== "ready" || !input.trim(), but pressing Enter in the textarea still callsonSubmit. If you want keyboard and mouse paths to match, you could short‑circuithandleKeyDownunder the same condition.docs/docs/waveai-modes.mdx (1)
31-36: Docs now match provider enums andai:azureapiversion, but double‑check for staleai:apiversionreferencesThe updated provider list with example anchors and the switch from
ai:apiversion→ai:azureapiversionin examples/field table look aligned with the code and schema. It’s a good improvement in clarity.I’d just recommend:
- Scanning the docs for any remaining mentions of
ai:apiversionin the context ofwaveai.jsonand Azure modes.- Verifying that the documented default (
2025-04-01-previewfor Azure legacy) and override semantics match what the provider code actually does.Also applies to: 74-81, 92-103, 277-328
frontend/app/view/waveconfig/waveconfig.tsx (1)
137-149: Editor resize handling and docs tooltip are well-integrated; consider minor cleanup
- The
ResizeObserver+debounce(100, ...)aroundeditor.layout()is a solid way to keep the editor sized correctly on container changes without thrashing.- Wrapping the docs link in a
Tooltipwith “View documentation” aligns with the rest of the UI’s affordances.Optional polish:
- If you ever see layout firing after unmount, you could call the debounced function’s cancel/flush (if exposed) in the cleanup alongside
resizeObserver.disconnect()to be extra-safe, though the currenteditorRefnull-check already avoids errors.Also applies to: 176-185
schema/waveai.json (1)
33-36: Schema updates align with types, but watch backward‑compat forai:apiversionand removed enumsThe schema changes look internally consistent:
ai:apitypeenum now includes"google-gemini"and drops the older Anthropic value.ai:azureapiversionandai:switchcompatmatch the new Go/TS fields and default Wave AI configs.However, with
additionalProperties: false:
- Any existing
waveai.jsonentries that still useai:apiversionfor Azure modes (from older docs) will now fail schema validation and lose their explicit version override.- Any configs that still specify
ai:apitype: "anthropic-messages"will also become invalid.It may be worth either:
- Providing a migration path (docs, release notes, or an automatic mapping from
ai:apiversion→ai:azureapiversion), and/or- Temporarily accepting the old keys/enums in the schema as deprecated aliases to avoid hard breaks for existing users.
Also applies to: 52-54, 67-77, 78-83
frontend/app/view/waveconfig/waveconfig-model.ts (1)
96-97: Inconsistent indentation.Line 97 uses tabs while line 96 uses spaces. This creates inconsistent formatting with the rest of the file.
name: "Wave AI Modes", path: "waveai.json", language: "json", description: "Local models and BYOK", - docsUrl: "https://docs.waveterm.dev/waveai-modes", + docsUrl: "https://docs.waveterm.dev/waveai-modes", validator: validateWaveAiJson,frontend/app/aipanel/aimode.tsx (2)
11-18: Consider using a more specific type forconfig.Using
anyhere loses type safety. If there's a defined type for AI mode configurations in the codebase, consider using it.
20-49: Remove unnecessarykeyprop on the button element.The
keyprop on line 23 is unnecessary sinceAIModeMenuItemis not directly rendered in a list context at this level - the parent handles thekeywhen mapping over configs.return ( <button - key={config.mode} onClick={onClick} disabled={isDisabled}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
docs/docs/waveai-modes.mdx(1 hunks)frontend/app/aipanel/aimode.tsx(3 hunks)frontend/app/aipanel/aipanelinput.tsx(2 hunks)frontend/app/aipanel/aipanelmessages.tsx(1 hunks)frontend/app/aipanel/waveai-model.tsx(1 hunks)frontend/app/view/waveconfig/waveconfig-model.ts(4 hunks)frontend/app/view/waveconfig/waveconfig.tsx(2 hunks)frontend/types/gotypes.d.ts(1 hunks)pkg/wconfig/defaultconfig/waveai.json(3 hunks)pkg/wconfig/settingsconfig.go(1 hunks)schema/waveai.json(3 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 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.
📚 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/aipanelinput.tsxfrontend/app/aipanel/aipanelmessages.tsxfrontend/app/aipanel/aimode.tsx
📚 Learning: 2025-11-01T00:57:23.025Z
Learnt from: sawka
Repo: wavetermdev/waveterm PR: 2504
File: frontend/app/aipanel/aipanel-contextmenu.ts:15-16
Timestamp: 2025-11-01T00:57:23.025Z
Learning: In the waveterm codebase, types defined in custom.d.ts are globally available and do not require explicit imports. Backend types defined in gotypes.d.ts are also globally available.
Applied to files:
frontend/app/view/waveconfig/waveconfig-model.ts
🧬 Code graph analysis (4)
frontend/app/aipanel/aipanelinput.tsx (2)
frontend/app/element/tooltip.tsx (1)
Tooltip(143-176)frontend/util/util.ts (1)
cn(502-502)
frontend/app/aipanel/waveai-model.tsx (2)
pkg/waveobj/wtype.go (1)
BlockDef(244-247)frontend/app/store/global.ts (1)
createBlock(820-820)
frontend/app/aipanel/aipanelmessages.tsx (1)
frontend/app/aipanel/aimode.tsx (1)
AIModeDropdown(121-254)
frontend/app/view/waveconfig/waveconfig.tsx (1)
frontend/app/element/tooltip.tsx (1)
Tooltip(143-176)
⏰ 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). (5)
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Build Docsite
- GitHub Check: Build for TestDriver.ai
- GitHub Check: merge-gatekeeper
🔇 Additional comments (16)
frontend/app/aipanel/aipanelinput.tsx (1)
7-7: Tooltip import wiring looks correctThe named
Tooltipimport matches how it’s used below; no issues here.frontend/types/gotypes.d.ts (1)
17-36: AIModeConfigType additions look consistent and correctAdding
"ai:azureapiversion"?: stringand"ai:switchcompat"?: string[]matches the backend struct and JSON schema and cleanly scopes Azure-specific versioning plus compatibility tags into the mode config surface. No issues from a typing or layering standpoint.pkg/wconfig/settingsconfig.go (1)
264-284: AIModeConfigType: SwitchCompat wiring is correct
SwitchCompat []string 'json:"ai:switchcompat,omitempty"'cleanly exposes the newai:switchcompatfield and stays in sync with the TS types and JSON schema. No issues in struct definition or tags.pkg/wconfig/defaultconfig/waveai.json (1)
11-13: Default Wave AI modes:ai:switchcompatlooks correctly populatedAdding
"ai:switchcompat": ["wavecloud"]to the Quick/Balanced/Deep presets gives the compatibility logic a clear signal without altering existing behavior (ai:capabilities,waveai:premium, etc.). The JSON remains valid and consistent with the new schema.Also applies to: 23-26, 36-39
frontend/app/aipanel/aipanelmessages.tsx (1)
64-66: AIModeDropdown now correctly uses compatibility mode in the panelWiring
compatibilityMode={true}here will make the main panel’s mode selector use the new compatibility-aware sections, which matches the updated AIModeDropdown behavior. No issues with the prop usage.frontend/app/aipanel/waveai-model.tsx (1)
550-558: openWaveAIConfig is consistent with existing block creation patternsThe new
openWaveAIConfighelper correctly opens awaveconfigview forwaveai.jsonusing the sameBlockDef+createBlockpattern asopenDiff. This is a clean, reusable entry point for the “Configure Modes” action.frontend/app/view/waveconfig/waveconfig.tsx (1)
44-53: ConfigSidebar: name/description rendering is a nice UX improvementRendering the file name and optional
file.descriptionon separate, truncated lines keeps the sidebar compact while surfacing more context. Click handling and selection styling remain unchanged, so behavior stays intact.frontend/app/view/waveconfig/waveconfig-model.ts (4)
12-12: LGTM!Import added for platform detection utility.
21-32: LGTM!Good addition of the optional
descriptionfield to theConfigFiletype, enabling richer metadata for configuration entries.
86-86: LGTM!Good UX improvements - "Sidebar Widgets" and "Tab Backgrounds" are more descriptive names that better communicate the purpose of these configuration files.
Also applies to: 103-103
82-82: No issue found—isWindows()is safe to call at module initialization.The function is a pure, synchronous check (
PLATFORM == PlatformWindows) with no runtime state dependencies. ThePLATFORMvariable has a safe default value (PlatformMacOS), so callingisWindows()beforesetPlatform()is invoked will simply returnfalsewithout side effects. There is no risk here.frontend/app/aipanel/aimode.tsx (5)
104-115: LGTM!Clean helper function for computing wave cloud sections.
117-119: LGTM!Good addition of the
AIModeDropdownPropsinterface with a sensible default value forcompatibilityMode.Also applies to: 121-121
157-165: LGTM!Good consolidation of selection handling logic with proper premium gating checks.
172-177: LGTM!Proper use of
fireAndForgetfor the async operation, and the dropdown closes after opening config.
241-248: LGTM!Good refactor extracting the click handler to
handleConfigureClick.
| {sections.map((section, sectionIndex) => { | ||
| const isFirstSection = sectionIndex === 0; | ||
| const isLastSection = sectionIndex === sections.length - 1; | ||
|
|
||
| return ( | ||
| <button | ||
| key={config.mode} | ||
| onClick={() => handleSelect(config.mode)} | ||
| disabled={isDisabled} | ||
| className={`w-full flex flex-col gap-0.5 px-3 ${ | ||
| isFirst ? "pt-1 pb-0.5" : isLast ? "pt-0.5 pb-1" : "pt-0.5 pb-0.5" | ||
| } ${ | ||
| isDisabled | ||
| ? "text-gray-500 cursor-not-allowed" | ||
| : "text-gray-300 hover:bg-gray-700 cursor-pointer" | ||
| } transition-colors text-left`} | ||
| > | ||
| <div className="flex items-center gap-2 w-full"> | ||
| <i className={makeIconClass(config["display:icon"] || "sparkles", false)}></i> | ||
| <span className={`text-sm ${isSelected ? "font-bold" : ""}`}> | ||
| {config["display:name"]} | ||
| {isDisabled && " (premium)"} | ||
| </span> | ||
| {isSelected && <i className="fa fa-check ml-auto"></i>} | ||
| </div> | ||
| {config["display:description"] && ( | ||
| <div className="text-xs text-muted pl-5" style={{ whiteSpace: "pre-line" }}> | ||
| {config["display:description"]} | ||
| </div> | ||
| <div key={section.sectionName}> | ||
| {!isFirstSection && <div className="border-t border-gray-600 my-2" />} | ||
| {showSectionHeaders && ( | ||
| <> | ||
| <div className={cn("pb-1 text-center text-[10px] text-gray-400 uppercase tracking-wide", isFirstSection ? "pt-2" : "pt-0")}> | ||
| {section.sectionName} | ||
| </div> | ||
| {section.isIncompatible && ( | ||
| <div className="text-center text-[11px] text-red-300 pb-1"> | ||
| (Start a New Chat to Switch) | ||
| </div> | ||
| )} | ||
| </> | ||
| )} | ||
| </button> | ||
| {section.configs.map((config, index) => { | ||
| const isFirst = index === 0 && isFirstSection && !showSectionHeaders; | ||
| const isLast = index === section.configs.length - 1 && isLastSection; | ||
| const isPremiumDisabled = !hasPremium && config["waveai:premium"]; | ||
| const isIncompatibleDisabled = section.isIncompatible || false; | ||
| const isDisabled = isPremiumDisabled || isIncompatibleDisabled; | ||
| const isSelected = currentMode === config.mode; | ||
| return ( | ||
| <AIModeMenuItem | ||
| key={config.mode} | ||
| config={config} | ||
| isSelected={isSelected} | ||
| isDisabled={isDisabled} | ||
| onClick={() => handleSelect(config.mode)} | ||
| isFirst={isFirst} | ||
| isLast={isLast} | ||
| /> | ||
| ); | ||
| })} | ||
| </div> | ||
| ); | ||
| })} |
There was a problem hiding this comment.
Disabled buttons still trigger onClick handler.
On lines 223-224, even when isDisabled is true, clicking the button will call handleSelect. While handleSelect has a premium check, it doesn't check for incompatibility. Consider returning early in handleSelect or preventing the click entirely for disabled items.
const handleSelect = (mode: string) => {
const config = aiModeConfigs[mode];
if (!config) return;
if (!hasPremium && config["waveai:premium"]) {
return;
}
+ // Don't allow selection in compatibility mode if mode is incompatible
+ // This is handled by button disabled state, but adding defensive check
model.setAIMode(mode);
setIsOpen(false);
};Alternatively, pass the disabled state to the click handler:
- onClick={() => handleSelect(config.mode)}
+ onClick={() => !isDisabled && handleSelect(config.mode)}📝 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.
| {sections.map((section, sectionIndex) => { | |
| const isFirstSection = sectionIndex === 0; | |
| const isLastSection = sectionIndex === sections.length - 1; | |
| return ( | |
| <button | |
| key={config.mode} | |
| onClick={() => handleSelect(config.mode)} | |
| disabled={isDisabled} | |
| className={`w-full flex flex-col gap-0.5 px-3 ${ | |
| isFirst ? "pt-1 pb-0.5" : isLast ? "pt-0.5 pb-1" : "pt-0.5 pb-0.5" | |
| } ${ | |
| isDisabled | |
| ? "text-gray-500 cursor-not-allowed" | |
| : "text-gray-300 hover:bg-gray-700 cursor-pointer" | |
| } transition-colors text-left`} | |
| > | |
| <div className="flex items-center gap-2 w-full"> | |
| <i className={makeIconClass(config["display:icon"] || "sparkles", false)}></i> | |
| <span className={`text-sm ${isSelected ? "font-bold" : ""}`}> | |
| {config["display:name"]} | |
| {isDisabled && " (premium)"} | |
| </span> | |
| {isSelected && <i className="fa fa-check ml-auto"></i>} | |
| </div> | |
| {config["display:description"] && ( | |
| <div className="text-xs text-muted pl-5" style={{ whiteSpace: "pre-line" }}> | |
| {config["display:description"]} | |
| </div> | |
| <div key={section.sectionName}> | |
| {!isFirstSection && <div className="border-t border-gray-600 my-2" />} | |
| {showSectionHeaders && ( | |
| <> | |
| <div className={cn("pb-1 text-center text-[10px] text-gray-400 uppercase tracking-wide", isFirstSection ? "pt-2" : "pt-0")}> | |
| {section.sectionName} | |
| </div> | |
| {section.isIncompatible && ( | |
| <div className="text-center text-[11px] text-red-300 pb-1"> | |
| (Start a New Chat to Switch) | |
| </div> | |
| )} | |
| </> | |
| )} | |
| </button> | |
| {section.configs.map((config, index) => { | |
| const isFirst = index === 0 && isFirstSection && !showSectionHeaders; | |
| const isLast = index === section.configs.length - 1 && isLastSection; | |
| const isPremiumDisabled = !hasPremium && config["waveai:premium"]; | |
| const isIncompatibleDisabled = section.isIncompatible || false; | |
| const isDisabled = isPremiumDisabled || isIncompatibleDisabled; | |
| const isSelected = currentMode === config.mode; | |
| return ( | |
| <AIModeMenuItem | |
| key={config.mode} | |
| config={config} | |
| isSelected={isSelected} | |
| isDisabled={isDisabled} | |
| onClick={() => handleSelect(config.mode)} | |
| isFirst={isFirst} | |
| isLast={isLast} | |
| /> | |
| ); | |
| })} | |
| </div> | |
| ); | |
| })} | |
| const isSelected = currentMode === config.mode; | |
| return ( | |
| <AIModeMenuItem | |
| key={config.mode} | |
| config={config} | |
| isSelected={isSelected} | |
| isDisabled={isDisabled} | |
| onClick={() => !isDisabled && handleSelect(config.mode)} | |
| isFirst={isFirst} | |
| isLast={isLast} | |
| /> | |
| ); |
🤖 Prompt for AI Agents
In frontend/app/aipanel/aimode.tsx around lines 200 to 240, disabled menu items
still trigger handleSelect because onClick is always assigned; update the code
so disabled items cannot call the handler — either (A) modify handleSelect to
accept a second parameter or inspect state and immediately return if the
selected mode is disabled (checking both premium and section incompatibility),
or (B) stop passing onClick for disabled items (pass undefined) or pass a no-op,
or pass isDisabled into the handler and have it return early; implement one of
these approaches and ensure UI feedback (no action) when isDisabled is true.
No description provided.