Resolve 4 Detail bugs (maxToolSteps, JWT caching, fallback concurrency, mergeFrames perf)#1213
Resolve 4 Detail bugs (maxToolSteps, JWT caching, fallback concurrency, mergeFrames perf)#1213toubatbrian wants to merge 4 commits intomainfrom
Conversation
🦋 Changeset detectedLatest commit: 176671d The changes in this PR will be included in the next version bump. This PR includes changesets to release 22 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1cb8c6de65
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| try { | ||
| let shouldSetCurrent = !checkRecovery; | ||
| for await (const chunk of stream) { |
There was a problem hiding this comment.
tryGenerate now relies only on for await (const chunk of stream) to detect provider failures, but LLMStream reports many failures by emitting llm.on('error') and then closing the output queue (see LLMStream.emitError), so iteration can end normally without throwing. In that case this method returns as success (often with zero/partial chunks), the adapter never marks the provider unavailable, and fallback to the next LLM is skipped. This regresses the core fallback behavior whenever a child stream fails via emitted error events rather than iterator exceptions.
Useful? React with 👍 / 👎.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
…into brian/bug-fix-2
1 participant
Summary
SpeechHandle._numStepsinitialization somaxToolStepslimit is actually enforced in realtime tool-call loopspendingLogsinPinoCloudExporterat 10k to prevent OOM on persistent export failureLLMerror listener inFallbackLLMStreamthat caused concurrent request cross-contaminationmergeFramesfrom O(N^2) array spread to O(N) pre-allocatedTypedArray.set()A fifth bug (Queue dropping falsy values) was already fixed on the current branch.
Changes
agents/src/voice/speech_handle.ts_numStepswas hardcoded to1, ignoring the_stepIndexconstructor arg. Now initialized from_stepIndexso themaxToolStepscheck inagent_activity.tssees the correct accumulated step count.agents/src/telemetry/otel_http_exporter.tsandagents/src/telemetry/pino_otel_transport.tsensureJwt()cached the token forever (if (this.jwt) return). Now tracksjwtExpiresAtand refreshes 1 hour before the 6-hour server TTL. Also capspendingLogsat 10k entries in the flush error handler to prevent unbounded memory growth.agents/src/llm/fallback_adapter.tsFallbackLLMStreamattached an error handler to the sharedLLMinstance, capturing errors from unrelated concurrent requests. Removed thellm.on('error')listener,streamErrorvariable, and post-loopif (streamError) throwcheck. Errors are already caught via thetry/catcharoundfor await (const chunk of stream).agents/src/utils.tsmergeFramesusednew Int16Array([...data, ...frame.data])in a loop (O(N^2)). Rewritten to pre-compute total size, allocate once, and copy withTypedArray.set()(O(N)). Also normalizedQueue.get()check fromtypeof item === 'undefined'toitem === undefined(no behavior change).Test plan
pnpm buildpassespnpm vitest run agents/src/utils.test.ts— 31 tests pass (covers Queue and mergeFrames)restaurant_agent.tsworks in Agent Playground (exercises speech handles + telemetry)