feat: Framework for adjusting prompts to best fix model contexts #6113
Conversation
odilitime
changed the title
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new dynamicPromptExecFromState method to handle structured LLM output with automatic validation, retry logic, and performance tracking. It replaces direct composePromptFromState + useModel + parseKeyValueXml patterns with a unified approach that includes schema-based parsing, validation codes, and metrics tracking.
Key changes:
- Adds
dynamicPromptExecFromStatemethod toIAgentRuntimeinterface with comprehensive schema validation and retry logic - Refactors message service methods (shouldRespond, messageHandler, multi-step workflows) to use the new dynamic prompt execution
- Exports utility functions (
upgradeDoubleToTriple,composeRandomUser) and changesStateexport from wildcard to named export
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/types/state.ts | Adds SchemaRow type definition for dynamic prompt schema |
| packages/core/src/types/runtime.ts | Adds dynamicPromptExecFromState method signature to IAgentRuntime interface |
| packages/core/src/types/index.ts | Changes from wildcard to named exports for State and SchemaRow types |
| packages/core/src/runtime.ts | Implements dynamicPromptExecFromState with validation, retry logic, and metrics tracking; adds utility imports |
| packages/core/src/utils.ts | Exports upgradeDoubleToTriple and composeRandomUser functions; includes formatting changes |
| packages/core/src/services/default-message-service.ts | Refactors shouldRespond, messageHandler, and multi-step workflows to use new dynamic prompt execution; removes composePromptFromState import; changes console.error to runtime.logger.error |
| packages/test-utils/src/mocks/runtime.ts | Adds mock implementation for dynamicPromptExecFromState; formatting changes to empty function bodies |
| packages/core/src/tests/message-service.test.ts | Adds mock for dynamicPromptExecFromState with schema-based response routing |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Quote property names in JSON format ("field": instead of field:)
- Add commas between properties using join with ',\n' separator
- Fixes invalid JSON example that would fail JSON.parse()
…cFromState - Fix validation bug: only validate codes that were actually added to schema based on contextCheckLevel (0=none, 1=first only, 2=both) - Fix unused 'thoughts' variable: now properly logged for debugging - Also conditionally clean up validation codes from response based on level
fix: dynamic prompt normalization follow-up
|
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 Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. ✨ 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 |
|
Claude finished @odilitime's task —— View job PR Review Complete ✅Todo List:
Review Results✅ APPROVED - This is a well-implemented, low-risk feature addition with comprehensive tests and proper error handling. Critical Checks ✅
Technical Assessment ✅Architecture:
TypeScript Quality:
Error Handling:
Test Coverage:
No Issues Found ❌All critical areas checked - no security vulnerabilities, breaking changes, or architectural problems identified. Minor Observations 💡
This PR introduces a valuable framework for handling context-limited models while maintaining backward compatibility and following all ElizaOS patterns. Recommendation: ✅ MERGE |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| }; | ||
|
|
||
| ensureListField('actions'); | ||
| ensureListField('providers'); |
There was a problem hiding this comment.
Bug: Empty array normalization breaks actions fallback logic
The _normalizeStructuredResponse method converts actions to an array, which breaks the fallback logic in default-message-service.ts. When the LLM returns an empty or missing actions value, toList converts it to an empty array []. Since empty arrays are truthy in JavaScript, the expression parsedXml.actions || ['IGNORE'] evaluates to [] instead of the intended fallback ['IGNORE']. Previously, parseKeyValueXml would return an empty string which is falsy and would correctly trigger the fallback.
Additional Locations (1)
|
Claude finished @odilitime's task —— View job PR Review Complete ✅Todo List:
Review Results✅ APPROVED - This is a well-implemented, low-risk feature addition with comprehensive tests and proper error handling. Critical Checks ✅
Technical Assessment ✅Architecture:
TypeScript Quality:
Error Handling:
Test Coverage:
No Issues Found ❌All critical areas checked - no security vulnerabilities, breaking changes, or architectural problems identified. Minor Observations 💡
This PR introduces a valuable framework for handling context-limited models while maintaining backward compatibility and following all ElizaOS patterns. Recommendation: ✅ MERGE |
| if (currentRetry <= maxRetries) { | ||
| this.logger.warn( | ||
| `dynamicPromptExecFromState retry ${currentRetry}/${maxRetries} after failure [${modelSchemaKey}]`, | ||
| allGood ? 'Parse failure' : 'Validation codes failed or missing required fields' |
There was a problem hiding this comment.
Bug: Inverted ternary makes "Parse failure" message unreachable
The ternary condition allGood ? 'Parse failure' : 'Validation codes failed...' is logically inverted. At this point in the retry loop, allGood is always false because: if parsing failed, it's set to false at line 2799; if validation or required fields failed, it's set to false at lines 2829/2871. The "Parse failure" branch is unreachable, causing all failures to be logged as "Validation codes failed" even when the actual issue was a parse error. The condition should check !responseContent to properly distinguish failure types.
|
Claude finished @odilitime's task —— View job PR Review Complete ✅Todo List:
Review Results✅ APPROVED - This is a well-implemented, low-risk feature addition with comprehensive tests and proper error handling. Critical Checks ✅
Technical Assessment ✅Architecture:
TypeScript Quality:
Error Handling:
Test Coverage:
No Issues Found ❌All critical areas checked - no security vulnerabilities, breaking changes, or architectural problems identified. Minor Observations 💡
This PR introduces a valuable framework for handling context-limited models while maintaining backward compatibility and following all ElizaOS patterns. Recommendation: ✅ MERGE |
|
Claude finished @odilitime's task —— View job PR Review Complete ✅Todo List:
Review Results✅ APPROVED - This is a well-implemented, low-risk feature addition with comprehensive tests and proper error handling. Critical Checks ✅
Technical Assessment ✅Architecture:
TypeScript Quality:
Error Handling:
Test Coverage:
Issues Found ❌❌ CRITICAL: Inverted ternary makes "Parse failure" message unreachable → Fix: Change Fixed Issues from Previous Reviews ✅✅ Empty array normalization - Now properly handles fallback with Minor Observations 💡
This PR introduces a valuable framework for handling context-limited models while maintaining backward compatibility and following all ElizaOS patterns. Recommendation: ✅ MERGE (after fixing the critical ternary bug) |
Summary
Adds
dynamicPromptExecFromState()toAgentRuntime- a schema-driven prompt execution system that detects context overflow in models with limited context windows.Problem
Models with lower context windows (4K-8K tokens) can silently truncate prompts, leading to hallucinated outputs. There was no way to detect when this happened.
Solution
Inject UUID validation codes at the start, middle, and end of prompts. The model must parrot these back correctly, proving it read the entire prompt.
Changes
packages/core/src/runtime.ts: NewdynamicPromptExecFromState()method with validation, retries, and metrics trackingpackages/core/src/services/default-message-service.ts: Refactored should-respond, single-shot, and multi-step handlers to use the new methodpackages/core/src/types/: AddedSchemaRowtype and interface extensionpackages/test-utils/: Updated mocksConfiguration
Risk
Low - New feature with configurable memory limits. Message service refactored to use it internally without breaking existing behavior.
Note
Adds dynamicPromptExecFromState with validation/retry/metrics and updates MessageService handlers to consume structured outputs, plus supporting types, utils, mocks, and tests.
dynamicPromptExecFromState(state, params, schema, options)with UUID validation codes, retry logic, JSON/XML parsing, normalization (lists/booleans), and two-level metrics + getters/clearers.upgradeDoubleToTriple,composeRandomUser, JSON/XML parsers, boolean parsing; generate cache/metrics keys.composePromptFromState+ raw model calls withdynamicPromptExecFromStateinshouldRespond, single-shot handler, multi-step decision, and summary.actionsto fallback['IGNORE']; improve error logging viaruntime.logger.SchemaRow; updateIAgentRuntimeto includedynamicPromptExecFromState; re-export{ SchemaRow, State }fromtypes/index.IGNOREwhen actions omitted).isRoomParticipant,dynamicPromptExecFromState) and minor logger/style tweaks.Written by Cursor Bugbot for commit 2beacb9. This will update automatically on new commits. Configure here.