feat: allow custom models#51
Merged
Merged
Conversation
8c032d6 to
fdae6f3
Compare
fdae6f3 to
b846107
Compare
Drop the `models.custom(...)` factory in favor of constructing `ModelDefinition(...)` directly, matching how the JS SDK exposes custom models via the `CustomModelDefinition` type. - `input_schema` becomes truly optional (`Optional[type[BaseModel]] = None`) instead of using `BaseModel` as a sentinel. - Add `queue_url_path` to `ModelDefinition`; populate it on the video and image registry entries and use it from `submit_job`. Submitting a model without `queue_url_path` raises `InvalidInputError` (mirrors the JS check in `queue/request.ts`). - Video registry `url_path` flips to `/v1/generate/<name>` to align with the JS SDK; queue submissions now go through `queue_url_path`.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 985db94. Configure here.
Each model now has a single `url_path` field that points at its primary
endpoint:
- realtime: `/v1/stream`
- image: `/v1/generate/<name>`
- video: `/v1/jobs/<name>`
Queue, process, and realtime clients all submit/connect to
`{base_url}{model.url_path}`. This drops the JS-style split between
`urlPath` and `queueUrlPath` — JS only carries both because TypeScript
forces every model to have a `urlPath`. In Python a single field
suffices, since video models are queue-only and image models are
process-only via the registry.
Custom models continue to work: pass whichever endpoint is appropriate
for the API you intend to call (e.g. `url_path="/v1/jobs/<name>"` for a
custom queue model).
The realtime unit test and two docstrings still referenced the removed `models.custom()` factory. Switch the test to constructing `ModelDefinition` directly and update the docstrings to match. Resolves the CI failure on Python 3.11 (and the Cursor Bugbot review).
Drop the "Custom models can omit an input schema..." comments — the `if model.input_schema is None:` branch is self-explanatory. Also tighten docstrings the PR over-extended: - `DecartClient.queue` / `QueueClient`: drop the second-line "SDK accepts any model definition..." preamble - `DecartClient.process`: drop the "backend validates whether the model supports this API" line and over-broad model list - `QueueClient.submit`: drop the "Video models and custom queue model definitions are supported" line - `CustomModelDefinition`: collapse the two-paragraph docstring to one line
Remove four tests that asserted process() accepts video/realtime ModelDefinitions and queue.submit() accepts image/realtime ModelDefinitions. They mocked the underlying request layer to pin "the SDK does not gate by registry bucket", but they don't reflect real usage — those calls would 404 against the backend because the url_path of one model is wrong for the other API.
- test_models.py: drop the validates-required-shape test (Pydantic's own concern), and trim the over-asserting custom-model test to the meaningful invariants (str name accepted, input_schema defaults None). - test_process.py: drop test_process_allows_custom_model_definition_for_realtime_url_path, same cross-bucket weirdness as the tests just removed. - test_queue.py: drop test_queue_custom_model_uses_url_path (URL construction is identical for registry and custom models, already covered by test_queue_includes_user_agent_header). Refocus the bouncer-error test as test_queue_submit_surfaces_backend_error using a registry video model — the "custom" framing was incidental. - Drop the misleading "accepts any model definition" file-level notes.
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.

Summary
Lets callers use models that aren't in the SDK's built-in registry (preview, experimental, private) by constructing a
ModelDefinitiondirectly. Mirrors the JS SDK'sCustomModelDefinition.Usage
Built-in helpers stay the same:
For non-registry models, construct a
ModelDefinitiondirectly:If
input_schemais omitted, inputs are forwarded as-is and the backend validates them.Validation
pytest -q— 128 passed (135 withdecart[realtime]extras)ModelDefinitionboth succeed viaclient.process()Note
Medium Risk
Expands supported model definitions beyond the SDK registry and changes request routing to rely on
model.url_path, which could surface new runtime errors if callers supply incorrect paths or inputs.Overview
Adds support for custom/non-registry models by letting callers construct
ModelDefinitiondirectly (exported asCustomModelDefinition) and makinginput_schemaoptional for passthrough validation.Updates
DecartClient.process()andQueueClient.submit()to accept anyModelDefinition[str], skip registry checks, and (when no schema is provided) forward non-Noneinputs as-is; queue submission now posts tobase_url + model.url_pathinstead of/v1/jobs/{model.name}. Realtime connect typing is loosened to accept arbitrary model names, and tests are updated/added to cover custom models and backend error surfacing.Reviewed by Cursor Bugbot for commit de8805b. Bugbot is set up for automated code reviews on this repo. Configure here.