Skip to content

Conversation

@daniel-lxs
Copy link
Member

@daniel-lxs daniel-lxs commented Jun 24, 2025

Description

Fixes #5075

This PR resolves the issue where LM Studio models incorrectly show a context length of "1" instead of their actual context window size.

Changes Made

  • Added fetchModel() method to LmStudioHandler that dynamically fetches model information from LM Studio using the existing getLMStudioModels() function
  • Updated getModel() method to return actual model info with correct context length instead of static defaults
  • Enhanced error handling with graceful fallback to default values when LM Studio is unavailable
  • Added comprehensive tests covering successful fetching, error scenarios, and backward compatibility
  • Maintained backward compatibility - existing functionality remains unchanged when fetching fails

Testing

  • ✅ All existing tests pass (2457 tests passed, 47 skipped)
  • ✅ New LM Studio-specific tests pass (12 tests passed)
  • ✅ Manual testing completed:
    • Verified fetchModel() correctly calls getLMStudioModels()
    • Verified getModel() returns actual model info after fetching
    • Verified graceful error handling when LM Studio is unavailable
  • ✅ Type checking passes
  • ✅ Linting passes

Verification of Acceptance Criteria

  • Context length detection: LM Studio models now show their actual context length instead of "1"
  • No breaking changes: Maintains backward compatibility with graceful fallback
  • Error handling: Properly handles cases where LM Studio is unavailable
  • Test coverage: Comprehensive tests ensure the fix works correctly

Technical Details

Root Cause: The LmStudioHandler.getModel() method was returning openAiModelInfoSaneDefaults (which has contextWindow: 1) instead of fetching actual model information from LM Studio.

Solution: The handler now dynamically fetches model information using the existing getLMStudioModels() function, which correctly parses the actual context length from LM Studio's model metadata.

Implementation Pattern: Follows the same pattern used by other providers (OpenRouter, Requesty) that implement a fetchModel() method for dynamic model information retrieval.

Files Changed

  • src/api/providers/lm-studio.ts - Added dynamic model fetching capability
  • src/api/providers/__tests__/lmstudio.spec.ts - Updated tests to cover new functionality

Checklist

  • Code follows project style guidelines
  • Self-review completed
  • Comments added for complex logic
  • No breaking changes
  • All tests pass
  • Type checking passes
  • Linting passes
  • Issue requirements fully addressed

Important

Fixes context length detection in LmStudioHandler by dynamically fetching model info and enhancing error handling.

  • Behavior:
    • LmStudioHandler now dynamically fetches model info using fetchModel() and getLMStudioModels().
    • getModel() returns actual model info with correct context length.
    • Graceful fallback to default values when LM Studio is unavailable.
  • Testing:
    • Added tests in lmstudio.spec.ts for successful fetching, error scenarios, and backward compatibility.
    • Manual testing verified fetchModel() and getModel() behavior.
  • Files Changed:
    • lm-studio.ts: Added dynamic model fetching and error handling.
    • lmstudio.spec.ts: Updated tests to cover new functionality.

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

- Add fetchModel() method to LmStudioHandler to dynamically fetch model info
- Update getModel() to return actual context length instead of default value of 1
- Add comprehensive tests for new functionality with proper error handling
- Maintain backward compatibility with graceful fallback to defaults

Fixes #5075
@daniel-lxs daniel-lxs requested review from cte, jr and mrubens as code owners June 24, 2025 18:34
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. bug Something isn't working labels Jun 24, 2025
@delve-auditor
Copy link

delve-auditor bot commented Jun 24, 2025

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

Security Overview
  • 🔎 Scanned files: 2 changed file(s)
Detected Code Changes
Change Type Relevant files
Bug Fix ► lmstudio.spec.ts
    Add comprehensive tests for LM Studio model info fetching
► lm-studio.ts
    Add fetchModel() method for dynamic model info
    Update getModel() to return actual context length

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.

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jun 24, 2025
@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Review] in Roo Code Roadmap Jun 24, 2025
@daniel-lxs daniel-lxs moved this from PR [Needs Review] to PR [Needs Prelim Review] in Roo Code Roadmap Jun 24, 2025
@mrubens mrubens merged commit 5bf7d00 into main Jun 24, 2025
23 checks passed
@mrubens mrubens deleted the fix/issue-5075-lmstudio-context-length branch June 24, 2025 18:44
@github-project-automation github-project-automation bot moved this from PR [Needs Prelim Review] to Done in Roo Code Roadmap Jun 24, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Jun 24, 2025
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 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.

Roo does not correctly detect context length for LM Studio models

3 participants