restXml protocol: reviewed implementation plan by chris-armstrong · Pull Request #37 · chris-armstrong/smaws

chris-armstrong · 2026-07-17T19:49:26Z

What this is

Plan-only PR for adding the Smithy aws.protocols#restXml protocol (S3, CloudFront, conformance test services). No restXml implementation yet — this PR lands only the reviewed implementation plan (refactorings/restxml-protocol.md + .todo.md).

Branch is based on the current main (which already contains the AwsQuery work via PRs #33/#34/#35). Diff against main is just the two plan files (835 lines, 0 deletions).

Plan changes from the original draft (revised after a three-agent review)

Re-baselined current-state inventory against main (AwsQuery now context-based; Protocol.t dispatch exists; codegen/AwsProtocolQuery.ml is the codegen template; shape count 21→23).
G6b reclassified blocker: xmlNamespace prefix is conformance-required (model uses xmlns:baz=…, xsi:…); added service-level default namespace.
Q3 resolved: httpPayload on a structure emits the structure's own root element as the body. httpPayload blob = raw, not base64.
Added missing conformance-required behaviors to scope: httpResponseCode, idempotencyToken auto-fill, enum/intEnum, per-binding timestamp defaults, httpQuery/httpQueryParams precedence, greedy-label / non-encoding, empty-prefix httpPrefixHeaders + httpHeader precedence, list-valued httpQueryParams, null/empty handling, xmlNamespace on list/map/members, endpoint/hostLabel host-prefix substitution.
noErrorWrapping root (<Error>) stated definitively; corrected the false claim that the targeted namespaces exercise it (they use the wrapped envelope; noErrorWrapping is S3 / Phase 9).
Folded the AwsQuery review failure modes into the design: skip-siblings in the error-envelope parse, Failure catch, separate RestXml.Error (not folded into the JSON-shaped AwsErrors), String.equal error dispatch, strengthened Phase 5/6 checkpoints, full-envelope error mock.
Banned the two requestCompression test IDs (out of scope; separate plan add_request_compression.md).
Resolved Q1–Q6.

Notes

Only markdown is changed; dune fmt (ocamlformat) is not applicable to this diff. The OCaml on main is already formatted to the project's pinned 0.27.0.
Environment note: this box's default opam switch is OCaml 5.5.0 (project requires >= 5.0); installed ocamlformat is 0.29.0 vs the project's pinned 0.27.0, so a full-tree dune fmt is not run here to avoid version-mismatch churn.

Status

Draft, assigned to @chris-armstrong for review of the plan before implementation begins (Phase 0 onward, per the plan's "stop after each phase" rule).

Land the restXml protocol implementation plan (`refactorings/restxml-protocol.md` + `.todo.md`), revised after a three-agent review (plan-vs-codebase audit, spec-conformance audit, guidelines/risk audit) against the current tree (AwsQuery is already on main via PRs #33/#34/#35). Key plan changes from the original draft: - Re-baseline the current-state inventory against main (AwsQuery is context-based; Protocol.t dispatch exists; AwsProtocolQuery.ml is the codegen template; appliesTo/Float/Stack.Empty fixes present; shape count 21->23). - Reclassify xmlNamespace `prefix` (G6b) as a blocker — the conformance model requires it; add service-level default namespace handling. - Resolve Q3: httpPayload on a structure emits the structure's own root element as the body (not bare members). Fix httpPayload blob = raw, not base64. - Add missing conformance-required behaviors to scope: httpResponseCode, idempotencyToken auto-fill, enum/intEnum, per-binding timestamp defaults, httpQuery/httpQueryParams precedence, greedy-label "/" non-encoding, empty-prefix httpPrefixHeaders + httpHeader precedence, list-valued httpQueryParams, null/empty handling, xmlNamespace on list/map/members, endpoint/hostLabel host-prefix substitution. - State the noErrorWrapping root (<Error>) definitively; correct the false claim that the targeted namespaces exercise it (they use the wrapped envelope; noErrorWrapping is for S3, Phase 9). - Fold the AwsQuery review failure modes into the design: skip-siblings in the error-envelope parse, Failure catch, separate RestXml.Error (not folded into the JSON-shaped AwsErrors), String.equal error dispatch, strengthened Phase 5/6 checkpoints, full-envelope error mock. - Ban the two requestCompression test IDs (out of scope; separate plan). - Resolve Q1-Q6. No restXml implementation yet; plan only.

Gets the restxml branch building green and adds the Phase 2 unit-test checkpoint from refactorings/restxml-protocol.md. Build fix: - Initialize the api-models-aws submodule (168 "No rule found" errors). - Drop stale promoted files in model_tests/protocols/restxml/ (operations.mli, restxml.mli) left by an older generator that emitted a closed RestXml.error type and an operations section in the .mli. The current generator skips both for RestXml (sdkgen.ml, gen_module.ml); dune's promote rule wasn't regenerating them because the targets already existed on disk. Deleting them lets dune regenerate them fresh. Phase 2 — HTTP binding helpers (smaws_lib/Http_bindings.ml): - New unit tests (smaws_lib_test/http_bindings_test.ml): greedy vs non-greedy '/' handling, path/query pct-encoding, @httpQuery-over-@httpQueryParams precedence (incl. list-valued maps), @httpHeader-over-@httpPrefixHeaders precedence (incl. empty prefix), @endpoint host-prefix substitution. - Fix percent_encode_path_segment ~greedy:false dropping '/' (split_on_char then concat "" lost the separator) — non-greedy labels now pct-encode '/' via Uri.pct_encode ~component:`Path so a single-segment label can't inject a path separator. - Fix substitute_host_prefix wrongly pct-encoding host labels: pct-escaping a host is never valid (produces e.g. "caf%C3%A9"), and the correct encoding for an internationalized label would be Punycode, which the uri library doesn't do and which is out of scope (no IDNA dep, no conformance coverage). @hostLabel values are DNS labels, so they are substituted raw. - Expose Http_bindings as Smaws_Lib.Http_bindings (Phase 7 codegen will need it public). Agent/tooling: - AGENTS.md: add "Dune invocation rules" — always set a tool-call timeout on dune, and check/clear _build/.lock before invoking (an aborted dune call can leave a stale lock blocking all later builds). - .gitignore: ignore .pi/ (agent-local config). Checkpoint: dune build + dune fmt + dune runtest smaws_lib_test green. restXml conformance (model_tests) still 170/171 failing — Phases 5-8 remain.

Phase 3 checkpoint (restxml-protocol.md §9): add the mock smoke test the plan requires — one success path plus an error path whose mock body keeps a trailing <RequestId> sibling of <Error> to exercise the skip-siblings guard (the AwsQuery-class trailing-sibling bug). Mirrors aws_query_response_test.ml: Alcotest with raw XML string fixtures, plus extra cases for the noErrorWrapping (S3) envelope and parse_error_struct. Writing the success-path test surfaced a real bug in RestXml.request's 2xx branch: it called Read.skip_to_end *after* the output deserializer consumed the document's single root element. At top level Xmlm.peek then hits end-of-input and raises Xmlm.Error, which Xml.Parse.run's catch-all turns into a spurious XmlParseError — so every valid success response would have been reported as a parse error once Phase 5+ wired real deserializers. Fix by factoring the 2xx flow into parse_response ~body ~output_deserializer that reads the DTD and hands the Xmlm.input to the deserializer without peeking past the root. request's 2xx branch now calls it. The deserializer owns consumption of the root and any siblings inside it (same contract AwsQuery.Response.parse_xml_ok_response relies on, where skip_to_end runs *inside* the root's Read.sequence callback, not after it). Phase 4 re-verified intact: SmithyHelpers.protocol has RestXml; protocol_of_traits maps AwsProtocolRestXmlTrait _ -> Some RestXml; gen_operations/gen_serialisers/gen_deserialisers branch on RestXml; Modules.ml has protocolRestXml/xml/xmlWrite/xmlParse; AwsProtocolRestXml.ml exists. dune build green with restXml services generating. Checkpoint: dune build + dune runtest smaws_lib_test green (new test 4/4: success parse, error recovers <Message> + skips trailing <RequestId>, noErrorWrapping direct children, parse_error_struct members + skip).

…back) Real AWS restXml responses carry the request id in two places: a response header (x-amzn-requestid for most services; x-amz-request-id / x-amz-requestid for S3) and, on error, the body <RequestId> sibling of <Error>. Phase 3 parsed the body <RequestId> (to drive skip-siblings) but request discarded it and never read response headers — so the id was not surfaced at all. Success (2xx) bodies have no <RequestId>, so for them the header is the only source. Mirror AwsQuery: reuse the shared Smaws_Lib.Response module ({ request_id : string option }, 'a t = { response; metadata }). - request_id_of_headers: case-insensitive lookup over [x-amzn-requestid; x-amz-request-id; x-amz-requestid], first non-empty value wins. - request_id_prefer_header: header wins, body <RequestId> as fallback (success path has no body id, so it's header-only there). - request_with_metadata: returns ('out Response.t, 'err * Response.metadata) result (Ok = { response; metadata } record, Error = ('err * metadata) tuple — exactly AwsQuery.request_with_metadata's shape). Success metadata = header id; error metadata = prefer_header ~header ~body. - request keeps its existing ('out, 'err) result signature (the Phase 4 generated stubs call it) and becomes a metadata-stripping wrapper. No codegen change; Phase 7 may switch the generated request to call request_with_metadata to surface the id to callers. Pure helpers unit-tested in restxml_response_test.ml (case-insensitive lookup, S3-style header, empty/absent, header-over-body precedence); body recovery stays covered by the existing parse_error_envelope test. request_with_metadata needs an HTTP context and is not runtime-mocked, same stance as AwsQuery.request_with_metadata. Plan updated: restxml-protocol.md §6.4 + Phase 3 bullet; todo Phase 3 item. Checkpoint: dune build + dune runtest smaws_lib_test green (RestXml response parsing now 6/6: 4 prior + 2 request-id).

chris-armstrong · 2026-07-18T11:05:01Z

+                   [
+                     (Nolabel, exp_ident "w");
+                     ( Nolabel,
+                       B.pexp_apply


This pattern appears several times, write a helper and reuse it everywhere

Added qualified_apply / qualified_ident in codegen/Ppx_util.ml and reused them across all ~28 sites in AwsProtocolRestXml.ml that built B.pexp_apply (B.pexp_ident (Location.mknoloc (make_lident ~names:...))). Verified the refactor produces byte-identical generated output (diffed generator output with/without the refactor).

…29.0 The generated model_tests/protocols/{json,query,shared,restxml} files committed in f4fa179 ("land Phases 0-4") were raw generator output never run through dune fmt — they used the pre-0.29.0 style ([@@deriving (show, eq)] with parens, compact record layout without semicolons, type-annotation attached to the closing brace). Run dune fmt under ocamlformat 0.29.0 (the version pinned in .ocamlformat) to normalize them: [@@deriving show, eq], multi-line record layout with semicolons and per-field [@ocaml.doc], type annotation on its own line. Pure formatting — no semantic changes (the generator is unchanged since f4fa179; my Phase 3 commits touched only the RestXml runtime, tests, and plan docs). Idempotent: dune fmt produces no further diff after this. Build green; dune runtest smaws_lib_test green (RestXml response parsing 6/6, all other executables pass). restxml was already on top of origin/main (merge-base = origin/main, behind 0 / ahead 4), so the rebase onto main was a no-op; this commit is the only new change.

chris-armstrong · 2026-07-18T11:36:11Z

+            Some (float_of_string (Smaws_Lib.Xml.Parse.Read.element i "doubleValue" ()))
+      | "floatValue" ->
+          r_float_value :=
+            Some (float_of_string (Smaws_Lib.Xml.Parse.Read.element i "floatValue" ()))


float_of _string is throwing - we should create a wrapper that rethrows the right exception our outer callers can catch and report the correct xml node that failed for debugging purposes. Int_of_string is the same

Deleted — query_deserializers.ml was stray dead code in the restxml dir (not compiled, not referenced). See reply on the thread above.

Addressed in 769d2d3 — the real generated deserialisers (xml_deserializers.ml) now report the failing element path. Rather than threading a path : string list (which would cons per descent), the leaf converters in the new Smaws_Lib.Xml.Parse.Primitive raise a path-less XmlDeserializeError and each enclosing element reader (Read.sequence/Read.sequences/Read.element_value/Read.elements_value) catches it and prepends its tag as the exception unwinds — zero happy-path allocation (only try..with frames), the path string is built only on failure. run formats it as "invalid <kind> at root/.../leaf: <value>". Same pattern for int/long/bigint/bigdecimal/bool/float/double/blob/timestamp. Unit test in smaws_lib_test/xml_parse_error_test.ml; restXml conformance failures unchanged (129, all pre-existing) and AwsQuery unaffected.

chris-armstrong · 2026-07-18T11:40:58Z

+  | Integer v ->
+      Smaws_Lib.Xml.Write.element w "integer" (fun w ->
+          Smaws_Lib.Xml.Write.text w (string_of_int v))
+  | String v -> Smaws_Lib.Xml.Write.element w "string" (fun w -> Smaws_Lib.Xml.Write.text w v)


Smaws_Lib.Xml.Write could be opened to reduce repetition

Done — the restXml serializer generator now emits open Smaws_Lib.Xml.Write at the top of generated xml_serializers.ml and references element / text / null unqualified. Regenerated model_tests/protocols/{shared,restxml}/xml_serializers.ml in 3f2a841.

chris-armstrong · 2026-07-18T11:54:43Z

+    independently of the smithy compliance fixtures:
+
+    - [parse_response] (the 2xx success path) must hand the [Xmlm.input] to the generated output
+      deserializer and return its value. Previously the 2xx branch called [Read.skip_to_end] *after*


Remove references to previous behaviour

Done — removed the "Previously the 2xx branch..." narration and the other "previous behaviour" references throughout the test comments; they now describe what each fixture verifies. 3f2a841

Runtime / hand-written code: - smaws_lib/Http_bindings.ml: replace Str regexes with a plain stdlib substring search (index_of_substring). Str uses process-global state for matched groups and is not safe to call concurrently with other Str users (e.g. Ini); the binding helpers only ever do literal substring match/replace, so no regex engine is needed. - smaws_lib/protocols_impl/RestXml.ml: drop phase/plan references from the `request` doc comment. - smaws_lib_test/restxml_response_test.ml: strip phase/plan references and "previous behaviour" narration from the test doc comments; describe what each fixture verifies instead. Dead code: - Remove model_tests/protocols/restxml/query_{de,}serializers.ml. They were committed by accident: not listed in the restxml dune `(modules ...)`, not produced by the restxml generate rule, and not referenced anywhere. This retires the three review threads on query_deserializers.ml. Generator: - codegen/Ppx_util.ml: add reusable `qualified_ident`, `qualified_apply`, and `exp_fun_ident_any` (a `fun arg _ -> exp` lambda) helpers. - codegen/AwsProtocolRestXml.ml: use them to replace the repeated `B.pexp_apply (B.pexp_ident (Location.mknoloc (make_lident ~names:...)))` pattern and the `fun i _ -> ...` scan-callback lambdas. Verified byte-identical generated output (diffed generator output with/without the refactor). - sdkgen/gen_serialisers.ml + AwsProtocolRestXml.Serialiser: emit `open Smaws_Lib.Xml.Write` in generated restXml serializers and reference `element`/`text`/`null` unqualified, removing the `Smaws_Lib.Xml.Write.` repetition. Regenerated model_tests/protocols/{shared,restxml}/xml_serializers.ml. Not addressed: the "open Smaws_Lib.Smithy_api.Types in generated types.ml" suggestion. That generator is shared by every protocol and would reformat the types.ml of all 168 generated SDKs plus every protocol-tests suite — a cross-cutting regeneration tracked separately rather than folded into this PR.

Address the review comment that bare [float_of_string]/[int_of_string] in generated restXml deserialisers raise [Failure] with no element context, so a malformed leaf reports only "float_of_string..." and not which node failed. Mirror [Json.DeserializeHelpers]' nested-context reporting, but allocation-free: instead of threading a [path : string list] (which would cons per descent), the leaf converters raise a path-less [Xml.DeserializeError] and each enclosing element reader catches it and prepends its tag as the exception unwinds. The happy path does zero heap allocation (only [try..with] frames, which are zero-cost until raised); the path string is built only on failure. [run] formats it as "invalid <kind> at <root>/.../<leaf>: <value>". Runtime (smaws_lib/Xml.ml): - [Parse.XmlDeserializeError] + [deserialize_error] record + [path_to_string]. - [Read.sequence]/[Read.sequences] decorate [XmlDeserializeError] with their tag (only that exception; other exceptions keep their own position). - [Read.element_value]/[Read.elements_value]: read text and run a converter function value (no per-call closure) with tag decoration. - [Parse.Primitive]: path-less leaf parsers (int/long/bigint/bigdecimal/ bool/float/double/blob/timestamp-iso/epoch/httpdate) raising [XmlDeserializeError]. Used by generated restXml deserialisers instead of bare [Stdlib.*_of_string] / [CoreTypes.*] / [Ptime.of_rfc3339]. - [run] catches [XmlDeserializeError] into [XmlParseError]. Generator (codegen/AwsProtocolRestXml.ml): - [primitive_conv] maps a smithy primitive target to the [Xml.Parse.Primitive.<kind>_of_string] converter ([Fun.id] for String). - Primitive members use [Read.element_value i tag conv]; primitive list items use [Read.elements_value i tag conv]; map keys/values use [Read.element_value]. Timestamp members use the format-specific [Primitive.timestamp_*_of_string] via [Read.element_value]. - Top-level primitive shapes route through [Xml.Parse.Primitive] too. - Drops [parse_primitive_from_string]. Regenerated model_tests/protocols/{shared,restxml}/xml_deserializers.ml. Test: smaws_lib_test/xml_parse_error_test.ml verifies a malformed float/int reports the full "root/.../leaf" path and that valid parses are unaffected. restXml conformance failures unchanged (129, all pre-existing); AwsQuery unaffected (shares [Read.sequence] but never raises [XmlDeserializeError]).

chris-armstrong · 2026-07-18T22:04:55Z

+      | "doubleValue" ->
+          r_double_value :=
+            Some
+              (Smaws_Lib.Xml.Parse.Read.element_value i "doubleValue"


This module could be opened at top to reduce boilerplate

Done in e6efa55 — generated restXml xml_deserializers.ml now opens Smaws_Lib.Xml.Parse at the top, so the reads are Read.element_value i "doubleValue" Primitive.double_of_string etc. instead of the fully-qualified Smaws_Lib.Xml.Parse.Read.element_value … Smaws_Lib.Xml.Parse.Primitive.double_of_string. The only remaining Smaws_Lib.Xml.Parse reference in the file is the open line itself.

chris-armstrong · 2026-07-18T22:06:09Z

+          r_epoch_seconds_on_target :=
+            Some
+              (Smaws_Lib.Xml.Parse.Read.sequence i "epochSecondsOnTarget"
+                 (fun i _ -> Shared.Xml_deserializers.epoch_seconds_of_xml i)


This module could be opened at To to reduce boilerplate (and moved inside Shared.Xml for consistencies)

Done in e6efa55 — open Smaws_Lib.Xml.Parse is now emitted at the top of generated xml_deserializers.ml, so Smaws_Lib.Xml.Parse.Read/…Primitive (and …Structure, required) are referenced unqualified as Read./Primitive./Structure./required. Re: "moved inside Shared.Xml for consistency" — could you clarify what you'd like moved/nested where? I read the main ask as the open and did that; happy to follow up on the nesting if you can spell out the target structure.

chris-armstrong · 2026-07-18T22:07:09Z

+    Smaws_Lib.Xml.Parse.Primitive.timestamp_iso_of_string ()
+
+let text_plain_blob_of_xml i =
+  Smaws_Lib.Xml.Parse.Primitive.blob_of_string (Smaws_Lib.Xml.Parse.Read.data i)


Open module at top to reduce boilerplate

Done in e6efa55 — the shared xml_deserializers.ml (also generated via the restXml path) now opens Smaws_Lib.Xml.Parse at the top and drops the Smaws_Lib.Xml.Parse. qualifier on every Read./Primitive./Structure. reference.

Address review comments asking to open the module at the top of generated xml_deserializers.ml to drop the repeated [Smaws_Lib.Xml.Parse.] qualifier now that the path-decoration refactor routes every leaf through Read/Primitive and every structure through Structure. - sdkgen/gen_deserialisers.ml: the RestXml branch now emits [open Smaws_Lib.Xml.Parse] before [open Types]. - codegen/AwsProtocolRestXml.ml (Deserialiser): shorten the emitted module paths to [Read] / [Structure] / [Primitive] and call [required] unqualified, so generated code reads [Read.element_value i tag Primitive.double_of_string] / [Structure.scanSequence] / [required ...] instead of the fully-qualified forms. [Smaws_Lib.Xml.Parse.XmlParseError] in the error-deserialiser (which is emitted to operations.ml, a different file) is left qualified. Regenerated model_tests/protocols/{shared,restxml}/xml_deserializers.ml: each now has one [open Smaws_Lib.Xml.Parse] line and no other [Smaws_Lib.Xml.Parse.] qualifiers. No collision: restXml/Shared Types define no [error] and the deserialisers reference neither [error] nor [XmlParseError]. restXml conformance failures unchanged (129, all pre-existing); AwsQuery (75) and codegen tests unaffected.

Apply the "open at the top, rename on conflict" preference to the last qualified Smaws_Lib.Xml.Parse reference: generated operations.ml now opens [Smaws_Lib.Xml.Parse] and matches [Error (XmlParseError msg)] unqualified instead of [Error (Smaws_Lib.Xml.Parse.XmlParseError msg)]. - sdkgen/gen_operations.ml: RestXml branch emits [open Smaws_Lib.Xml.Parse]. - codegen/AwsProtocolRestXml.ml (Operations): the XmlParseError pattern is emitted as a bare constructor (resolved via the open) instead of the fully-qualified [Smaws_Lib.Xml.Parse.XmlParseError]. No symbol clash: operations.ml uses [error] only as a value parameter (separate namespace from Xml.Parse's [error] type), and the regular variant constructor [XmlParseError] does not collide with the [ `XmlParseError ] polymorphic-variant tag used elsewhere in the file. restXml conformance failures unchanged (129, all pre-existing); smaws_lib_test green.

@fmt

The SDK generate rules (sdks/*/dune) already run [ocamlformat -i] over the generator output, so SDK files are ocamlformat-formatted and stable across builds. The model_tests protocol rules (restxml/shared/query/json) did not — they wrote raw [Ppxlib.Pprintast] output, so the committed files were raw and every [dune build] drifted them against any [dune fmt]-formatted state. Add [(:ocf %{bin:ocamlformat})] and a [(run %{ocf} -i <targets>)] step to each model_tests generate rule, matching the SDK rules. Regenerate: the model_tests generated files are now ocamlformat-formatted and stable (a rebuild reproduces the committed bytes, no drift). [dune build @fmt] passes; tests unchanged (restXml 129 pre-existing, query 75, json 110, codegen/smaws_lib_test green).

chris-armstrong · 2026-07-19T08:53:29Z

+                     ( Nolabel,
+                       qualified_apply
+                         ~names:
+                           [ "Smaws_Lib"; "Protocols"; "AwsQuery"; "Serialize"; "float_to_string" ]


Should not use AswQuery converter: move to shared and alias

…heckpoint Phase 5 (Serialiser, §7.1) — finishes the request-body serializer so the conformance model's @mixin-flattened *Request/*Response pairs serialize at all: - mixin flattening at parse time (structureShapeDetails gained `mixins`; Smithy.resolve_mixins flattens mixin members + traits, own wins) — without this every request body serializer was a no-op; - member wrapping fix: complex members (structure/union/non-flattened list/map/set) are wrapped in their <tag_name> element (the previous catch-all emitted bare content, and the flattened branches double-wrapped); - @xmlNamespace (prefix + on list/map/member) on the wrapping element; Xml.Write.element/element_with_ns emit namespaced elements as literal names + xmlns attrs so Write.make () (no ns_prefix) works; - @xmlAttribute collected onto the wrapping element (option attrs omitted); - NaN/Infinity/-Infinity float sentinels via RestXml.Serialize.float_field_to_string; - flattened list/map emit repeated <tag_name> siblings (flattened-map key/value @xmlName retained); empty containers render as empty elements; null options omitted, empty strings preserved. - round-trip/parse-back test (smaws_lib_test/restxml_serialize_test.ml) covers double-precision, sentinel strings, namespaced + prefixed + attributed + empty-container emission. Phase 6 (Deserialiser checkpoint, §7.2; G6c, G19): - no partial functions on the happy path: Xml.Parse.Primitive.* raise XmlDeserializeError (folded to Error by Xml.Parse.run); Read.element_value decorates a parse failure with the enclosing element tag; `required` raises XmlMissingElement. enum/union `failwith` and List.find_map_exn are unhappy-path / codegen-time only. - runtime response-header readers for @httpHeader/@httpPrefixHeaders members: RestXml.header_value (case-insensitive single-header lookup) and RestXml.prefix_headers ~prefix (empty prefix → every header keyed by full name; non-empty → suffix as key; original casing preserved). Verified against the smithy fixtures that @httpHeader-over-@httpPrefixHeaders precedence is a serialise-side rule, so these readers do not exclude specifically-bound names on the deserialize side. - parse unit test (smaws_lib_test/restxml_deserialize_test.ml): http-date guarded Scanf (success + failure → Error), epoch-seconds integer + fractional, primitive parsers success + failure via run, Read.element_value path decoration, and the header readers (case insensitivity, empty + non-empty prefix). All assertions use explicit Result matching. The codegen WIRING of the header readers into the generated output/error deserializers (root-element consumption + http overlay + @httpPayload-raw) is deferred to Phase 7: those http traits only appear on operation input/output/error shapes, and the top-level output deserializer must consume the body root (named after the output shape / @xmlName / service-level @xmlNamespace), which the per-shape <shape>_of_xml (positioned inside its wrapper by the parent's Read.sequence) cannot do for the body-root case. That wrapper belongs in the Phase 7 Operations generator. Phase 6's checkpoint (build + parse unit test) is met without it. dune build @ALL green; dune runtest smaws_lib_test codegen_test green.

chris-armstrong self-assigned this Jul 17, 2026

chris-armstrong force-pushed the restxml branch from 33e5474 to c85aa92 Compare July 17, 2026 20:15

chris-armstrong changed the title ~~restXml protocol: reviewed plan + AwsQuery merge~~ restXml protocol: reviewed implementation plan Jul 17, 2026

chris-armstrong added 3 commits July 18, 2026 09:51

chris-armstrong commented Jul 18, 2026

View reviewed changes

chris-armstrong added 2 commits July 18, 2026 12:36

chris-armstrong commented Jul 18, 2026

View reviewed changes

chris-armstrong added 3 commits July 18, 2026 22:14

chris-armstrong commented Jul 19, 2026

View reviewed changes

Conversation

chris-armstrong commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this is

Plan changes from the original draft (revised after a three-agent review)

Notes

Status

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

chris-armstrong commented Jul 17, 2026 •

edited

Loading