Ship py.typed and hold the SDK to strict typing - #48
Merged
Conversation
Covers the three cases Steven flagged as untested and credential-free: - PARAMETER_FIELDS keeping the Schema name when the Python field is escaped or snake_cased - camelCase parameters becoming snake_case fields with the wire key unchanged - to_input() / serialisation shape, including omission of unset fields The to_input tests execute BASE_CLASS_SOURCE rather than importing the committed fragment/sdk/typed_entries, so they test what codegen emits now instead of whenever it last ran. Subclassing the generated module first meant removing `description=self.description` from the renderer left all tests green. Claude-Session: https://claude.ai/code/session_01J1SSSkp8AgFGobLLaEAN6K
Widening the entries argument to Sequence let a tuple through the type
checker, but ariadne's base client only recurses into variables with
isinstance(value, list). A tuple skipped conversion and reached json.dumps
holding model objects:
list -> {"entries": [{"entry": {...}, "ik": "a"}]}
tuple -> TypeError: Object of type OrderPlacedV1 is not JSON serializable
The plugin now also rewrites the method body's variables assignment to
{"entries": list(entries)}, so tuples, generators and mixed sequences all
serialise. Warns if that assignment cannot be found, matching the other
two warnings in this plugin.
resolve_class_names guards class-name collisions, but nothing guarded field
names within a class. A Schema declaring both user_id and userId rendered
the field twice; pydantic kept the last one and both wire keys took the
same value, with no warning:
{"user_id": "VALUE", "userId": "VALUE"}
_safe_field_name cannot see its siblings, so the check belongs in
_extract_parameters where the parameter list is assembled. Later
collisions take a numeric suffix, the same shape resolve_class_names uses
one level up, and a warning names the parameter that moved.
PARAMETER_FIELDS still maps each Schema name to its own field, so the wire
payload is unchanged:
{"user_id": "SNAKE", "userId": "CAMEL"}
collect_annotations copies annotations off the generated client methods, so
it depends on RewriteUnsetTypeMethodArguments having already collapsed
Union[Optional[X], UnsetType] into Optional[X]. That only held because of
where GenerateTypedLedgerEntries sits in the plugin list, with nothing
saying so. Listed earlier it emitted:
memo: Union[Optional[str], UnsetType] = None
into a module that imports neither name, and the generated SDK failed at
import with NameError.
collect_annotations now raises and names the cause. Raising rather than
warning because the output is an unimportable package, not a model with a
weaker type. helpers.py documents why the order matters.
The typed-entry tests assert on render_module's output as a string, which cannot tell a working module from one that merely contains the right substrings. Nothing imported the per-entry classes, so a duplicated field declaration or an annotation the header does not import passed every test. tests/test_typed_entries_generated.py renders into a throwaway package and imports it, then uses the classes: the optional-parameter branch (no snapshot query has a nullable parameter), field-name collisions, escaped names, and to_entry_inputs. It also exercises every model in the committed snapshot, which is the artifact a customer gets. tests/test_typed_entries_warnings.py covers the paths where codegen degrades rather than fails. Each leaves a working but quietly worse SDK, and the warning is the only signal, so a silent version of any of them passed the whole suite. tests/type_checks/ asserts what a caller sees, which no runtime test can: a runtime test passes just as happily against `entries: Any`. Negative cases are written as `# type: ignore[...]` and warn_unused_ignores makes them fail if the call ever starts passing, so the signature cannot loosen unnoticed. CI ran mypy -p fragment only, leaving both tests/ and the mypy_path setting unchecked. `make typecheck` now covers tests/ as well, and typecheck plus the offline tests run in a job with no secrets -- on forks the credentialed job errors on every test, so nothing ran at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
vigneshwerv
force-pushed
the
claude/typed-entries-test-coverage
branch
from
August 10, 2026 19:13
fbcd5fe to
1a1baa4
Compare
Merging typed-batch-ledger-entries dropped the `pytestmark` line from tests/test_add_ledger_entries.py. Nothing failed loudly: `make unit` deselected 1 test instead of 3, ran the two credential-bound tests offline, and they errored on missing environment variables. conftest now marks anything whose fixture closure includes `credentials`, so the marker follows from what a test actually needs. A new integration test cannot forget it and a merge cannot drop it. The per-module markers are redundant under that rule and are removed, leaving one mechanism.
Without a PEP 561 marker, type checkers skip the installed package outright.
Against a consumer of the published SDK, mypy reports only:
error: Skipping analyzing "fragment.sdk.client": module is installed, but
missing library stubs or py.typed marker [import-untyped]
and then `ik=123`, `schema_key=None` and a call to a method that does not
exist all pass. The typed batch payloads are built so callers get type errors,
and no installed consumer could see one. With the marker, the same file fails
on the nonexistent method.
poetry already ships anything under `packages`, so the marker file alone is
enough; no `include` entry is needed. tests/test_packaging.py asserts the
built wheel carries it, because `poetry install` puts the source tree on the
path and so passes whether or not the build is configured to ship it.
fragment.sdk, fragment.sync_sdk, fragment.client and fragment.exceptions now
typecheck under strict. That took four annotations: `refresh_token` and both
`__init__`s were untyped, and `self.token` was inferred as None, which made
`self.token["expires_in"]` an error. The generated modules already passed.
The codegen package stays on the default settings; it is build tooling, not
something a customer imports.
The strict flags are spelled out rather than `strict = true`, which mypy only
honours globally.
Edits are to fragment/client/, the source ariadne copies into both SDKs; the
four generated copies are updated to match.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The strict settings this branch adds surface an error the SDK already had:
`self.token` is declared `dict | None`, so reading the attribute straight
back after assigning it is not narrowed.
Value of type "dict[str, Any] | None" is not indexable
Assign through a local instead. Applied to both hand-written base clients
and regenerated, which covers the copies in fragment/sdk, fragment/sync_sdk
and the snapshot.
fragment/sync_sdk/async_client.py is patched by hand because nothing
regenerates it: codegen writes sync_client.py for that package, and this is
a leftover from before the sync base client existed. Nothing imports it.
vigneshwerv
force-pushed
the
claude/sdk-py-typed
branch
from
August 10, 2026 19:42
83e923e to
900a3ed
Compare
vigneshwerv
changed the base branch from
claude/typed-entries-test-coverage
to
dev
August 10, 2026 19:42
vigneshwerv
enabled auto-merge (squash)
August 10, 2026 19:44
vigneshwerv
disabled auto-merge
August 10, 2026 19:44
vigneshwerv
approved these changes
Aug 10, 2026
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
2 participants
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.
Stacked on #47 (base
claude/typed-entries-test-coverage), which is itself stacked on #45. It extends thetypechecktarget and the credential-free CI job that #47 introduces, so it would conflict as a sibling.Typing only; no behaviour changes.
The SDK is invisible to type checkers
It ships no PEP 561 marker. Installed into a clean venv, this is everything mypy has to say about a consumer:
Not reduced precision — the module is skipped. In that state all three of these pass:
That undermines the premise of #45. The typed batch payloads exist so callers get type errors on a wrong parameter, and today no
pip installed consumer can see one.tests/type_checks/in #47 only works because it imports the snapshot from source.With
fragment/py.typed, the same file fails on the nonexistent method.No
includeentry is needed. I added one first, then checked: poetry ships anything underpackages, with and without it. Dead config, so it's out.tests/test_packaging.pyasserts the built wheel carries the marker, sincepoetry installputs the source tree on the path and would pass either way.The shipped SDK now typechecks under strict
fragment.sdk,fragment.sync_sdk,fragment.clientandfragment.exceptions. It took four annotations:refresh_token, both__init__sself.tokenNone, makingself.token["expires_in"]an errorThe generated modules already passed.
fragment.codegenstays on the defaults — build tooling, not something a customer imports.Flags are spelled out rather than
strict = true, which mypy only honours globally. Setting it in an override section silently applies it everywhere.Edits are to
fragment/client/, the source ariadne copies into both SDKs; the four generated copies are updated to match byte-for-byte.Verification
fragment/py.typedrefresh_tokenin a shipped copyself.tokenannotationEnd to end: built the wheel, installed it into a separate venv, ran mypy on a consumer file. Fails on the bad call with the marker, skips the module without it.
Not in scope
Custom scalars all land on
Any—SafeString(30 uses),DateTime,JSON,LastMoment;ledger_ik: Anyappears 16 times inclient.py. That's whyik=123still passes above. ariadne-codegen has a[tool.ariadne-codegen.scalars]section for this, but it rewrites every generated signature and is semver-visible — parameters that accept anything today would start rejecting it. Worth its own PR and a decision on theDateTime/JSONmappings.🤖 Generated with Claude Code