Skip to content

feat(attributes): add AttributeCollection with CRUD and reference values#473

Merged
prasad-albert merged 1 commit into
nextfrom
worktree-feat-attributes
May 22, 2026
Merged

feat(attributes): add AttributeCollection with CRUD and reference values#473
prasad-albert merged 1 commit into
nextfrom
worktree-feat-attributes

Conversation

@prasad-albert
Copy link
Copy Markdown
Collaborator

@prasad-albert prasad-albert commented Apr 30, 2026

Summary

  • Adds AttributeCollection (client.attributes) with full CRUD (create, get_by_id, get_by_ids, get_all, search, update, delete) and reference value methods (add_values, get_values, get_bulk_values, delete_values, clear_values)
  • Introduces Attribute, AttributeScope, ValidationItem, AttributeValue, AttributeValueRange, AttributeDefinition, and AttributeValuesResponse resource models
  • Deprecates InventoryCollection.get_specs() and add_specs() via @deprecated (removed in 2.0); pins typing_extensions>=4.6.1 as a direct dependency
  • Adds migration guide (docs/migration/specs-to-attributes.md) and restructures migration docs into per-topic directory following the substances-v4 pattern

Notes

  • add_values is upsert — internally deletes matching attribute IDs before PUT so callers never see duplicate-ID errors
  • GET /attributes/values (bulk) returns 500 from the backend; test is marked xfail pending backend fix
  • typing_extensions.deprecated chosen over raw warnings.warn

@claude
Copy link
Copy Markdown

claude Bot commented Apr 30, 2026

Code Review

Issues Found

  • [src/albert/collections/attributes.py:484] create() is missing @validate_call. Every public collection method must have it for runtime validation — all sibling methods (get_by_id, get_by_ids, delete, search, add_values, etc.) have it but create does not.

  • [src/albert/collections/attributes.py:503] update() is missing @validate_call for the same reason.

  • [src/albert/collections/inventory.py:~256] The add_specs() deprecation message tells users to call client.attributes.set_values(), which does not exist. The correct replacement is client.attributes.add_values().

  • [src/albert/collections/attributes.py:504-507] update() docstring contains implementation details: "Enum value changes are handled transparently via a separate enums endpoint before applying any field-level patches." Per project standards, docstrings must describe behaviour from the caller's perspective, never internal mechanics.

  • [src/albert/collections/attributes.py:503] update() is missing the required Notes section listing updatable fields. Per AGENTS.md: "Every update() method must have a Notes section listing the fields that can be updated." _updatable_attributes = {"reference_name", "parameters", "validation"} (plus unit_id based on the patch logic) should be documented there.

Summary

5 issues found. The wrong method name in the deprecation message is a user-facing correctness bug; the missing @validate_call decorators break the runtime validation contract that every public collection method is expected to provide.

@prasad-albert
Copy link
Copy Markdown
Collaborator Author

Code Review

Issues Found

  • [src/albert/collections/attributes.py:484] create() is missing @validate_call. Every public collection method must have it for runtime validation — all sibling methods (get_by_id, get_by_ids, delete, search, add_values, etc.) have it but create does not.
  • [src/albert/collections/attributes.py:503] update() is missing @validate_call for the same reason.
  • [src/albert/collections/inventory.py:~256] The add_specs() deprecation message tells users to call client.attributes.set_values(), which does not exist. The correct replacement is client.attributes.add_values().
  • [src/albert/collections/attributes.py:504-507] update() docstring contains implementation details: "Enum value changes are handled transparently via a separate enums endpoint before applying any field-level patches." Per project standards, docstrings must describe behaviour from the caller's perspective, never internal mechanics.
  • [src/albert/collections/attributes.py:503] update() is missing the required Notes section listing updatable fields. Per AGENTS.md: "Every update() method must have a Notes section listing the fields that can be updated." _updatable_attributes = {"reference_name", "parameters", "validation"} (plus unit_id based on the patch logic) should be documented there.

Summary

5 issues found. The wrong method name in the deprecation message is a user-facing correctness bug; the missing @validate_call decorators break the runtime validation contract that every public collection method is expected to provide.

@claude fix all the issues found in the review

@claude
Copy link
Copy Markdown

claude Bot commented Apr 30, 2026

Claude finished @prasad-albert's task in 1m 1s —— View job


Fixed all 5 review issues

  • Add @validate_call to create()src/albert/collections/attributes.py:156
  • Add @validate_call to update()src/albert/collections/attributes.py:176
  • Fix wrong method name in add_specs() deprecation message (set_valuesadd_values) — src/albert/collections/inventory.py:283
  • Remove implementation details from update() docstring (removed the enums endpoint sentence)
  • Add Notes section to update() docstring listing reference_name, parameters, validation, unit_id

All changes committed in c881de8 and pushed to the branch.

Comment thread docs/migration/specs-to-attributes.md Outdated
@prasad-albert prasad-albert force-pushed the worktree-feat-attributes branch 3 times, most recently from b3f63a8 to 09f4c86 Compare May 20, 2026 09:57
@prasad-albert prasad-albert force-pushed the worktree-feat-attributes branch from dadd57a to af71685 Compare May 22, 2026 13:04
@prasad-albert prasad-albert merged commit 1064207 into next May 22, 2026
2 checks passed
@prasad-albert prasad-albert deleted the worktree-feat-attributes branch May 22, 2026 14:45
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