fix: resolve model alias naming conflict in SDK generation#109
Merged
Conversation
Fixes compilation error caused by naming collision between the models package and the Models SDK class. Root cause: - New model alias endpoints (/models/aliases/*) were missing the x-speakeasy-group override - Speakeasy generated separate Models class in models_.py - Created conflict: `from opperai import models` vs `self.models: "Models"` - Result: pylint error "undefined variable 'models'" in sdk.py:252 Solution: - Added x-speakeasy-group: language_models to all 6 alias endpoints: * create_alias, list_aliases, get_alias * update_alias, delete_alias, get_alias_by_name - All model operations now unified under LanguageModels SDK - Eliminated models_.py and naming conflict Changes: - Modified .speakeasy/speakeasy-modifications-overlay.yaml - Deleted src/opperai/models_.py - All model alias methods moved to language_models.py - SDK compiles successfully, version bumped to 1.6.3
There was a problem hiding this comment.
Opper Python SDK – PR #109 Review
| Area | Observation | Recommendation |
|---|---|---|
| Naming-conflict fix | Deleted src/opperai/models_.py and grouped alias endpoints via x-speakeasy-group. Build now passes. |
✅ Looks good – resolves compile error. |
| Parameter rename | embeddings.py (input_ → input) shadows Python built-in. |
Non-blocking: consider input_data for clarity & to keep built-in usable. |
| Dataclass immutability | Error classes changed from frozen=True to unsafe_hash=True, making them mutable. |
Ensure no code relies on previous immutability; otherwise fine. |
| OAuth2 scopes | Default oauth2_scopes arg switched from [] → None. |
👍 Cleaner default; no issues. |
| Docs & version | Extensive doc regeneration and version bump 1.3.0 → 1.6.4. | Confirm downstream clients pin >=1.6.4 before release. |
Overall the patch is solid, fixes the blocking compile issue, and introduces helpful rerank & alias docs. Suggestions above are non-blocking.
Status: Approving – great work!
Reviewed by agentobot[bot]
There was a problem hiding this comment.
Addendum – Minor Follow-Up Suggestions
| Area | Observation | Recommendation |
|---|---|---|
| Version bump text | _version.py sets __version__ = "1.6.4", while PR body mentions 1.6.3 |
Update PR description or bump note to 1.6.4 so changelog matches code. |
| Error mapping | entries.py handles 400/401/404/422/500 but omits 409 & 429 |
Consider mapping HTTP 409 (Conflict) and 429 (Rate Limit) to existing ConflictError / custom error for completeness. |
| Built-in shadowing | embeddings.py uses parameter name input |
Non-blocking: rename to input_data to avoid masking built-in input(). |
These are non-blocking and do not affect approval status already granted. Thanks for the quick fix!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The SDK failed to compile with error:
This was caused by a naming collision between:
modelspackage (for data types/models)ModelsSDK class (for model alias management)Root Cause
When the new model alias endpoints (
/models/aliases/*) were added to the API, they were missing thex-speakeasy-group: language_modelsoverride in the Speakeasy overlay configuration. This caused Speakeasy to generate a separateModelsSDK class inmodels_.py, creating a conflict with the existingfrom opperai import modelspackage import.Solution
Added
x-speakeasy-group: language_modelsto all 6 model alias endpoints in.speakeasy/speakeasy-modifications-overlay.yaml:create_aliaslist_aliasesget_aliasupdate_aliasdelete_aliasget_alias_by_nameThis unifies all model operations under the
LanguageModelsSDK class, eliminating the naming conflict.Changes
.speakeasy/speakeasy-modifications-overlay.yamlwith missing group overridessrc/opperai/models_.py(conflicting Models SDK)language_models.pyTesting
python -m py_compile)Release Notes
This fix resolves a critical compilation error and should be released ASAP. After merge, the automated release workflow will publish v1.6.3 to PyPI.