Polish hasSuccessfulToolCall: tests, docs, and impl symmetry#263
Polish hasSuccessfulToolCall: tests, docs, and impl symmetry#263sethconvex wants to merge 1 commit into
Conversation
Follow-ups to #241 from the post-merge review. - hasSuccessfulToolCall now reads from `step.content` and matches a `tool-result` part by toolName, mirroring the pattern in willContinue. Previously it read `step.toolResults` — which already excludes errors by type, so behavior is unchanged in practice, but reading from the same source as willContinue removes the asymmetry the reviewer flagged and is more obviously correct on inspection. - Expand JSDoc on hasSuccessfulToolCall: explicitly note that tool-error parts do not match, and that the helper is evaluated only against the last step (consistent with how stopWhen is applied after each step). - Clarify the comment on the willContinue fallback: under AI SDK v6 step.content is always defined, so the optional chain and the `?? step.toolResults.length` fallback are defensive (for non-v6 / mock callers), not load-bearing for the v6 fix. - Add src/client/utils.test.ts with unit tests covering: * hasSuccessfulToolCall returns true on tool-result, false on tool-error, false on a different toolName, false on empty steps, and only inspects the last step. * willContinue does not stop when a tool-error fills in for a missing tool-result, stops when an output is genuinely missing, and stops when finishReason isn't tool-calls. - Add CHANGELOG entry under Unreleased noting the new public helper and the willContinue fix.
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository: get-convex/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ 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 |
commit: |
2 participants
Summary
Follow-ups to #241 (
hasSuccessfulToolCall/willContinuefix) from the post-merge review.Implementation
hasSuccessfulToolCallreads fromstep.content(matchingtool-resultparts bytoolName) instead ofstep.toolResults. AI SDK'sTypedToolResultalready excludes error variants by type, so the prior implementation was correct in practice — but reading from the same source aswillContinueremoves the asymmetry flagged in review and is more obviously correct on inspection.tool-errorparts do not match, and that the helper only inspects the last step (consistent with howstopWhenis applied per-step).willContinuefallback: thestep.content?.filter(...).length ?? step.toolResults.lengthpattern is defensive — under AI SDK v6step.contentis always defined (Array<ContentPart<TOOLS>>, non-optional), so the fallback is for non-v6 / mock callers, not load-bearing for the v6 bug fix.Tests (
src/client/utils.test.ts, new)hasSuccessfulToolCall:trueon atool-resultpart with matching namefalsewhen only atool-erroris present for the named toolfalsewhen no part matches the namefalseon emptystepswillContinue:tool-errorfills in for a missingtool-result(the regression fix: handle tool validation errors in stopWhen and willContinue #241 fixed)finishReasonis nottool-callsVerified: with the polished
hasSuccessfulToolCallimpl reverted to the originalstep.toolResults-based code, the first test fails — so the test file actually catches the asymmetry, not just rubber-stamps it.CHANGELOG
## Unreleasedsection documenting the publichasSuccessfulToolCallhelper (with usage example) and thewillContinuefix.Test plan
npx tsc --noEmit(src/) — cleannpm test— 270 passed (262 existing + 8 new)npm run lint— cleannpm run typecheck(full) — fails on the pre-existingexample/convex/setup.test.tsissue addressed by Fix typecheck failure in example/convex/setup.test.ts #262, unrelated to this PRNotes
hasSuccessfulToolCallreturns the same booleans before and after; only the internal source-of-truth shifts.