Skip to content

Conversation

@mrubens
Copy link
Collaborator

@mrubens mrubens commented Jul 8, 2025

This updates the architect prompt and also fixes #5468


Important

Updates architect mode prompt to create a todo list, adds isEmpty() utility, and enhances getPromptComponent() and getModeSelection() functions with tests.

  • Behavior:
    • Updates architect mode prompt to create a todo list using update_todo_list tool instead of a detailed plan in system-prompt snapshots.
    • Emphasizes creating clear, actionable todo lists over markdown documents.
  • Functions:
    • Adds getPromptComponent() in system.ts to filter out empty objects using isEmpty().
    • Updates getModeSelection() in modes.ts to merge prompt components with built-in modes.
  • Utilities:
    • Introduces isEmpty() in object.ts to check if an object is empty.
  • Tests:
    • Adds tests for getPromptComponent() in get-prompt-component.spec.ts.
    • Adds tests for isEmpty() in object.spec.ts.
    • Adds tests for getModeSelection() with empty prompt components in modes-empty-prompt-component.spec.ts.

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

@mrubens mrubens requested review from cte and jr as code owners July 8, 2025 17:14
@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Jul 8, 2025
@mrubens mrubens force-pushed the have_architect_create_todo_list branch from f169469 to 799d897 Compare July 8, 2025 17:14
@delve-auditor
Copy link

delve-auditor bot commented Jul 8, 2025

No security or compliance issues detected. Reviewed everything up to 4f31a24.

Security Overview
  • 🔎 Scanned files: 19 changed file(s)
Detected Code Changes
Change Type Relevant files
Enhancement ► prompts/system.ts
    Implement smarter partial merging of prompt components
► modes.ts
    Update mode selection logic for prompt components
► object.ts
    Add isEmpty utility function
Refactor ► usePromptHistory.ts
    Improve prompt history navigation
► reminder.ts
    Update reminder section formatting

Reply to this PR with @delve-auditor followed by a description of what change you want and we'll auto-submit a change to this PR to implement it.

@delve-auditor
Copy link

delve-auditor bot commented Jul 8, 2025

No security or compliance issues detected. Reviewed everything up to f16946947a32554a354a6854d7bf2a0b96d8f5ae.

Security Overview
  • 🔎 Scanned files: 18 changed file(s)
Detected Code Changes
Change Type Relevant files
Enhancement ► src/core/prompts/system.ts
    Add getPromptComponent helper function
► src/utils/object.ts
    Add isEmpty utility function
► src/shared/modes.ts
    Update architect mode instructions
Bug Fix ► webview-ui/src/components/chat/hooks/usePromptHistory.ts
    Fix handling of history navigation with modifiers

Reply to this PR with @delve-auditor followed by a description of what change you want and we'll auto-submit a change to this PR to implement it.

@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Jul 8, 2025
Copy link
Member

@daniel-lxs daniel-lxs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

I noticed in the test case that when a promptComponent only has customInstructions but no roleDefinition, the mode ends up with an empty roleDefinition. Is this intentional? Would it make sense to merge partial promptComponents with the built-in mode defaults, so that missing fields (like roleDefinition) fall back to the built-in values rather than empty strings?

@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Review] in Roo Code Roadmap Jul 8, 2025
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jul 8, 2025
@hannesrudolph hannesrudolph added PR - Needs Review and removed Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. labels Jul 8, 2025
@mrubens mrubens merged commit 0606c56 into main Jul 8, 2025
11 checks passed
@mrubens mrubens deleted the have_architect_create_todo_list branch July 8, 2025 17:59
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Jul 8, 2025
@github-project-automation github-project-automation bot moved this from PR [Needs Review] to Done in Roo Code Roadmap Jul 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

Bug: Mode-specific Custom Instructions section missing from system prompt when field is empty/default (Regression from PR #3791)

4 participants