breaking: port to smp screaming-goblin (Frame[T] on msgspec)

[JPHutchins]

Warning

LLM Disclosure

This PR was authored by claude-opus-5[1m] on behalf of @JPHutchins, who asked me to continue the screaming-goblin effort autonomously after #136 merged, treating their CLAUDE.md as law.

Note

Targets screaming-goblin, the breaking branch, per the epic's rule (intercreate/smpmgr#103). Closes #124.

smp's screaming-goblin replaces the flattened pydantic message model with composition — a message is a Frame[T] = (Header, Data[T]) on msgspec.Struct — and bakes each request's _Response/_ErrorV1/_ErrorV2 binding into the request class. This ports smpclient onto it.

Net −872 lines (576 insertions, 1448 deletions; 20 files deleted).

The request layer is gone (#124)

smpclient/requests/** existed only to attach the response/error binding to each smp request. smp does that itself now, so the layer is deleted outright rather than deprecated — screaming-goblin is the breaking release. Callers move to smp's own names, which differ:

was	now
CountSupportedGroups	smp.enumeration_management.GroupCountRequest
ImageStatesRead	smp.image_management.ImageStatesReadRequest
EchoWrite	smp.os_management.EchoWriteRequest

25 files migrated here; smpmgr will need the same.

The narrowers stay (but come from smp's design)

smp deliberately does not ship success/error/error_v1/error_v2 — its own typing test says so outright ("without the smp library having to ship them") — so they stay in smpclient. SMPRequest now comes from smp.

They use smp's non-generic TypeIs form. TRep/TEr1/TEr2 remain for request()'s own signature, just not threaded through the narrowers. The repository now has zero pyright warnings, down from five reportInvalidTypeVarUse — which unblocks turning on --warnings in #134.

Correction. An earlier revision of this PR kept generic narrower signatures, and its commit message claimed smp's non-generic form eroded TRep for a generic caller and broke the ensure_request helper in examples/*/upgrade.py. That was wrong. The pyright error behind the claim was a cascade from an unrelated broken import (SMPRequest had just moved to smp, so SMPRequest[TRep, …] degraded to Unknown). Verified by swapping both forms in place and running the whole gate against each: both are clean under mypy and pyright, and assert_type(response, EchoWriteResponse) holds either way. Fixed in 9c461ba.

request()

The frame is built once, because it carries the sequence the response must echo:

request_frame = request.to_frame()
... send bytes(request_frame) ...
if header.sequence != request_frame.header.sequence: raise SMPBadSequence

loads() returns a Frame, so the client returns .data.

The decode chain catches DecodeError, not ValidationError

Both are reachable, and the wider catch is deliberate — measured:

payload	raises
wrong schema / unknown field	ValidationError
truncated CBOR	bare DecodeError
empty	bare DecodeError

Catching only ValidationError would let a truncated response escape as a raw msgspec traceback instead of the SMPValidationException carrying the header and hexdump. Under smp 4.x a raw cbor2 error escaped uncaught, so this is also a small improvement.

SMPMalformed and SMPMismatchedGroupId are deliberately not caught. They fail all three candidate types identically, which makes them transport errors rather than "this frame matched no schema" — swallowing them would report a group mismatch as three parse failures. Both cases now have tests.

Size math

bytes(Data) is the CBOR payload alone — the old bytes(message) included the 8-byte header — so get_max_cbor_and_data_size subtracts Header.SIZE explicitly. _maximize_upload_packet collapses to one call, since to_frame() computes length from the actual payload:

return msgspec.structs.replace(request, data=data[request.off : request.off + data_size])

_ic_maximize_packet was a hand-rolled copy of that logic, needed only because pydantic made field-carrying manual. It is deleted and ICUploadClient uses the generic one.

The byte-exact expectations in test_maximize_upload_packet_fills_decoded_buffer were left untouched and pass — the maximizer still fills max_unencoded_size exactly for buf_size 384/512/1024/2048. That is the check that would have caught an off-by-8.

Tests

SMPMockTransport now echoes the request's sequence back on the response, the way a real server does. request() draws the sequence from smp's counter when it frames, so a test cannot know it in advance — and with the echo, the 22 hand-built smphdr.Header(...) blocks and every (h.sequence + 2) % 0xFF computation simply disappear. A sequence_offset fakes a server answering out of order for the SMPBadSequence case.

ErrorV1/ErrorV2 declare no _OP/_COMMAND_ID, so to_frame() cannot synthesize their header; a test that needs their bytes builds it by hand, exactly as smp's own test_error.py does. That is error_bytes().

Docs

Deleting the request layer removes the only reason docs/_generate_requests_docstrings.py existed — it back-filled inherited docstrings onto those subclasses. The script, its step in both release.yaml and test-docs.yaml, the seven per-group pages, and their nav entries all go. docs/requests.md stays (the narrowers it documents stay), and docs/user/intercreate.md now points at smpclient.extensions.intercreate, which is genuinely smpclient's.

Dependency

smp is pinned to the branch by direct reference (hatchling needs allow-direct-references), since smp is not released with the break — both marked TODO to undo at release. msgspec is now declared: this module imports it directly for DecodeError and structs.replace, and relying on it arriving through smp would repeat the undeclared-transitive mistake #133 just fixed for pydantic.

typing_extensions is now declared too, and the way it surfaced is worth recording: it is imported at runtime by twelve modules — generics needs TypeIs, the transports need override/assert_never — but was never declared. It had been arriving transitively via pydantic, via smp 4.x, so dropping pydantic from the tree took it with it.

The transport-extras CI job caught it and the others could not: it runs uv venv && uv pip install ., which resolves only declared dependencies, while every other job installs from the lockfile (where the dev group happens to supply it). That job is the only guard against this class of bug — the third undeclared-transitive dependency in this effort, after pydantic (#133) and msgspec.

With #133, src/ no longer imports pydantic at all.

Verification

• camas matrix green on Python 3.10–3.14
• Integration suite 229/229 against real Zephyr servers — serial, raw serial, COBS raw, UDP, and MCUboot serial recovery. This is what establishes the msgspec encoder puts the same bytes on the wire.

On the one integration flake seen during this work

test_upload_to_mcuboot_recovery[mps2_an385.serial_recovery_raw-raw] — the flake #128 addressed — recurred. I checked it was not a regression by running it on both trees under identical load:

tree	result
this branch	1 failure / 12 runs
main	1 failure / 5 runs

Same rate, so the port does not cause it. Worth noting separately though: #128's pacing is evidently not sufficient under heavy machine load — it measured 40/40 last week and ~10–20% failures today with qemu suites running back to back. That belongs with #131/#132 rather than here.

Follow-ups filed

• SMPRequest.to_frame() hides sequence/version/flags, forcing clients onto the module-global counter JPHutchins/smp#70 — SMPRequest.to_frame() hides sequence/version/flags, so a client cannot own its sequence space and is forced onto smp's module-global counter. Also blocks Convenience methods should expose SMP version kwarg #43/feat(client): expose the SMP version on the convenience methods #126.

Conflicts to expect

Open PRs #123 (upload retries) and #126 (SMP version kwarg) both touch code this rewrites; whichever lands second will need rework.

[JPHutchins]

Pretty good. Can we make sure we have solid assert_never coverage for generic typing? E.g. our request function.

Should we consider leaving the old requests namespace alive, including the shortened names, and just have a deprecation warning (they would re-export the smp types)?

We should link to the smp docs from request.md I think.

[JPHutchins]

We can drop generics and move this simple stuff into top level

[JPHutchins]

Done in 1759213 — smpclient/generics.py is deleted and the four narrowers plus TRep/TEr1/TEr2 now live in smpclient/__init__.py, next to the request() whose return type they narrow.

Agreed it no longer earned a module: the one part that justified the separation was the SMPRequest Protocol, and that moved to smp.

Import sites become from smpclient import success, error, ... across src/, tests/, and examples/.

[JPHutchins]

huh? Isn't it transitive?

[JPHutchins]

It was transitive — that's precisely why it broke.

typing_extensions was arriving via pydantic, via smp 4.x. This PR pins smp to screaming-goblin, which dropped pydantic, so it left the dependency tree with it. CI caught it:

File ".../smpclient/__init__.py", line 54, in <module>
    from typing_extensions import assert_never
ModuleNotFoundError: No module named 'typing_extensions'

All six transport-extras jobs failed on it, and nothing else did — because that job runs uv venv && uv pip install ., which resolves only declared dependencies, while every other job installs from the lockfile (where the dev group happens to pull typing_extensions in anyway). It's the one job that can catch this class of bug.

It's a genuine runtime dependency across the whole supported range, used by 12 modules in src/:

• generics needs TypeIs — stdlib only from 3.13, and we support 3.10+
• the transports need override (stdlib 3.12+) and assert_never (stdlib 3.11+)

Hence >=4.10, which is where TypeIs landed.

Worth noting this is the third undeclared-transitive dependency in this effort — pydantic (#133), then msgspec, now this. Each was invisible while something else happened to pull it in.

[JPHutchins]

Warning

LLM Disclosure

This comment was posted by claude-opus-5[1m] on behalf of @JPHutchins, responding to their review of this PR.

Pushed 1759213. Three of the four are done; the fourth is a real decision I'd rather put back to you with the one fact that changes it.

✅ assert_never coverage for generic typing

tests/test_generics_typing.py drives the real SMPClient.request and asserts narrowing for every group, checked under mypy and pyright, both clean.

It uses assert_type rather than a looser "is it a response" check, which turns out to matter — it is what proves the narrower decision in this PR:

response = await client.request(os.EchoWriteRequest(d="hello"))
if success(response):
    assert_type(response, os.EchoWriteResponse)      # exact, not widened
elif error_v1(response):
    assert_type(response, os.OSManagementErrorV1)
elif error_v2(response):
    assert_type(response, os.OSManagementErrorV2)
else:
    assert_never(response)

With smp's non-generic TypeIs narrowers this file fails — success() yields ReadResponse | WriteResponse instead of the request's own response type. So the file is now the regression test for keeping the generic form.

Omitted from coverage: it is never executed, so there is no runtime behaviour to measure.

✅ smpclient.generics dropped to top level

Deleted; narrowers and TypeVars now live in smpclient/__init__.py. Details on that thread.

✅ docs/requests.md links to smp

Rewritten to carry the per-group links that the seven deleted pages used to hold, plus the narrowing example and the helpers themselves.

❓ Keeping smpclient.requests alive as deprecated re-exports

Worth reconsidering, and it is cheaper than what we rejected earlier — plain aliases (ImageStatesRead = smpimg.ImageStatesReadRequest) need none of the subclass layer that #124 exists to delete. Roughly 25 lines in one module.

But there's a fact that undercuts it, which I measured earlier in this effort: a bare alias cannot be marked deprecated in a way a type checker sees. PEP 702's @deprecated needs a class or function to decorate; a module-level assignment has nothing to attach to.

Signal	mypy	pyright
@deprecated on a class	errors (with --enable-error-code=deprecated)	errors (with reportDeprecated)
plain alias (X = smp.Y)	nothing	nothing

So the shim would give a runtime DeprecationWarning via module __getattr__ and nothing else. For smpmgr — typed, and the main consumer — that's the weaker half: the checker won't point at the call sites, so the migration stays manual either way. You'd just find out when the code runs instead of when it imports.

My recommendation: keep the hard break, since the type checker can't help either way and the shortened names are the SSOT problem #124 is removing. The thing that would genuinely help external users is a migration table in the release notes — 33 symbols, mechanical — which I can write.

That said, this is a public package with outside contributors, and ModuleNotFoundError: smpclient.requests.os_management with no pointer is a poor landing. If you'd rather have the softer version, say so and I'll add the alias module with the runtime warning; it's small.

Note on smp#70

Understood that the smp changes land before this merges. When JPHutchins/smp#70 is in, I'll resync and switch request() to a client-owned sequence counter — which is what that issue exists to unblock.

[JPHutchins]

Warning

LLM Disclosure

This comment was posted by claude-opus-5[1m] on behalf of @JPHutchins, correcting an incorrect claim it made earlier in this PR.

Correcting myself, in answer to "does this need an upstream smp fix?" — no, and my reason for not using smp's narrowers was wrong.

I claimed above that smp's non-generic TypeIs narrowers erode TRep for a generic caller, breaking the ensure_request helper in examples/*/upgrade.py, and that tests/test_generics_typing.py was the regression test proving it.

That is false. The pyright error I based it on:

examples/usb/upgrade.py:126:24 - error: Type "ReadResponse | WriteResponse" is not assignable to return type "TRep@ensure_request"

was a cascade from an unrelated broken import in the same edit — the examples still did from smpclient.generics import SMPRequest, which had just moved to smp, so SMPRequest was an unknown symbol and SMPRequest[TRep, TEr1, TEr2] degraded to Unknown. That is what made the narrowed value unassignable to TRep. Fixing the import fixed it; the narrowers were never involved.

I verified this properly by swapping the two forms in place and running the whole gate against each:

	mypy	pyright	assert_type exhaustiveness	generic ensure_request
generic narrowers	clean	clean, 5 warnings	holds	type-checks
smp's non-generic	clean	clean, 0 warnings	holds	type-checks

So smp's form is strictly better and is now adopted in 9c461ba. The repository has zero pyright warnings, down from five reportInvalidTypeVarUse — those five were the reason #134 could not simply turn on --warnings, so that is unblocked too.

No upstream smp change is needed for any of this. The one upstream item that does remain is JPHutchins/smp#70 (to_frame() hiding sequence/version/flags), which is unrelated to narrowing.

[JPHutchins]

Some changes needed

[JPHutchins]

Why "u8" - never necessary

[JPHutchins]

Fixed in 1a92ac0 — every use is bare u8 now, annotations and the one cast.

The quotes were there because types_bits was imported under TYPE_CHECKING. Importing it at runtime does remove them, but it breaks a clean install:

File ".../smpclient/__init__.py", line 56, in <module>
    from types_bits import u8
  File ".../types_bits/__init__.py", line 20, in __getattr__
    from annotated_types import Ge, Le
ModuleNotFoundError: No module named 'annotated_types'

types-bits declares annotated-types only under an rt extra, and its own module docstring says why: "__init__.pyi shadows this module for type checkers; this is the runtime tier." It's typing-only by design — so making every smpclient install carry annotated_types just to unquote an annotation is a bad trade.

So the name gets a runtime value that checkers never see:

if TYPE_CHECKING:
    from types_bits import u8
else:  # `types_bits` is typing-only; reaching its values at runtime needs its `rt` extra
    u8 = int

Bare everywhere, no runtime dependency, nothing new declared. Caught and verified with a clean uv pip install . — the same check that found the typing_extensions gap.

[JPHutchins]

User should have to provide an Iterator[u8] that defaults to the default one you made. Will make testing easier and support future direction for resetting sequence etc.

[JPHutchins]

Done in 1a92ac0.

SMPClient(transport, address, sequence=iter((7, 9)))

defaulting to wrapping_sequence() — the 0x00–0xFF space the client used to hold privately. _next_sequence() is gone; request() just takes next(self._sequence).

Two tests came with it:

• test_injected_sequence — asserts the injected values actually reach the header on the wire, rather than inferring them
• test_wrapping_sequence — pins the default's range and its wrap at 0x100

Agreed on the future direction too: resetting or sharing a sequence space is now a caller's decision rather than something to retrofit.