Skip to content

fix(lots): add workflow_id update support - #656

Open
prasad-albert wants to merge 3 commits into
mainfrom
fix/lots-workflow-id-update
Open

fix(lots): add workflow_id update support#656
prasad-albert wants to merge 3 commits into
mainfrom
fix/lots-workflow-id-update

Conversation

@prasad-albert

Copy link
Copy Markdown
Collaborator

What

Callers can set and read workflow_id on lots via client.lots.update() and get_by_id(). The field is populated from the API's Workflows array on GET.

Why

The lots API supports PATCH on workflowId, but the SDK omitted it from Lot and _updatable_attributes, so callers could not assign a workflow to a lot through the SDK.

How

Initial assigns emit UPDATE (not ADD) because the API only allows workflowId under the UPDATE allow-list. storage_location create/update examples now use EntityLink instead of a partial StorageLocation.

Testing

  • uv run ruff format .
  • uv run ruff check . --fix
  • uv run pytest tests/collections/test_lots.py::test_update_workflow_id -n 4 -v

SDK Changes

Lot.workflow_id is now readable after GET (from Workflows) and writable via LotCollection.update().

Expose workflowId as a patchable Lot field, populate it from the Workflows array on GET, and clarify storage_location EntityLink usage in docs.

Co-authored-by: Cursor <cursoragent@cursor.com>
@claude

claude Bot commented Aug 4, 2026

Copy link
Copy Markdown

Code Review

Issues Found

  • tests/collections/test_lots.py:112-125 test_update_workflow_id consumes the seeded_workflows fixture, but this file is pinned to pytestmark = pytest.mark.xdist_group("inventory"). Per tests/TESTING.md, seeded_workflows (and its chain: seeded_parameter_groups, seeded_parameters, static_consumeable_parameter) is owned by the tasks group. Pulling it into an inventory-grouped file forces the inventory worker to rebuild the entire workflow seeding chain and mixes two independent heavy fixture chains in one module, which TESTING.md explicitly warns against ("If two tests in one file genuinely need different heavy chains, that is a sign they belong in different files"). Move this test to a tasks-grouped file, or seed a lightweight workflow locally.
  • src/albert/resources/lots.py:58, 138, 142 Field/class docstrings leak backend specifics ("returned on lot GET responses", "Populated on GET from the Workflows array", "Workflow associations from the API"). Per AGENTS.md, docstrings should describe caller-facing behavior, not HTTP/GET internals.

Summary

2 issue(s) found. The xdist_group mismatch is the substantive one: it breaks the repo's parallel-test fixture-grouping contract and duplicates heavy seeding on the inventory worker. The ADD->UPDATE patch transform, model_validator population logic, and set-once handling are correct; workflow_id is properly gated by model_fields_set in the base diff.

Use the built-in WFL1 workflow in test_update_workflow_id so the
inventory worker no longer pulls in the full tasks fixture chain.

Remove backend-specific language from LotWorkflowLink, workflow_id,
and workflows field docstrings.
@claude

claude Bot commented Aug 4, 2026

Copy link
Copy Markdown

Looks good - no bugs or correctness issues found.

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.

2 participants