fix(python-client): serialize write_attribute struct values by TLV tag, not field name

[lboue]

Type of change

• 🐛 Fix — corrects a defect or wrong behavior
• ✨ Feature — adds new functionality or capability

Description

write_attribute() in the Python client passed dataclass values (e.g. list[PresetStruct] for the Thermostat Presets/Schedules attributes) straight through to the outgoing WebSocket message, relying on orjson's native dataclass serialization. orjson keys a dataclass by its Python field name (e.g. "presetHandle"), but the server's WRITE_ATTRIBUTE handler (convertWebSocketTagBasedToMatter in ws-controller) expects struct members keyed by their numeric TLV tag ("0"), matching the format already used for attribute reports.

Because of that mismatch, every struct field falls into an "unknown key, keep as-is" fallback that skips type conversion — most importantly, octstr handle fields (presetHandle/scheduleHandle) stay as raw base64 strings instead of being decoded to binary. matter.js then rejects the resulting malformed struct with Status 141 (INVALID_DATA_TYPE) when trying to TLV-encode it. This reproduces even for a byte-for-byte unchanged round-trip write (confirmed against a live device), and is unrelated to the AtomicRequest Begin/Commit sequence, which both report success independently.

Fix: adds dataclass_to_tag_dict() (common/helpers/util.py), which recursively converts a dataclass instance — including nested lists, e.g. Schedule.transitions — into a dict keyed by each field's ClusterObjectFieldDescriptor.Tag, and uses it in write_attribute(). dataclass_to_dict() (field-name-keyed, used by send_device_command for DEVICE_COMMAND payloads) is untouched, since the server's command-invoke path (convertCommandDataToMatter) intentionally expects name-keyed data — the two message types use different wire conventions on the server side.

Backing evidence

• Related issue: write_attribute: struct/list-of-struct writes fail with Status 141 (INVALID_DATA_TYPE) due to name-keyed vs tag-keyed JSON mismatch #957
• This fix/change is backed by a complete log file — attached here or in the linked issue (not just a few quoted lines).

Issue #957 contains the exact raw WebSocket request/response JSON captured directly from the wire for the failing write (both the malformed name-keyed request and the server's Status: 141 response), plus the precise server-side source lines (Converters.ts, ControllerCommandHandler.ts) that explain why that shape is rejected. That is the complete, reproducible evidence this fix is based on.

Checklist

• I understand the code I am submitting and can explain how it works (AI policy)
• Tests added or updated to cover the change (python_client/tests/test_util.py)
• npm test passes — not applicable, this change touches only python_client/ (pure Python), no TypeScript packages
• npm run format-verify and npm run lint pass — not applicable for the same reason; ran the Python equivalents instead: pytest python_client/tests/ (130 passed) and ruff check/ruff format --check on all changed files (clean, aside from a pre-existing repo-wide CPY001 copyright-notice warning present on every file in the project, unrelated to this change)
• CHANGELOG updated

[Copilot]

Pull request overview

Fixes Python client write_attribute() serialization for struct / list-of-struct values to match the server’s tag-keyed WRITE_ATTRIBUTE wire format (TLV tag numbers as object keys), preventing type-conversion from being skipped (notably octstr handle fields) and avoiding INVALID_DATA_TYPE failures on valid writes.

Changes:

• Add dataclass_to_tag_dict() helper to recursively convert CHIP cluster dataclass structs into TLV tag-keyed dictionaries.
• Use dataclass_to_tag_dict() in MatterClient.write_attribute() so outgoing WS payloads match the server’s expected struct shape.
• Add unit tests validating tag-keyed behavior and recursion through nested list members; update changelog.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
python_client/matter_server/common/helpers/util.py	Adds dataclass_to_tag_dict() to produce TLV tag-keyed dicts for struct writes.
python_client/matter_server/client/client.py	Routes write_attribute() values through dataclass_to_tag_dict() before sending.
python_client/tests/test_util.py	Adds tests covering tag-keying, recursive list handling, and leaving dataclass_to_dict() unchanged.
CHANGELOG.md	Documents the Python client fix and its impact on struct writes.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (1)

python_client/matter_server/common/helpers/util.py:104

• The docstring claims CHIP cluster dataclasses are "the only kind ever passed to write_attribute()", but write_attribute() is also used for scalar values (and dataclass_to_tag_dict() explicitly supports non-dataclass inputs). Rewording avoids an inaccurate guarantee while keeping the rationale about descriptors/tags.

    Every generated CHIP cluster dataclass (the only kind ever passed to
    write_attribute()) exposes a `descriptor` with a Tag per field, so the
    tag lookup always succeeds in practice. A dataclass field is only ever
    kept name-keyed if `descriptor`/`GetFieldByLabel` is missing/unset for
    that field, which should not happen for real cluster structs; this is a

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (1)

packages/ws-controller/src/server/Converters.ts:110

• convertWebSocketTagBasedToMatter now always builds/looks up the wire-name member map for every struct conversion, even when all keys are numeric TLV tags (the common case for tag-based WRITE_ATTRIBUTE). This adds avoidable overhead on a hot path; the wire-name map can be initialized lazily only when a non-numeric key is actually encountered.

        const memberById = getStructMembersById(model);
        // Python clients before matter-server 1.3.0 serialized struct fields by wire field name
        // (e.g. "presetHandle") instead of TLV tag, so unrecognized non-numeric keys are resolved
        // against the same name that convertMatterToWebSocketNameBased emits.
        const memberByWireName = getStructMembersByWireFieldName(model, clusterModel);
        for (const key of valueKeys) {
            const tag = parseInt(key);
            const member = Number.isNaN(tag) ? memberByWireName.get(key) : memberById.get(tag);

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

[Apollon77]

Pushed a follow-up commit on top of this PR — thanks @lboue, both sides of the fix were sound. What it adds:

Null semantics — the main gap: unset optional fields traveled as explicit null (orjson emits null for every unset dataclass field; the new tag-keyed helper did too). matter.js rejects null on optional non-nullable members, so partial-struct writes (e.g. a heat-only schedule transition) still failed. Now:

• Server: WRITE_ATTRIBUTE struct conversion treats null for an optional non-nullable member as omitted — same guard the command-invoke path already had. This is what actually unblocks pre-1.3.0 name-keyed clients.
• Python client: dataclass_to_tag_dict() omits None fields (matching CHIP's TLV encoder); NullValue still travels as null.

Key parsing hardening — only purely-numeric keys are treated as TLV tags ("5x" no longer resolves to tag 5), and the name fallback also accepts legacy matter.js property names (e.g. iPv4Addresses) which the server dual-emits since #927.

Tests — converter unit tests for the null/key rules (incl. a wire-name≠propertyName case via GeneralDiagnostics.NetworkInterface), a client test pinning the tag-keyed write_attribute payload, and end-to-end struct-list write round-trips in both integration suites (tag-keyed and name-keyed) via a new UserLabel cluster on the test light device.

All gates green: npm run format / lint / build / full npm test (incl. both integration suites), pytest 132 passed, ruff clean.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (3)

packages/ws-controller/test/ConvertersTest.ts:40

• This test name hard-codes a version boundary ("pre-1.3.0") which is easy to stale. Consider naming it based on the wire format being accepted (wire-field-name keyed struct members) instead of a specific release number.

    it("falls back to wire field names for pre-1.3.0 python clients that serialized by name", () => {

packages/ws-controller/src/server/Converters.ts:107

• The comment hard-codes a version boundary ("before matter-server 1.3.0") that is likely to become inaccurate and make the rationale harder to trust later. Consider rewording it to be version-agnostic (e.g., "Older Python clients serialized struct fields by wire field name"), since the code already detects the format by key shape (numeric vs non-numeric).

        // Python clients before matter-server 1.3.0 serialized struct fields by wire field name
        // (e.g. "presetHandle") instead of TLV tag, so unrecognized non-numeric keys are resolved
        // against the same name that convertMatterToWebSocketNameBased emits.
        const memberByWireName = getStructMembersByWireFieldName(model, clusterModel);

packages/ws-controller/test/ConvertersTest.ts:28

• This test name embeds a specific version claim ("matter-server >=1.3.0") that may quickly become wrong. Renaming it to describe the behavior being tested (tag-keyed struct members) keeps the intent accurate over time.

This issue also appears on line 40 of the same file.

    it("resolves struct members by numeric TLV tag (matter-server >=1.3.0 python client)", () => {

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (2)

CHANGELOG.md:22

• The PR description/checklist says the change “touches only python_client/” and marks npm run format-verify/npm run lint/npm test as “not applicable”, but this PR also changes TypeScript packages (packages/ws-controller/ and packages/matter-server/). Please update the PR description/checklist (and ensure the TS-side format/lint/build/tests are run) so reviewers/CI expectations match the actual scope.

- Fix: Improves ICD UI handling when deactivating the LIT mode
- Fix: `WRITE_ATTRIBUTE` now also accepts struct members keyed by wire field name, not only by TLV tag
- Fix: (lboue) Python client `write_attribute()` now serializes struct/list-of-struct values keyed by TLV tag instead of field name as the server's `WRITE_ATTRIBUTE` handler expects

packages/ws-controller/src/server/Converters.ts:110

• parseInt should be called with an explicit radix to avoid legacy parsing edge cases (and to match the rest of the codebase’s use of Number.parseInt(..., 10) in newer code).

            const isTag = /^\d+$/.test(key);
            const member = isTag ? memberById.get(parseInt(key)) : memberByWireName.get(key);

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (1)

python_client/matter_server/common/helpers/util.py:120

• dataclass_to_tag_dict() currently omits any field whose value is None unconditionally. For non-optional (mandatory) fields this diverges from the CHIP SDK’s own TLV encoding behavior (ClusterObjects.ClusterObjectFieldDescriptor.PutFieldToTLV), which raises when a non-optional field is None. Silently dropping a mandatory field can mask caller bugs and may result in malformed partial structs being sent to the server.

        for field in cached_fields(type(obj_in)):
            value = getattr(obj_in, field.name)
            if value is None:
                continue

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (3)

packages/ws-controller/test/ConvertersTest.ts:40

• This test description hard-codes a version boundary ("pre-1.3.0") that may not match actual client/server release versioning. Consider rephrasing to simply describe the legacy behavior (name-keyed struct members).

    it("falls back to wire field names for pre-1.3.0 python clients that serialized by name", () => {

packages/ws-controller/src/server/Converters.ts:106

• The comment refers to “Python clients before matter-server 1.3.0”, which is ambiguous (server vs client versioning) and may become inaccurate over time. Consider rephrasing to describe the wire format difference without tying it to a specific version number.

        // Python clients before matter-server 1.3.0 serialized struct fields by wire field name
        // (e.g. "presetHandle") instead of TLV tag, so unrecognized non-numeric keys are resolved
        // against the same name that convertMatterToWebSocketNameBased emits.

packages/ws-controller/test/ConvertersTest.ts:28

• The test name bakes in a specific version relationship ("matter-server >=1.3.0 python client") that isn’t enforced by the code and may become misleading. Prefer describing the behavior (tag-keyed struct members) without a version qualifier.

This issue also appears on line 40 of the same file.

    it("resolves struct members by numeric TLV tag (matter-server >=1.3.0 python client)", () => {