Add retry logic, caching, streaming progress, and model comparison features - #617
Conversation
…omparison features Add four major feature enhancements to the SDK: 1. Enhanced Error Handling & Retry Logic - Add retry policy system with exponential backoff - Support configurable retry strategies - Provide predefined retry policies (aggressive, balanced, conservative, none) - Integrate retry logic into ClientPort RPC calls 2. Model Caching Layer - Create ModelMetadataCache with TTL and max entries - Implement CacheManager for multiple named caches - Add cache statistics and invalidation methods - Support lazy loading with getOrCompute 3. Streaming Response Progress - Add StreamingProgressTracker for token/chunk tracking - Implement progress callbacks with throttling - Calculate token rate and time estimates - Support completion and error handlers 4. Model Comparison Tools - Create ModelComparisonRunner for multi-model testing - Collect comprehensive performance metrics - Support multiple export formats (text, JSON, CSV) - Handle timeouts and failures gracefully Includes comprehensive test suites for all features and detailed documentation with usage examples and best practices. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
I have read the CLA Document and I hereby sign the CLA GitHub Copilot seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff2bd63ba9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| #### Basic Usage | ||
| ```typescript | ||
| import { LMStudioClient, predefinedRetryPolicies } from "@lmstudio/sdk"; |
There was a problem hiding this comment.
Export the documented SDK APIs
The guide tells users to import predefinedRetryPolicies from @lmstudio/sdk, and later does the same for the new cache/progress/comparison APIs, but publish/sdk/src/index.ts does not re-export any of these symbols and packages/lms-client/src/index.ts does not export the new client modules either. In the published package these examples fail at compile time; additionally LMStudioClient's strict constructor schema has no retryPolicy option, so this documented configuration cannot be used through the SDK entry point.
Useful? React with 👍 / 👎.
| const response = await Promise.race([ | ||
| respond(config.input), | ||
| new Promise<never>((_, reject) => | ||
| setTimeout(() => reject(new Error("Model timeout")), config.timeoutMs ?? 30000), |
There was a problem hiding this comment.
Clear comparison timeout timers
When respond(config.input) resolves before the timeout, the setTimeout created here is never cleared. A successful comparison in a short-lived CLI/script will keep the Node event loop alive until every pending timeout fires (30s by default, or timeoutMs per model), even though compare() has already returned.
Useful? React with 👍 / 👎.
| const fastest = successfulMetrics.reduce((prev, current) => | ||
| prev.latencyMs < current.latencyMs ? prev : current, | ||
| ); |
There was a problem hiding this comment.
Handle all-failure comparisons without throwing
If every model rejects or times out (or the input model list is empty), successfulMetrics is empty and this reduce throws before a ModelComparisonResult can be returned. That breaks the advertised failure-handling path because callers lose the collected failure metrics instead of receiving a result with successCount: 0.
Useful? React with 👍 / 👎.
| if (config.useJitter) { | ||
| const jitter = delay * Math.random(); | ||
| delay = delay + jitter; |
There was a problem hiding this comment.
Cap jittered delays at maxDelayMs
The delay is capped before jitter, then jitter adds up to another full delay on top of that. With the default useJitter: true, a policy with maxDelayMs: 10000 can sleep for almost 20 seconds, which violates the documented maximum delay and makes the predefined policy caps unreliable.
Useful? React with 👍 / 👎.
| public reset(): void { | ||
| this.tokenCount = 0; | ||
| this.content = ""; | ||
| this.lastUpdateTime = Date.now(); |
There was a problem hiding this comment.
After reset() is used to reuse a tracker, tokenCount and content are cleared but the immutable startTime remains the original constructor time. The next stream's getProgress() and complete() therefore include all idle time before the reset, overstating elapsed time and underreporting tokens/sec.
Useful? React with 👍 / 👎.
|
I have read the CLA Document and I hereby sign the CLA |
|
recheck |
Overview
This PR adds four major feature enhancements to the lmstudio-js SDK to improve reliability, performance, and usability. These features are designed to work independently but complement each other when used together.
Changes
1. Enhanced Error Handling & Retry Logic
packages/lms-common/src/retryPolicy.tsClientPort.callRpc()for automatic RPC retry support2. Model Caching Layer
packages/lms-client/src/cache/ModelMetadataCache.tsgetOrCompute()for lazy loading with automatic cachingCacheManagerfor managing multiple named caches3. Streaming Response Progress
packages/lms-client/src/streaming/ProgressTracker.ts4. Model Comparison Tools
packages/lms-client/src/tools/ModelComparison.tsImplementation Details
Quality & Testing
Documentation
Integration Points
packages/lms-communication-client/src/ClientPort.tsto support retry policiespackages/lms-common/src/index.tsto export new retry utilitiesFiles Changed
Total: 2,518 lines added (features, tests, documentation)