-
Notifications
You must be signed in to change notification settings - Fork 1
SDK API Redesign - Clearer and More Intuitive API #73
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
This commit implements a redesigned SDK API that addresses several issues with the current design: **Issues Fixed:** - Confusing naming: "process" didn't clearly indicate sync operation - Unnecessary nesting: queue.* methods created extra API surface - Type constraints mismatched with actual capabilities - Unclear callback naming: onStatusChange vs onProgress **New API:** - client.generate() - synchronous generation (replaces process()) - client.submit() - async job submission (replaces queue.submit()) - client.submitAndWait() - async with auto-polling (replaces queue.submitAndPoll()) - client.getJobStatus() - check job status (replaces queue.status()) - client.getJobResult() - get job result (replaces queue.result()) - onProgress callback - more intuitive than onStatusChange **Key Features:** - Full backward compatibility: old API still works but deprecated - Clearer method names that communicate intent - Flatter API structure without unnecessary nesting - Future-proof: organized by capability (sync/async) not model type - Comprehensive tests for new API - Migration guide and examples **Files Changed:** - packages/sdk/src/index.ts: Add new unified API methods - packages/sdk/src/shared/unified-types.ts: New type definitions - packages/sdk/README.md: Document new API with examples - packages/sdk/tests/unit.test.ts: Add comprehensive test coverage - examples/: New example files demonstrating new API - SDK_REDESIGN_PROPOSAL.md: Design proposal and rationale - MIGRATION_GUIDE.md: Complete migration guide **Testing:** All existing tests pass + new tests for new API methods. Builds successfully with no TypeScript errors. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Remove unused import of 'z' from zod in unified-types.ts - Fix type safety in submitAndWait by using proper generic types - Remove unnecessary 'as any' type assertions - All linting checks now pass with no warnings Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit addresses all feedback from PR review: **1. Remove Redundant Type Definitions** - Replaced `SyncCapableModelDefinition` and `AsyncCapableModelDefinition` with single `GenerationCapableModelDefinition` type - Both were identical, causing confusion about their purpose - New unified type clearly indicates models support both sync and async **2. Remove Unnecessary Async Wrapper** - Changed `generate` from async wrapper to direct assignment: `const generate = process` - Consistent with other method aliases like `submit = queue.submit` - Eliminates unnecessary overhead **3. Extract Duplicated Type Helpers** - Created shared `type-helpers.ts` with generic type manipulation utilities - Replaced duplicate `PickDocumentedInputs` and `MergeDocumentedInputs` in 3 files (process/types.ts, queue/types.ts, unified-types.ts) - New generic `PickDocumentedFields<TDocumented, TInferred>` and `MergeDocumentedFields<TDocumented, TInferred>` helpers - Reduces maintenance burden and ensures consistency **Files Changed:** - packages/sdk/src/shared/type-helpers.ts (new) - shared type utilities - packages/sdk/src/process/types.ts - use shared helpers - packages/sdk/src/queue/types.ts - use shared helpers - packages/sdk/src/shared/unified-types.ts - use shared helpers, consolidate types - packages/sdk/src/index.ts - update exports, fix generate method **Testing:** - All 64 tests still passing - TypeScript compilation successful - Linting and formatting clean Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
| * Optional `AbortSignal` for canceling the request. | ||
| */ | ||
| signal?: AbortSignal; | ||
| } & MergeDocumentedFields<ProcessInputs & ModelSpecificInputs<T>, InferModelInputs<T>>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate type definitions for GenerateOptions and SubmitOptions
Low Severity
GenerateOptions and SubmitOptions are structurally identical types with the exact same definition: { model: T; signal?: AbortSignal; } & MergeDocumentedFields<ProcessInputs & ModelSpecificInputs<T>, InferModelInputs<T>>. The only differences are the JSDoc comments. SubmitOptions could be defined as a simple type alias (e.g., type SubmitOptions<T extends GenerationCapableModelDefinition = GenerationCapableModelDefinition> = GenerateOptions<T>) to reduce code duplication and maintenance burden.
Additional Locations (1)
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
| realtimeClientRef.current?.disconnect(); | ||
| }; | ||
| }, []); | ||
| }, [prompt]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding prompt dependency causes unnecessary reconnection on change
Medium Severity
Adding prompt to the first useEffect's dependency array causes the entire connection setup, including camera access, to re-run on every prompt change. This results in unnecessary reconnections, visual flickering, repeated camera permission requests, and a camera resource leak because the previous stream isn't properly stopped.
ref API-667
Summary
Redesigned the SDK API to provide clearer method names, flatter structure, and better organization while maintaining full backward compatibility.
What Changed
New API Methods
client.generate()- synchronous generation (replacesprocess())client.submit()- async job submission (replacesqueue.submit())client.submitAndWait()- async with auto-polling (replacesqueue.submitAndPoll())client.getJobStatus()- check job status (replacesqueue.status())client.getJobResult()- get job result (replacesqueue.result())onProgresscallback - more intuitive thanonStatusChangeOld API (Deprecated but Working)
client.process()- deprecated, useclient.generate()client.queue.*methods - deprecated, use new flat methodsonStatusChange- deprecated, useonProgressWhy This Change?
The previous API had several issues:
queue.*methods created extra API surfaceonStatusChangewasn't as intuitive asonProgressExample: Before and After
Before (Old API)
After (New API)
Files Changed
packages/sdk/src/index.ts- Add new unified API methodspackages/sdk/src/shared/unified-types.ts- New type definitionspackages/sdk/README.md- Document new API with examplespackages/sdk/tests/unit.test.ts- Add comprehensive test coverageexamples/- New example files demonstrating new APISDK_REDESIGN_PROPOSAL.md- Design proposal and rationaleMIGRATION_GUIDE.md- Complete migration guideBackward Compatibility
✅ All old methods still work!
✅ No breaking changes
✅ Gradual migration supported
The old API is marked as deprecated and will be removed in v1.0.0.
Testing
Documentation
MIGRATION_GUIDE.mdSDK_REDESIGN_PROPOSAL.mdNext Steps
🤖 Generated with Claude Code
Note
Introduces a flatter, clearer SDK API while preserving backward compatibility.
client.generate()(sync),client.submit(),client.submitAndWait(),client.getJobStatus(),client.getJobResult();submitAndWaitmapsonProgress(and supports deprecatedonStatusChange)shared/unified-types.ts,shared/type-helpers.ts) and updatesprocess/queueoption types to merge schema-inferred types with documented fieldspackages/sdk/src/index.ts; keeps deprecatedclient.process()andclient.queue.*aliasesMIGRATION_GUIDE.mdandSDK_REDESIGN_PROPOSAL.mdpromptWritten by Cursor Bugbot for commit f626b32. This will update automatically on new commits. Configure here.