Skip to content

feat!: type RegistryClient.create_adagents return as CreateAdagentsResponse#937

Merged
bokelley merged 2 commits into
mainfrom
claude/issue-934-type-create-adagents
Jun 7, 2026
Merged

feat!: type RegistryClient.create_adagents return as CreateAdagentsResponse#937
bokelley merged 2 commits into
mainfrom
claude/issue-934-type-create-adagents

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

@bokelley bokelley commented Jun 7, 2026

Closes #934

What

Types RegistryClient.create_adagents as CreateAdagentsResponse instead of dict[str, Any]. The response is now parsed via self._parse(CreateAdagentsResponse, resp.json(), ...), matching the other typed RegistryClient methods (e.g. publish_community_mirror_adagents).

CreateAdagentsResponse was added to the registry OpenAPI in adcp#5385 and regenerated into adcp.types.registry by #929. This typing was deferred during the #929 retype because the existing TestCreateAdagents tests asserted dict access against an incomplete mock payload.

The generated model fits the /api/adagents/create response cleanly: success / data (success, adagents_json, validation) / timestamp. The two TestCreateAdagents tests now use a complete CreateAdagentsResponse-shaped fixture and assert on typed model attributes.

Commits

  • style: — black-format tests/test_registry_new.py. The file had drifted out of compliance with the pinned black 24.10.0 pre-commit hook (unrelated to this change); split out so the functional diff stays focused. No CI gate enforces black on tests/, but the local pre-commit hook reformats on commit.
  • feat!: — the actual typing change + test updates (breaking public-API change; see below).

Breaking change

BREAKING CHANGE: RegistryClient.create_adagents now returns a CreateAdagentsResponse Pydantic model instead of dict[str, Any]. Callers using subscript access (result["success"], result["data"]) must switch to attribute access (result.success, result.data). The model matches the /api/adagents/create wire contract (adcp#5385).

Verification

  • uv run --extra dev pytest tests/test_registry_new.py tests/test_registry.py -q → 289 passed
  • make lint → passed
  • make typecheck → passed
  • tests/test_public_api.py → 20 passed (no public-API export changes)

🤖 Generated with Claude Code

@bokelley bokelley enabled auto-merge (squash) June 7, 2026 12:05
Copy link
Copy Markdown
Contributor

@aao-ipr-bot aao-ipr-bot Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right change, wrong semver signal. The retype is correct and the model matches the wire contract exactly — but this is a breaking public-API return-type change shipped under a refactor: prefix that release-please treats as non-releasing, so it would land on PyPI invisibly.

RegistryClient is a public export (src/adcp/__init__.py:80,689). create_adagents goes from dict[str, Any] to a Pydantic CreateAdagentsResponse. Pydantic v2 models aren't subscriptable, so any adopter doing result["success"] / result["data"] gets a TypeError. The proof it's breaking is in this very diff: the old tests asserted result["success"] and had to be rewritten to result.success (diff L828→L831). Package is 6.3.0b8 — post-1.0, breaking changes carry weight.

The commit is refactor(registry): with no ! and no BREAKING CHANGE: footer. Per release-please-config.json, refactor is absent from changelog-sections and has no patch-bump override — it triggers no version bump and no changelog entry. So this breaking change ships folded into the next unrelated feat/fix release with zero signal to adopters pinned to >=6.3. That's the block.

Fix (mechanical): re-prefix the functional commit refactor(registry)!: with a BREAKING CHANGE: footer, or use feat(registry)!: so release-please actually cuts a release that reflects the break. Include a one-line migration note: result["data"]result.data, result["success"]result.success. That's the only thing standing between this and approval.

Things I checked

  • Model fidelity (ad-tech-protocol-expert: sound). CreateAdagentsResponse (src/adcp/types/registry.py:2028-2031) requires success/data/timestamp, matching OpenAPI required: [success, data, timestamp] (schemas/registry-openapi.yaml:1184-1187). Nested CreateAdagentsData and AdagentsValidationResult required sets line up 1:1. No required↔optional inversions.
  • No happy-path parse failure. The include_timestamp request param governs a timestamp embedded inside the generated adagents_json document — not the response envelope. The envelope timestamp is unconditionally required by the schema, so create_adagents(...) with include_timestamp=False does not raise a ValidationError through _parse. This was my first suspicion; it's clean.
  • Forward-compat intact. RegistryBaseModel uses extra='allow' (src/adcp/types/base.py:278) — new server fields are preserved, not dropped. No discriminated unions in this model tree, so no fallback-arm regression risk.
  • Test fixture is schema-valid. _CREATE_ADAGENTS_RESPONSE (diff L797-812) supplies every required field at each level. test_sends_optional_params correctly swapped its {} mock (which _parse now rejects) for the full fixture (diff L837-838). Necessary, not gratuitous.
  • Commit hygiene. style: (deb33145) is pure black reformat touching only tests/test_registry_new.py, cleanly separated from the functional refactor: (27a61b49). The reformat changed no assertion semantics. Splitting it out was the right call.

Follow-ups (non-blocking — file as issues)

  • validate_adagents (src/adcp/registry.py:1498-1507) still returns raw dict[str, Any] directly above the now-typed create_adagents. A sibling response model exists. Worth finishing the migration in a follow-up — same #934-style deferral.

Re-request review once the functional commit carries the breaking marker and a migration note. Everything else is ready.

bokelley and others added 2 commits June 7, 2026 08:36
The file drifted out of black compliance (the pinned black 24.10.0
pre-commit hook reformats it). Apply the formatter on its own so the
follow-up create_adagents typing change stays a focused diff.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…sponse

adcp#5385 added CreateAdagentsResponse to the registry OpenAPI and #929
regenerated it into adcp.types.registry. Parse the /api/adagents/create
response into the typed model via _parse, matching the other typed
RegistryClient methods, and update the TestCreateAdagents tests to assert
on model attributes.

Closes #934

BREAKING CHANGE: RegistryClient.create_adagents now returns a CreateAdagentsResponse Pydantic model instead of dict[str, Any]. Callers using subscript access (result["success"], result["data"]) must switch to attribute access (result.success, result.data). The model matches the /api/adagents/create wire contract (adcp#5385).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@bokelley bokelley force-pushed the claude/issue-934-type-create-adagents branch from 27a61b4 to cd9b3dc Compare June 7, 2026 12:36
@bokelley bokelley changed the title refactor(registry): type create_adagents return as CreateAdagentsResponse feat!: type RegistryClient.create_adagents return as CreateAdagentsResponse Jun 7, 2026
@bokelley
Copy link
Copy Markdown
Contributor Author

bokelley commented Jun 7, 2026

Re-signaled as feat! with a BREAKING CHANGE footer (PR title + commit + body) so release-please treats it as a releasing breaking change and documents the dict->model migration. Thanks @aao-ipr-bot.

@bokelley
Copy link
Copy Markdown
Contributor Author

bokelley commented Jun 7, 2026

Noted — the feat! re-signal with the BREAKING CHANGE: footer looks correct for release-please's major-bump detection. No further action from triage.


Generated by Claude Code

Copy link
Copy Markdown
Contributor

@aao-ipr-bot aao-ipr-bot Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean fix. Wires the already-merged CreateAdagentsResponse model into the one RegistryClient method that was still returning a raw dict — the last typing gap from the #929 retype, deferred only because the old tests asserted dict access against an incomplete mock.

Things I checked

  • _parse wiring matches the sibling pattern. return self._parse(CreateAdagentsResponse, resp.json(), "Adagents create") at src/adcp/registry.py:1533 is the same shape as publish_community_mirror_adagents. The operation string reuses the label already passed to the request helper. Right shape.
  • Model fits the wire contract. CreateAdagentsResponse (src/adcp/types/registry.py:2028) requires success/data/timestamp; CreateAdagentsData (:2019) requires success/adagents_json/validation; AdagentsValidationResult requires discovery_method. The _CREATE_ADAGENTS_RESPONSE fixture supplies all of them with a valid discovery_method: "direct". Validates cleanly.
  • No forward-compat regression. RegistryBaseModel is extra="allow" (src/adcp/types/base.py:278) and neither subclass overrides it — a registry that adds a top-level or data-nested field is preserved, not rejected. ad-tech-protocol-expert: sound. Only genuinely-missing required fields fail.
  • Behavior change is fail-closed, and that's the point. Previously a malformed 200 body round-tripped as a raw dict; now it raises RegistryError via model_validate. That matches every other typed RegistryClient method. test_sends_optional_params had to swap its {} mock for the full fixture precisely because of this — required fix, not churn.
  • Breaking signal is correct. dict[str, Any]CreateAdagentsResponse flips callers from subscript to attribute access — a genuine public-API break. Shipped as feat!: with a BREAKING CHANGE: footer and a migration note in the body. The semver signal matches the diff.
  • SuccessLiteral.boolean_True assertion is correct, not a smell. SuccessLiteral (:890) is a single-member generated enum; identity comparison against the member is the right check.
  • Largest-file rule on tests/test_registry_new.py (420/234). Read it — the bulk is black reflow churn (collapsing/expanding _mock_response(...) call sites) with no logic changes. The only semantic edits are the new fixture and the TestCreateAdagents assertion rewrites. Splitting the reformat into its own style: commit was the right call.

Minor nits (non-blocking)

  1. RegistryBaseModel has no strict-mode hook. Unlike AdCPBaseModel, registry parsing is unconditionally lenient (extra="allow" with no ADCP_STRICT_VALIDATION escape), so a renamed/dropped registry field won't surface in tests the way protocol-type renames do. Correct call for forward-compat here — noting it for whoever next wonders why registry drift is quieter than protocol drift.

LGTM.

@bokelley bokelley merged commit 8e6018b into main Jun 7, 2026
27 checks passed
@bokelley bokelley deleted the claude/issue-934-type-create-adagents branch June 7, 2026 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

type RegistryClient.create_adagents return as CreateAdagentsResponse

1 participant