Skip to content

Conversation

@ajgreyling
Copy link

@ajgreyling ajgreyling commented Jan 24, 2026

Related GitHub Issue

Closes: #10935

Description

This PR fixes vLLM compatibility issues with the Harmony provider by addressing two key problems:

  1. Removing unsupported strict parameter: vLLM doesn't support OpenAI's strict mode parameter for function calling. The base class adds this parameter, but Harmony (which uses vLLM) needs it removed to prevent warnings and potential errors.

  2. Preserving optional parameters: The base class's convertToolSchemaForOpenAI method converts all properties to required fields for OpenAI strict mode. However, vLLM requires that optional parameters (especially those with nullable types like ["string", "null"]) remain optional. This is critical for tools like browser_action that have optional parameters.

Implementation details:

  • convertToolsForOpenAI override: Removes the strict parameter from all function tools after conversion, ensuring vLLM compatibility.
  • convertToolSchemaForOpenAI override:
    • Preserves the original required array structure
    • Converts nullable types (e.g., ["string", "null"]) to non-nullable types (e.g., "string")
    • Excludes nullable properties from the required array, keeping them optional
    • Recursively processes nested objects and arrays
    • Ensures additionalProperties: false is set (required by OpenAI Responses API)

This fix ensures that tools with optional parameters work correctly with vLLM backends while maintaining compatibility with the OpenAI-compatible API format.

Test Procedure

Unit Tests Added:

  1. convertToolsForOpenAI tests:

    • Verifies strict parameter is removed from function tools
    • Ensures all other tool properties are preserved
    • Tests MCP tool handling (which have strict: false from parent)
    • Verifies non-function tools pass through unchanged
    • Tests undefined input handling
  2. Optional parameter preservation test:

    • Tests browser_action tool with nullable optional parameters (url, coordinate)
    • Verifies nullable types are converted to non-nullable
    • Ensures only required fields (action) are in the required array
    • Confirms optional parameters remain optional for vLLM compatibility

Running tests:

npx vitest run src/api/providers/__tests__/harmony.spec.ts

All existing tests continue to pass, and the new tests verify the vLLM compatibility fixes.

Pre-Submission Checklist

  • Issue Linked: This PR is linked to GitHub Issue [ENHANCEMENT] Add Harmony AI Provider Support for GPT-OSS Models #10935.
  • Scope: Changes are focused on fixing vLLM compatibility issues with the Harmony provider.
  • Self-Review: Code has been reviewed for correctness and follows existing patterns.
  • Testing: Comprehensive unit tests have been added to cover the new functionality.
  • Documentation Impact: No user-facing documentation changes required (internal implementation fix).
  • Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Screenshots / Videos

N/A - Backend implementation fix with no UI changes.

Documentation Updates

  • No documentation updates are required.

This is an internal implementation fix that improves compatibility with vLLM backends. No user-facing documentation changes are needed.

Additional Notes

Key design decisions:

  1. Why override instead of modifying base class: The base class behavior is correct for OpenAI's strict mode, but Harmony uses vLLM which doesn't support strict mode. Overriding allows Harmony-specific behavior without affecting other providers.

  2. Nullable type handling: The implementation converts ["string", "null"] to "string" while tracking which properties were nullable. This ensures the required array only includes truly required fields, not optional nullable ones.

  3. Backward compatibility: The changes maintain full backward compatibility with existing tool schemas while fixing the vLLM-specific issues.

Testing considerations:

  • Tests cover both regular tools and MCP tools
  • Tests verify the critical browser_action use case with nullable optional parameters
  • All edge cases (undefined, non-function tools, nested objects) are covered

Get in Touch

Contact me on Discord @ ajgreyling


Important

Adds Harmony provider to handle GPT-OSS models with vLLM compatibility, including new handler, settings, and tests.

  • Behavior:
    • Adds HarmonyHandler in harmony.ts to handle GPT-OSS models with vLLM compatibility.
    • Removes strict parameter in convertToolsForOpenAI() to prevent vLLM warnings.
    • Preserves optional parameters in convertToolSchemaForOpenAI().
  • Files:
    • Adds harmony.ts to src/api/providers/.
    • Updates index.ts in src/api/ and src/api/providers/ to include HarmonyHandler.
    • Modifies provider-settings.ts to add Harmony provider settings.
  • Tests:
    • Adds harmony.spec.ts, harmony-edge-cases.spec.ts, and harmony-roo-code-integration.spec.ts for unit and integration tests.
    • Includes tests for convertToolsForOpenAI() and optional parameter handling.
  • UI:
    • Updates ApiOptions.tsx and constants.ts to include Harmony provider in settings.
    • Adds Harmony.tsx for Harmony provider settings UI.
  • Misc:
    • Updates README.md to fix formatting.
    • Updates package.json to a new pnpm version.

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

@roomote
Copy link
Contributor

roomote bot commented Jan 24, 2026

Rooviewer Clock   See task on Roo Cloud

Review complete. Found 3 issues that should be addressed before merging:

  • Protected method convertToolsForOpenAI called directly in tests - use a test wrapper class pattern (see Cerebras tests for example)
  • Development/debug files (test-harmony-api.ts, test_harmony_toolcall_fix.py) placed in project root - consider removing or relocating
  • Unrelated packageManager version change in package.json - consider reverting or submitting separately

Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues.

},
]

const converted = handler.convertToolsForOpenAI(tools)
Copy link
Contributor

Choose a reason for hiding this comment

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

The convertToolsForOpenAI method is protected in the parent class, so calling it directly on the handler instance will cause TypeScript compilation errors. The Cerebras tests demonstrate the correct pattern: create a test wrapper class that exposes the protected method via a public method. For example:

class TestHarmonyHandler extends HarmonyHandler {
    public testConvertToolsForOpenAI(tools: any[] | undefined) {
        return this.convertToolsForOpenAI(tools)
    }
}

Then use testHandler.testConvertToolsForOpenAI(tools) in tests.

Fix it with Roo Code or mention @roomote and request a fix.

@@ -0,0 +1,113 @@
import OpenAI from "openai"
Copy link
Contributor

Choose a reason for hiding this comment

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

This development/debug script and the accompanying test_harmony_toolcall_fix.py are placed in the project root. Consider either removing these files from the PR (they appear to be local development tools) or moving them to an appropriate location such as scripts/ or a documentation directory if they serve as examples.

Fix it with Roo Code or mention @roomote and request a fix.

"name": "roo-code",
"packageManager": "pnpm@10.8.1",
"packageManager": "pnpm@10.28.1",
"engines": {
Copy link
Contributor

Choose a reason for hiding this comment

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

This packageManager version change from pnpm@10.8.1 to pnpm@10.28.1 appears unrelated to the Harmony provider feature. Consider reverting this change or submitting it as a separate PR to keep the feature scope focused.

Fix it with Roo Code or mention @roomote and request a fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

[ENHANCEMENT] Add Harmony AI Provider Support for GPT-OSS Models

2 participants