feat(design-sync): document schema v2 with body tree (CE-26)

[linardsb]

Summary

This PR adds version "2.0" of EmailDesignDocument. A v2 document carries a typed body layout tree of nine node types next to the unchanged flat sections. The DSL compiler spike (CE-19 #437) can then build on an existing, schema-validated contract instead of inventing one.

The v1 schema is also relaxed in two places. Before this change, every committed converter document failed v1 validation.

Nothing produces a v2 document yet. The tree builder, the compiler and the feature flag are in CE-19 and CE-20.

What changed

• feat(design-sync): document schema v2 with body tree (CE-26)

  • app/design_sync/dsl/nodes.py: frozen dataclasses for the nine node types.
    • Containers: wrapper, section, column. Leaves: text, image, button, divider, spacer, raw.
    • The text, image and button leaves reuse DocumentText, DocumentImage and DocumentButton unchanged.
    • When loading, the type tag is checked against the parent's allowed children (spec S2) before the node is parsed.
    • TextNode requires font_stack to end in a generic family (spec S3).
  • EmailDesignDocument (app/design_sync/email_design_document.py):
    • It gains a field body: tuple[BodyNode, ...] = ().
    • from_json and validate pick their rules from version. A v1 document that carries body, or any unknown version, raises ValueError. The import service already turns that into the legacy re-fetch.
    • to_json writes body only for v2.
  • data/schemas/email-design-document-v2.json: v1 plus body and new $defs (node_*, container_style, hex6, sizing).
  • data/schemas/email-design-document-v1.json: tokens.typography.maxItems raised from 200 to 1000, and dividers declared on $defs/column.
  • /validate-document: docstring updated to cover v2, and the SDK regenerated (openapi.json and sdk.gen.ts, one line each).
  • Ledger (.agents/deferred-items.json):
    • closes phase-53.7-typography-maxitems-cap;
    • adds a note to ce-6-sizing-horizontal-dropped-at-document-bridge (left open for CE-16);
    • adds ce-26-whole-file-documents-exceed-v1-caps.

• fix(design-sync): align v2 loader with schema and point errors at fields (CE-26 review), c9e7ab15. See Review fixes below.

Files: 15 changed, 3772 insertions, 31 deletions (git diff --stat origin/main...HEAD, at c9e7ab15).

Validation

observed — make check-full on the review-fix tree (uncommitted on e8f6799e, committed unchanged as c9e7ab15), exit 0:

pytest:  9261 passed, 0 failed, 123 skipped
vitest:  780 passed, 0 failed
window:  2026-10-05T12:00:38Z -> 2026-10-05T12:06:28Z

The 30 extra tests are the review-fix tests (derived: 9261 − 9231). The two runs below are the pre-fix head.

observed — make ci-fe, at e8f6799e, exit 0:

vitest:  780 passed, 0 failed
window:  2026-10-05T09:40:56Z -> 2026-10-05T09:42:11Z

observed — make check-full, at e8f6799e, exit 0:

pytest:  9231 passed, 0 failed, 123 skipped
vitest:  780 passed, 0 failed
window:  2026-10-05T09:42:14Z -> 2026-10-05T09:49:31Z

The checks below are converter-fix evidence. All were run at e8f6799e unless marked otherwise.

• Converter HTML is byte-identical on all 7 cases (5–10 and reframe) (observed).
  • Command: regression_runner.run_case_conversion with DESIGN_SYNC__SECTION_CACHE_ENABLED=false.
  • The output was compared with diff -r against a dump taken at b9cccdf8 before any edit; it printed nothing.
• A3 fidelity is unchanged (derived): the HTML is identical, so the render input is identical.
• Ladder: pytest test_converter_data_regression.py -k ladder gave 7 passed and 0 skipped (observed).
• Typography token count is 234 on all 7 cases (observed: len(doc.tokens.typography) on each case's from_legacy document).
• RED run before any code change (observed, at base b9cccdf8 plus the new tests only): 11 failed, 62 passed.
  • test_case_v1_document_validates failed on all 7 cases.
  • Typography maxItems failed on every case; cases 5, 8, 9 and 10 also failed with 'dividers' was unexpected.
• Mutation checks (observed):
  • Setting the v1 typography cap back to 200 turns all 7 corpus validations red, plus the shared-$defs parity test. The v2 round-trip test stays green.
  • Dropping body from v2 required turns test_v2_missing_body_fails_validation and test_root_properties_v1_plus_body red.
• Persisted rows (observed, local dev DB, run on pre-squash commit 483e7fbe, no longer in the branch history; its tree differs from e8f6799e only by the one-line schema() cache change):
  • Both design_token_snapshots.document_json rows load and save back unchanged (sort_keys comparison).
  • validate() no longer reports a typography error on them. It still reports sections cap errors, logged as the new ledger entry.
• Manual check: GET /api/v1/design-sync/schema/v1 on a local uvicorn served typography.maxItems 1000 and column.dividers (observed, on pre-squash commit 483e7fbe, as above).
  • POST /validate-document returned 401 locally, because it needs a viewer login.
  • The v2 → valid: true path is covered by test_validate_document_accepts_v2, which runs under TestClient.

Schema contract change

1. v1 was relaxed, not reshaped. The issue asked for an "unchanged v1 file"; this change was made by user decision on 2026-10-05, because every committed case failed v1 validation (see the RED run above).
2. A relaxation cannot make a valid v1 document invalid (derived). Only one maxItems bound rises and one optional property is added, so every constraint a document met before still holds.
3. A stale cached schema cannot let an invalid document through (derived).
   • GET /schema/v1 is served with Cache-Control: public, max-age=86400 (routes.py:875-878), so a client may hold the old, stricter copy for up to 24 h.
   • The old copy rejects more documents than the new one and never accepts one the new one rejects.

Notes for the reviewer

These are intentional deviations from the plan .agents/plans/ce-26-document-schema-v2.md. Each is recorded under the plan's AMENDMENTS.

• The validate_document docstring says (1.0, 2.0). The falsy-numeric-trap pre-commit hook rejects the text 1.0 or 2.0.
• v2 was generated from v1 with json.dumps(indent=2), not hand-copied. The generator is not committed. The two files differ in layout only; TestSchemaParity compares the parsed JSON and catches drift.
• Each container's contains clause also requires "type": "object" and type. Without that, a non-object item could satisfy contains vacuously.
• The v2 tests and node tests could not be written RED-first, because they import dsl.nodes, which did not exist on the base. The mutation checks show these tests can fail.
• Unknown node types raise NodeType's own ValueError ('carousel' is not a valid NodeType), without the Malformed EmailDesignDocument: prefix. import_service.py:239 still catches it.
• schema() calls _load_schema("1.0"), so it shares the validator's cache key instead of loading v1 a second time.
• Five tests beyond the plan:
  • an empty v2 body validates;
  • each node def's type const matches NodeType;
  • a section under a column is rejected;
  • wrong-length padding or radius is rejected;
  • a node without type raises KeyError.
• Not done here: sizing_horizontal on the v1 dataclasses (CE-16), a /schema/v2 endpoint (DSL-1), and raising the whole-file caps (new ledger entry).

Review fixes (round 1, .claude/code-reviews/pr-475-review.md)

• F1: validate() and from_json now accept the same lengths. nodes._px loads a whole float (12.0) as int and rejects a bool or a fraction. That covers radius, padding, thickness, width and height. Test: test_v2_length_fields_agree_with_schema, 24 cases.
• F2: the child unions dispatch on type with if/then instead of oneOf, so an error names the failing field. The reviewer's divider #CCCCCC now gives one 88-character error at body.0.children.0.children.0.children.3.color; it was 1917 characters at body.0 (observed). validate() keeps the first and last 120 characters of a longer message. The schema accepts the same documents (observed: 18 valid and invalid shapes checked against both schema versions, 0 disagreements). AC 3 is amended: the error is now at the node's type path, not at body.0. Tests: test_v2_errors_point_at_the_bad_field, test_v2_error_message_is_capped, plus the two amended path tests.
• F3: from_json maps AttributeError to ValueError. Test: test_v2_malformed_style_raises_value_error.
• F5: ledger code_refs carry line numbers.
• F6: pre-squash SHAs are now labelled as such (above).
• F4: dropped as Low. As a side effect of F1, "padding": "1234" is now rejected. F7: stamp the ledger after merge.

Linked

Closes #466 · Epic #439 · Ledger: closes phase-53.7-typography-maxitems-cap, adds ce-26-whole-file-documents-exceed-v1-caps

Opened as a draft; CI's ready job flips it when every ci.yml job (backend, frontend, sdk-check, trivy, migrations, integration, migration-lint, commit-lint, e2e-smoke) and codeql are green on this head. A red job leaves it here with the failing check on the PR.

🤖 Generated with Claude Code

[qodo-code-review]

ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing

[linardsb]

Verdict: APPROVE (with recommendations). Recorded as a comment because GitHub does not allow an author to approve their own PR.

PR #475 review: feat(design-sync): document schema v2 with body tree (CE-26)

Head e8f6799e · Base main @ b9cccdf859d2172429e3d0ace7b1fc8b463ecd6d (equal to the live origin/main tip after git fetch, 2026-10-05) · Round 1

Summary

This PR adds EmailDesignDocument version "2.0", which carries a typed body layout tree of nine node types. It also relaxes two places in the v1 schema so the committed converter documents pass validation. There are no Critical or High issues. Two Medium issues are worth fixing before CE-19 builds on this contract:

• F1: one document passes the schema but fails to load.
• F2: a v2 validation error message contains the whole failing subtree.

Both were reproduced on this head. The documented deviations (plan AMENDMENTS, PR "Notes for the reviewer") were treated as intentional and are not flagged.

Issues

Critical: none

High: none

Medium

F1 app/design_sync/dsl/nodes.py:122 (ContainerStyle.from_json): the schema and the loader disagree on what counts as an integer length.

• jsonschema 2020-12 treats 12.0 as an integer, so a container with "radius": 12.0 passes validate().
• The loader then tests isinstance(radius, int), which fails for a float. It falls through to _four(12.0) and raises TypeError, which from_json turns into ValueError.
• Observed on this head: validate() → [], then from_json → ValueError: Malformed EmailDesignDocument: object of type 'float' has no len().
• The reverse case: radius: true loads as 1 (a bool is an int in Python), but the schema rejects it.
• Float padding, thickness, height and width pass both checks but keep their float values in fields typed int. That breaks S3 "Lengths: int px" for the CE-19/20 compiler.
• Fix: branch on isinstance(radius, list) and treat anything else as a scalar. Coerce whole-number floats to int (or reject them) in one helper used for every length field. Add a test with radius: 12.0.

F2 app/design_sync/email_design_document.py:1522 (validate) with the oneOf items in data/schemas/email-design-document-v2.json: a v2 error message repeats the whole failing subtree.

• jsonschema's oneOf error is "<instance repr> is not valid under any of the given schemas", and validate() passes it through unchanged.
• Observed: changing one divider color to #CCCCCC, deep inside the test fixture, gives one 1917-character error at path body.0. The message is the Python repr of the whole wrapper.
• With real raw.html payloads (each up to 100 kB) the /validate-document response grows with the size of the document. The message also does not point to the bad field.
• Fix: dispatch on type with if/then per node def so errors reach the real path. Or report jsonschema.exceptions.best_match(error.context) for oneOf errors and cap the message length.
• Either fix moves error paths below body.0, so AC 3, test_v2_unknown_node_type_fails_validation and test_v2_illegal_nesting_fails would need amending. Record that as a plan amendment.

Low

F3 app/design_sync/email_design_document.py:1504: "style": [] (or a string) raises AttributeError (observed). from_json catches only KeyError/TypeError, and import_service.py:240 catches ValueError/KeyError, so a malformed persisted v2 row would surface as a 500 instead of the legacy re-fetch. The same class of bug already exists on v1 types, and today the only producer is our own to_json. Fix: add AttributeError to the except.

F4 app/design_sync/dsl/nodes.py:79 (_four): any sequence of length 4 is accepted, so "padding": "1234" loads as ('1','2','3','4'). Also, "body": {} loads as (). Both are consistent with the plan's rule that the loader does not validate and the schema owns values. But _four reads as a shape check, and here it is not one. Either check isinstance(value, list) or leave it and rely on the schema.

F5 .agents/deferred-items.json:1508-1509: two of the three code_refs on ce-26-whole-file-documents-exceed-v1-caps have no :<line>, which the ledger schema's <file>:<line> (<symbol>) form requires.

F6 PR body and report: SHA provenance.

• The report says make check-full ran on 483e7fbe.
• The PR body says the persisted-row and manual checks ran "on the tree of 483e7fbe".
• After the squash, 483e7fbe and c188dc61 are not in the branch history (observed: git log origin/main..HEAD shows only e8f6799e).
• The gate figures themselves hold on this head (see Validation), so this is wording only. Cite e8f6799e or the CI run, or say that the SHAs are pre-squash.

F7 .agents/deferred-items.json:586, :1505: the pending SHA placeholders follow the repo's stamp-after-squash pattern. They need the follow-up stamp after merge (the deferred-items skill).

Validation

Check	Result	Provenance
make check-full @ e8f6799e	exit 0	observed (this review)
pytest	9231 passed, 123 skipped, 7 xfailed	observed; matches the PR body
mypy	no issues, 1403 files	observed
pyright	0 errors	observed
vitest (inside the gate)	780 passed	observed; matches the PR body
make lint rewrites	none (git status showed only pre-existing unrelated edits)	observed
CI (14 checks) on e8f6799e	all pass	observed (gh pr checks 475)
Code scanning (Semgrep, CodeQL)	0 open alerts on refs/pull/475/merge	observed (gh api …/code-scanning/alerts)

Numbers pass

• 15 files, +3385/−25: matches git diff --stat origin/main...HEAD (observed).
• 234 typography tokens on every case, and the v1 document validates: re-observed with a script over discover_cases() (from_legacy → to_json → validate() == []; from_json → to_json round-trips).
• 9231 passed / 780 passed: re-observed by the gate above.
• The RED run (11 failed, 62 passed), the 7/7 byte identity and the mutation checks: not re-run. They are plausible from the diff, because no converter emit path changed. Each is labelled observed with a named command.
• A3 unchanged: correctly labelled derived (identical HTML gives an identical render input).
• The "relaxation cannot invalidate a valid v1 doc" and "stale cached schema is only stricter" guarantees: re-derived and they hold. The change raises one maxItems and adds one optional property, and that cannot make a previously valid document invalid.

Constraint pass

• grep for frozen/no-changes in the plan returns only the frozen-dataclass rule, which the code follows.
• The F1 and F2 fixes do not conflict with any acceptance criterion. F2 needs an AC 3 amendment, as noted above.

Loader vs schema: where they disagree

Each row is from the reviewer agent. F1–F3 were re-observed in this review.

Fragment	validate()	from_json	Status
radius: 12.0	accepts	rejects	F1
radius: true	rejects	accepts	F1
float lengths	accepts	accepts, but keeps a float in an int field	F1
style: []	rejects	AttributeError	F3
font_stack with no generic family	accepts	rejects (S3)	documented
empty wrapper/section children	rejects	accepts	documented (Task 3 GOTCHA)
hex case, maxItems, raw.reason	rejects	accepts	documented ("schema owns values")
v1 with body, unknown version, v2 without body	rejects	rejects	correct

What's good

• The S2 child tables in nodes.py and the schema's oneOf lists match the spec exactly. Nesting depth is bounded (body → wrapper → section → column → leaf), so neither the loader nor the validator can recurse without limit.
• Checking the tag before parsing gives clear errors such as "section not allowed in a column".
• version is read inside the try. A missing type becomes a ValueError, so import_service's legacy fallback still fires.
• __post_init__ stops a non-2.0 document from carrying body.
• The validator cache is keyed by version and guards against a non-hashable version.
• The tests run on the real case-5 payload and all 7 corpus cases. The tests themselves were mutation-checked.
• TestSchemaParity guards against the v1 and v2 schemas drifting apart.
• The SDK diff is exactly the two expected lines.
• The ledger closure is backed by a real-fixture test, as its closes_when requires.

Recommendation

Approve. Nothing blocks the merge: there is no Critical or High finding, and the gate is green on this head. Fix F1 and F2 in this PR (preferred, since this PR defines the contract) or as the first task of CE-19. F3–F6 are small cleanups. F7 is the usual post-merge stamp.

[linardsb]

Verdict: APPROVE. Recorded as a comment because GitHub does not allow an author to approve their own PR.

PR #475 review, round 2: feat(design-sync): document schema v2 with body tree (CE-26)

Head c9e7ab15 · Base main @ b9cccdf859d2172429e3d0ace7b1fc8b463ecd6d (equal to the live origin/main tip after git fetch, 2026-10-05) · Round 2

Summary

Round 2 covers fix commit c9e7ab15 (round-1 F1, F2, F3, F5 and F6). The base has not moved since round 1, so the guarantees pass does not apply. In the fix-mechanism pass, each fix closes its finding, and none of them lets the schema accept a document it used to reject. The reviewer found 0 Critical, 0 High, 0 Medium and 3 Low issues. Round-1 F7 (stamp the ledger SHA after merge) carries forward.

Fix-mechanism pass

Round-1 fix	What the mechanism newly permits	Evidence
F2: if/then dispatch replaces oneOf	Nothing. Each of the four item schemas keeps type: object, required: [type] and an enum on type, and every if also requires type: object, so an unknown tag, a missing tag or a non-object item still fails. Each fails exactly once, at the right path.	observed, scratchpad probe at c9e7ab15: carousel → body.0.type … is not one of; no type → body.0: 'type' is a required property; 5 → body.0: 5 is not of type 'object'; ["wrapper"] → rejected. All four also fail to load (ValueError).
F2: _cap_message	The message is capped, but the path is not (L3).	observed: a 100 kB tokens.modes key gives a 100,040-character error line
F1: _px	The schema and the loader now agree on the type of every length. They still disagree on values: negative lengths and an explicit null (L1).	observed: thickness: -1 and divider width: null fail validate() but load
F3: AttributeError → ValueError	It also catches a coding error in any nested parser (L2). TypeError already had this scope.	derived: the try covers email_design_document.py:1488-1517
F5: ledger code_refs	n/a	observed (reviewer, git show HEAD:): every line number points at the named symbol

Issues

Critical / High / Medium

None.

Low

• L1 .agents/plans/ce-26-document-schema-v2.md:465: "The loader and validate() now accept the same lengths" claims more than is true. It holds for types but not for values: thickness: -1 and width: null pass the loader and fail validate() (observed, above). The test only covers {12, 12.0, 12.5, true}. Under the CLAUDE.md claims rule, this sentence is a guarantee. Fix: reword it to "accept the same numeric types (value bounds stay the schema's job)".
• L2 app/design_sync/email_design_document.py:1516: catching AttributeError across the whole deserialisation tree means a typo in a parser now looks like malformed data. import_service.py:240-244 then logs design_sync.document_json_invalid without the exception and falls back silently. Fix (optional): add error=str(exc) to that warning.
• L3 app/design_sync/email_design_document.py:1531-1534: _cap_message bounds the message but not the path. A user-supplied tokens.modes key goes into absolute_path, so the error line is as large as the input (observed: 100,040 characters). v1 behaves the same, and the size does not grow beyond the input. Fix (optional): cap the whole line.

Numbers pass

Claim	Source	Re-derived
30 new tests (9261 − 9231)	PR body :40, fix report	observed: test_document_v2.py collects 54 at c9e7ab15 and 24 at e8f6799e, so 30
17 of 24 length cases RED before the fix	fix report F1	derived (reviewer): 6 whole float + 5 fraction + 6 bool = 17
15 files, +3772/−31	PR body :29	observed: git diff --stat origin/main...HEAD
import_service.py:239/:240	PR body :92, fix report F3	observed: 239 is the from_json call, 240 is except (ValueError, KeyError)
88-character error at the deep divider path	PR body :104	Not re-run. The path shape is consistent with the probe paths above.
18 shapes, 0 disagreements	PR body :104, plan :466	Not re-run. The six shapes probed here behave the same as oneOf (derived from the round-1 semantics).

Validation

Check	Result
CI on c9e7ab15 (14 checks, including Backend and Frontend)	all pass (observed, gh pr checks 475)
pytest: test_document_v2, dsl/, test_email_design_document, test_golden_roundtrip	152 passed (observed)
mypy on dsl/ and email_design_document.py	clean (observed)
pyright, the same files plus test_document_v2.py	0 errors (observed)
ruff check and format --check	clean (observed)
Code scanning, open alerts on refs/pull/475/merge	none (observed: the API returned an empty list, not 403/404)
make check-full	Not re-run this round. The author's run on the fix tree was green, and CI's Backend job is green on this head.

What is done well

• The F2 dispatch adds type: object inside each if. Without it, a non-object item would match every if and trigger every then.
• The F1 test checks agreement in both directions at all six sites, and asserts type(loaded) is int.
• The fix report uses a discarded parity run (the one that compared HEAD with itself) as its example of a run that measured nothing.
• AC 3 is marked as amended in the plan, not silently rewritten.

Recommendation

Approve. Fixing L1 is a one-line wording change and can go in with the F7 post-merge stamp. L2 and L3 are optional. After merge, F7: stamp pending in ce-26-whole-file-documents-exceed-v1-caps and phase-53.7-typography-maxitems-cap. The pending SHA of ce-6-sizing-horizontal-dropped-at-document-bridge from #474 also still needs stamping.