Skip to content

Conversation

@ChuKhaLi
Copy link
Contributor

@ChuKhaLi ChuKhaLi commented May 21, 2025

Related GitHub Issue

Closes: #2558

Description

  • modify the logic get roleDefinition and baseInstructions in get system prompt on system.ts file
  • the logic follow PromptsView.tsx, as custom mode's instruction > user prompt > built-in

Test Procedure

Type of Change

  • 🐛 Bug Fix: Non-breaking change that fixes an issue.
  • New Feature: Non-breaking change that adds functionality.
  • 💥 Breaking Change: Fix or feature that would cause existing functionality to not work as expected.
  • ♻️ Refactor: Code change that neither fixes a bug nor adds a feature.
  • 💅 Style: Changes that do not affect the meaning of the code (white-space, formatting, etc.).
  • 📚 Documentation: Updates to documentation files.
  • ⚙️ Build/CI: Changes to the build process or CI configuration.
  • 🧹 Chore: Other changes that don't modify src or test files.

Pre-Submission Checklist

  • Issue Linked: This PR is linked to an approved GitHub Issue (see "Related GitHub Issue" above).
  • Scope: My changes are focused on the linked issue (one major feature/fix per PR).
  • Self-Review: I have performed a thorough self-review of my code.
  • Code Quality:
    • My code adheres to the project's style guidelines.
    • There are no new linting errors or warnings (npm run lint).
    • All debug code (e.g., console.log) has been removed.
  • Testing:
    • New and/or updated tests have been added to cover my changes.
    • All tests pass locally (npm test).
    • The application builds successfully with my changes.
  • Branch Hygiene: My branch is up-to-date (rebased) with the main branch.
  • Documentation Impact: I have considered if my changes require documentation updates (see "Documentation Updates" section below).
  • Changeset: A changeset has been created using npm run changeset if this PR includes user-facing changes or dependency updates.
  • Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Screenshots / Videos

Documentation Updates

Does this PR necessitate updates to user-facing documentation?

  • No documentation updates are required.

Additional Notes

Get in Touch


Important

Fixes prompt generation precedence by introducing getModeSelection to prioritize custom instructions and role definitions.

  • Behavior:
    • Updates generatePrompt and SYSTEM_PROMPT in system.ts to use getModeSelection for determining roleDefinition and baseInstructions.
    • Ensures precedence: custom mode's instruction > user prompt > built-in.
  • Functions:
    • Adds getModeSelection in modes.ts to encapsulate logic for selecting roleDefinition and baseInstructions.
  • Tests:
    • Adds tests for getModeSelection in modes.test.ts to verify correct precedence handling and fallback behavior.

This description was created by Ellipsis for 7dda1f7d588a9a9d7c468f8800bdc4ea6d3f2d4d. You can customize this summary. It will automatically update as commits are pushed.

@ChuKhaLi ChuKhaLi requested review from cte and mrubens as code owners May 21, 2025 18:25
@ChuKhaLi ChuKhaLi marked this pull request as draft May 21, 2025 18:33
@ChuKhaLi ChuKhaLi marked this pull request as ready for review May 21, 2025 18:44
@hannesrudolph hannesrudolph moved this from New to PR [Pre Approval Review] in Roo Code Roadmap May 22, 2025
@ChuKhaLi ChuKhaLi requested a review from mrubens May 23, 2025 17:43
@hannesrudolph hannesrudolph moved this from PR [Needs Review] to TEMP in Roo Code Roadmap May 26, 2025
@daniel-lxs daniel-lxs moved this from TEMP to PR [Needs Review] in Roo Code Roadmap May 27, 2025
@hannesrudolph hannesrudolph added the Followup Needs followup from support or code team label May 27, 2025
@daniel-lxs
Copy link
Member

daniel-lxs commented May 27, 2025

Hey @ChuKhaLi, this looks good to me, can we solve the merge conflicts?

@daniel-lxs daniel-lxs moved this from PR [Needs Preliminary Review] to PR [Needs Review] in Roo Code Roadmap May 27, 2025
@daniel-lxs daniel-lxs removed the Followup Needs followup from support or code team label May 27, 2025
@ChuKhaLi ChuKhaLi force-pushed the fix/prompt-precedence branch from 2baf398 to e8feed7 Compare May 28, 2025 00:15
@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label May 28, 2025
@ChuKhaLi
Copy link
Contributor Author

Hey @ChuKhaLi, this looks good to me, can we solve the conflict issues?

done, pls have a look

@daniel-lxs
Copy link
Member

daniel-lxs commented May 28, 2025

Thank you! LGTM

Makes sense to be consistent and actually use the role definitions and instructions present in the custom mode files when the file exists.

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label May 28, 2025
@mrubens mrubens merged commit 171307c into RooCodeInc:main May 28, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working lgtm This PR has been approved by a maintainer PR - Needs Review size:L This PR changes 100-499 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

UI Prefers global modes json over built-in modes but agents do not

4 participants