Skip to content

feat: Generate typed nested resource classes - #602

Merged
razor-x merged 7 commits into
mainfrom
claude/acs-credential-namespacing-l3q9m3
Aug 12, 2026
Merged

feat: Generate typed nested resource classes#602
razor-x merged 7 commits into
mainfrom
claude/acs-credential-namespacing-l3q9m3

Conversation

@razor-x

@razor-x razor-x commented Aug 12, 2026

Copy link
Copy Markdown
Member

Summary

This PR refactors the code generation system to create properly typed nested resource classes instead of using generic Dict[str, Any] for complex nested objects. All nested objects now inherit from ResourceMapping to provide both type safety and backward-compatible dictionary-style access.

Key Changes

  • New ResourceMapping base class: Added seam/utils/resource_mapping.py to provide a base class for nested resource dataclasses that supports dictionary-style access (__getitem__, __contains__, keys(), values(), items(), get()) for backward compatibility while maintaining type safety.

  • Generated typed nested classes: Updated code generation to create nested @dataclass classes for complex properties across all resource types:

    • AcsEntrance: Added AkilesMetadata, AssaAbloyVostioMetadata, and other provider-specific metadata classes
    • AcsUser: Converted access_schedule from Dict[str, Any] to typed AccessSchedule class
    • Device: Significantly expanded with typed nested classes (3156 lines added)
    • AccessCode, AccessGrant, ConnectedAccount, UnmanagedAccessCode, UnmanagedAccessGrant, SeamEvent, UnmanagedDevice, AcsAccessGroup, AcsCredential, Phone, AccessMethod, UnmanagedAccessMethod, AcsSystem, and others: Added typed Errors and other nested classes
  • Code generation updates: Modified codegen/lib/layouts/resources.ts and codegen/layouts/partials/resource-dataclass.hbs to:

    • Support nested class generation with proper indentation
    • Track nested classes and their properties in the layout context
    • Generate from_dict() class methods for deserialization
    • Inherit from ResourceMapping for nested classes
  • Type checking: Added mypy configuration to justfile for type checking the generated resources with specific error code exclusions.

  • PEP 561 compliance: Added seam/py.typed marker file to indicate the package supports type hints.

Implementation Details

  • Each nested class includes comprehensive docstrings with parameter descriptions
  • All nested classes implement from_dict() class methods for proper deserialization from API responses
  • The ResourceMapping base class provides transparent dictionary-style access while maintaining type safety
  • Unknown fields in API responses are silently dropped during deserialization
  • Nested classes properly handle optional fields with None defaults
  • Added regression test in test/nested_resource_test.py to verify nested objects are typed and drop unknown fields

https://claude.ai/code/session_0149sGKk8V2fn6dLvzNor86n

razor-x and others added 7 commits August 5, 2026 22:07
Nested resource objects now hydrate as typed mapping-compatible dataclasses and unknown nested API fields are stripped. Free-form record properties remain mappings.

BREAKING CHANGE: Nested properties are typed objects rather than dict subclasses. dict(...) and isinstance(value, dict) no longer work for them; typoed attributes now raise AttributeError instead of returning and inserting an empty mapping; undocumented nested fields are stripped.
Nested dataclasses were emitted at module level with their root resource
name flattened onto the property name, so `device.properties.battery`
became `DeviceBattery`.

Because that name only ever used the root resource name, distinct shapes
at different depths collapsed onto one class. The first shape reached won,
and every other path silently got the wrong fields:

  - `device.properties.battery` took the shape of
    `device.properties.accessory_keypad.battery`, dropping `status`.
  - `device.properties.available_climate_presets[].ecobee_metadata` (and
    the `current_` / `default_climate_setting` variants) took the shape of
    `device.properties.ecobee_metadata`, dropping `climate_ref`,
    `is_optimized`, and `owner`.
  - the `phone_session` credential/entrance `assa_abloy_vostio_metadata`
    and `visionline_metadata` pairs collapsed onto each other, as did
    `device.errors` and `phone_session.errors` against their nested
    namesakes.

Emit nested dataclasses inside the class that owns them instead, named
after the property alone, so the enclosing class disambiguates them:

  AcsCredential.AssaAbloyVostioMetadata
  Device.Properties.Battery
  Device.Properties.AccessoryKeypad.Battery

That removes all nine collisions by construction, keeps each resource
module exporting just its resource, and matches the layout the Ruby SDK
generates. `from_dict` reaches nested classes through `cls`, so no
qualified paths appear in the generated bodies.

The generator now builds a class tree rather than a flat map, and raises
if a nested class would shadow a name the module depends on, if two
siblings would claim the same name, or if the shape nests implausibly
deep.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0149sGKk8V2fn6dLvzNor86n
`just lint` runs pylint, which exits non-zero on any message. Three
findings predate this branch and fail the Lint job on both Python 3.11 and
3.12:

  - `unused-argument` on the generated `from_dict` of a nested class with
    no documented properties, where the body is just `return cls()`.
  - `no-member` on the test that asserts attribute access raises, which
    reads a deliberately absent attribute.
  - `use-implicit-booleaness-not-comparison` on `device.errors == []`,
    where the point is the exact empty-list value rather than falsiness.

Suppress the first two narrowly and assert the third by type and length.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0149sGKk8V2fn6dLvzNor86n
`mergeResourceProperties` kept the first occurrence of each property name
and discarded the rest. That was invisible while merged shapes stayed
`Dict[str, Any]`, but this branch turns them into dataclasses whose
`from_dict` reads field by field, so the dropped fields were discarded
from responses at hydration:

  - `ActionAttempt.Result` carried one field, `was_confirmed_by_device`.
    An `ENCODE_ACS_CREDENTIAL` attempt's `acs_credential_on_encoder` and
    `acs_credential_on_seam`, and a `CREATE_INSTANT_KEY` attempt's
    `instant_key_url`, were silently thrown away.
  - `AcsUser.PendingMutations.From` carried three of its eight fields, and
    the same pattern hit `from`/`to` across ten resources.

Merge recursively and take the union instead, descending into nested
objects and list items so a field survives no matter how deep the variants
disagree. Variants of a nested discriminated list are concatenated and
merged by whoever consumes the list.

The merged dataclass is wider than any single variant, which is the
tradeoff for one class per union: a `LOCK_DOOR` result exposes
`instant_key_url` as None. `main` had the same looseness via DeepAttrDict,
where every key was untyped and possibly absent, so this keeps the
response data intact rather than dropping it.

Merging now raises if variants disagree on a property's shape rather than
silently picking one. The only disagreements in the blueprint today are
`action_attempt.error.type` and `result.errors[].error_code`, both string
against enum, which map to the same Python type; anything structural
fails generation.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0149sGKk8V2fn6dLvzNor86n
Merging kept the first occurrence's description along with its shape. Each
variant documents a property for its own case, so the surviving text
describes one arm of a union the dataclass no longer distinguishes. Of 271
merged properties, 82 had conflicting descriptions.

Most were merely narrow: `from` on an access code's pending mutations read
"Previous code configuration" on a shape that also covers names. Four were
wrong outright, describing the opposite of what the field means:

    :ivar is_device_error: Indicates that the error is not a device error.

Keep a description only when every variant that documents the property
agrees, which also picks the real text when the others are blank. Deprecate
the merged property if any variant deprecates it, so a warning is never
lost to a variant that omits it.

The per-field documentation inside a merged dataclass is unaffected, since
those fields come from a single variant each. Only the summary of a shape
the variants describe differently goes away.

The type packages document each variant correctly and the TypeScript SDK
renders them as a union, so it keeps the precise text. Resolving this
upstream would flatten those descriptions and cost that accuracy, so the
loss stays here, in the SDKs that merge.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0149sGKk8V2fn6dLvzNor86n
@razor-x razor-x changed the title Generate typed nested resource classes with ResourceMapping feat: Generate typed nested resource classes with ResourceMapping Aug 12, 2026
@razor-x
razor-x marked this pull request as ready for review August 12, 2026 17:49
@razor-x
razor-x requested a review from a team as a code owner August 12, 2026 17:49
@razor-x razor-x changed the title feat: Generate typed nested resource classes with ResourceMapping feat: Generate typed nested resource classes Aug 12, 2026
@razor-x
razor-x merged commit fe79a84 into main Aug 12, 2026
18 checks passed
@razor-x
razor-x deleted the claude/acs-credential-namespacing-l3q9m3 branch August 12, 2026 17:52
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.

3 participants