fix(core): prevent duplicate LLM calls when message contains URL or triggers providers#6528
fix(core): prevent duplicate LLM calls when message contains URL or triggers providers#6528nicolasdma wants to merge 216 commits intoelizaOS:v2.0.0from
Conversation
Implement comprehensive hot reload functionality for backend code changes during development. When TypeScript files in watched packages are modified, the system automatically rebuilds the CLI and restarts the server. Core implementation: - Watch all CLI dependency packages (cli, core, server, api-client, plugin-bootstrap, plugin-sql, config) - Debounce file changes (300ms) to prevent rapid rebuilds - Graceful server shutdown and restart on code changes - Health check verification after rebuilds to detect crashes - Rebuild queueing for changes during active rebuilds Technical details: - Use Bun.spawn() for all process execution per project standards - Comprehensive TypeScript type annotations with JSDoc - Exit event listeners for proper SIGKILL fallback - Directory existence checks to handle optional packages gracefully - Full test coverage with 13 passing tests using temp directories The dev environment now provides: - Automatic backend rebuild on TypeScript file changes - Server health verification after each rebuild - Graceful error handling and process cleanup - Clear logging for all rebuild operations Usage: bun run dev Addresses all PR review feedback from @cursor, @greptile-apps, and @claude
Co-authored-by: sayonara <sayonara@elizalabs.ai>
- Add .catch() handler on child.exited promise - Ensure setTimeout always calls resolve() after SIGKILL - Wrap child.kill() in try-catch for error handling - Match the pattern used in cleanup() function This prevents the rebuild from freezing if the process doesn't respond to signals or if child.exited rejects.
Co-authored-by: sayonara <sayonara@elizalabs.ai>
…r extraction - Add retry logic for XML parsing with exponential backoff (1s, 2s, 4s... capped at 8s) - Add summary generation retry with graceful fallback message - Add parameter extraction from multi-step decision template - Add formatActionsWithParams() for LLM parameter schema information - Add bounds checking for retry counts (1-10 range) - Add comprehensive tests for retry and parameter scenarios
Enhanced JSON parameter parsing to ensure only non-null objects are accepted, with appropriate logging for invalid types. Updated related test and provider logic to match stricter validation and improved code consistency.
Added explicit checks and warnings for array-type parameters in both DefaultMessageService and multi-step tests. Metadata properties in accumulated state are now prefixed with underscores to avoid collisions with action parameters.
Improves logging granularity and clarity for multi-step message processing, including step-by-step info, provider/action execution, and summary generation. Adds streaming support for summary output in multi-step mode and ensures streaming contexts are correctly managed for both single-shot and multi-step flows. Minor code formatting and consistency improvements applied.
Replaces 'state' with 'accumulatedState' in provider and action calls to ensure correct state is used. Adds logic to prevent duplicate streaming of summary output to the user on retry attempts, tracking streaming status with 'hasStreamedToUser'.
Marks streaming as started before sending content to prevent duplicate streams on retries in DefaultMessageService. Updates action parameter formatting to handle invalid definitions and provide defaults for missing type or description.
Updates submodule from 797ad22 (init commit) to 4a33c0c (Add ElizaOS scenario testing rules) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…entries - Add array check to prevent malformed parameter output - Filter invalid param entries upfront to avoid dangling headers
- Added methods to StreamingContext for managing streaming state, including reset, getStreamedText, and isComplete. - Improved DefaultMessageService to utilize intelligent retry logic for streaming, allowing for continuation prompts when text extraction is incomplete. - Updated tests to validate new streaming context features and ensure correct behavior during retries. - Enhanced IStreamExtractor interface with reset and flush methods for better state management.
- Removed redundant '[MULTI-STEP]' prefix from logging messages in DefaultMessageService to enhance clarity and readability. - Updated various log statements to maintain consistency while preserving essential information about the multi-step processing flow. - other: fix docs
Replace ACTIONS_REGEX with indexOf-based extraction in detectResponseStrategy. This eliminates potential polynomial-time regex matching on malicious inputs.
Ensures continuation response is properly streamed to user in single-shot mode, matching the behavior of multi-step continuation at line 1862.
…nt results Previously, calling getStreamedText() multiple times would return different results because flush() empties the buffer. Now the flushed content is accumulated into streamedText, ensuring consistent results across calls.
This commit refactors the error handling and shutdown logic in the development server scripts. It ensures that processes are properly cleaned up and that errors are logged and handled more effectively. Co-authored-by: sayonara <sayonara@elizalabs.ai>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
…riggers providers Remove providers length check from isSimple determination. Providers enrich state (already handled at L863-871) but don't require re-generating the response. The previous check incorrectly treated "has providers" as "not simple", causing messages with URLs to go through the actions path and trigger a second LLM call via the REPLY action handler. Relates to elizaOS#6486
681b3d8 to
3a304b6
Compare
|
if it has providers, it's not simple. That is correct. |
|
The ATTACHMENTS provider gets set during the initial runSingleShotCore pass (after URL detection). In reply.ts around line 70: is the second LLM call expected when actions: ["REPLY"] and providers are present? |
Relates to
Relates to #6486
Risks
Low — single condition removed from a boolean expression. No new code, no new logic paths.
Background
What does this PR do?
Removes the
providers.lengthcheck from theisSimpledetermination inpackages/typescript/src/services/message.ts.What kind of change is this?
Bug fix (non-breaking change that fixes an issue).
Changes
When a user sends a message containing a URL, the LLM's first call (via
runSingleShotCore) generates and streams the response text. It also returnsproviders: ["ATTACHMENTS"]because the prompt template instructs it to when it sees URLs.The
isSimplecheck at line 1883 previously treated "has providers" as "not simple":This caused the message to go through the
actionspath, which triggered the REPLY action handler (reply.ts:60), making a second LLM call withreplyTemplateand streaming another response — duplicating output and doubling token costs.The fix: Remove the providers check. Providers enrich state (already handled at L863-871) but don't require re-generating the response.
isSimpleshould only care about whether the action is REPLY (text already generated and streamed) vs something else (needs execution).Testing
Greptile Summary
Removes the
providers.lengthcheck from theisSimpleboolean expression, preventing duplicate LLM calls when messages contain URLs or trigger provider enrichment.Key Changes:
(!responseContent.providers || responseContent.providers.length === 0)fromisSimplecheck at line 1883actions: ["REPLY"]now correctly use the "simple" path regardless of provider presenceImpact:
runSingleShotCorecall is now correctly used as final outputConfidence Score: 5/5
Important Files Changed
isSimpledetermination to prevent duplicate LLM calls when URLs trigger provider enrichmentLast reviewed commit: 681b3d8
(2/5) Greptile learns from your feedback when you react with thumbs up/down!