Skip to content

fix(units): units_v4 family-ref aliases, legacy setup, merge wait - #784

Merged
lkubie merged 3 commits into
lkubie/unitsv4from
cakeagents/sdk-168
Sep 24, 2026
Merged

lkubie merged 3 commits into
lkubie/unitsv4from
cakeagents/sdk-168

Conversation

@lkubie

@lkubie lkubie commented Sep 24, 2026

Copy link
Copy Markdown
Collaborator

What

U1 RELEASE BLOCKER fixed (top priority): the units v4 endpoints serialize unit-family references as {familyId, familyName}, but UnitFamilyV4Ref required id, so create/get_by_id/get_by_ids/update/get_compatible raised ValidationError on every convertible unit and search silently dropped them via the paginator's per-item fallback.

Audit §5.40 findings addressed (units v4 vs invent-core services/master-data):

  • U1 (release blocker): UnitFamilyV4Ref accepts both wire spellings via validation_alias=AliasChoices("id", "familyId") / AliasChoices("name", "familyName"). Serialization stays on the id/name keys, so SDK request payloads are unchanged.
  • U3: merge() gained wait: bool = False; with wait=True it polls the returned job via the existing poll_worker_job helper (the pattern used by tasks), raising on failure or timeout.
  • U4: the Custom (Legacy) setup flow is reachable. UnitV4.type is now optional (legacy units carry no type until setup and previously failed validation on read), and update() emits the setup payload: type plus refUnit/refUnitExp/refUnitValue for a convertible setup or unitFamilies for a non-convertible one. Changing or clearing type after setup raises client-side.
  • merge(webhook_method=…) is now Literal["POST", "GET"], matching the backend WebhookMethod enum (was an unvalidated str).

Why

SDK-168. Verified against invent-core integration branch: unit-family-ref.dto.ts (familyId/familyName), unit.dto.ts + unit-search-item.dto.ts (same ref shape on GET/search), units.service.ts dispatchUpdate/executeSetupPath (setup triggers on a Custom (Legacy) record with no type when the patch carries type), and merge-unit.dto.ts (webhook POST/GET enum + @IsUrl).

How

  • AliasChoices on the ref model fields; unit test payloads corrected from the assumed {id, name} shape to the real {familyId, familyName} shape, plus a pin that both spellings validate and serialization is stable.
  • Setup support lives in _generate_merge_patch: a type change is only legal when the existing unit has none (legacy), and the family-change gate now evaluates the effective (post-setup) type.
  • wait=True calls poll_worker_job(session=…, job_id=…) before returning the job id; default behavior is unchanged.

Testing

  • ruff format --check . + ruff check .: clean (398 files).
  • pytest tests/unit tests/test_exceptions.py: 73 passed, including new merge-patch tests for both legacy setup paths and post-setup type-change guards, and alias-tolerance pins.
  • pytest tests/collections/test_units_v4.py tests/collections/test_unit_families_v4.py tests/collections/test_units.py --collect-only: 22 collected. Integration suites remain skip-marked (units v4 API not yet in the test environment) and no integration credentials were available, so they were not run.

Not in this PR

  • U6 (typed exceptions for 409/412/415/428): main's exceptions.py still maps only 400/401/403/404/500/502, so these are landing via SDK-170 (in progress on another branch); noted here as a dependency rather than duplicated.
  • U2 (paginator silent item-skipping strict mode), U5/U7 (backend-side), and additive search-surface widening are out of scope.

Cake session: https://agents.ai.albertinventdev.com/sessions/910a4725-9404-4336-81dd-e30bfef0ce73

Units v4 endpoints serialize family references as {familyId, familyName}
but UnitFamilyV4Ref required id, so every convertible unit raised a
ValidationError on create/get/update/get_compatible and was silently
dropped from search pages. Accept both spellings via AliasChoices while
keeping serialization on the id/name keys.
Custom (Legacy) units carry no type until setup, so the required type
field made them fail validation on read, and _generate_merge_patch never
emitted type or the SI-mapping fields the setup flow needs. type is now
optional on the model, and setting it on a legacy unit emits the setup
payload (type plus ref_unit/ref_unit_exp for convertible, unit_families
for non-convertible). Changing or clearing type after setup raises.
merge() returned as soon as the job was accepted with no way to track
completion. A wait parameter now polls the job via poll_worker_job, the
pattern used elsewhere in the SDK, raising on failure or timeout. The
webhook_method parameter is typed Literal["POST", "GET"] to match the
backend enum instead of an unvalidated str.
@claude

claude Bot commented Sep 24, 2026

Copy link
Copy Markdown

Code Review

Issues Found

  • PR title type: fix(units) under-labels the change. Adding the new wait parameter to merge() and tightening webhook_method to a Literal[POST, GET] are new caller-visible capabilities, which OPINIONS.md classifies as feat, not fix. The alias fix (U1) and making type optional on read are genuine fixes, so the PR is mixed; consider feat(units) since release-please derives the changelog from the title.

Summary
1 minor issue found (commit-type labeling). No bugs or correctness issues in the code: the merge-patch model_fields_set gating with inequality guards, the legacy-setup type guards, AliasChoices serialization staying on id/name, and the poll_worker_job wiring are all correct and well covered by the new tests.

@lkubie
lkubie merged commit bae3cf8 into lkubie/unitsv4 Sep 24, 2026
4 checks passed
@lkubie
lkubie deleted the cakeagents/sdk-168 branch September 24, 2026 22:26
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.

1 participant