QVAC-21805 feat: generate Python pydantic models + typed method stubs from the SDK contract#3100
Conversation
02d252a to
3bd44a1
Compare
|
Rebased onto the updated Added real progress support on the Python side:
|
|
Simplified the progress e2e test per feedback: dropped the ad-hoc local HTTP server. It now points |
6f8b5dd to
ad72019
Compare
|
Added the two Tier-2 API-parity pieces (from the JS-vs-Python gap audit, tracked in Asana):
Also rebuilt this worktree's own Not in scope here (tracked separately under the "Full-fidelity" epic per user direction): |
|
Reduced the PoC to exactly what it should be: The demo functions build typed requests and get typed pydantic responses back via Verified for real against a spawned worker: heartbeat, completion (via the registry constant), embed, transcribe, and textToSpeechStream all produce correct typed output. One honest gap: |
2a4667c to
4b35cec
Compare
55362e7 to
eff8999
Compare
|
Rebased onto the updated |
|
Added Verified for real against a spawned worker: loads |
|
Converted the whole Python SDK to asyncio-native, matching the JS SDK's Promise/async-iterator model, rather than bridging a sync client API onto an async wire library (relevant for QVAC-21806's
Verified for real against a spawned worker: heartbeat, |
317191a to
ea37864
Compare
fd3f50b to
feb6336
Compare
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
…, docstring) - Widen CI's black --check to src/qvac (excluding _generated/models/) instead of an explicit file list that omitted api.py/methods.py/ models.py/schemas.py -- methods.py was already failing black unnoticed. Reformat it now that the check actually covers it. - Fix stale CI comment describing the PoC tests' skip condition (they gate on a built worker existing, not just the env var). - Update README's method-stub examples to their real async signatures; note qvac.schemas also exports the constant enums. - Add an explanatory comment to the empty except in QvacWorker.close() (best-effort teardown, timeout non-fatal).
…, docstring) - Widen CI's black --check to src/qvac (excluding _generated/models/) instead of an explicit file list that omitted api.py/methods.py/ models.py/schemas.py -- methods.py was already failing black unnoticed. Reformat it now that the check actually covers it. - Fix stale CI comment describing the PoC tests' skip condition (they gate on a built worker existing, not just the env var). - Update README's method-stub examples to their real async signatures; note qvac.schemas also exports the constant enums. - Add an explanatory comment to the empty except in QvacWorker.close() (best-effort teardown, timeout non-fatal).
7d00ecf to
e2578ae
Compare
arun-mani-j
left a comment
arun-mani-j
left a comment
There was a problem hiding this comment.
Looks good overall, some minor nits.
…egistry constants from the SDK contract
- scripts/generate.py runs datamodel-code-generator against packages/sdk/contract/{schema.json,manifest.json,models.json}
to produce pydantic models (_generated/models/**) and one typed stub per RPC method (_generated/methods.py),
grouped by call shape (request-reply / server-stream / duplex)
- typed constant enums (ModelType, PluginId, ...) generated from the contract's model registry
- public re-export namespaces (qvac.models, qvac.methods, qvac.schemas) over the generated internals
- CI: pr-checks-sdk-python.yml runs generate.py --check to catch drift between the contract and committed output
…PI wrapper layer - qvac._transport.Transport: asyncio Protocol defining call/call_stream/call_duplex, the abstract surface the generated method stubs call through - qvac.api: hand-written convenience wrappers mirroring the JS SDK's client-side API (options split from wire params, e.g. onProgress/logger callbacks)
…ode-mirroring quickstart - poc_heartbeat.py / poc_transport.py: a Transport implementation over the SDK worker's wire protocol, used to smoke-test the generated stubs end-to-end against a real worker - test_poc_smoke.py / test_poc_progress.py: unary and progress-streaming round trips (e.g. a real streamed loadModel) against a spawned worker, using registry:// modelSrc - python_quickstart.py mirrors the SDK's Node quickstart
… methods - test_poc_methods.py converts poc_heartbeat.py's manual demo functions (embed, transcribe, completion_stream, transcribe_stream, text_to_speech_stream) into real pytest assertions against a spawned worker, closing part of the test-coverage gap flagged in review. - Fixes transcribe_stream along the way: the wire format is 16kHz mono s16le PCM fed at roughly real-time cadence, not f32 fed as fast as possible -- parakeet's streaming session only decodes audio paced close to real time (confirmed against transcription-parakeet's own duplex-streaming tests) and never emits partials otherwise, which is exactly the "one honest gap" this PR's own history already flagged.
…method Adds real end-to-end tests (against a spawned worker, real models, real fixtures) for: classify, ocr_stream, translate, rag (ingest + search + workspace cleanup), diffusion_stream, upscale_stream, bci_transcribe / bci_transcribe_stream, get_model_info, get_loaded_model_info, download_asset, and finetune. - classify needs an empty-string modelSrc to hit the addon's bundled weights fallback (`config.modelPath ?? (params.modelPath || undefined)`) -- any non-empty placeholder is treated as a literal path and fails to resolve, since the plugin skips normal modelSrc resolution (`skipPrimaryModelPathValidation`). - RagRequest/RagResponse are RootModel unions over `operation`, unlike most other methods' flat response shapes -- construct requests via the top-level RagRequest wrapper and unwrap `.root` on responses. - finetune requires an explicit learningRate: omitting it hits a native GGML_ASSERT(opt_pars.adamw.alpha > 0.0f) crash with no addon-side default. Mirrors the SDK e2e suite's own known-good finetune-executor.ts config (tiny fixture dataset, 1 epoch). - video_stream is deliberately not covered here: its cheapest legal config still loads an ~11GB text encoder with no existing e2e precedent to validate against; left as a known gap rather than guessing at a slow, unverified test.
Rebased onto upstream/main, which had moved forward with unrelated sdcpp-generation config additions (main-gpu GPU pinning, LTX-2 video layout support) since this branch forked. CI's pull_request check tests the merge of this branch with main, not this branch's tip alone, so the committed _generated tree needs to reflect main's contract changes too -- regenerated and verified against the merged state.
datamodel-code-generator already runs black internally by default (--formatters defaults to [black, isort]); the only reason its output disagreed with a bare `black` invocation was single- vs double-quote normalization. Pass --use-double-quotes to the generator so its own black pass matches ours exactly, then drop the black-check workflow's _generated/models/ exclusion entirely -- one consistent style across generated and hand-written code, verified with a fresh --use-double-quotes run diffing byte-for-byte against a plain `black` pass (only quote characters change, same line count).
The previous fix hand-wrote the name->value record (MP3/M4A/etc), which
review correctly flagged as a second source of truth that could drift
from @/constants/audio's SUPPORTED_AUDIO_FORMATS array. Derive the
record programmatically instead (".mp3" -> "MP3") so there's exactly
one place the format list lives. Verified byte-identical contract
export output and a clean sdk typecheck.
`license = { text = "Apache-2.0" }` is deprecated per current PyPA
guidance; the SPDX string form is now recommended and correctly
emits a `License-Expression` metadata field (verified by building the
wheel and inspecting METADATA).
Rebased onto upstream/main, which picked up "accept non-string JSON Schema enum values in tool schemas" (#3278) -- widens CompletionStreamRequest/BatchCompletionStreamRequest's tool-parameter enum field from list[str] to list[str | float | bool | None]. Regenerated and verified against the merged contract.
- ruff (E/F/I/UP, E501 ignored -- black owns wrapping) and mypy (check_untyped_defs, pydantic.mypy plugin) as new dev/gen extras - generate.py: --use-annotated + --base-class qvac._generated_base (fixes conint()/constr() used as bare type annotations, and coerces enum-typed fields' JSON-Schema-literal defaults via validate_default) - generate.py now runs ruff's import-sort fix on datamodel-code-generator's output, since --base-class's injected import isn't isort-clean on its own - py.typed marker so mypy can check the installed package by name - apply ruff's auto-fixes (import order, typing.AsyncIterable/AsyncIterator -> collections.abc) across hand-written src/tests files; noqa the intentional `import *` + `__all__` re-export pattern in methods.py/models.py - CI: add lint (ruff) and typecheck (mypy) steps; document tooling in README
- test_poc_methods.py: narrow RagResponse.root (a 9-member union) with isinstance before accessing operation-specific fields -- .processed/ .results only exist on RagResponseIngest/RagResponseSearch, not the other variants - test_api.py: FakeTransport.call_duplex wasn't a generator (no yield), so it structurally returned Coroutine instead of AsyncIterator and didn't conform to the Transport protocol it's meant to fake; add a None-check before indexing transport.sent - poc_heartbeat.py: assert self._writer/_reader are set before use in _send_frame/_recv (both only ever called after start() connects them); cast embedding to its flat shape (request always sends a single string) - python_quickstart.py: assert model_id is set after the load loop (previously could stay None if no terminal reply arrived, then get passed straight into unload_model); rename the completion-loop's `event` to `delta_event` so it doesn't collide with the load loop's - test_generate.py: assert spec_from_file_location's result isn't None before use; fix a pytest fixture's return annotation (Path -> Iterator[Path]) to match that it's actually a generator
2e47338 to
765abea
Compare
Line wrap left over from the SupportedAudioFormat derive-from-source change was failing the sdk pod's prettier --check. No logic change.
…oolDialect Main merge pulled in QVAC-22274's getLoadedModelInfo contract change (optional toolDialect enum on LocalLoadedModelInfo); regenerate so the committed client matches contract/schema.json again.
86f09a7
…gates Rebasing onto merged #3100 pulls in ruff+mypy CI for sdk-python; the bare-rpc transport predates it. Bring it into line: - ruff: typing.AsyncIterable/AsyncIterator/Sequence -> collections.abc, drop redundant forward-ref quotes, sort the test import block - mypy: assert post-connect invariants (rpc/writer set) and add a _require_rpc guard so the Transport methods don't deref Optional; add a scoped ignore_missing_imports override for the optional, git-only bare_rpc/compact_encoding extras - test: load the Qwen3 completion model with an explicit n_ctx -- it's a thinking model, so the worker reserves context for the reasoning trace and the metadata-default budget overflowed even a tiny prompt
3 participants
🎯 What problem does this PR solve?
📝 How does it solve it?
packages/sdk-python.scripts/generate.pyrunsdatamodel-code-generatoragainstcontract/schema.json/manifest.json/models.jsonintosrc/qvac/_generated/models/, resolves each manifest method's request/response title to wherever it landed, then renders a flat__init__.pyre-export andmethods.py— one typed function per manifest entry, grouped by call shape (request-reply / server-stream / duplex), plus typed constant enums (ModelType,PluginId, ...) from the contract's model registry:qvac._transport.Transportis an asyncioProtocoleach stub calls through (call/call_stream/call_duplex). Doesn't implement the actual socket transport — that's QVAC-21806 (bare-rpc-python integration, QVAC-21806 feat: prototype bare-rpc-python as the wire transport #3208), stacked on this branch; anything implementing the protocol can back the stubs.qvac.api: hand-written convenience wrappers mirroring the JS SDK's client-side API surface (options split from wire params, e.g.onProgress/loggercallbacks).generate.py --checkfails clearly on drift..github/workflows/pr-checks-sdk-python.yml: standalone workflow (nopackage.json, can't joinpr-checks-sdk-pod.yml) runninggenerate.py --check, a scopedblack --check, and pytest.🧪 How was it tested?
poc_heartbeat.py/poc_transport.py) implements theTransportprotocol and smoke-testsheartbeat/state/progress-streaming (a real streamedloadModelviaregistry://modelSrc) against a real spawned SDK worker — round-trips correctly. Skipped unlessQVAC_POC_SDK_DIRis set, never blocks CI.allOf-of-union not merging, and meaningless positional names on nested schemas.Dependencies
main— the contract-export work (QVAC-21804 feat: contract export — Zod schemas to JSON Schema + method manifest #3085) is merged. This branch was rebased onto currentmainand squashed to drop the 13 commits that duplicated QVAC-21804 feat: contract export — Zod schemas to JSON Schema + method manifest #3085's now-merged content.