-
Notifications
You must be signed in to change notification settings - Fork 2.8k
refactor: decouple tools from system prompt #9784
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Re-reviewed the latest commits. The previously flagged issue has been resolved. No new issues found.
Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues. |
hannesrudolph
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall this is a well-structured refactor that achieves its stated goal of decoupling tool-specific instructions from system prompt sections. The changes reduce coupling and make the codebase simpler and more maintainable. Found one cleanup issue noted below.
Note: The PR has merge conflicts that need to be resolved before merging.
Review completed. Found 1 minor cleanup issue. This refactoring successfully decouples tool-specific instructions from the system prompt by moving them into the tool descriptions themselves. The changes simplify the codebase while maintaining the same functionality.
Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues. |
The system prompt sections no longer change based on which tools are available. All tool-specific instructions are now in the tool descriptions themselves. - codebase_search: Added CRITICAL usage instructions about using it first - write_to_file: Added guidance about preferring other editing tools - search_files: Added regex pattern crafting guidance - browser_action: Added web development usage guidance - rules.ts: Removed getEditingInstructions(), all tool conditionals - capabilities.ts: Removed all tool availability conditionals - tool-use-guidelines.ts: Removed codeIndexManager conditional guidelines - objective.ts: Removed all parameters and conditional instructions - system.ts: Updated calls to match new simplified signatures - Deleted mode-aware-sections.spec.ts (tests for removed behavior) - Updated sections.spec.ts for new signatures - Updated snapshot files
66d632a to
3f5c534
Compare
Summary
The system prompt sections no longer change based on which tools are available. All tool-specific instructions are now in the tool descriptions themselves. This simplifies the codebase and makes tool behavior self-contained.
Changes
Tool Descriptions Updated (XML & Native)
System Prompt Sections Simplified
getEditingInstructions(), all editing tool conditionals, browser_action conditionals. Function signature simplified from 8 params to 3.Tests Updated
mode-aware-sections.spec.ts(was testing removed behavior)sections.spec.tsfor new signaturesBenefits
Testing
All 4456 tests pass (339 test files)
Important
Refactor system prompt to decouple tool-specific instructions, simplifying the codebase and enhancing predictability.
codebase_searchmust be used first for unexplored code areas.rules.ts,capabilities.ts,tool-use-guidelines.ts, andobjective.ts.rules.ts,capabilities.ts,tool-use-guidelines.ts, andobjective.ts.sections.spec.tsandobjective.spec.tsto reflect changes.mode-aware-sections.spec.tsas it tested removed behavior.sections.spec.tsfor new function signatures.This description was created by
for 9c3781e. You can customize this summary. It will automatically update as commits are pushed.