fix(propertydata): property data audit fixes (SDK-160)

[lkubie]

What

Fixes the five SMALL SDK findings from the backend API audit section 5.6 (propertydata):

1. update_property_on_inventory UnboundLocalError: existing_id was only assigned inside the match loop, so an item with no value for the column crashed instead of adding the value as the docstring promises. The variable is now initialized and, when no property-data record exists for the column, the value is added via the create path (PATCH add requires an existing PTD id; verified against api-propertydata patchPropertyData.js).
2. visibleTrialNo type: TaskPropertyCreate defaulted visible_trial_number to the string "1" in an after-validator, so it serialized as a string. The API schema types it as number (docs/fields.yaml VisibleTrialNo). Now defaults to int 1.
3. Response model gaps: PropertyData dropped valueNumeric/valueString (extra="ignore"). The GET handler (getPropertyDataV2.js) returns them inside DataColumns[].PropertyData; both fields are now declared on the model.
4. Dead params: removed the history query param from add_properties_to_task (bulkTrial.js derives history from the request body and never reads the query param), and deprecated the no-op data_template_id on void_interval_data/unvoid_interval_data (voidUnvoidPTD never reads it), matching the in-file deprecation pattern used for return_fields/return_facets.
5. Positional zip misalignment: add_properties_to_task zipped request properties to the POST response by position, but the API returns existing trials before new trials and drops rejected values, so mixed batches misaligned and failed values looked registered. Response items are now paired back to request properties by (interval, data column, column sequence, visible trial number).

Why

SDK-160. Each finding causes silent data corruption or a hard crash on documented call patterns.

How

Minimal diffs confined to collections/property_data.py and resources/property_data.py; contracts verified against the api-propertydata handlers (cloned for reference). Not changed (out of ticket scope): the MEDIUM items (PATCH 206 partial failures swallowed, single-interval client guard, search-item strictness), the [API]-side findings, and Trial.visible_trial_number defaulting to 1 when the task GET omits visibleTrialNo.

Testing

No integration credentials available locally, so:

• ruff format + ruff check --fix: clean (386 files unchanged, all checks passed)
• pytest tests/collections/test_property_data.py --collect-only -q: 14 collected (incl. new test_update_property_on_inventory_adds_then_updates)
• pytest tests/core tests/utils tests/unit -q: 116 passed (incl. 2 new field-coverage tests pinning valueNumeric/valueString on PropertyData and numeric visibleTrialNo serialization)
• Direct model-level verification of the visibleTrialNo wire type and the key-based pairing against a shuffled response

Integration tests added but not run live: test_update_property_on_inventory_adds_then_updates (group tasks, writes only to a data column unused by other tests in the file).

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

[claude]

Looks good - no bugs or correctness issues found.

[prasad-albert]

LGTM! Verified backend contract parity, backward compatibility, caller ergonomics, and SDK conventions. Ready to merge.

[claude]

Looks good - no bugs or correctness issues found.

[claude]

Code Review

Issues Found

• collections/property_data.py:1219-1239 The new key-based pairing in add_properties_to_task compares raw response dict values against typed request-model values, so a semantic match can miss. prop.visible_trial_number is always an int (model-validated), but the key uses item_data.get("visibleTrialNo") straight from the JSON before Pydantic coercion; if the API returns visibleTrialNo as a string, 1 != "1" and every item fails to pair. Similarly prop.data_column.column_sequence is optional (None when a caller omits it) while the response columnId is server-populated, causing a None vs value mismatch. When pairing fails, prop is None, so DataTemplate is silently not attached, which directly feeds the calculated-property patch step on the next lines. This path has no live-API test coverage.

Summary

1 issue found. Not a crash (failed pairs degrade gracefully to no DataTemplate), but if the response field types/presence do not exactly match the request-model types, the core fix silently misaligns and DataTemplate attachment is lost. Worth verifying the actual POST response shape (type of visibleTrialNo, presence of columnId) against the live endpoint before merge.

[claude]

Looks good - no bugs or correctness issues found.

[claude]

Code Review

Issues Found

• src/albert/resources/property_data.py:107-111 Naming inconsistency: the new PropertyData fields for wire aliases valueNumeric/valueString are named value_numeric/value_string, but the sibling model PropertyValue (same file, lines 153-156) already models the identical wire fields as numeric_value/string_value. Same wire keys, two different Python attribute names across models. Not a bug, but a pattern deviation worth aligning for caller consistency.
• src/albert/collections/property_data.py:1219-1242 The identity-pairing in add_properties_to_task keys on visible_trial_number and column_sequence, both of which can differ between request and response. visible_trial_number defaults to 1 for new trials (set by the model validator), but for a newly created trial the server may echo a server-assigned visibleTrialNo; likewise column_sequence is optional (default=None), so a caller omitting it produces None in the request key while the response returns the real columnId. In either case the lookup misses, prop is None, and DataTemplate is silently not attached to the registered property (a subtle regression from the old positional attach). Worth confirming against a live add of a new trial and an add that omits column_sequence.

Summary
2 issue(s) found. Both are low severity: one cosmetic naming deviation, one potential silent DataTemplate-attach miss in edge cases of the new pairing logic. The primary audit fixes are correct.

[lkubie]

CircleCI Integration Tests failed

Branch: cakeagents/sdk-160 (65c6ded)
Build: https://circleci.com/gh/albert-labs/albert-python/8617

=========================== short test summary info ============================
FAILED tests/integration/collections/test_substance_v4.py::test_update_metadata_multiselect_list_field@projects - albert.exceptions.BadRequestError: PATCH 'https://staging.albertinventdev.com/api/v4/substances/metadata/SUB1f1b901e-13f1-6670-9aa0-bbedd0572994' failed with status code 400 (Bad Request). Errors: [{'msg': 'CAS API request failed (no response received)'}]
Body:
b'{"data": [{"operation": "add", "attribute": "Metadata.test_substances_list_field", "newValue": [{"id": "LST121102", "value": "TEST Substances List Field Option 0"}]}]}'
SKIPPED [10] tests/integration/collections/test_cas.py: CAS collection is deprecated in favor of Substance v4; the testing tenant has migrated to substance_v4.
SKIPPED [1] tests/integration/collections/test_data_templates.py:193: No eligible user returned by users.search() for ACL update.
SKIPPED [1] tests/integration/collections/test_data_templates.py:731: No extensions configured in tenant
SKIPPED [1] tests/integration/collections/test_inventory.py:197: LLM search is currently not working as expected.
SKIPPED [1] tests/integration/collections/test_lots.py:91: No second active user available for team tests
SKIPPED [1] tests/integration/collections/test_btdataset.py:23: created_by=USR226595 returned no datasets on this tenant (seeded DST1524)
SKIPPED [1] tests/integration/collections/test_reports.py:48: Report Queries not loaded into testing environment yet
SKIPPED [1] tests/integration/collections/test_sds.py:105: Tenant cannot generate SDS for seeded formula: POST 'https://staging.albertinventdev.com/api/v2/documentgenerator/sds' failed with status code 422 (Unprocessable Entity). Errors: [{'msg': 'ERROR_MISSING_INFO_CAS', 'casIDs': ['SDK-Test-6c1cd231-fcb6-425d-83b4-8fa26f4849f4-64-17-5']}]
SKIPPED [1] tests/integration/collections/test_teams.py:90: No second active user available for team tests
SKIPPED [1] tests/integration/collections/test_teams.py:116: No second active user available for team tests
SKIPPED [15] tests/integration/collections/test_chats.py: Chats API is not yet available in the test environment.
SKIPPED [1] tests/integration/collections/test_users.py:92: No search hits could be hydrated (stale index IDs)
SKIPPED [1] tests/integration/collections/test_files.py:33: Breakthrough file namespace not available or upload not visible
SKIPPED [1] tests/integration/resources/test_sheets.py:221: Seeded Process Design has no rows to reference
XFAIL tests/integration/collections/test_design_runs.py::test_create_optimization_returns_generate_insight@datatemplates - api-designruns is not live in the TEN0 test environment.
XFAIL tests/integration/collections/test_design_runs.py::test_create_doe_returns_smart_doe_insight@datatemplates - api-designruns is not live in the TEN0 test environment.
XFAIL tests/integration/collections/test_design_runs.py::test_validate_optimization_returns_response@datatemplates - api-designruns is not live in the TEN0 test environment.
XFAIL tests/integration/collections/test_design_runs.py::test_validate_doe_returns_response@datatemplates - api-designruns is not live in the TEN0 test environment.
XFAIL tests/integration/collections/test_design_runs.py::test_create_optimization_accepts_weighted_objectives@datatemplates - api-designruns is not live in the TEN0 test environment.
XFAIL tests/integration/collections/test_design_runs.py::test_validate_optimization_accepts_weighted_objectives@datatemplates - api-designruns is not live in the TEN0 test environment.
XFAIL tests/integration/collections/test_design_runs.py::test_validate_optimization_accepts_plain_criterion@datatemplates - api-designruns is not live in the TEN0 test environment.
XFAIL tests/integration/collections/test_design_runs.py::test_validate_optimization_treats_a_null_weight_as_unweighted@datatemplates - api-designruns is not live in the TEN0 test environment.
XFAIL tests/integration/collections/test_smart_datasets.py::test_smart_dataset_get_data@datatemplates - No DWH available in TEN0 test environment.
XFAIL tests/integration/collections/test_smart_datasets.py::test_smart_dataset_get_data_with_filters@datatemplates - No DWH available in TEN0 test environment.
XFAIL tests/integration/collections/test_tasks.py::test_get_and_set_block_rules@tasks - increased intervals is not live on ten0 test env
XFAIL tests/integration/collections/test_tasks.py::test_create_with_combinations_integration@tasks - increased intervals is not live on ten0 test env
XFAIL tests/integration/collections/test_activities.py::test_activity_search - GET /api/v3/activities/search 500 OpenSearch on TEN0 staging: https://linear.app/albert-invent/issue/SEA-221
XPASS tests/integration/collections/test_smart_datasets.py::test_smart_dataset_create_with_build@datatemplates - No DWH available in TEN0 test environment.
XPASS tests/integration/collections/test_targets.py::test_target_create_with_between_parameter_filter@datatemplates - Requires api-targets backend to accept operator/value-pair TargetParameter.value (ML-1207). Remove decorator once deployed.
XPASS tests/integration/collections/test_batch_data.py::test_create_batch_data@tasks - Batch data creation is currently not idempotent, so this test may fail if batch data already exists for the task.
XPASS tests/integration/collections/test_substance_v4.py::test_search_pagination@projects - api-substance-v4 search pagination intermittently returns duplicate items across pages (MoleculeEngineering/api-substance-v4#57).
XPASS tests/integration/collections/test_tasks.py::test_generate_block_combinations_integration@tasks - increased intervals is not live on ten0 test env
= 1 failed, 354 passed, 37 skipped, 13 xfailed, 5 xpassed, 36 warnings in 541.27s (0:09:01) =

[lkubie]

CircleCI Integration Tests failed

Branch: cakeagents/sdk-160 (65c6ded)
Build: https://circleci.com/gh/albert-labs/albert-python/8663

=========================== short test summary info ============================
FAILED tests/integration/collections/test_substance_v4.py::test_update_metadata@projects - albert.exceptions.BadRequestError: PATCH 'https://staging.albertinventdev.com/api/v4/substances/metadata/SUB1f1b9169-8a0f-6360-869c-2729c3c30abc' failed with status code 400 (Bad Request). Errors: [{'msg': 'CAS API request failed (no response received)'}]
Body:
b'{"data": [{"operation": "add", "attribute": "notes", "newValue": "sdk test note"}, {"operation": "add", "attribute": "casSmiles", "newValue": "CCO"}, {"operation": "add", "attribute": "Metadata.test_substances_string_field", "newValue": "sdk test value"}]}'
SKIPPED [10] tests/integration/collections/test_cas.py: CAS collection is deprecated in favor of Substance v4; the testing tenant has migrated to substance_v4.
SKIPPED [1] tests/integration/collections/test_data_templates.py:193: No eligible user returned by users.search() for ACL update.
SKIPPED [1] tests/integration/collections/test_data_templates.py:731: No extensions configured in tenant
SKIPPED [1] tests/integration/collections/test_inventory.py:197: LLM search is currently not working as expected.
SKIPPED [1] tests/integration/collections/test_lots.py:91: No second active user available for team tests
SKIPPED [1] tests/integration/collections/test_btdataset.py:23: created_by=USR226595 returned no datasets on this tenant (seeded DST1545)
SKIPPED [1] tests/integration/collections/test_sds.py:105: Tenant cannot generate SDS for seeded formula: POST 'https://staging.albertinventdev.com/api/v2/documentgenerator/sds' failed with status code 422 (Unprocessable Entity). Errors: [{'msg': 'ERROR_MISSING_INFO_CAS', 'casIDs': ['SDK-Test-d462c8d0-60f0-4f32-ba5a-a0781a533281-64-17-5']}]
SKIPPED [1] tests/integration/collections/test_teams.py:90: No second active user available for team tests
SKIPPED [1] tests/integration/collections/test_teams.py:116: No second active user available for team tests
SKIPPED [1] tests/integration/collections/test_reports.py:48: Report Queries not loaded into testing environment yet
SKIPPED [15] tests/integration/collections/test_chats.py: Chats API is not yet available in the test environment.
SKIPPED [1] tests/integration/collections/test_users.py:92: No search hits could be hydrated (stale index IDs)
SKIPPED [1] tests/integration/collections/test_files.py:33: Breakthrough file namespace not available or upload not visible
SKIPPED [1] tests/integration/resources/test_sheets.py:221: Seeded Process Design has no rows to reference
XFAIL tests/integration/collections/test_design_runs.py::test_create_optimization_returns_generate_insight@datatemplates - api-designruns is not live in the TEN0 test environment.
XFAIL tests/integration/collections/test_design_runs.py::test_create_doe_returns_smart_doe_insight@datatemplates - api-designruns is not live in the TEN0 test environment.
XFAIL tests/integration/collections/test_design_runs.py::test_validate_optimization_returns_response@datatemplates - api-designruns is not live in the TEN0 test environment.
XFAIL tests/integration/collections/test_design_runs.py::test_validate_doe_returns_response@datatemplates - api-designruns is not live in the TEN0 test environment.
XFAIL tests/integration/collections/test_design_runs.py::test_create_optimization_accepts_weighted_objectives@datatemplates - api-designruns is not live in the TEN0 test environment.
XFAIL tests/integration/collections/test_design_runs.py::test_validate_optimization_accepts_weighted_objectives@datatemplates - api-designruns is not live in the TEN0 test environment.
XFAIL tests/integration/collections/test_design_runs.py::test_validate_optimization_accepts_plain_criterion@datatemplates - api-designruns is not live in the TEN0 test environment.
XFAIL tests/integration/collections/test_design_runs.py::test_validate_optimization_treats_a_null_weight_as_unweighted@datatemplates - api-designruns is not live in the TEN0 test environment.
XFAIL tests/integration/collections/test_smart_datasets.py::test_smart_dataset_get_data@datatemplates - No DWH available in TEN0 test environment.
XFAIL tests/integration/collections/test_smart_datasets.py::test_smart_dataset_get_data_with_filters@datatemplates - No DWH available in TEN0 test environment.
XFAIL tests/integration/collections/test_activities.py::test_activity_search - GET /api/v3/activities/search 500 OpenSearch on TEN0 staging: https://linear.app/albert-invent/issue/SEA-221
XFAIL tests/integration/collections/test_tasks.py::test_get_and_set_block_rules@tasks - increased intervals is not live on ten0 test env
XFAIL tests/integration/collections/test_tasks.py::test_create_with_combinations_integration@tasks - increased intervals is not live on ten0 test env
XPASS tests/integration/collections/test_smart_datasets.py::test_smart_dataset_create_with_build@datatemplates - No DWH available in TEN0 test environment.
XPASS tests/integration/collections/test_targets.py::test_target_create_with_between_parameter_filter@datatemplates - Requires api-targets backend to accept operator/value-pair TargetParameter.value (ML-1207). Remove decorator once deployed.
XPASS tests/integration/collections/test_batch_data.py::test_create_batch_data@tasks - Batch data creation is currently not idempotent, so this test may fail if batch data already exists for the task.
XPASS tests/integration/collections/test_substance_v4.py::test_search_pagination@projects - api-substance-v4 search pagination intermittently returns duplicate items across pages (MoleculeEngineering/api-substance-v4#57).
XPASS tests/integration/collections/test_tasks.py::test_generate_block_combinations_integration@tasks - increased intervals is not live on ten0 test env
= 1 failed, 354 passed, 37 skipped, 13 xfailed, 5 xpassed, 36 warnings in 616.18s (0:10:16) =

[lkubie]

Both failures are substance metadata PATCHes failing with 400 'CAS API request failed (no response received)' - the staging CAS service was unreachable during the run. Environmental, unrelated to the property_data diff. Rerun once CAS recovers.

[lkubie]

CircleCI Integration Tests failed

Branch: cakeagents/sdk-160 (65c6ded)
Build: https://circleci.com/gh/albert-labs/albert-python/8669

=========================== short test summary info ============================
FAILED tests/integration/collections/test_substance_v4.py::test_update_metadata_multiselect_list_field@projects - albert.exceptions.BadRequestError: PATCH 'https://staging.albertinventdev.com/api/v4/substances/metadata/SUB1f1b92c1-e473-6a00-8d3b-b6dc246e12e8' failed with status code 400 (Bad Request). Errors: [{'msg': 'CAS API request failed (no response received)'}]
Body:
b'{"data": [{"operation": "delete", "attribute": "Metadata.test_substances_list_field", "oldValue": [{"id": "LST121102", "value": "TEST Substances List Field Option 0"}]}]}'
SKIPPED [10] tests/integration/collections/test_cas.py: CAS collection is deprecated in favor of Substance v4; the testing tenant has migrated to substance_v4.
SKIPPED [1] tests/integration/collections/test_data_templates.py:193: No eligible user returned by users.search() for ACL update.
SKIPPED [1] tests/integration/collections/test_data_templates.py:731: No extensions configured in tenant
SKIPPED [1] tests/integration/collections/test_inventory.py:197: LLM search is currently not working as expected.
SKIPPED [1] tests/integration/collections/test_lots.py:91: No second active user available for team tests
SKIPPED [1] tests/integration/collections/test_btdataset.py:23: created_by=USR226595 returned no datasets on this tenant (seeded DST1549)
SKIPPED [1] tests/integration/collections/test_reports.py:48: Report Queries not loaded into testing environment yet
SKIPPED [1] tests/integration/collections/test_teams.py:90: No second active user available for team tests
SKIPPED [1] tests/integration/collections/test_teams.py:116: No second active user available for team tests
SKIPPED [15] tests/integration/collections/test_chats.py: Chats API is not yet available in the test environment.
SKIPPED [1] tests/integration/collections/test_sds.py:105: Tenant cannot generate SDS for seeded formula: POST 'https://staging.albertinventdev.com/api/v2/documentgenerator/sds' failed with status code 422 (Unprocessable Entity). Errors: [{'msg': 'ERROR_MISSING_INFO_CAS', 'casIDs': ['SDK-Test-2010f67f-051c-4d1e-98ac-de7a126d88d8-64-17-5']}]
SKIPPED [1] tests/integration/collections/test_files.py:33: Breakthrough file namespace not available or upload not visible
SKIPPED [1] tests/integration/collections/test_users.py:92: No search hits could be hydrated (stale index IDs)
SKIPPED [1] tests/integration/resources/test_sheets.py:221: Seeded Process Design has no rows to reference
XFAIL tests/integration/collections/test_design_runs.py::test_create_optimization_returns_generate_insight@datatemplates - api-designruns is not live in the TEN0 test environment.
XFAIL tests/integration/collections/test_design_runs.py::test_create_doe_returns_smart_doe_insight@datatemplates - api-designruns is not live in the TEN0 test environment.
XFAIL tests/integration/collections/test_design_runs.py::test_validate_optimization_returns_response@datatemplates - api-designruns is not live in the TEN0 test environment.
XFAIL tests/integration/collections/test_design_runs.py::test_validate_doe_returns_response@datatemplates - api-designruns is not live in the TEN0 test environment.
XFAIL tests/integration/collections/test_design_runs.py::test_create_optimization_accepts_weighted_objectives@datatemplates - api-designruns is not live in the TEN0 test environment.
XFAIL tests/integration/collections/test_design_runs.py::test_validate_optimization_accepts_weighted_objectives@datatemplates - api-designruns is not live in the TEN0 test environment.
XFAIL tests/integration/collections/test_design_runs.py::test_validate_optimization_accepts_plain_criterion@datatemplates - api-designruns is not live in the TEN0 test environment.
XFAIL tests/integration/collections/test_design_runs.py::test_validate_optimization_treats_a_null_weight_as_unweighted@datatemplates - api-designruns is not live in the TEN0 test environment.
XFAIL tests/integration/collections/test_smart_datasets.py::test_smart_dataset_get_data@datatemplates - No DWH available in TEN0 test environment.
XFAIL tests/integration/collections/test_smart_datasets.py::test_smart_dataset_get_data_with_filters@datatemplates - No DWH available in TEN0 test environment.
XFAIL tests/integration/collections/test_tasks.py::test_get_and_set_block_rules@tasks - increased intervals is not live on ten0 test env
XFAIL tests/integration/collections/test_tasks.py::test_create_with_combinations_integration@tasks - increased intervals is not live on ten0 test env
XFAIL tests/integration/collections/test_activities.py::test_activity_search - GET /api/v3/activities/search 500 OpenSearch on TEN0 staging: https://linear.app/albert-invent/issue/SEA-221
XPASS tests/integration/collections/test_smart_datasets.py::test_smart_dataset_create_with_build@datatemplates - No DWH available in TEN0 test environment.
XPASS tests/integration/collections/test_batch_data.py::test_create_batch_data@tasks - Batch data creation is currently not idempotent, so this test may fail if batch data already exists for the task.
XPASS tests/integration/collections/test_targets.py::test_target_create_with_between_parameter_filter@datatemplates - Requires api-targets backend to accept operator/value-pair TargetParameter.value (ML-1207). Remove decorator once deployed.
XPASS tests/integration/collections/test_tasks.py::test_generate_block_combinations_integration@tasks - increased intervals is not live on ten0 test env
XPASS tests/integration/collections/test_substance_v4.py::test_search_pagination@projects - api-substance-v4 search pagination intermittently returns duplicate items across pages (MoleculeEngineering/api-substance-v4#57).
= 1 failed, 354 passed, 37 skipped, 13 xfailed, 5 xpassed, 36 warnings in 628.92s (0:10:28) =