feat!: generated request models validate requests before the wire (0.3.0) - #13
Merged
Conversation
Request models need to send exactly what the caller set: server defaults must keep governing omitted fields, and PUT /llm-providers relies on an explicit null api_key meaning "keep the stored key", so to_wire() excludes unset fields rather than None values. Unknown fields pass through so the SDK can lag the platform without blocking callers. datamodel-code-generator joins the SDK dev group for the generator.
The two users of ignore_params existed only to hide a kwarg-to-body rename (email.find_batch) and a shared wrapper over two routes (segment). Both go away once request models carry the exact field names, so the stamp shrinks to (method, path, openapi). Moved the two per-route exclusions into check_contract.py's global IGNORE_PARAMS set so the contract check keeps passing until Task 14 rewrites the request side.
The platform's pydantic request models import geo/propelauth/litellm and ship no shared package, so the SDK generates its own from the OpenAPI spec instead. Query-param routes get a synthesized <Resource><Method>Params schema; JSON-body routes reuse the platform component name. anyOf-null and per-item constraints are folded so datamodel-codegen emits plain annotated fields instead of RootModel wrappers, and deprecated params stay out as they have since 0.1.1.
Popping from a set left the transitive schema order dependent on PYTHONHASHSEED, so the generated class order could differ between runs and --check would report drift against a file nobody had touched. Only one schema is pulled transitively today, so this was latent rather than live, but it arms itself the moment a request component gains two sibling refs.
Generated from the dev spec: prod still exposes the discogen, validate, contacts-generate and provider bodies under FastAPI's inline Body_* names, which dev has already renamed. The release PR to main stays blocked by gen_requests.py --check until that deploys, the same gate check_contract.py imposes today.
Enum-valued params (match mode, public link source) are now rejected locally with a pydantic ValidationError instead of a server 422.
min_match_confidence's 50-100 range is now enforced client-side; the hand-written 'only send it when set' branch goes away because to_wire already sends only what the caller set.
The 39-parameter discover/search signatures collapse into ContactFilters / ContactsSearchParams, and bulk_match now validates each query item instead of forwarding raw dicts.
find_batch drops its contacts= shim; FindEmailBatchRequest.requests is the platform's own field name and validates each contact up front.
save_results' action enum and the tag/name length limits now fail locally; query_id stays a keyword argument since path params are not part of the body schema.
LLMProviderUpdateRequest carries the platform's required-but-nullable api_key, so the hand-rolled 'always send api_key' branch is gone: to_wire sends whatever the caller set, null included.
Domain/persona list bounds (1-10000), context_mode and search_context_size enums are checked before the job is submitted.
The 41-parameter signature was copied three times (resource, sync client, async client); DiscoverParams is generated from the spec, so new filters no longer need four hand edits.
…_file GET /segment and POST /segment are different routes with different params; one wrapper over both is why ignore_params existed. Each gets its own method and model. domains is a comma-separated string, as the spec declares it, instead of a list the SDK joined behind the caller's back.
check_contract.py reads each method's request model off its annotation and diffs the field set against the spec's query params and body properties, so a platform field the SDK never declared fails the run just like an SDK field the platform dropped. gen_requests.py --check catches the generated file going stale on the same schedule. Regenerating for that new gate reorders two classes in requests.py; the line multiset is unchanged.
…exit 2 Bad option and --param values now surface as the same ValidationError exit-2 path a server 422 takes, before any request is sent. A bare --param string for a list field is wrapped the way the API reads it.
Option values are validated against the spec before the request goes out; --param keeps working as the escape hatch and now also validates known keys instead of forwarding them blind.
Seniority/department/industry values are checked against the spec's enums locally; the tests that relied on made-up values now use real ones.
llm-providers update keeps sending an explicit null api_key: the platform reads null as 'keep the stored key' and the field is required.
…odels segment dispatches to segment/segment_file explicitly instead of relying on the removed SDK wrapper; --domain values are joined into the comma-separated string the spec declares. call_typed goes with the last call site: the SDK no longer raises TypeError for unknown kwargs, and unknown --param keys pass through by design.
Breaking SDK surface (models instead of kwargs) warrants the minor bump; CLI pins the SDK exactly as before.
…mple Missed in the 0.3.0 doc sweep — discover() now takes DiscoverParams, not icp_text= directly.
…ange, and dev-spec drift check CHANGELOG was silent about two 0.3.0 behavior changes callers will hit: append now requires --dataset client-side, and GET /segment sends query_id as repeated params instead of comma-joined. README/CONTRIBUTING didn't explain that gen_requests.py --check (like check_contract.py) only passes against the dev spec until the platform deploys prod.
…rated file --check was leaking ruff's "Fixed N errors" lines into its own stdout because the repo sets show-fixes=true; pass --no-show-fixes explicitly. Also, importing discolike.resources at module scope meant deleting the committed requests.py crashed with a traceback instead of the intended "stale; run: ..." message, since check() never got a chance to guard for it. Deferred those imports into the functions that need them so a missing file is reported cleanly.
…d missing annotations test_email_find_batch_rejects_more_than_500_before_any_request duplicated the CLI test above it without exercising the guard its name claimed (MAX_BATCH_CONTACTS fires first). Retargeted it as a direct FindEmailBatchRequest.model_validate test. Also: discover.py's ANN401 noqa said the kwargs are forwarded to typed resource/client methods, but they go to build_request as a dict; fixed the comment. Added -> None return annotations to test_gen_requests.py and test_requests_module.py to match the rest of the suite, and dropped an unused routes fixture arg.
The deferred-import workaround for a deleted committed requests.py traded a stale repo convention violation for a marginal UX improvement. A missing _generated/requests.py means a broken checkout; the ModuleNotFoundError is the honest failure and imports belong at module scope. Keeps the --no-show-fixes fix from the same pass.
…g identity fields EmailJobResult.result is EnumerationOutput | ValidationOutput | None because the model is shared by find and verify batches. The example read find-only fields off the union, which ty rejects; isinstance narrowing replaces the getattr workaround.
--contact already rejects blank parts locally, but a CSV row with an empty first_name/last_name/domain was blanked to "" and sent to the API, which rejected it server-side. The spec puts no min_length on those fields, so the generated model cannot catch it; the CSV reader has to.
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.
Why
Bad requests currently fail as server 422s. Every SDK method now takes one generated pydantic request model, so shape errors (missing required field, out-of-range value, unknown enum) raise
pydantic.ValidationErrorlocally before any HTTP call. The server stays the authority on cross-field/semantic rules; the client validates shape.Models are generated from the platform OpenAPI spec rather than mirrored from platform pydantic sources — those import
geo,constants,propelauth,litellmand have no shared package.What changed (0.3.0, breaking)
scripts/gen_requests.py: walks the@api_routeregistry, keeps platform names for JSON bodies (FindEmailRequest,ContactFilters, …), synthesizes<Resource><Method>Paramsfor query routes (MatchCompanyParams,DiscoverParams), runs datamodel-code-generator, ruff-formats,--checkfor drift. Output committed todiscolike/_generated/requests.py, re-exported viadiscolike.requests.DiscolikeRequestbase:extra="allow"(unknown fields pass through — escape hatch when the SDK lags the platform),to_wire()=exclude_unsetso explicitNonereaches the wire (LLMProviderUpdateRequest.api_keyis required-nullable).client.match.company(MatchCompanyParams(name=...)). Path params andfilestay keyword-only.enrich.segmentsplit intosegment(GET) /segment_file(POST multipart).check_contract.py: request side reads the model off the annotation, bidirectional field diff. CI also runsgen_requests.py --checkunder the sameDEV_SPEC_URL/prod fallback.build_request; pydantic errors exit 2;--param KEY=VALUErides onextra="allow".appendnow requires--dataset.--contacts-filerejects empty cells locally.Gates
check_contract.pygen_requests.py --checkBody_*inline names)Prod failures are the known platform deploy lag (same gate as #12): the committed models track dev until prod ships
DiscoGenProcessRequest,LLMProviderCreateRequest,SearchProviderRequest, etc. Documented in README/CONTRIBUTING so nobody "fixes" it by regenerating from prod.pytest: 385 passed. ruff, ruff format, ty clean (incl.examples/,scripts/).Not in this PR
gen_requests.pydrops query params on a route that also has a JSON body (none exist;check_contractwould flag one)._merge_paramslives indiscover.py, imported by 8 CLI modules.examples/is outside CI'sty checktarget set.Greptile Summary
This breaking 0.3.0 release replaces SDK method keyword arguments with generated Pydantic request models, adding local request validation and consistent serialization before transport.
Confidence Score: 5/5
The PR appears safe to merge based on the reviewed changes, with no concrete actionable defects identified.
The generated request boundary, resource serialization, synchronous and asynchronous paths, CLI validation handling, and contract-generation workflow remain internally consistent, while the notable production-spec fallback and breaking input changes are explicitly documented.
Important Files Changed
Reviews (1): Last reviewed commit: "fix(cli): reject empty cells in --contac..." | Re-trigger Greptile
Context used (4)