Skip to content

Conversation

@cte
Copy link
Collaborator

@cte cte commented Aug 19, 2025

Important

Enhance RooHandler to expose reasoning tokens, update model description, and add dynamic provider URL configuration.

  • Behavior:
    • RooHandler in roo.ts now yields reasoning tokens in createMessage().
    • Updates rooModels in roo.ts to support prompt caching and modifies the model description.
    • Adds ROO_CODE_PROVIDER_URL to .env.sample for dynamic base URL configuration.
  • Refactoring:
    • Extracts createStream() from createMessage() in base-openai-compatible-provider.ts for better code organization.
  • Tests:
    • Updates roo.spec.ts to test new reasoning token behavior and prompt cache support.

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

@cte cte requested review from jr and mrubens as code owners August 19, 2025 08:42
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. enhancement New feature or request labels Aug 19, 2025
@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Aug 19, 2025
Copy link
Contributor

@roomote roomote bot left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution! I've reviewed the changes and found some areas that need attention.

Critical Issue:

  • The test file src/api/providers/__tests__/roo.spec.ts needs to be updated to include test coverage for the new reasoning_content functionality. This is a significant feature that requires proper testing.

Other feedback provided inline on the changed files.

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Aug 19, 2025
@mrubens
Copy link
Collaborator

mrubens commented Aug 19, 2025

@roomote-agent please fix the failing test

@roomote
Copy link
Contributor

roomote bot commented Aug 19, 2025

Hi @mrubens! I see the request to fix the failing test. Let me investigate the issue and fix it for you. 🔧

The getModel() method in RooHandler was incorrectly using the roo/sonic model info as fallback for unknown models. This fix ensures that unknown models get a proper fallback with supportsPromptCache set to false, which aligns with the test expectations.
@roomote
Copy link
Contributor

roomote bot commented Aug 19, 2025

Hi @mrubens! ✅ I have successfully fixed the failing test.

Summary of the fix:

The test "should handle unknown model ID with fallback info" in src/api/providers/tests/roo.spec.ts was failing because the RooHandler.getModel() method was incorrectly returning the roo/sonic model info as fallback for unknown models.

What I changed:

  • Updated src/api/providers/roo.ts line 74 to remove the fallback to rooDefaultModelId model info
  • This ensures unknown models get proper fallback info with supportsPromptCache: false (matching test expectations)

Test results:

platform-unit-test (ubuntu-latest) - Now passing (was failing before)
✅ All provider tests passing locally

The integration test failure appears to be unrelated to this change and was already failing before my fix.

Commit: 7ed6620

@mrubens mrubens merged commit 532728e into main Aug 19, 2025
9 checks passed
@mrubens mrubens deleted the cte/roo-sonic-thinking branch August 19, 2025 14:01
@github-project-automation github-project-automation bot moved this from Triage to Done in Roo Code Roadmap Aug 19, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Aug 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. lgtm This PR has been approved by a maintainer size:M This PR changes 30-99 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

5 participants