when the AI edits a file it now triggers a rebuild and the AI gets the tool output #2546
Conversation
…e build output returned...
WalkthroughThis pull request introduces a new restart-builder-and-wait RPC workflow and integrates AI streaming status synchronization with the UI. The changes span frontend and backend: new RPC command types and handlers are added to support restarting a builder and awaiting its build result; AI streaming state is introduced as an observable atom synced to a global store; the builder preview tab error UI is enhanced with AI integration options; the automatic app-update event trigger is removed from builder initialization; and Go backend tools are updated to pass builderId parameters for incremental build triggering. The BuilderController is extended with build result channel handling to propagate build outcomes. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20-25 minutes
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
frontend/app/aipanel/waveai-model.tsx (1)
55-55: Type and naming consistency for the new atom.For clarity, match nearby atoms:
- Add explicit type annotation.
- Consider naming it isAIStreamingAtom to signal it’s an atom.
- isAIStreaming = jotai.atom(false); + isAIStreaming: jotai.PrimitiveAtom<boolean> = jotai.atom(false);frontend/app/aipanel/aipanel.tsx (1)
257-260: Tighten effect: strict equality and complete deps.Use strict comparison and include model in deps to avoid stale closure after a singleton reset.
- useEffect(() => { - globalStore.set(model.isAIStreaming, status == "streaming"); - }, [status]); + useEffect(() => { + globalStore.set(model.isAIStreaming, status === "streaming"); + }, [status, model]);frontend/builder/tabs/builder-previewtab.tsx (2)
40-45: Clip large build logs before sending to AI.Current join of all lines can create huge prompts (cost/latency). Clip to last N lines or max chars, and indicate truncation.
- const getBuildContext = () => { - const filteredLines = outputLines.filter((line) => !line.startsWith("[debug]")); - const buildOutput = filteredLines.join("\n").trim(); - return `Build Error:\n\`\`\`\n${displayMsg}\n\`\`\`\n\nBuild Output:\n\`\`\`\n${buildOutput}\n\`\`\``; - }; + const getBuildContext = () => { + const filtered = outputLines.filter((l) => !l.startsWith("[debug]")); + const MAX_LINES = 500; + const tail = filtered.slice(Math.max(0, filtered.length - MAX_LINES)); + const buildOutput = tail.join("\n").trim(); + const note = filtered.length > tail.length ? `\n\n[truncated to last ${MAX_LINES} lines]` : ""; + return `Build Error:\n\`\`\`\n${displayMsg}\n\`\`\`\n\nBuild Output:\n\`\`\`\n${buildOutput}\n\`\`\`${note}`; + };
66-81: Guard actions on chat readiness (optional UI polish).Buttons are hidden during streaming, but handleSubmit no-ops unless status is "ready" or "error". Consider disabling buttons unless chat is actionable to avoid user confusion.
pkg/wshrpc/wshserver/wshserver.go (1)
1054-1081: Server flow looks correct; consider large BuildOutput and timeouts.
- BuildOutput can be large; consider truncating or relying solely on GetBuilderOutputCommand/events to avoid oversized RPC responses.
- Ensure ctx carries caller timeout; document expected long runtimes so clients set RpcOpts.Timeout accordingly.
pkg/buildercontroller/buildercontroller.go (1)
444-458: Consider documenting the independent build timeout behavior.The build runs with a fixed 60-second timeout (independent of the caller's context), which means:
- If the caller cancels via
ctx, this method returns immediately- But the build continues running in the background for up to 60 seconds
- This matches the
Startmethod's behavior (line 186) and prevents partial buildsWhile this design is safe (buffered channel + non-blocking sends prevent leaks), it's somewhat subtle. Consider adding a comment explaining this intentional independence.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
frontend/app/aipanel/aipanel.tsx(1 hunks)frontend/app/aipanel/waveai-model.tsx(1 hunks)frontend/app/store/wshclientapi.ts(1 hunks)frontend/builder/store/builder-apppanel-model.ts(0 hunks)frontend/builder/tabs/builder-previewtab.tsx(3 hunks)frontend/types/gotypes.d.ts(2 hunks)pkg/aiusechat/tools_builder.go(7 hunks)pkg/aiusechat/usechat.go(1 hunks)pkg/buildercontroller/buildercontroller.go(5 hunks)pkg/wshrpc/wshclient/wshclient.go(1 hunks)pkg/wshrpc/wshrpctypes.go(3 hunks)pkg/wshrpc/wshserver/wshserver.go(1 hunks)
💤 Files with no reviewable changes (1)
- frontend/builder/store/builder-apppanel-model.ts
🧰 Additional context used
🧠 Learnings (4)
📓 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-01-22T01:28:41.417Z
Learnt from: esimkowitz
Repo: wavetermdev/waveterm PR: 1790
File: pkg/remote/fileshare/wshfs/wshfs.go:122-122
Timestamp: 2025-01-22T01:28:41.417Z
Learning: The RpcClient in pkg/remote/fileshare/wshfs/wshfs.go is initialized and handled downstream by either main-server or wshcmd-connserver, as documented in the package comment.
Applied to files:
pkg/wshrpc/wshclient/wshclient.go
📚 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.gopkg/aiusechat/tools_builder.go
📚 Learning: 2025-10-21T05:09:26.916Z
Learnt from: sawka
Repo: wavetermdev/waveterm PR: 2465
File: frontend/app/onboarding/onboarding-upgrade.tsx:13-21
Timestamp: 2025-10-21T05:09:26.916Z
Learning: In the waveterm codebase, clientData is loaded and awaited in wave.ts before React runs, ensuring it is always available when components mount. This means atoms.client will have data on first render.
Applied to files:
frontend/app/aipanel/waveai-model.tsx
🧬 Code graph analysis (9)
pkg/wshrpc/wshclient/wshclient.go (3)
frontend/app/store/wshclientapi.ts (1)
RestartBuilderAndWaitCommand(476-478)pkg/wshutil/wshrpc.go (1)
WshRpc(47-61)pkg/wshrpc/wshrpctypes.go (3)
CommandRestartBuilderAndWaitData(1022-1024)RpcOpts(353-359)RestartBuilderAndWaitResult(1026-1030)
pkg/aiusechat/usechat.go (1)
pkg/aiusechat/tools_builder.go (2)
GetBuilderWriteAppFileToolDefinition(75-127)GetBuilderEditAppFileToolDefinition(151-232)
frontend/app/store/wshclientapi.ts (1)
pkg/wshrpc/wshrpctypes.go (3)
CommandRestartBuilderAndWaitData(1022-1024)RpcOpts(353-359)RestartBuilderAndWaitResult(1026-1030)
pkg/wshrpc/wshserver/wshserver.go (6)
frontend/app/store/wshclientapi.ts (1)
RestartBuilderAndWaitCommand(476-478)pkg/wshrpc/wshclient/wshclient.go (1)
RestartBuilderAndWaitCommand(572-575)pkg/wshrpc/wshrpctypes.go (2)
CommandRestartBuilderAndWaitData(1022-1024)RestartBuilderAndWaitResult(1026-1030)pkg/buildercontroller/buildercontroller.go (1)
GetOrCreateController(70-87)pkg/wstore/wstore_rtinfo.go (1)
GetRTInfo(128-138)pkg/waveobj/waveobj.go (1)
MakeORef(71-76)
pkg/aiusechat/tools_builder.go (5)
pkg/buildercontroller/buildercontroller.go (1)
GetOrCreateController(70-87)pkg/wstore/wstore_rtinfo.go (1)
GetRTInfo(128-138)pkg/waveobj/waveobj.go (1)
MakeORef(71-76)pkg/waveobj/wtype.go (1)
OType_Builder(32-32)pkg/aiusechat/uctypes/usechat-types.go (1)
ToolDefinition(80-94)
frontend/types/gotypes.d.ts (1)
pkg/wshrpc/wshrpctypes.go (2)
CommandRestartBuilderAndWaitData(1022-1024)RestartBuilderAndWaitResult(1026-1030)
pkg/wshrpc/wshrpctypes.go (2)
frontend/app/store/wshclientapi.ts (1)
RestartBuilderAndWaitCommand(476-478)pkg/wshrpc/wshclient/wshclient.go (1)
RestartBuilderAndWaitCommand(572-575)
frontend/builder/tabs/builder-previewtab.tsx (2)
frontend/app/aipanel/waveai-model.tsx (1)
WaveAIModel(43-566)frontend/builder/store/builder-buildpanel-model.ts (1)
BuilderBuildPanelModel(11-73)
pkg/buildercontroller/buildercontroller.go (4)
pkg/waveappstore/waveappstore.go (2)
ParseAppId(40-51)GetAppDir(73-80)tsunami/build/build.go (1)
GetAppName(105-108)pkg/wavebase/wavebase.go (1)
GetWaveAppElectronExecPath(129-131)pkg/utilds/multireaderlinebuffer.go (1)
MakeMultiReaderLineBuffer(17-27)
⏰ 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: Build for TestDriver.ai
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (12)
pkg/wshrpc/wshclient/wshclient.go (1)
571-575: New RPC looks correct; confirm caller sets an adequate timeout.Build/restart can run long; ensure callers pass opts.Timeout to avoid premature client-side cancellation. Consider documenting expected duration.
frontend/app/store/wshclientapi.ts (1)
475-479: LGTM; ensure callers pass a sensible timeout.Restart-and-wait may be lengthy; have UIs set opts.timeout appropriately to avoid default timeouts interrupting the build.
frontend/types/gotypes.d.ts (2)
382-386: Type addition is consistent with existing conventions.Matches generator patterns and Go json tags. No issues.
917-923: Result type matches server shape.Fields and casing align with backend.
frontend/builder/tabs/builder-previewtab.tsx (1)
52-57: Nice UX touch.“Ask AI to Fix” composes context and triggers submit; pairs well with the streaming gate.
pkg/wshrpc/wshrpctypes.go (3)
160-172: Constant additions align with existing naming.No issues spotted.
338-338: Interface extension is correct.Matches server implementation signature.
1022-1030: Data/result types look good.Field names and tags match TS definitions and server return shape.
pkg/buildercontroller/buildercontroller.go (4)
45-49: LGTM - Clean result type definition.The
BuildResultstruct clearly captures build outcomes with appropriate field types and JSON tags that follow the existing codebase conventions.
192-192: LGTM - Backward compatible enhancement.Passing
nilfor the result channel preserves the originalStartbehavior while enabling the new result-reporting capability for other code paths.
273-286: LGTM - Safe result delivery pattern.The non-blocking send with
select/defaultappropriately prevents goroutine blocking. The nil check ensures backward compatibility with existing callers.
396-416: LGTM - Comprehensive error reporting.The enhanced error handler properly captures both the error message and partial build output, providing valuable debugging context. The non-blocking send pattern is consistent with the success path.
| }) | ||
| bc.lock.Unlock() | ||
|
|
||
| time.Sleep(500 * time.Millisecond) |
There was a problem hiding this comment.
Remove or justify the hardcoded 500ms sleep.
This hardcoded delay appears to be ensuring the UI processes the reset event (line 435) before build output arrives. However, this is a problematic pattern:
- It introduces noticeable latency (500ms) on every restart
- The backend shouldn't assume specific UI processing times
- The
Startmethod has no equivalent delay, suggesting this isn't actually necessary - If there's a genuine race condition, it should be addressed with proper synchronization (e.g., acknowledgment from the event subscriber) rather than an arbitrary delay
Consider removing this sleep. If there's a specific race condition this is addressing, please document it and implement proper synchronization instead.
🤖 Prompt for AI Agents
In pkg/buildercontroller/buildercontroller.go around line 442, remove the
hardcoded time.Sleep(500 * time.Millisecond) used after emitting the reset
event; instead either eliminate the delay entirely (recommended) or replace it
with proper synchronization: emit the reset event and block on an explicit
acknowledgment from the event subscriber (e.g., a response channel, an "ack"
event on the same event bus, or a condition variable) with a sensible timeout
and logging on timeout; if you keep any waiting logic document the specific race
it addresses in a comment on that block and add tests to cover the ack/timeout
behavior so we don't rely on arbitrary sleeps.
No description provided.