Improve harness tool-call wait UX#128
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. To trigger a review, include ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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.
Code Review
This pull request introduces a non-streaming generation path via Agent.generate() and adds new observation hooks to runAgentLoop, including onStreamPart and onTextBeforeToolCall for buffering assistant text before tool boundaries. Feedback highlights that Agent.generate() incorrectly attempts to use the stopWhen option, which is unsupported by generateText, and fails to resolve functional instructions. Additionally, the getTextDelta utility and associated test mocks should be updated to use the standard textDelta property from the AI SDK to ensure compatibility.
e2c7619 to
c942605
Compare
The harness now exposes generic stream observation hooks and a non-streaming generate path so chat surfaces can acknowledge work before tools run without hard-coded domain branches. The loop keeps observer hooks isolated from stream control while documentation and regression tests cover the public API. The minimal-agent local state directory is ignored so live smoke artifacts do not leak into future diffs. Constraint: Upstream change must stay generic and package-local; no Bori/MGW/weather-specific behavior or new dependencies. Rejected: Deterministic fallback acknowledgement generation | caller-specific behavior belongs outside the upstream harness. Rejected: Leaving pre-tool hooks as private loop casts | consumers could not use the DX through exported RunAgentLoopOptions types. Confidence: high Scope-risk: moderate Directive: Keep pre-tool acknowledgements driven by model text deltas and public hooks, not package-level domain heuristics. Tested: pnpm -F @ai-sdk-tool/harness test Tested: pnpm -F @ai-sdk-tool/harness typecheck Tested: pnpm -F @ai-sdk-tool/harness build Tested: pnpm run typecheck:root Tested: pnpm exec ultracite check packages/harness Tested: LIVE_AI_BASE_URL=https://llm.vhh.sh/v1 LIVE_AI_MODEL=minpeter/gpt-5.4-mini node --conditions=@ai-sdk-tool/source --import tsx packages/minimal-agent/.minimal-agent/harness-pretool-live.ts
c942605 to
8f1ca79
Compare
The review found that the pre-tool text hook exposed raw AI SDK boundary parts too directly, including a boundary without a guaranteed toolName. This keeps the generic hook but normalizes its callback payload, narrows loop-only consumers to LoopAgent, and documents the timing/backpressure and async-instruction boundaries. Constraint: Preserve the existing model-agnostic harness direction and avoid new dependencies. Rejected: Drop tool-input-end as a boundary | it would hide a useful early flush point rather than making the contract explicit. Rejected: Keep Agent as the only loop-facing type | it would force generate() onto loop-only test doubles and custom agents. Confidence: high Scope-risk: narrow Directive: Keep onTextBeforeToolCall payload harness-normalized when AI SDK stream part shapes drift. Tested: pnpm -F @ai-sdk-tool/harness test Tested: pnpm -F @ai-sdk-tool/harness typecheck Tested: pnpm -F @ai-sdk-tool/harness build Tested: pnpm run check Tested: pnpm run typecheck Tested: pnpm run build Tested: pnpm run test Tested: final code-review APPROVE and architecture CLEAR via delegated reviewers
Summary
runAgentLoophooks for every stream part and buffered pre-tool assistant textAgent.generate()as the non-streaminggenerateTextcounterpart toAgent.stream()@ai-sdk-tool/harnesspackages/minimal-agent/.minimal-agentlocal runtime/smoke artifactsVerification
pnpm -F @ai-sdk-tool/harness testpnpm -F @ai-sdk-tool/harness typecheckpnpm -F @ai-sdk-tool/harness buildpnpm run typecheck:rootpnpm exec ultracite check packages/harnessLIVE_AI_BASE_URL=https://llm.vhh.sh/v1 LIVE_AI_MODEL=minpeter/gpt-5.4-mini node --conditions=@ai-sdk-tool/source --import tsx packages/minimal-agent/.minimal-agent/harness-pretool-live.tspreToolText:광화문 현재 날씨를 확인해볼게요.sawPreToolText=true,preToolBeforeToolExecute=true,preToolBeforeToolCallHook=trueNotes