From dee8b7010188d7dd0e2e332fa40384ba99dab071 Mon Sep 17 00:00:00 2001 From: Jeremy Bedient Date: Wed, 26 Aug 2026 10:32:05 -0400 Subject: [PATCH 1/4] Generate email templates from a spec file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `kizen messages templates create --spec-file ` builds a complete email template — `craft_json` and the compiled, Outlook-safe `content` HTML — from one declarative spec; `update --spec-file ` rewrites an existing one the same way, as an alternative to that command's raw `--craft-json-file`/`--content-file` PATCH path. Both fields come from one pass over one node tree, so a spec can never describe one without the other — no flag and no spec key accepts a raw `craft_json` or `content` value. A spec's rows pick one of 4 column layouts by name and cells hold text/image/button/divider blocks, both closed sets, so an unsupported layout or block kind is a clear error rather than a silent partial template. An image block names a local PNG/JPEG file; it's uploaded publicly readable (`is_public=true` on `POST /api/s3/success`) so recipients can actually load it, and its pixel dimensions are read from the file's own header bytes, no new dependency. `--dry-run` resolves images offline instead of uploading, so it never writes. `messages templates craft-config` previews the `{craft_json, content}` pair offline, with `--out-html` to drop the compiled body somewhere a browser can open it. Reuses `tools/form_ui.py`'s Root/Section/Row/Cell assembly via two new, additive hooks (`cell_props`, `block_assembler`); forms/layouts output is unchanged. Fixed during review: uploaded images were coming back non-public and 404ing for real recipients (`upload_file()` now takes `is_public`); the compiled CSS's column-width media query was inverted, so every non-1- column layout rendered stacked instead of side-by-side in most mail clients (`.mj-column-per-N` width is now a base rule, with the mobile-collapse moved into the media query, matching MJML's own convention); `--dry-run` was performing a real upload with no signal to the user (dry-run now resolves images offline, same as `craft-config`). A real test send opened in Outlook is still the only way to fully confirm rendering — nothing offline substitutes for that, and it wasn't done here. --- CHANGELOG.md | 24 + scripts/cli-tree-baseline.txt | 88 +- src/kizen_builder/api/files.py | 31 +- src/kizen_builder/cli/messages.py | 177 +++- .../docs/specs/email-templates.md | 152 ++- src/kizen_builder/models/spec/__init__.py | 24 + .../models/spec/email_templates.py | 139 +++ src/kizen_builder/tools/email_craft.py | 898 ++++++++++++++++++ src/kizen_builder/tools/form_ui.py | 77 +- src/kizen_builder/tools/planners/messages.py | 100 +- tests/drift/test_email_template_roundtrip.py | 168 ++++ tests/test_cli_email_templates.py | 194 ++++ tests/test_email_craft.py | 492 ++++++++++ tests/test_email_craft_upload.py | 144 +++ tests/test_email_template_planners.py | 177 ++++ tests/test_email_template_spec.py | 114 +++ tests/test_smart_connectors_authoring.py | 23 + 17 files changed, 2967 insertions(+), 55 deletions(-) create mode 100644 src/kizen_builder/models/spec/email_templates.py create mode 100644 src/kizen_builder/tools/email_craft.py create mode 100644 tests/drift/test_email_template_roundtrip.py create mode 100644 tests/test_cli_email_templates.py create mode 100644 tests/test_email_craft.py create mode 100644 tests/test_email_craft_upload.py create mode 100644 tests/test_email_template_planners.py create mode 100644 tests/test_email_template_spec.py diff --git a/CHANGELOG.md b/CHANGELOG.md index a9287f7..14d68f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,30 @@ called out explicitly under **Changed** or **Removed**. Generating a template from a spec file is still not wired; see `kizen docs show email-templates`. +- **`kizen messages templates create --spec-file ` builds a complete email + template — `craft_json` and the compiled, Outlook-safe `content` HTML — + from one declarative spec.** `update --spec-file ` rewrites an + existing template the same way, as an alternative to its existing + field-level `--craft-json-file`/`--content-file` PATCH path. Both fields + come from one pass over one node tree, so a spec can never describe one + without the other — no flag and no spec key accepts a raw `craft_json` or + `content` value. + + A spec's rows pick one of 4 column layouts by name (`1 Column`, `2 + Columns`, `2 Columns (1/3 and 2/3)`, `2 Columns (2/3 and 1/3)`) and cells + hold `text`/`image`/`button`/`divider` blocks — both closed sets, so an + unsupported layout or block kind is a clear error, never a silent partial + template. An `image` block names a local PNG/JPEG file; it's uploaded + (`source="public_image"`, publicly readable so recipients can load it) and + its real pixel dimensions are read from the file's own header bytes — no + new dependency. `--dry-run` resolves images offline instead of uploading, + so it never writes. `messages templates craft-config` previews the + `{craft_json, content}` pair offline, with `--out-html` to drop the + compiled body somewhere a browser (or Outlook) can open it. + + A real test send opened in Outlook is still the only way to confirm actual + rendering — nothing offline can substitute for that. + ### Fixed - **`kizen upgrade --check` can now find a release tag from a `uv tool diff --git a/scripts/cli-tree-baseline.txt b/scripts/cli-tree-baseline.txt index ba4bec8..188eaeb 100644 --- a/scripts/cli-tree-baseline.txt +++ b/scripts/cli-tree-baseline.txt @@ -2264,11 +2264,17 @@ │ --help Show this message and exit. │ ╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ ╭─ Commands ───────────────────────────────────────────────────────────────────────────────────────╮ -│ list List email templates (pass one's name or UUID as `--template`). │ -│ get Show one email template, including how well its two content fields agree. │ -│ clone Copy an email template, both content fields included. │ -│ update PATCH one email template's fields. │ -│ delete Delete an email template. │ +│ list List email templates (pass one's name or UUID as `--template`). │ +│ get Show one email template, including how well its two content fields agree. │ +│ create Create an email template from a spec file — `craft_json` and `content` │ +│ built together from one pass over the spec's `sections`, so the two can │ +│ never be authored out of sync. │ +│ clone Copy an email template, both content fields included. │ +│ update PATCH one email template's fields. │ +│ delete Delete an email template. │ +│ craft-config Preview the `{craft_json, content}` pair a spec would produce — │ +│ offline, no live calls, runs the exact emitter `create`/`update │ +│ --spec-file` use. │ ╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ Usage: messages templates clone [OPTIONS] {source} @@ -2293,6 +2299,54 @@ Wire format (the two coupled content fields): see `kizen docs show email-templates` + Usage: messages templates craft-config [OPTIONS] + + Preview the `{craft_json, content}` pair a spec would produce — offline, no live calls, runs the + exact emitter `create`/`update --spec-file` use. + + Unlike `dashboards dashlet-config`, this command's output is **not** + meant to be pasted into a create/update spec: any Image block is + resolved without uploading (no live calls here), so its `fileId`/`src` + are obvious placeholder tokens, not real ones. Use `--out-html` to drop + the compiled body somewhere a browser (or Outlook) can open it. + +╭─ Options ────────────────────────────────────────────────────────────────────────────────────────╮ +│ --spec-file Path to a JSON email-template spec. Omit to list the available block │ +│ kinds and column layouts instead. │ +│ --out-html Write the compiled `content` HTML to this file. │ +│ --help Show this message and exit. │ +╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ + + Spec shape: see `kizen docs show email-templates` + + + Usage: messages templates create [OPTIONS] + + Create an email template from a spec file — `craft_json` and `content` built together from one + pass over the spec's `sections`, so the two can never be authored out of sync. + + No flag and no spec key accepts a raw `craft_json` or `content` value — + that hand-author-both-and-hope-they-agree foot-gun is exactly what this + command exists to close. `sender_type` is always `"business"`, the only + value ever observed live. Any Image block's local file is uploaded for + real before the plan is built (there's no way to know its real + `fileId`/`src`/`naturalWidth`/`naturalHeight` otherwise) — except under + `--dry-run`, which resolves images offline instead (placeholder + `fileId`/`src`, real dimensions read from the file's own header bytes) + so a dry run never writes. + +╭─ Options ────────────────────────────────────────────────────────────────────────────────────────╮ +│ --spec-file Path to a JSON email-template spec. Default: read from stdin. │ +│ --dry-run Show the plan without applying. │ +│ --yes -y Skip the y/N confirmation prompt. │ +│ --json Emit JSON (plan with --dry-run, results otherwise). │ +│ --help Show this message and exit. │ +╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ + + Spec shape: see `kizen docs show email-templates`. Generate one with `messages templates + craft-config`. + + Usage: messages templates delete [OPTIONS] {template} Delete an email template. @@ -2345,11 +2399,15 @@ PATCH one email template's fields. - The server stores both content fields verbatim and compiles neither - from the other (confirmed live 2026-08-25), so sending `craft_json` - without `content` — or the reverse — leaves the builder showing one - email while recipients receive another. Pass both together, and check - the result with `messages templates get`. + Two ways to update: `--spec-file` rebuilds `craft_json`/`content` from a + spec, both fields together, the same way `create` does. The raw path + (`--craft-json-file`/`--content-file`/`--name`/`--subject`) PATCHes + fields verbatim instead — the server stores both content fields + independently and compiles neither from the other (confirmed live + 2026-08-25), so sending `craft_json` without `content` on the raw path — + or the reverse — leaves the builder showing one email while recipients + receive another. Check the result either way with `messages templates + get`. ╭─ Arguments ──────────────────────────────────────────────────────────────────────────────────────╮ │ * template Template name or UUID. [required] │ @@ -2359,6 +2417,10 @@ │ --subject Set the subject line. │ │ --craft-json-file Path to a JSON file to send as `craft_json`. │ │ --content-file Path to an HTML file to send as `content`. │ +│ --spec-file Path to a JSON email-template spec — rebuilds │ +│ `craft_json`/`content` together the same way `create` does. │ +│ Mutually exclusive with │ +│ --name/--subject/--craft-json-file/--content-file. │ │ --dry-run Show the plan without applying. │ │ --yes -y Skip the y/N confirmation prompt. │ │ --json Emit JSON (plan with --dry-run, results otherwise). │ @@ -4369,6 +4431,12 @@ ===== messages templates clone ===== +===== messages templates craft-config ===== + + +===== messages templates create ===== + + ===== messages templates delete ===== diff --git a/src/kizen_builder/api/files.py b/src/kizen_builder/api/files.py index 8a5983a..dcc2d3a 100644 --- a/src/kizen_builder/api/files.py +++ b/src/kizen_builder/api/files.py @@ -33,6 +33,14 @@ # The ``source`` a smart connector's reference/sample file is uploaded under. SMART_CONNECTOR_IMPORT = "smart_connector_import" +# The ``source`` an email-template Image block's file is uploaded under — +# confirmed live 2026-08-25 from the Kizen email builder's own browser +# network trace, then confirmed end-to-end through `upload_file()` itself. +# `source` is a server-validated closed choice (~30 other plausible names +# all rejected live); this is the only one confirmed to work for an image. +# See `kizen docs show email-templates`. +PUBLIC_IMAGE = "public_image" + def download_file( config: EnvConfig, file_id: str, timeout: float = 120.0 @@ -71,6 +79,7 @@ def upload_file( *, source: str = SMART_CONNECTOR_IMPORT, content_type: str | None = None, + is_public: bool = False, timeout: float = 300.0, ) -> dict[str, Any]: """Upload a local file and return the registered Kizen ``File`` dict. @@ -79,7 +88,12 @@ def upload_file( wants. ``content_type`` is guessed from the extension when omitted; the guess is signed into the S3 policy, so a mismatch between what's declared here and what S3 receives fails the upload rather than uploading something - mislabeled. + mislabeled. ``is_public`` sets ``POST /api/s3/success``'s own + ``is_public`` field (confirmed live 2026-08-25 via ``GET + /api/docs/schema``, "Whether the S3 object is public (default: false)") + — omitted means false, matching the field's own documented default. A + caller that needs an unauthenticated recipient to load the file (an + email's ``Image.src``, say) must pass ``is_public=True`` explicitly. """ src = Path(path) if not src.is_file(): @@ -131,12 +145,25 @@ def upload_file( etag = (s3_resp.headers.get("etag") or "").strip('"') # Leg 3: register the File with Kizen (form-encoded). + data = {"uuid": s3object_id, "key": key, "name": src.name, "etag": etag} + if is_public: + data["is_public"] = "true" registered = client.post( "/api/s3/success", params={"source": source}, - data={"uuid": s3object_id, "key": key, "name": src.name, "etag": etag}, + data=data, headers={"Content-Type": "application/x-www-form-urlencoded"}, ) if not isinstance(registered, dict) or not registered.get("id"): raise KizenAPIError(0, f"unexpected s3/success response: {registered!r}") return registered + + +def delete_file(client: KizenClient, file_id: str) -> Any: + """DELETE /api/files/{id} — confirmed live 2026-08-25 (a follow-up + download of the same id 404s afterward, so this is a real delete, not a + soft no-op). Needed for drift teardown: `GET /api/files` is broken + (301s to plain HTTP, then 404s), so there is no other way to find or + remove a file a drift run uploaded. + """ + return client.delete(f"/api/files/{file_id}") diff --git a/src/kizen_builder/cli/messages.py b/src/kizen_builder/cli/messages.py index 11235c3..3345d00 100644 --- a/src/kizen_builder/cli/messages.py +++ b/src/kizen_builder/cli/messages.py @@ -9,17 +9,21 @@ from pathlib import Path import typer +from pydantic import ValidationError from rich.table import Table from kizen_builder import output as out -from kizen_builder.cli._mutations import _run_mutation +from kizen_builder.cli._mutations import _read_spec, _run_mutation from kizen_builder.cli._shared import ( JSON_OPTION, OUTPUT_OPTION, app, cli_errors, console, + err_console, ) +from kizen_builder.models.spec.email_templates import EmailTemplateDef +from kizen_builder.tools import email_craft from kizen_builder.tools import messages as message_tools from kizen_builder.tools.planners import messages as message_planners @@ -130,6 +134,71 @@ def table() -> None: out.render(fmt, json_data={**summary, "id": detail.get("id")}, table=table) +def _validate_email_spec(spec_dict: dict) -> EmailTemplateDef: + try: + return EmailTemplateDef.model_validate(spec_dict) + except ValidationError as e: + for err in e.errors(): + loc = ".".join(str(p) for p in err.get("loc", ())) or "spec" + msg = err.get("msg", "invalid value") + err_console.print(f"[red]spec error:[/red] {loc}: {msg}") + raise typer.Exit(code=1) from e + + +@messages_templates_app.command( + "create", + epilog="Spec shape: see `kizen docs show email-templates`. " + "Generate one with `messages templates craft-config`.", +) +def messages_templates_create( + spec_file: str = typer.Option( + "", + "--spec-file", + help="Path to a JSON email-template spec. Default: read from stdin.", + ), + dry_run: bool = typer.Option( + False, "--dry-run", help="Show the plan without applying." + ), + yes: bool = typer.Option( + False, "--yes", "-y", help="Skip the y/N confirmation prompt." + ), + json_out: bool = typer.Option( + False, "--json", help="Emit JSON (plan with --dry-run, results otherwise)." + ), +) -> None: + """Create an email template from a spec file — `craft_json` and `content` + built together from one pass over the spec's `sections`, so the two can + never be authored out of sync. + + No flag and no spec key accepts a raw `craft_json` or `content` value — + that hand-author-both-and-hope-they-agree foot-gun is exactly what this + command exists to close. `sender_type` is always `"business"`, the only + value ever observed live. Any Image block's local file is uploaded for + real before the plan is built (there's no way to know its real + `fileId`/`src`/`naturalWidth`/`naturalHeight` otherwise) — except under + `--dry-run`, which resolves images offline instead (placeholder + `fileId`/`src`, real dimensions read from the file's own header bytes) + so a dry run never writes. + """ + spec_dict, from_stdin = _read_spec(spec_file, what="email template") + with cli_errors(ValueError, FileNotFoundError): + spec = _validate_email_spec(spec_dict) + resolved_sections = ( + email_craft.offline_resolve_spec_images(spec) + if dry_run + else email_craft.resolve_spec_images(spec) + ) + _run_mutation( + lambda: message_planners.plan_create_template_from_spec( + spec, resolved_sections + ), + dry_run=dry_run, + yes=yes, + json_out=json_out, + stdin_consumed=from_stdin, + ) + + @messages_templates_app.command( "clone", epilog="Wire format (the two coupled content fields): see `kizen docs show email-templates`", @@ -175,6 +244,13 @@ def messages_templates_update( content_file: str = typer.Option( None, "--content-file", help="Path to an HTML file to send as `content`." ), + spec_file: str = typer.Option( + "", + "--spec-file", + help="Path to a JSON email-template spec — rebuilds `craft_json`/`content` " + "together the same way `create` does. Mutually exclusive with " + "--name/--subject/--craft-json-file/--content-file.", + ), dry_run: bool = typer.Option( False, "--dry-run", help="Show the plan without applying." ), @@ -187,12 +263,46 @@ def messages_templates_update( ) -> None: """PATCH one email template's fields. - The server stores both content fields verbatim and compiles neither - from the other (confirmed live 2026-08-25), so sending `craft_json` - without `content` — or the reverse — leaves the builder showing one - email while recipients receive another. Pass both together, and check - the result with `messages templates get`. + Two ways to update: `--spec-file` rebuilds `craft_json`/`content` from a + spec, both fields together, the same way `create` does. The raw path + (`--craft-json-file`/`--content-file`/`--name`/`--subject`) PATCHes + fields verbatim instead — the server stores both content fields + independently and compiles neither from the other (confirmed live + 2026-08-25), so sending `craft_json` without `content` on the raw path — + or the reverse — leaves the builder showing one email while recipients + receive another. Check the result either way with `messages templates + get`. """ + raw_flags_used = ( + name is not None or subject is not None or craft_json_file or content_file + ) + if spec_file and raw_flags_used: + err_console.print( + "[red]error:[/red] --spec-file cannot be combined with " + "--name/--subject/--craft-json-file/--content-file." + ) + raise typer.Exit(code=2) + + if spec_file: + spec_dict, from_stdin = _read_spec(spec_file, what="email template") + with cli_errors(ValueError, FileNotFoundError): + spec = _validate_email_spec(spec_dict) + resolved_sections = ( + email_craft.offline_resolve_spec_images(spec) + if dry_run + else email_craft.resolve_spec_images(spec) + ) + _run_mutation( + lambda: message_planners.plan_update_template( + template, spec=spec, resolved_sections=resolved_sections + ), + dry_run=dry_run, + yes=yes, + json_out=json_out, + stdin_consumed=from_stdin, + ) + return + patch: dict[str, object] = {} if name is not None: patch["name"] = name @@ -233,6 +343,61 @@ def messages_templates_delete( ) +@messages_templates_app.command( + "craft-config", + epilog="Spec shape: see `kizen docs show email-templates`", +) +def messages_templates_craft_config( + spec_file: str = typer.Option( + "", + "--spec-file", + help="Path to a JSON email-template spec. Omit to list the available " + "block kinds and column layouts instead.", + ), + out_html: str = typer.Option( + "", "--out-html", help="Write the compiled `content` HTML to this file." + ), +) -> None: + """Preview the `{craft_json, content}` pair a spec would produce — + offline, no live calls, runs the exact emitter `create`/`update + --spec-file` use. + + Unlike `dashboards dashlet-config`, this command's output is **not** + meant to be pasted into a create/update spec: any Image block is + resolved without uploading (no live calls here), so its `fileId`/`src` + are obvious placeholder tokens, not real ones. Use `--out-html` to drop + the compiled body somewhere a browser (or Outlook) can open it. + """ + if not spec_file: + t = Table(title="Block kinds") + t.add_column("kind") + for kind in email_craft.known_block_kinds(): + t.add_row(kind) + console.print(t) + + lt = Table(title="Column layouts (v1)") + lt.add_column("layout") + lt.add_column("columns") + for name in email_craft.known_layouts(): + columns = email_craft.COLUMN_LAYOUTS[name].columns + lt.add_row(name, ", ".join(str(c) for c in columns)) + console.print(lt) + return + + spec_dict, _ = _read_spec(spec_file, what="email template") + with cli_errors(ValueError, FileNotFoundError): + spec = _validate_email_spec(spec_dict) + resolved_sections = email_craft.offline_resolve_spec_images(spec) + sections = email_craft.assemble_sections(resolved_sections) + craft_json, content = email_craft.build_email_content(sections) + + if out_html: + Path(out_html).write_text(content) + err_console.print(f"[dim]wrote compiled content to {out_html}[/dim]") + + out.emit_json({"craft_json": craft_json, "content": content}) + + @messages_app.command("create") def messages_create( api_name: str = typer.Argument(..., help="Automation api_name."), diff --git a/src/kizen_builder/docs/specs/email-templates.md b/src/kizen_builder/docs/specs/email-templates.md index d049169..8f0d116 100644 --- a/src/kizen_builder/docs/specs/email-templates.md +++ b/src/kizen_builder/docs/specs/email-templates.md @@ -11,6 +11,89 @@ Two distinct resources, easy to confuse: --template ` creates the automation message a `notify_member_via_email` step points at. +## Building a template from a spec file — `messages templates create`/`update --spec-file` + +`craft_json` (the editable tree) and `content` (the compiled, Outlook-safe +HTML that is actually sent) are independent stored fields, and the server +compiles neither from the other on `POST` or `PATCH` — see "Two content +fields that must be kept in sync" below. So a spec never names either field +directly; both are built together, from one pass over one node tree, by +`kizen messages templates create --spec-file ` (and `update +--spec-file `, an alternative to that command's raw +`--craft-json-file`/`--content-file` PATCH path). Preview what a spec would +produce, offline, with `messages templates craft-config --spec-file +[--out-html ]` — no args lists the available block kinds and layouts. + +### Spec shape + +```json +{ + "name": "Newsletter", + "subject": "This month's update", + "sections": [ + { + "background_color": "#FFFFFF", + "rows": [ + { + "layout": "2 Columns", + "cells": [ + {"blocks": [{"kind": "text", "html": "

Left column

"}]}, + {"blocks": [{"kind": "image", "file": "/path/to/logo.png", "alt": "Logo"}]} + ] + } + ] + } + ] +} +``` + +- `sections[].rows[].layout` is one of **4 v1 presets**, a closed enum — a + typo'd name is a spec-validation error, not a bad fractions array to catch + later. `cells` must have exactly the count the preset needs. + + | preset | `columns` | `Cell.__width` | + |---|---|---| + | `1 Column` | `[1]` | `1` | + | `2 Columns` | `[0.5, 0.5]` | `0.5`, `0.5` | + | `2 Columns (1/3 and 2/3)` | `[0.3333333333333333, 0.6666666666666666]` | same | + | `2 Columns (2/3 and 1/3)` | `[0.6666666666666666, 0.3333333333333333]` | same | + + Confirmed live 2026-08-25, byte-exact — not rounded, not recomputed as + `1/3`. Five more presets (`3`/`4`/`5`/`6 Columns`, `3 Columns (gutters)`) + are confirmed live but out of scope for this spec format; naming one is a + clear error, not a silent reshape. +- `cells[].blocks[].kind` is one of `text`, `image`, `button`, `divider` — a + closed set, same reasoning. There is no `attachments` kind and no raw-HTML + escape hatch (no `HTMLBlock` on this surface at all, confirmed live). + - `text`: `{"kind": "text", "html": "

...

"}` — embedded verbatim in + both outputs. + - `image`: `{"kind": "image", "file": "", "alt": "", "link": "", "width": 150}`. + `file` is a **local path**, not a `file_id` — there is no CLI surface to + look one up afterward (`GET /api/files` is broken; see below), so the + spec captures the upload at the point it happens. **PNG and JPEG only** + (pixel dimensions are read from the file's own header bytes, no + dependency); GIF/WebP/SVG are rejected outright. Uploaded with + `is_public=true` (confirmed live 2026-08-25 via `GET /api/docs/schema` + on `POST /api/s3/success`) so the emitted `src` is reachable by a real + recipient, not just an authenticated session. Uploading happens for real + when a `create`/`update --spec-file` is actually applied; under + `--dry-run` the CLI resolves images offline instead (a placeholder + `fileId`/`src`, real dimensions still read from the file's own header + bytes) so a dry run never writes. + - `button`: `{"kind": "button", "label": "...", "url": "...", "color": null}`. + - `divider`: `{"kind": "divider", "color": null}`. +- `sender_type` and `from_name_type` are not spec keys. They are hard-coded + to `"business"`/`"default"`, the only values ever observed live — see + "Other top-level fields" below. There is no `--sender-type` flag. + **`from_name_type` is required on `POST`**, confirmed live 2026-08-25: a + create without it 400s (`{"from_name_type": ["This field is required."]}`) + — the earlier PATCH-only probing hadn't surfaced this since PATCH only + needs the fields actually being changed. + +No flag and no spec key on `create` accepts a raw `craft_json` or `content` +value, on purpose — that's the exact "hand-author both fields and hope they +agree" foot-gun this whole surface exists to close. + ## Automation messages: create them *from a template* A `notify_member_via_email` step's config has no subject/body — it is a bare @@ -43,9 +126,10 @@ template `Text` blocks. ## Email template wire format Confirmed live 2026-07-21 from a real save captured out of the Kizen email -template builder — `PATCH /api/messages/templates/{id}`. Read, clone, update -and delete are CLI-wired (`kizen messages templates`); generating a template -from a spec file is not. +template builder — `PATCH /api/messages/templates/{id}`. Read, clone, +create, update and delete are all CLI-wired (`kizen messages templates`) — +see "Building a template from a spec file" above for `create`/`update +--spec-file`/`craft-config`. ### Two content fields that must be kept in sync @@ -124,18 +208,32 @@ Block props confirmed live 2026-08-25: - **`Text`** — copy lives in `custom.text` as an HTML string, not in props. `content` embeds that markup **verbatim**, so comparing the two means tag-stripping both sides. -- **`Image`** — `src` is host-absolute - (`https:///api/files//download`) alongside a `fileId`, so an - image reference is **environment-bound**; moving a template between envs - needs the `src` rewritten. `naturalWidth`/`naturalHeight` are the uploaded - file's real pixel dimensions and are not derivable from a spec — they have - to be read off the image. +- **`Image`** — `src` is host-absolute, so an image reference is + **environment-bound**; moving a template between envs needs `src` + rewritten. Two URL schemes are both confirmed live on a plain `Image` + node, not just on `Attachments`: `https:///api/files/{fileId}/download` + (used by the "All Rows" capture and most templates) and + `https:///api/public/s3/{fileId}/download` (seen on one image in + another template) — either is accepted; `messages templates create`/ + `craft-config` always emit the `/api/files/` form. `naturalWidth`/ + `naturalHeight` are the uploaded file's real pixel dimensions and are not + derivable from a spec — they have to be read off the image (this surface's + spec-file emitter parses them from the file's own PNG/JPEG header bytes). + **The uploaded file must be `is_public: true`** or `src` 404s for anyone + without an authenticated Kizen session — confirmed live 2026-08-25 (`POST + /api/s3/success`'s own `is_public` field, `GET /api/docs/schema`; both URL + schemes 200 unauthenticated once set, 404 on both when not). The + spec-file emitter's upload path sets it; a raw `upload_file()` call + elsewhere in this repo does not unless asked (see `api/files.py`). - **`Button`** — `{url, label, action: "url", color, textColor, fontSize, fontFamily, alignment, borderSize, borderColor, borderRadius, padding{Top,Left,Right,Bottom}, textStyles: [], openLinkInNewTab}` plus the - `container*` set. + `container*` set. The emitter's compiled `content` markup for this node + (`_render_button`) was checked byte-exact against a real Button in a + Kizen-authored template, read-only, 2026-08-26. - **`Divider`** — `{size, color, width, alignment, borderStyle}` plus the - `container*` set. + `container*` set. Same verification: `_render_divider`'s output matches a + real captured Divider's compiled markup byte-exact. - **`Attachments`** — `props.attachments` is a list of **full file records** (id, key, url, name, size_bytes, content_type, thumbnail_url, `is_public`, and an `employee` object naming the uploader), plus an @@ -155,19 +253,25 @@ merge-field span to an existing `Text` node). Generating genuinely **new** structure — new Sections/Rows/Images/Buttons from scratch, the way forms/layouts/dashboards builders do — additionally requires hand-producing matching Outlook-safe compiled HTML for `content`, with the -node ids threaded through it. Not attempted, and meaningfully higher-stakes -than the other craft.js surfaces: a wrong `content` is what real recipients -receive, not an editor-only concern. Since the server will not compile it for -you (see the table above), that emitter has to live somewhere. - -`kizen messages templates clone` is the safe path in the meantime: it copies -both content fields together, so the copy is internally consistent by -construction. Build the design once in the builder UI, then clone and -surgically edit it. - -Everything needed to build the generation slice — node shapes, the coupling -rule, the compile findings — is in this document. It is deliberately the only -home for those facts. +node ids threaded through it. **Built** by `tools/email_craft.py`'s +`build_email_content()`, wired to `messages templates create`/`update +--spec-file`/`craft-config` — see "Building a template from a spec file" +above. It covers `Text`/`Image`/`Button`/`Divider` and the 4 v1 column +presets; `Attachments` and the other 5 presets are confirmed live but still +unbuilt (their `columns`/markup are pre-captured for a follow-on). This +stayed meaningfully higher-stakes than the other craft.js surfaces even once +built: a wrong `content` is what real recipients actually receive, not an +editor-only concern, and nothing offline can substitute for opening a real +test send in Outlook. + +`kizen messages templates clone` is still the safe path for copying an +existing design: it copies both content fields together, so the copy is +internally consistent by construction. Build the design once in the builder +UI (or with `create --spec-file`), then clone and surgically edit it. + +Everything needed to build on the generation slice — node shapes, the +coupling rule, the compile findings, the spec-file format — is in this +document. It is deliberately the only home for those facts. ## See also diff --git a/src/kizen_builder/models/spec/__init__.py b/src/kizen_builder/models/spec/__init__.py index 8757bbd..03e679c 100644 --- a/src/kizen_builder/models/spec/__init__.py +++ b/src/kizen_builder/models/spec/__init__.py @@ -130,6 +130,19 @@ ColumnTemplateDef, LayoutDef, ) +from kizen_builder.models.spec.email_templates import ( + ColumnPreset, + COLUMN_FRACTIONS, + TextBlockDef, + ImageBlockDef, + ButtonBlockDef, + DividerBlockDef, + BlockDef, + CellDef, + RowDef, + SectionDef, + EmailTemplateDef, +) from kizen_builder.models.spec.activities import ( ActivityFieldType, AssociationMode, @@ -295,6 +308,17 @@ def build_payload_meta(model: BaseModel) -> dict[str, Any]: "QuickFilterDef", "ColumnTemplateDef", "LayoutDef", + "ColumnPreset", + "COLUMN_FRACTIONS", + "TextBlockDef", + "ImageBlockDef", + "ButtonBlockDef", + "DividerBlockDef", + "BlockDef", + "CellDef", + "RowDef", + "SectionDef", + "EmailTemplateDef", "ActivityFieldType", "AssociationMode", "ActivityFieldDef", diff --git a/src/kizen_builder/models/spec/email_templates.py b/src/kizen_builder/models/spec/email_templates.py new file mode 100644 index 0000000..49f2e37 --- /dev/null +++ b/src/kizen_builder/models/spec/email_templates.py @@ -0,0 +1,139 @@ +"""Spec models for `messages templates create/update --spec-file`. + +Backs `tools/email_craft.py`'s emitter, which builds `craft_json` and the +compiled `content` HTML from one pass over this tree — see that module's +docstring for why the two can't be authored separately. Wire-format facts +(node shapes, the `section-` coupling rule) live in +`docs/specs/email-templates.md`, not here; this module only encodes what a +spec author is allowed to write. + +v1 ships four column presets and four leaf block kinds. Both closed sets are +enforced by construction: `ColumnPreset` is a `Literal`, so a typo'd preset +name is a spec-validation error, not a bad fractions array to catch later, +and `BlockDef` is a discriminated union on `kind`, so an unsupported kind +(`attachments`, anything else) fails with the literal list of valid kinds +rather than a silent skip. `Attachments` and the other five column presets +(`3`/`4`/`5`/`6 Columns`, `3 Columns (gutters)`) are confirmed live but out +of v1 scope — see the work item that shipped this module. +""" + +from __future__ import annotations + +from typing import Annotated, Literal + +from pydantic import BaseModel, ConfigDict, Field + +# The 4 v1 layouts, confirmed live 2026-08-25 — exact `Row.props.columns` / +# `Cell.props.__width` fractions, not rounded or recomputed. See +# docs/specs/email-templates.md. +ColumnPreset = Literal[ + "1 Column", + "2 Columns", + "2 Columns (1/3 and 2/3)", + "2 Columns (2/3 and 1/3)", +] + +COLUMN_FRACTIONS: dict[str, tuple[float, ...]] = { + "1 Column": (1,), + "2 Columns": (0.5, 0.5), + "2 Columns (1/3 and 2/3)": (0.3333333333333333, 0.6666666666666666), + "2 Columns (2/3 and 1/3)": (0.6666666666666666, 0.3333333333333333), +} + + +class TextBlockDef(BaseModel): + """Rich-text copy. `html` is embedded verbatim in both `craft_json` + (`custom.text`) and the compiled `content` — see the coupling rule in + `docs/specs/email-templates.md`.""" + + model_config = ConfigDict(extra="forbid") + + kind: Literal["text"] = "text" + html: str + + +class ImageBlockDef(BaseModel): + """An image, uploaded from a local file at plan time. + + `file` is a path on disk, not a `file_id` — there is no CLI surface to + look one up after the fact (`GET /api/files` is broken; see + docs/specs/email-templates.md), so the spec captures the upload at the + point it happens. `naturalWidth`/`naturalHeight` are read from the + file's own header bytes, never guessed. + """ + + model_config = ConfigDict(extra="forbid") + + kind: Literal["image"] = "image" + file: str + alt: str = "" + link: str = "" + width: int | None = Field( + default=None, + description="Display width in px. Defaults to 150 (form_ui's default).", + ) + + +class ButtonBlockDef(BaseModel): + model_config = ConfigDict(extra="forbid") + + kind: Literal["button"] = "button" + label: str + url: str + color: str | None = None + + +class DividerBlockDef(BaseModel): + model_config = ConfigDict(extra="forbid") + + kind: Literal["divider"] = "divider" + color: str | None = None + + +BlockDef = Annotated[ + TextBlockDef | ImageBlockDef | ButtonBlockDef | DividerBlockDef, + Field(discriminator="kind"), +] + + +class CellDef(BaseModel): + model_config = ConfigDict(extra="forbid") + + blocks: list[BlockDef] = Field(default_factory=list) + + +class RowDef(BaseModel): + """One row. `layout` picks a closed-enum column preset; the emitter (not + this model — see `tools/planners/messages.py`) rejects a row whose cell + count doesn't match the preset with a `PlanError`, at plan time rather + than as a silent reshape.""" + + model_config = ConfigDict(extra="forbid") + + layout: ColumnPreset = "1 Column" + cells: list[CellDef] + + +class SectionDef(BaseModel): + model_config = ConfigDict(extra="forbid") + + rows: list[RowDef] = Field(default_factory=list) + background_color: str = "#FFFFFF" + + +class EmailTemplateDef(BaseModel): + """Top-level spec for `messages templates create/update --spec-file`. + + Deliberately has no `craft_json`/`content` key of any kind — those two + fields are always derived from `sections` by `tools/email_craft.py`, so + a hand-authored pair can never be passed in and go out of sync. `subject` + is optional (kept blank if omitted, matching what the builder allows). + `sender_type` is not a field here — it is hard-coded to `"business"` by + the planner; see `docs/specs/email-templates.md`. + """ + + model_config = ConfigDict(extra="forbid") + + name: str = Field(min_length=1) + subject: str = "" + sections: list[SectionDef] = Field(default_factory=list) diff --git a/src/kizen_builder/tools/email_craft.py b/src/kizen_builder/tools/email_craft.py new file mode 100644 index 0000000..b01dbd3 --- /dev/null +++ b/src/kizen_builder/tools/email_craft.py @@ -0,0 +1,898 @@ +"""Build an email template's `craft_json` and compiled `content` from one spec. + +**Why this has to be one function, not two.** `craft_json` (the editable +craft.js tree the builder shows) and `content` (the Outlook-safe HTML that +actually gets sent) are stored as independent fields — the server compiles +neither from the other, confirmed live both for `PATCH` and `POST` (see +`docs/specs/email-templates.md`). The compiled HTML carries a +`section-` class for every `Section` **and** every `Row` node, with +no orphans on either side in every live capture. So the two fields are +coupled by node id, not merely parallel: build the tree once, mint each id +once, and derive both outputs from that single pass. Building the tree and +then compiling HTML in a second pass that mints its *own* ids produces a +template whose builder view and real output silently disagree — the exact +failure this module exists to make impossible. + +`build_email_content()` is the one entry point that upholds that invariant. +Everything else here is either structural reuse of `tools.form_ui` (the +`Root`/`Section`/`Row`/`Cell` assembly is identical topology, threaded +through the `cell_props`/`block_assembler` hooks added there for this +module) or email-specific: this surface's own `Text`/`Image`/`Button`/ +`Divider` prop shapes (email's `Button`/`Divider` props differ from the +forms surface's — see `docs/specs/email-templates.md`), the v1 column-preset +table (byte-exact `columns`/`__width` fractions and compiled-HTML markup, +confirmed live 2026-08-25), and image upload (`api/files.py::upload_file` +with `source="public_image"`, plus reading `naturalWidth`/`naturalHeight` +straight from the uploaded file's own header bytes). + +v1 scope only: `Text`, `Image`, `Button`, `Divider` leaf blocks, and the 4 +column presets in `COLUMN_LAYOUT` below (`1 Column`, `2 Columns`, `2 Columns +(1/3 and 2/3)`, `2 Columns (2/3 and 1/3)`). `Attachments` and the other 5 +presets (`3`/`4`/`5`/`6 Columns`, `3 Columns (gutters)`) are confirmed live +but out of scope — anything using them fails loudly rather than silently +degrading. There is no raw-HTML escape hatch on this surface (no +`HTMLBlock`, confirmed live directly against the builder). +""" + +from __future__ import annotations + +from collections.abc import Callable +from html import escape +from pathlib import Path +from typing import Any + +from kizen_builder.api import files as files_api +from kizen_builder.api.client import KizenClient +from kizen_builder.config import load_env_config +from kizen_builder.models.spec.email_templates import ( + ButtonBlockDef, + DividerBlockDef, + EmailTemplateDef, + ImageBlockDef, + TextBlockDef, +) +from kizen_builder.tools import form_ui + +# --------------------------------------------------------------------------- +# Root/container prop shapes +# --------------------------------------------------------------------------- + +# Confirmed live 2026-08-25: a form page's Root props minus `tabletBreak`, +# in two independent captures. Kept as its own literal (not derived from +# form_ui's private _ROOT_PROPS) so this module documents its own contract +# against docs/specs/email-templates.md rather than silently tracking +# whatever forms does next. +EMAIL_ROOT_PROPS: dict[str, Any] = { + "containerBackgroundColor": "rgba(0,0,0,0)", + "containerBackgroundImageName": "", + "containerBackgroundPositionX": "0%", + "containerBackgroundPositionY": "0%", + "containerBackgroundSize": "auto", + "containerBackgroundRepeat": "repeat", + "containerBorderColor": "rgba(74,86,96,1)", + "containerBorderStyle": "solid", + "containerBorderWidth": "0", + "containerBorderRadius": True, + "containerBorderTopLeftRadius": "4", + "containerBorderTopRightRadius": "4", + "containerBorderBottomLeftRadius": "4", + "containerBorderBottomRightRadius": "4", + "containerMarginTop": "0", + "containerMarginRight": "0", + "containerMarginBottom": "0", + "containerMarginLeft": "0", + "containerPaddingTop": "0", + "containerPaddingRight": "0", + "containerPaddingBottom": "0", + "containerPaddingLeft": "0", + "backgroundColor": "#F8FAFF", + "width": "100", + "maxWidth": "900", + "alignment": "center", + "mobileBreak": "414", + "color": "rgba(74,86,96,1)", + "fontFamily": "Arial", + "fontSize": "14", + "linkColor": "rgba(82,142,249,1)", + "lineHeight": "1.25", +} + +# Same `container*` vocabulary every leaf block on this surface carries, +# same values form_ui._CONTAINER_DEFAULTS uses for forms — no live evidence +# these differ per block on the email surface. +_CONTAINER_DEFAULTS: dict[str, Any] = { + "containerBackgroundColor": "rgba(0,0,0,0)", + "containerBackgroundImageName": "", + "containerBackgroundPositionX": "0%", + "containerBackgroundPositionY": "0%", + "containerBackgroundSize": "auto", + "containerBackgroundRepeat": "repeat", + "containerBorderColor": "rgba(74,86,96,1)", + "containerBorderStyle": "solid", + "containerBorderWidth": "0", + "containerBorderRadius": False, + "containerBorderTopLeftRadius": "4", + "containerBorderTopRightRadius": "4", + "containerBorderBottomLeftRadius": "4", + "containerBorderBottomRightRadius": "4", + "containerMarginTop": "0", + "containerMarginRight": "0", + "containerMarginBottom": "0", + "containerMarginLeft": "0", + "containerPaddingTop": "10", + "containerPaddingRight": "10", + "containerPaddingBottom": "10", + "containerPaddingLeft": "10", +} + +# --------------------------------------------------------------------------- +# v1 column presets — byte-exact, confirmed live 2026-08-25. Do not round or +# recompute; see the work item's "Live probe findings". +# --------------------------------------------------------------------------- + + +class ColumnLayout: + __slots__ = ("preset", "columns", "classes", "media_widths", "mso_widths_px") + + def __init__( + self, + preset: str, + columns: tuple[float, ...], + classes: tuple[str, ...], + media_widths: tuple[str, ...], + mso_widths_px: tuple[float, ...], + ) -> None: + self.preset = preset + self.columns = columns + self.classes = classes + self.media_widths = media_widths + self.mso_widths_px = mso_widths_px + + +# 880px content width in every case observed (900 Root maxWidth - 20px padding). +CONTENT_WIDTH_PX = 880.0 + +COLUMN_LAYOUTS: dict[str, ColumnLayout] = { + "1 Column": ColumnLayout( + "1 Column", + (1,), + ("mj-column-per-100",), + ("100%",), + (880.0,), + ), + "2 Columns": ColumnLayout( + "2 Columns", + (0.5, 0.5), + ("mj-column-per-50", "mj-column-per-50"), + ("50%", "50%"), + (440.0, 440.0), + ), + "2 Columns (1/3 and 2/3)": ColumnLayout( + "2 Columns (1/3 and 2/3)", + (0.3333333333333333, 0.6666666666666666), + ("mj-column-per-33-333332", "mj-column-per-66-666664"), + ("33.333332%", "66.666664%"), + (293.3333, 586.6666), + ), + "2 Columns (2/3 and 1/3)": ColumnLayout( + "2 Columns (2/3 and 1/3)", + (0.6666666666666666, 0.3333333333333333), + ("mj-column-per-66-666664", "mj-column-per-33-333332"), + ("66.666664%", "33.333332%"), + (586.6666, 293.3333), + ), +} + +# The other 5 presets are pre-captured groundwork for a follow-on item, not +# built here. Naming one is a clear, immediate error, not a silent skip. +_OUT_OF_SCOPE_LAYOUTS = ( + "3 Columns", + "3 Columns (gutters)", + "4 Columns", + "5 Columns", + "6 Columns", +) + + +def known_layouts() -> list[str]: + """The v1 closed enum of layout names, in display order.""" + return list(COLUMN_LAYOUTS) + + +def known_block_kinds() -> list[str]: + """The v1 closed set of leaf block kinds this emitter supports.""" + return ["text", "image", "button", "divider"] + + +# --------------------------------------------------------------------------- +# Spec-builder helpers — plain dicts consumed by build_email_content(). +# Mirrors tools.form_ui's cell()/row()/section() naming, but this surface's +# own leaf-block shapes (email Button/Divider props differ from forms' — +# see the module docstring) and its own layout validation. +# --------------------------------------------------------------------------- + + +def text_block(html: str) -> dict[str, Any]: + return {"kind": "text", "html": html} + + +def image_block( + *, + file_id: str, + src: str, + name: str, + alt: str = "", + link: str = "", + width: int | None = None, + natural_width: int | None = None, + natural_height: int | None = None, +) -> dict[str, Any]: + return { + "kind": "image", + "file_id": file_id, + "src": src, + "name": name, + "alt": alt, + "link": link, + "width": width, + "natural_width": natural_width, + "natural_height": natural_height, + } + + +def button_block(label: str, url: str, *, color: str | None = None) -> dict[str, Any]: + return {"kind": "button", "label": label, "url": url, "color": color} + + +def divider_block(color: str | None = None) -> dict[str, Any]: + return {"kind": "divider", "color": color} + + +def cell(blocks: list[dict[str, Any]]) -> dict[str, Any]: + return {"blocks": blocks} + + +def row(cells: list[dict[str, Any]], layout: str = "1 Column") -> dict[str, Any]: + """One row using a v1 column preset by name. + + Raises ``ValueError`` — never a silent reshape — for an unknown preset + name or a cell count that doesn't match it, naming the valid presets or + the expected count. The planner (``tools.planners.messages``) catches + this and re-raises as a ``PlanError``. + """ + if layout in _OUT_OF_SCOPE_LAYOUTS: + raise ValueError( + f"layout {layout!r} is confirmed live but out of v1 scope " + f"(see the work item's pre-captured groundwork). Supported: " + f"{', '.join(known_layouts())}" + ) + preset = COLUMN_LAYOUTS.get(layout) + if preset is None: + raise ValueError( + f"unknown row layout {layout!r}. Supported: {', '.join(known_layouts())}" + ) + if len(cells) != len(preset.columns): + raise ValueError( + f"layout {layout!r} needs {len(preset.columns)} cell(s), got {len(cells)}" + ) + return {"cells": cells, "columns": list(preset.columns), "layout": layout} + + +def section( + rows: list[dict[str, Any]], *, background_color: str = "#FFFFFF" +) -> dict[str, Any]: + return {"rows": rows, "background_color": background_color} + + +# --------------------------------------------------------------------------- +# Image upload + header-byte pixel dimensions +# --------------------------------------------------------------------------- + + +def _png_dimensions(data: bytes) -> tuple[int, int]: + # Signature (8 bytes) + IHDR chunk: length(4) type(4) width(4) height(4). + if len(data) < 24 or data[:8] != b"\x89PNG\r\n\x1a\n": + raise ValueError("not a valid PNG (bad signature)") + width = int.from_bytes(data[16:20], "big") + height = int.from_bytes(data[20:24], "big") + return width, height + + +# JPEG SOF (start-of-frame) markers that carry dimensions. Excludes DHT +# (0xC4), JPG (0xC8), DAC (0xCC) — same-range bytes that are NOT SOF markers. +_JPEG_SOF_MARKERS = frozenset( + {0xC0, 0xC1, 0xC2, 0xC3, 0xC5, 0xC6, 0xC7, 0xC9, 0xCA, 0xCB, 0xCD, 0xCE, 0xCF} +) +# Markers with no following length/payload — skip straight past them. +_JPEG_STANDALONE_MARKERS = frozenset({0x01, 0xD8, 0xD9} | set(range(0xD0, 0xD8))) + + +def _jpeg_dimensions(data: bytes) -> tuple[int, int]: + if len(data) < 4 or data[0:2] != b"\xff\xd8": + raise ValueError("not a valid JPEG (bad SOI marker)") + pos = 2 + n = len(data) + while pos < n - 1: + if data[pos] != 0xFF: + raise ValueError("malformed JPEG: expected a marker") + marker = data[pos + 1] + pos += 2 + while marker == 0xFF and pos < n: # padding fill bytes between markers + marker = data[pos] + pos += 1 + if marker in _JPEG_STANDALONE_MARKERS: + continue + if pos + 2 > n: + break + seg_len = int.from_bytes(data[pos : pos + 2], "big") + if marker in _JPEG_SOF_MARKERS: + if pos + 7 > n: + break + height = int.from_bytes(data[pos + 3 : pos + 5], "big") + width = int.from_bytes(data[pos + 5 : pos + 7], "big") + return width, height + pos += seg_len + raise ValueError("no SOF0/SOF2 segment found in JPEG") + + +def read_image_dimensions(data: bytes) -> tuple[int, int, str]: + """Return ``(width, height, content_type)`` read from the file's own + header bytes. PNG and JPEG only — both are real cases on this surface + (every image already stored in the target environment is PNG, but the + browser trace that settled the ``source`` question was a JPEG upload). + GIF/WebP/SVG fail loudly as unsupported rather than being silently + mis-parsed; SVG especially has no pixel dimensions to read this way at + all. + """ + if data[:8] == b"\x89PNG\r\n\x1a\n": + w, h = _png_dimensions(data) + return w, h, "image/png" + if data[:3] == b"\xff\xd8\xff": + w, h = _jpeg_dimensions(data) + return w, h, "image/jpeg" + if data[:6] in (b"GIF87a", b"GIF89a"): + raise ValueError("GIF is not supported on this surface — PNG or JPEG only") + if data[:4] == b"RIFF" and data[8:12] == b"WEBP": + raise ValueError("WebP is not supported on this surface — PNG or JPEG only") + if data[:5] == b" dict[str, Any]: + """Upload a local PNG/JPEG for use in an Image block and return the + resolved block fields (``file_id``, ``src``, ``name``, ``natural_width``, + ``natural_height``). + + A real write — reuses ``api.files.upload_file`` with + ``source=files_api.PUBLIC_IMAGE`` and ``is_public=True``, confirmed live + 2026-08-25 (without ``is_public``, the upload defaults to non-public and + the resulting `src` 404s for any recipient without an authenticated + session — see `docs/specs/email-templates.md`). Callers outside + ``tools/planners/`` only (planners never write — see ``CLAUDE.md``); the + CLI only calls this for a real apply — under ``--dry-run`` it calls + :func:`offline_resolve_spec_images` instead, which uploads nothing. + ``base_url`` is the target env's own base URL (``EnvConfig.base_url``) — + ``Image.src`` is host-absolute, confirmed live, so a template is + environment-bound. + """ + src_path = Path(path) + data = src_path.read_bytes() + width, height, _content_type = read_image_dimensions(data) + registered = files_api.upload_file( + client, src_path, source=files_api.PUBLIC_IMAGE, is_public=True + ) + file_id = registered["id"] + src = f"{base_url}/api/files/{file_id}/download" + return { + "file_id": file_id, + "src": src, + "name": src_path.name, + "natural_width": width, + "natural_height": height, + } + + +# --------------------------------------------------------------------------- +# craft_json assembly — reuses tools.form_ui's Root/Section/Row/Cell shell +# via the cell_props/block_assembler hooks added there for this module. +# --------------------------------------------------------------------------- + + +def _cell_props(width: float | None) -> dict[str, Any]: + # Confirmed live: Cell.props is {"__width": }, redundant with + # (and must agree with) the parent Row's own columns entry. + return {"__width": width} + + +def _assemble_email_block( + block: dict[str, Any], parent_id: str, content: dict[str, Any] +) -> str: + kind = block["kind"] + node_id = form_ui._new_id() + + if kind == "text": + node = { + "type": {"resolvedName": "Text"}, + "isCanvas": False, + "props": dict(_CONTAINER_DEFAULTS), + "displayName": "Text", + "custom": {"text": block["html"]}, + "parent": parent_id, + "hidden": False, + "nodes": [], + "linkedNodes": {}, + } + elif kind == "image": + node = { + "type": {"resolvedName": "Image"}, + "isCanvas": False, + "props": { + **_CONTAINER_DEFAULTS, + "size": "dynamic", + "unit": "pixel", + "height": None, + "width": block.get("width") or 150, + "display": "flex", + "position": "center", + "alt": block.get("alt", ""), + "link": block.get("link", ""), + "src": block["src"], + "name": block["name"], + "fileId": block["file_id"], + "naturalHeight": block.get("natural_height"), + "naturalWidth": block.get("natural_width"), + "dimension": "width", + }, + "displayName": "Image", + "custom": {}, + "parent": parent_id, + "hidden": False, + "nodes": [], + "linkedNodes": {}, + } + elif kind == "button": + node = { + "type": {"resolvedName": "Button"}, + "isCanvas": False, + "props": { + **_CONTAINER_DEFAULTS, + "url": block.get("url", ""), + "label": block["label"], + "action": "url", + "color": block.get("color") or "rgba(0,51,160,1)", + "textColor": "rgba(255,255,255,1)", + "fontSize": "16", + "fontFamily": "Arial", + "alignment": "center", + "borderSize": "0", + "borderColor": "rgba(0,0,0,1)", + "borderRadius": "8", + "paddingTop": "10", + "paddingLeft": "20", + "paddingRight": "20", + "paddingBottom": "10", + "textStyles": [], + "openLinkInNewTab": True, + }, + "displayName": "Button", + "custom": {}, + "parent": parent_id, + "hidden": False, + "nodes": [], + "linkedNodes": {}, + } + elif kind == "divider": + node = { + "type": {"resolvedName": "Divider"}, + "isCanvas": False, + "props": { + **_CONTAINER_DEFAULTS, + "size": "3", + "color": block.get("color") or "rgba(78,193,145,1)", + "width": "100", + "alignment": "center", + "borderStyle": "solid", + }, + "displayName": "Divider", + "custom": {}, + "parent": parent_id, + "hidden": False, + "nodes": [], + "linkedNodes": {}, + } + else: + raise ValueError( + f"unsupported email block kind: {kind!r}. Supported: " + f"{', '.join(known_block_kinds())}" + ) + + content[node_id] = node + return node_id + + +# --------------------------------------------------------------------------- +# content (compiled HTML) — walks the SAME craft_json dict build_content_tree +# just returned, using its dict keys as node ids. No second id-minting pass. +# --------------------------------------------------------------------------- + + +def _resolved_name(node: dict[str, Any]) -> str: + t = node.get("type") + return str(t.get("resolvedName")) if isinstance(t, dict) else str(t) + + +def _layout_for_columns(columns: list[float]) -> ColumnLayout: + for layout in COLUMN_LAYOUTS.values(): + if list(layout.columns) == list(columns): + return layout + raise ValueError(f"no v1 column layout matches columns={columns!r}") + + +def _render_button(node: dict[str, Any]) -> str: + p = node["props"] + return ( + '' + "
' + f'' + f"{escape(p['label'])}
" + ) + + +def _render_divider(node: dict[str, Any]) -> str: + p = node["props"] + return ( + f'

' + ) + + +def _render_image(node: dict[str, Any]) -> str: + p = node["props"] + img = ( + f'{escape(p.get(' + ) + link = p.get("link") + if link: + return f'{img}' + return img + + +def _render_block(node: dict[str, Any]) -> str: + name = _resolved_name(node) + if name == "Text": + # Embedded verbatim, not stripped — see craft_summary()'s _plain_text, + # which tag-strips both sides before comparing. + return f'
{node["custom"]["text"]}
' + if name == "Image": + return _render_image(node) + if name == "Button": + return _render_button(node) + if name == "Divider": + return _render_divider(node) + raise ValueError(f"cannot compile HTML for unsupported node type {name!r}") + + +def _render_cell(cell_id: str, craft_json: dict[str, Any]) -> str: + node = craft_json[cell_id] + return "".join(_render_block(craft_json[bid]) for bid in node["nodes"]) + + +def _render_row(row_id: str, craft_json: dict[str, Any]) -> tuple[str, str]: + """Return (body_html, style_rule) for one Row.""" + node = craft_json[row_id] + columns = node["props"]["columns"] + layout = _layout_for_columns(columns) + cell_ids = [node["linkedNodes"][f"column-{i + 1}"] for i in range(len(columns))] + + parts = [f'
'] + parts.append( + '' + ) + else: + parts.append( + f'" + ) + parts.append( + f'
' + ) + parts.append(_render_cell(cid, craft_json)) + parts.append("
") + parts.append("") + parts.append("
") + + style_rule = f".section-{row_id} {{ max-width:{CONTENT_WIDTH_PX}px; }}" + return "".join(parts), style_rule + + +def _render_section( + section_id: str, craft_json: dict[str, Any] +) -> tuple[str, list[str]]: + node = craft_json[section_id] + bg = node["props"].get("containerBackgroundColor", "#FFFFFF") + rows_html: list[str] = [] + style_rules = [f".section-{section_id} {{ background-color:{bg}; }}"] + for row_id in node["nodes"]: + row_html, row_style = _render_row(row_id, craft_json) + rows_html.append(row_html) + style_rules.append(row_style) + body = f'
' + "".join(rows_html) + "
" + return body, style_rules + + +def _column_base_width_rules(craft_json: dict[str, Any]) -> list[str]: + """Unconditional `.mj-column-per-N` width rules — MJML's own convention. + + Each column `
` also carries a hardcoded inline `width:100%` (see + `_render_row`), which is the fallback for clients that ignore `" + ) + return ( + "" + '' + "" + '' + '' + '' + "" + style_block + "" + '' + "".join(bodies) + "" + ) + + +# --------------------------------------------------------------------------- +# The entry point +# --------------------------------------------------------------------------- + + +def build_email_content(sections: list[dict[str, Any]]) -> tuple[dict[str, Any], str]: + """Return ``(craft_json, content)`` from ONE id-assignment pass. + + ``sections`` is built from :func:`section`/:func:`row`/:func:`cell`/ + ``*_block()`` above (or straight from an :class:`EmailTemplateDef` via + ``tools.planners.messages``). Never expose a way to build one output + without the other — see the module docstring for why. + """ + craft_json = form_ui.build_content_tree( + sections, + root_props=EMAIL_ROOT_PROPS, + cell_props=_cell_props, + block_assembler=_assemble_email_block, + ) + content = _compile_html(craft_json) + return craft_json, content + + +def assemble_sections(resolved_sections: list[dict[str, Any]]) -> list[dict[str, Any]]: + """Turn the plain-dict tree :func:`resolve_spec_images` (or + :func:`offline_resolve_spec_images`) returns into the + :func:`section`/:func:`row`/:func:`cell`/``*_block()`` spec + :func:`build_email_content` consumes. + + This is where a row's cell count is checked against its layout preset — + :func:`row` raises ``ValueError`` on a mismatch. Callers under + ``tools/planners/`` catch that and re-raise as a ``PlanError`` (a + validation error at plan time, not a silent reshape); `craft-config` + lets it surface as a plain CLI error since it isn't planning anything. + """ + sections: list[dict[str, Any]] = [] + for s in resolved_sections: + rows: list[dict[str, Any]] = [] + for r in s["rows"]: + cells: list[dict[str, Any]] = [] + for c in r["cells"]: + blocks: list[dict[str, Any]] = [] + for b in c["blocks"]: + kind = b["kind"] + if kind == "text": + blocks.append(text_block(b["html"])) + elif kind == "image": + blocks.append( + image_block( + file_id=b["file_id"], + src=b["src"], + name=b["name"], + alt=b.get("alt", ""), + link=b.get("link", ""), + width=b.get("width"), + natural_width=b.get("natural_width"), + natural_height=b.get("natural_height"), + ) + ) + elif kind == "button": + blocks.append( + button_block(b["label"], b["url"], color=b.get("color")) + ) + elif kind == "divider": + blocks.append(divider_block(b.get("color"))) + else: + raise ValueError( + f"unsupported block kind: {kind!r}. Supported: " + f"{', '.join(known_block_kinds())}" + ) + cells.append(cell(blocks)) + rows.append(row(cells, layout=r["layout"])) + sections.append( + section(rows, background_color=s.get("background_color", "#FFFFFF")) + ) + return sections + + +# --------------------------------------------------------------------------- +# Spec resolution — walks an EmailTemplateDef into the plain-dict shape +# assemble_sections() turns into row()/cell()/*_block() calls. Only the +# image leg differs between the live and offline (craft-config) callers, so +# both share this walk. +# --------------------------------------------------------------------------- + +# `craft-config`'s offline output has no live upload behind it — these tokens +# make that obvious rather than looking like a real id a spec could paste in. +OFFLINE_FILE_PLACEHOLDER = "" +OFFLINE_HOST_PLACEHOLDER = "" + + +def _walk_blocks( + spec: EmailTemplateDef, + resolve_image: Callable[[ImageBlockDef], dict[str, Any]], +) -> list[dict[str, Any]]: + sections: list[dict[str, Any]] = [] + for s in spec.sections: + rows: list[dict[str, Any]] = [] + for r in s.rows: + cells: list[dict[str, Any]] = [] + for c in r.cells: + blocks: list[dict[str, Any]] = [] + for b in c.blocks: + if isinstance(b, TextBlockDef): + blocks.append({"kind": "text", "html": b.html}) + elif isinstance(b, ButtonBlockDef): + blocks.append( + { + "kind": "button", + "label": b.label, + "url": b.url, + "color": b.color, + } + ) + elif isinstance(b, DividerBlockDef): + blocks.append({"kind": "divider", "color": b.color}) + elif isinstance(b, ImageBlockDef): + blocks.append({"kind": "image", **resolve_image(b)}) + else: # pragma: no cover - the discriminated union rejects this + raise ValueError(f"unsupported block: {b!r}") + cells.append({"blocks": blocks}) + rows.append({"layout": r.layout, "cells": cells}) + sections.append({"rows": rows, "background_color": s.background_color}) + return sections + + +def resolve_spec_images(spec: EmailTemplateDef) -> list[dict[str, Any]]: + """Upload every local file an ``EmailTemplateDef``'s Image blocks + reference and return ``spec.sections`` as the plain nested dicts + ``tools.planners.messages`` turns into a plan. + + A real write (see :func:`upload_email_image`) — call this from the CLI + layer for a real apply, never from ``tools/planners/``. Under + ``--dry-run`` the CLI calls :func:`offline_resolve_spec_images` instead, + so a dry run uploads nothing — see that function. + """ + config = load_env_config() + with KizenClient(config) as client: + + def resolve_image(b: ImageBlockDef) -> dict[str, Any]: + info = upload_email_image(client, config.base_url, b.file) + return {**info, "alt": b.alt, "link": b.link, "width": b.width} + + return _walk_blocks(spec, resolve_image) + + +def offline_resolve_spec_images(spec: EmailTemplateDef) -> list[dict[str, Any]]: + """Offline counterpart for `messages templates craft-config`: no + network call of any kind. Reads each local image's header bytes for + ``naturalWidth``/``naturalHeight`` (that part needs no upload) but + stands in obvious placeholder tokens for ``fileId``/``src`` — this + output previews the compiled HTML, it is not meant to be pasted into a + create/update spec. + """ + + def resolve_image(b: ImageBlockDef) -> dict[str, Any]: + path = Path(b.file) + width, height, _ct = read_image_dimensions(path.read_bytes()) + return { + "file_id": OFFLINE_FILE_PLACEHOLDER, + "src": f"https://{OFFLINE_HOST_PLACEHOLDER}/api/files/{OFFLINE_FILE_PLACEHOLDER}/download", + "name": path.name, + "natural_width": width, + "natural_height": height, + "alt": b.alt, + "link": b.link, + "width": b.width, + } + + return _walk_blocks(spec, resolve_image) diff --git a/src/kizen_builder/tools/form_ui.py b/src/kizen_builder/tools/form_ui.py index 5e415f4..ebaf35a 100644 --- a/src/kizen_builder/tools/form_ui.py +++ b/src/kizen_builder/tools/form_ui.py @@ -76,6 +76,7 @@ import json import uuid +from collections.abc import Callable from typing import Any # --------------------------------------------------------------------------- @@ -496,14 +497,27 @@ def _assemble_block( def _assemble_cell( - cell_spec: dict[str, Any], parent_id: str, content: dict[str, Any] + cell_spec: dict[str, Any], + parent_id: str, + content: dict[str, Any], + *, + width: float | None = None, + cell_props: Callable[[float | None], dict[str, Any]] | None = None, + block_assembler: Callable[[dict[str, Any], str, dict[str, Any]], str] | None = None, ) -> str: + """``cell_props``/``block_assembler`` are additive hooks: both default to + today's behaviour (``props: {}``, this module's own block dispatch), so + forms/layouts callers that don't pass them are unaffected. A surface with + a different ``Cell.props`` shape (email's ``{"__width": }`` — + see ``tools.email_craft``) or different leaf-block prop shapes passes its + own callables instead of forking this function.""" + assemble = block_assembler or _assemble_block cell_id = _new_id() - block_ids = [_assemble_block(b, cell_id, content) for b in cell_spec["blocks"]] + block_ids = [assemble(b, cell_id, content) for b in cell_spec["blocks"]] content[cell_id] = { "type": {"resolvedName": "Cell"}, "isCanvas": True, - "props": {}, + "props": cell_props(width) if cell_props else {}, "displayName": "Cell", "custom": {}, "parent": parent_id, @@ -515,13 +529,28 @@ def _assemble_cell( def _assemble_row( - row_spec: dict[str, Any], parent_id: str, content: dict[str, Any] + row_spec: dict[str, Any], + parent_id: str, + content: dict[str, Any], + *, + cell_props: Callable[[float | None], dict[str, Any]] | None = None, + block_assembler: Callable[[dict[str, Any], str, dict[str, Any]], str] | None = None, ) -> str: row_id = _new_id() cells = row_spec["cells"] n = len(cells) or 1 columns = row_spec.get("columns") or [1.0 / n] * n - cell_ids = [_assemble_cell(c, row_id, content) for c in cells] + cell_ids = [ + _assemble_cell( + c, + row_id, + content, + width=columns[i] if i < len(columns) else None, + cell_props=cell_props, + block_assembler=block_assembler, + ) + for i, c in enumerate(cells) + ] content[row_id] = { "type": {"resolvedName": "Row"}, "isCanvas": False, @@ -543,10 +572,24 @@ def _assemble_row( def _assemble_section( - section_spec: dict[str, Any], parent_id: str, content: dict[str, Any] + section_spec: dict[str, Any], + parent_id: str, + content: dict[str, Any], + *, + cell_props: Callable[[float | None], dict[str, Any]] | None = None, + block_assembler: Callable[[dict[str, Any], str, dict[str, Any]], str] | None = None, ) -> str: section_id = _new_id() - row_ids = [_assemble_row(r, section_id, content) for r in section_spec["rows"]] + row_ids = [ + _assemble_row( + r, + section_id, + content, + cell_props=cell_props, + block_assembler=block_assembler, + ) + for r in section_spec["rows"] + ] content[section_id] = { "type": {"resolvedName": "Section"}, "isCanvas": True, @@ -568,7 +611,11 @@ def _assemble_section( def build_content_tree( - sections: list[dict[str, Any]], *, root_props: dict[str, Any] | None = None + sections: list[dict[str, Any]], + *, + root_props: dict[str, Any] | None = None, + cell_props: Callable[[float | None], dict[str, Any]] | None = None, + block_assembler: Callable[[dict[str, Any], str, dict[str, Any]], str] | None = None, ) -> dict[str, Any]: """Assemble a ``Root`` → ``Section`` → ``Row`` → ``Cell`` → block camelCase craft.js tree from :func:`section`/:func:`row`/:func:`cell`/ @@ -587,9 +634,21 @@ def build_content_tree( ``height``/``maxWidth``/``hasShadow``/``tabletBreak``/``mobileBreak`` (matching the dashboard static-content dashlet's Root shape, just camelCased instead of snake_case). + + ``cell_props``/``block_assembler`` are additive hooks for a surface whose + ``Cell.props`` or leaf-block prop shapes differ from this module's forms + defaults — see ``tools.email_craft``, which needs ``Cell.props`` to carry + ``{"__width": }`` and its own Button/Divider prop construction. + Both default to ``None``, which reproduces today's exact output — the + call sites in this file and in ``tools/layouts.py`` are unaffected. """ content: dict[str, Any] = {} - section_ids = [_assemble_section(s, "ROOT", content) for s in sections] + section_ids = [ + _assemble_section( + s, "ROOT", content, cell_props=cell_props, block_assembler=block_assembler + ) + for s in sections + ] content["ROOT"] = { "type": {"resolvedName": "Root"}, "isCanvas": True, diff --git a/src/kizen_builder/tools/planners/messages.py b/src/kizen_builder/tools/planners/messages.py index 56dae91..2802077 100644 --- a/src/kizen_builder/tools/planners/messages.py +++ b/src/kizen_builder/tools/planners/messages.py @@ -10,10 +10,26 @@ from kizen_builder.api.client import KizenClient from kizen_builder.config import load_env_config +from kizen_builder.models.spec.email_templates import EmailTemplateDef +from kizen_builder.tools import email_craft from kizen_builder.tools.automations import get_automation from kizen_builder.tools.messages import craft_summary, resolve_template from kizen_builder.tools.plans import Plan, PlanError, PlanOperation +# Only value ever observed live for a created template (see +# docs/specs/email-templates.md). No enum is declared anywhere in the repo +# (BCLI-015 left this field unwired since no create path existed until now), +# so this is hard-coded rather than exposed as an unguessable --sender-type +# flag. +_DEFAULT_SENDER_TYPE = "business" + +# `create_automation_message_from_template` (api/messages.py) already sends +# this for the automation-message resource; `POST /api/messages/templates` +# turns out to require it too — confirmed live 2026-08-25 the hard way (a +# `400 {"from_name_type": ["This field is required."]}` from a create +# missing it, not something the earlier probe's PATCH-only testing surfaced). +_DEFAULT_FROM_NAME_TYPE = "default" + # Copied onto a clone; everything else is server-assigned (id, created, # updated, is_editable) or a back-reference that must not be carried over. _CLONED_FIELDS = ( @@ -116,13 +132,89 @@ def plan_clone_template(source: str, new_name: str) -> Plan: ) -def plan_update_template(template: str, patch: dict[str, Any]) -> Plan: +def plan_create_template_from_spec( + spec: EmailTemplateDef, resolved_sections: list[dict[str, Any]] +) -> Plan: + """Plan creating an email template whose ``craft_json``/``content`` are + built from a spec, not hand-authored. + + ``resolved_sections`` comes from ``tools.email_craft.resolve_spec_images`` + — image blocks are already uploaded by the time this runs (a real write, + but not one this function performs; see that function's docstring for + why it can't happen here — planners never write, ``CLAUDE.md``). Building + the tree and compiling the HTML both happen inside + ``email_craft.build_email_content()``, in one pass over one set of ids, + so the two fields cannot go out of sync. + """ + config = load_env_config() + try: + sections = email_craft.assemble_sections(resolved_sections) + craft_json, content = email_craft.build_email_content(sections) + except ValueError as e: + raise PlanError(str(e)) from e + + payload: dict[str, Any] = { + "name": spec.name, + "subject": spec.subject, + "type": "email", + "sender_type": _DEFAULT_SENDER_TYPE, + "from_name_type": _DEFAULT_FROM_NAME_TYPE, + "craft_json": craft_json, + "content": content, + } + op = PlanOperation( + action="create", + kind="email_template", + key=spec.name, + preview={ + "env": config.name, + "name": spec.name, + "subject": spec.subject, + "sections": len(spec.sections), + "craft_json": f"{len(craft_json)} nodes", + "content": f"{len(content)} chars", + }, + payload=payload, + ) + return Plan.build( + env=config.name, + summary=f"Create email template '{spec.name}' from spec", + operations=[op], + ) + + +def plan_update_template( + template: str, + patch: dict[str, Any] | None = None, + *, + spec: EmailTemplateDef | None = None, + resolved_sections: list[dict[str, Any]] | None = None, +) -> Plan: """Plan a PATCH of one email template's fields. - ``patch`` is applied verbatim, including explicit ``None`` values — - that is deliberate, since clearing ``content`` is the way to ask - whether the server recompiles it from ``craft_json``. + Two mutually exclusive input modes, matching the CLI's two update paths: + + - ``patch`` — the raw field-level PATCH (``--craft-json-file``/ + ``--content-file``/``--name``/``--subject``), applied verbatim + including explicit ``None`` values (clearing ``content`` is the way to + ask whether the server recompiles it from ``craft_json`` — it doesn't). + - ``spec``/``resolved_sections`` — rebuilds both content fields from a + spec file the same way ``create`` does (``--spec-file``); overrides + ``name``/``subject`` too if the spec sets them. """ + if spec is not None: + try: + sections = email_craft.assemble_sections(resolved_sections or []) + craft_json, content = email_craft.build_email_content(sections) + except ValueError as e: + raise PlanError(str(e)) from e + patch = { + "name": spec.name, + "subject": spec.subject, + "craft_json": craft_json, + "content": content, + } + config, tmpl = _resolve(template) if not patch: raise PlanError("nothing to update — pass at least one field") diff --git a/tests/drift/test_email_template_roundtrip.py b/tests/drift/test_email_template_roundtrip.py new file mode 100644 index 0000000..94c8eba --- /dev/null +++ b/tests/drift/test_email_template_roundtrip.py @@ -0,0 +1,168 @@ +"""Drift test: `messages templates create --spec-file`'s payload round-trips +against a real environment — the same code path the CLI uses end to end: +spec -> `email_craft.resolve_spec_images()` (a real image upload, +`source="public_image"`) -> `plan_create_template_from_spec()` -> `apply_plan()`. + +Everything created is registered with `scratch` immediately after it's +created, per the module-level convention in `tests/drift/conftest.py`. +""" + +from __future__ import annotations + +import struct +import zlib + +import httpx +import pytest + +from kizen_builder.api import files as files_api +from kizen_builder.api import messages as messages_api +from kizen_builder.models.spec.email_templates import EmailTemplateDef +from kizen_builder.tools import email_craft as ec +from kizen_builder.tools.messages import craft_summary +from kizen_builder.tools.planners import messages as message_planners +from kizen_builder.tools.plans import apply_plan +from tests.drift.conftest import debris_name + +pytestmark = pytest.mark.drift + + +def _make_png(width: int, height: int) -> bytes: + sig = b"\x89PNG\r\n\x1a\n" + + def chunk(tag: bytes, data: bytes) -> bytes: + return ( + struct.pack(">I", len(data)) + + tag + + data + + struct.pack(">I", zlib.crc32(tag + data) & 0xFFFFFFFF) + ) + + ihdr = struct.pack(">IIBBBBB", width, height, 8, 2, 0, 0, 0) + raw = (b"\x00" + b"\xff\x00\x00" * width) * height + idat = zlib.compress(raw) + return sig + chunk(b"IHDR", ihdr) + chunk(b"IDAT", idat) + chunk(b"IEND", b"") + + +def test_create_template_from_spec_roundtrips_live( + drift_client, drift_config, scratch, tmp_path +): + png_path = tmp_path / "logo.png" + png_path.write_bytes(_make_png(9, 7)) + + spec = EmailTemplateDef.model_validate( + { + "name": debris_name("email-template"), + "subject": "Drift check — safe to delete", + "sections": [ + { + "background_color": "#FFFFFF", + "rows": [ + { + "layout": "1 Column", + "cells": [ + {"blocks": [{"kind": "text", "html": "

Hello

"}]} + ], + }, + { + "layout": "2 Columns (1/3 and 2/3)", + "cells": [ + { + "blocks": [ + { + "kind": "image", + "file": str(png_path), + "alt": "logo", + } + ] + }, + { + "blocks": [ + { + "kind": "button", + "label": "Go", + "url": "https://example.com", + } + ] + }, + ], + }, + ], + } + ], + } + ) + + # Real write #1: the image upload (source="public_image", is_public=True). + resolved_sections = ec.resolve_spec_images(spec) + image_block = resolved_sections[0]["rows"][1]["cells"][0]["blocks"][0] + file_id = image_block["file_id"] + assert file_id, "upload_email_image did not return a file id" + scratch.track("file", file_id, lambda: files_api.delete_file(drift_client, file_id)) + assert image_block["natural_width"] == 9 + assert image_block["natural_height"] == 7 + + # The emitted Image.src must be reachable by a real recipient — no Kizen + # auth headers on this request at all. Without is_public=True on the + # upload, this 404s (the finding this test exists to catch). + unauth = httpx.get(image_block["src"]) + assert unauth.status_code == 200, ( + f"uploaded image is not publicly readable: {unauth.status_code} " + f"for {image_block['src']}" + ) + + # Real write #2: the template itself, via the exact planner + apply_plan + # the CLI's `messages templates create` command uses. + plan = message_planners.plan_create_template_from_spec(spec, resolved_sections) + result = apply_plan(plan) + assert result.all_ok, [r.message for r in result.results if r.status != "ok"] + template_id = result.results[0].server_uuid + assert template_id + scratch.track( + "email template", + template_id, + lambda: messages_api.delete_template(drift_client, template_id), + ) + + # Read it back and run the same drift check `messages templates get` does. + live = messages_api.get_template(drift_client, template_id) + summary = craft_summary(live) + assert summary["structure_coupled"] is True, summary + assert summary["text_in_sync"] is True, summary + assert summary["coupled"] is True + + row_nodes = [ + n + for n in live["craft_json"].values() + if isinstance(n, dict) and n.get("type", {}).get("resolvedName") == "Row" + ] + fractions_seen = {tuple(n["props"]["columns"]) for n in row_nodes} + assert (1,) in fractions_seen + assert (0.3333333333333333, 0.6666666666666666) in fractions_seen + + # The stored content's column-width rule must be a BASE rule, not only + # inside the mobile @media query — confirms the fix against the real + # stored payload, not just the offline emitter. + style_start = live["content"].index('", 1)[0] + if "@media" not in style: + return style, "" + base, media = style.split("@media only screen and (max-width:480px){", 1) + # Strip exactly the one closing brace that ends the @media block itself + # (not a rule's own closing brace). + return base, media[:-1] if media.endswith("}") else media + + +def _resolved_name(node: dict) -> str: + t = node.get("type") + return t.get("resolvedName") if isinstance(t, dict) else t + + +def _ids_of(craft_json: dict, *kinds: str) -> set[str]: + return {nid for nid, node in craft_json.items() if _resolved_name(node) in kinds} + + +# --------------------------------------------------------------------------- +# Synthetic PNG/JPEG builders (no external dependency, no captured data) +# --------------------------------------------------------------------------- + + +def _make_png(width: int, height: int) -> bytes: + sig = b"\x89PNG\r\n\x1a\n" + + def chunk(tag: bytes, data: bytes) -> bytes: + return ( + struct.pack(">I", len(data)) + + tag + + data + + struct.pack(">I", zlib.crc32(tag + data) & 0xFFFFFFFF) + ) + + ihdr = struct.pack(">IIBBBBB", width, height, 8, 2, 0, 0, 0) + raw = (b"\x00" + b"\xff\x00\x00" * width) * height + idat = zlib.compress(raw) + return sig + chunk(b"IHDR", ihdr) + chunk(b"IDAT", idat) + chunk(b"IEND", b"") + + +def _make_jpeg(width: int, height: int) -> bytes: + soi = b"\xff\xd8" + sof0_payload = ( + struct.pack(">B", 8) + + struct.pack(">H", height) + + struct.pack(">H", width) + + struct.pack(">B", 1) + + struct.pack(">BBB", 1, 0x11, 0) + ) + sof0 = b"\xff\xc0" + struct.pack(">H", len(sof0_payload) + 2) + sof0_payload + return soi + sof0 + + +# --------------------------------------------------------------------------- +# Column-layout table — byte-exact, confirmed live 2026-08-25 +# --------------------------------------------------------------------------- + + +def test_v1_layouts_are_exactly_the_four_confirmed_live(): + assert ec.known_layouts() == [ + "1 Column", + "2 Columns", + "2 Columns (1/3 and 2/3)", + "2 Columns (2/3 and 1/3)", + ] + + +def test_column_fractions_are_byte_exact_not_rounded(): + assert ec.COLUMN_LAYOUTS["1 Column"].columns == (1,) + assert ec.COLUMN_LAYOUTS["2 Columns"].columns == (0.5, 0.5) + assert ec.COLUMN_LAYOUTS["2 Columns (1/3 and 2/3)"].columns == ( + 0.3333333333333333, + 0.6666666666666666, + ) + assert ec.COLUMN_LAYOUTS["2 Columns (2/3 and 1/3)"].columns == ( + 0.6666666666666666, + 0.3333333333333333, + ) + # The spec model's own table must agree with the emitter's. + for name, layout in ec.COLUMN_LAYOUTS.items(): + assert COLUMN_FRACTIONS[name] == layout.columns + + +def test_two_thirds_one_third_compiled_markup_matches_live_probe(): + sections = [ + ec.section( + [ + ec.row( + [ + ec.cell([ec.text_block("

a

")]), + ec.cell([ec.text_block("

b

")]), + ], + layout="2 Columns (1/3 and 2/3)", + ) + ] + ) + ] + _craft_json, content = ec.build_email_content(sections) + base_css, media_css = _split_style_block(content) + assert "mj-column-per-33-333332" in content + assert "mj-column-per-66-666664" in content + # The column widths are BASE (unconditional) rules — this is what makes + # the row render side by side at desktop width, not just under 480px. + assert "width:33.333332% !important; max-width:33.333332%;" in base_css + assert "width:66.666664% !important; max-width:66.666664%;" in base_css + assert "33.333332%" not in media_css + assert "66.666664%" not in media_css + # The media query's job is to COLLAPSE both columns to full width. + assert ( + ".mj-column-per-33-333332 { width:100% !important; max-width:100%; }" + in media_css + ) + assert ( + ".mj-column-per-66-666664 { width:100% !important; max-width:100%; }" + in media_css + ) + assert "width:293.3333px;" in content + assert "width:586.6666px;" in content + + +def test_one_column_and_two_column_compiled_markup_matches_live_probe(): + sections = [ + ec.section( + [ + ec.row([ec.cell([ec.text_block("

x

")])], layout="1 Column"), + ec.row( + [ + ec.cell([ec.text_block("

a

")]), + ec.cell([ec.text_block("

b

")]), + ], + layout="2 Columns", + ), + ] + ) + ] + _craft_json, content = ec.build_email_content(sections) + base_css, media_css = _split_style_block(content) + assert "mj-column-per-100" in content + assert "width:880.0px;" in content + # 2 Columns: one base rule + one media-collapse rule + one div per + # column = 4 occurrences of the class name. + assert content.count("mj-column-per-50") == 4 + assert ".mj-column-per-50 { width:50% !important; max-width:50%; }" in base_css + assert ".mj-column-per-50 { width:100% !important; max-width:100%; }" in media_css + assert "width:50%" not in media_css + assert content.count("width:440.0px;") == 2 + + +def test_button_and_divider_compiled_markup_matches_a_real_captured_template(): + """Verified 2026-08-26, read-only, against a real Kizen-authored + template on `cli-testing`: `_render_button`/`_render_divider`'s output + is byte-exact against that template's actual compiled `content` for a + Button/Divider node carrying the same props. The real template's own + copy/URL/color never enter this repo (personal-data rule) — the values + below are synthetic, chosen to exercise the same code path.""" + sections = [ + ec.section( + [ + ec.row( + [ + ec.cell( + [ + ec.button_block( + "Click Here", "https://example.com", color="#1B64F2" + ) + ] + ), + ec.cell([ec.divider_block("#E5E7EB")]), + ], + layout="2 Columns", + ) + ] + ) + ] + _craft_json, content = ec.build_email_content(sections) + assert ( + '
Click Here
' + ) in content + assert ( + '

' + ) in content + + +def test_exactly_one_tr_per_row_regardless_of_column_count(): + sections = [ + ec.section( + [ + ec.row( + [ec.cell([]), ec.cell([])], + layout="2 Columns", + ) + ] + ) + ] + _craft_json, content = ec.build_email_content(sections) + # One opening mso for the row's own mso table, not one per column. + assert content.count("") == 1 + + +# --------------------------------------------------------------------------- +# The coupling rule: Section/Row ids <-> section- classes, both ways +# --------------------------------------------------------------------------- + + +def test_every_section_and_row_id_has_a_matching_class_and_vice_versa(): + sections = [ + ec.section( + [ + ec.row([ec.cell([ec.text_block("

one

")])], layout="1 Column"), + ec.row( + [ + ec.cell([ec.button_block("Go", "https://example.com")]), + ec.cell([ec.divider_block()]), + ], + layout="2 Columns", + ), + ] + ), + ec.section( + [ec.row([ec.cell([ec.text_block("

two

")])], layout="1 Column")] + ), + ] + craft_json, content = ec.build_email_content(sections) + node_ids = _ids_of(craft_json, "Section", "Row") + html_classes = set(_SECTION_CLASS.findall(content)) + assert node_ids == html_classes + # 2 sections + 3 rows (2 in the first section, 1 in the second). + assert len(node_ids) == 5 + + +def test_craft_summary_reports_coupled_and_text_in_sync_for_emitted_output(): + """The real end-to-end check: run the drift detector this item reuses + (`craft_summary`, unmodified) against this emitter's own output.""" + sections = [ + ec.section( + [ + ec.row( + [ec.cell([ec.text_block("

Hello World

")])], + layout="1 Column", + ), + ec.row( + [ + ec.cell([ec.button_block("Go", "https://example.com")]), + ec.cell([ec.divider_block()]), + ], + layout="2 Columns", + ), + ] + ) + ] + craft_json, content = ec.build_email_content(sections) + summary = craft_summary({"craft_json": craft_json, "content": content}) + assert summary["structure_coupled"] is True + assert summary["text_in_sync"] is True + assert summary["coupled"] is True + + +def test_text_block_html_is_embedded_verbatim_in_content(): + html = '

Hi there

' + sections = [ + ec.section([ec.row([ec.cell([ec.text_block(html)])], layout="1 Column")]) + ] + _craft_json, content = ec.build_email_content(sections) + assert html in content + + +# --------------------------------------------------------------------------- +# Cell.props.__width — the additive form_ui hook this item adds +# --------------------------------------------------------------------------- + + +def test_cell_props_carries_double_underscore_width_matching_row_columns(): + sections = [ + ec.section( + [ + ec.row( + [ + ec.cell([ec.text_block("

a

")]), + ec.cell([ec.text_block("

b

")]), + ], + layout="2 Columns (2/3 and 1/3)", + ) + ] + ) + ] + craft_json, _content = ec.build_email_content(sections) + cells = [n for n in craft_json.values() if _resolved_name(n) == "Cell"] + widths = sorted(c["props"]["__width"] for c in cells) + assert widths == sorted([0.6666666666666666, 0.3333333333333333]) + + +def test_form_ui_cell_props_hook_defaults_to_empty_dict_props(): + """Regression: the hook this module relies on must default to today's + forms/layouts behaviour when not passed — see + tests/test_form_ui_payloads.py and tests/test_layout_custom_content.py + for the full guard.""" + tree = form_ui.build_content_tree( + [form_ui.section([form_ui.row([form_ui.cell([form_ui.text_block("x")])])])] + ) + cell = next( + n for n in tree.values() if n.get("type", {}).get("resolvedName") == "Cell" + ) + assert cell["props"] == {} + + +# --------------------------------------------------------------------------- +# Validation: row/layout, unsupported block kind — never a silent reshape +# --------------------------------------------------------------------------- + + +def test_row_rejects_cell_count_mismatch_for_its_layout(): + with pytest.raises(ValueError, match="needs 2 cell"): + ec.row([ec.cell([])], layout="2 Columns") + + +def test_row_rejects_unknown_layout_name(): + with pytest.raises(ValueError, match="unknown row layout"): + ec.row([ec.cell([])], layout="Fancy Layout") + + +@pytest.mark.parametrize( + "layout", + ["3 Columns", "3 Columns (gutters)", "4 Columns", "5 Columns", "6 Columns"], +) +def test_row_rejects_out_of_v1_scope_layouts_by_name(layout): + with pytest.raises(ValueError, match="out of v1 scope"): + ec.row([ec.cell([])], layout=layout) + + +def test_unsupported_block_kind_fails_loudly(): + with pytest.raises(ValueError, match="unsupported email block kind"): + ec.build_email_content( + [ + ec.section( + [ec.row([ec.cell([{"kind": "attachments"}])], layout="1 Column")] + ) + ] + ) + + +# --------------------------------------------------------------------------- +# Image header-byte dimension parsing — PNG and JPEG, GIF/WebP/SVG rejected +# --------------------------------------------------------------------------- + + +def test_png_dimensions_read_from_header_bytes(): + png = _make_png(37, 21) + assert ec.read_image_dimensions(png) == (37, 21, "image/png") + + +def test_jpeg_dimensions_read_from_sof0_segment(): + jpg = _make_jpeg(64, 48) + assert ec.read_image_dimensions(jpg) == (64, 48, "image/jpeg") + + +@pytest.mark.parametrize( + "label,data", + [ + ("gif", b"GIF89a" + b"\x00" * 20), + ("webp", b"RIFF\x00\x00\x00\x00WEBP" + b"\x00" * 20), + ("svg", b''), + ("bogus", b"not an image at all, just text"), + ], +) +def test_unsupported_image_formats_fail_loudly(label, data): + with pytest.raises(ValueError): + ec.read_image_dimensions(data) + + +def test_offline_resolve_spec_images_reads_local_dims_without_uploading(tmp_path: Path): + png_path = tmp_path / "logo.png" + png_path.write_bytes(_make_png(64, 32)) + spec = EmailTemplateDef.model_validate( + { + "name": "Synthetic", + "sections": [ + { + "rows": [ + { + "layout": "1 Column", + "cells": [ + { + "blocks": [ + { + "kind": "image", + "file": str(png_path), + "alt": "logo", + } + ] + } + ], + } + ] + } + ], + } + ) + resolved = ec.offline_resolve_spec_images(spec) + sections = ec.assemble_sections(resolved) + craft_json, _content = ec.build_email_content(sections) + image_node = next(n for n in craft_json.values() if _resolved_name(n) == "Image") + assert image_node["props"]["naturalWidth"] == 64 + assert image_node["props"]["naturalHeight"] == 32 + assert image_node["props"]["fileId"] == ec.OFFLINE_FILE_PLACEHOLDER + assert image_node["props"]["alt"] == "logo" + + +# --------------------------------------------------------------------------- +# Golden fixture — deterministic ids, byte-exact output for a small synthetic +# template. Regression net for accidental format drift in the emitter. +# --------------------------------------------------------------------------- + + +def test_golden_output_for_a_small_synthetic_template(monkeypatch: pytest.MonkeyPatch): + counter = iter(f"{i:024x}" for i in range(1, 50)) + monkeypatch.setattr(form_ui, "_new_id", lambda: next(counter)) + + sections = [ + ec.section( + [ec.row([ec.cell([ec.text_block("

Hello

")])], layout="1 Column")], + background_color="#EEEEEE", + ) + ] + craft_json, content = ec.build_email_content(sections) + + root_id = "ROOT" + section_id = "000000000000000000000001" + row_id = "000000000000000000000002" + cell_id = "000000000000000000000003" + text_id = "000000000000000000000004" + + assert set(craft_json) == {root_id, section_id, row_id, cell_id, text_id} + assert craft_json[root_id]["nodes"] == [section_id] + assert craft_json[section_id]["nodes"] == [row_id] + assert craft_json[row_id]["linkedNodes"] == {"column-1": cell_id} + assert craft_json[cell_id]["props"] == {"__width": 1} + assert craft_json[text_id]["custom"]["text"] == "

Hello

" + + assert f'class="section-{section_id}"' in content + assert f'class="section-{row_id}"' in content + assert f".section-{section_id} {{ background-color:#EEEEEE; }}" in content + assert "

Hello

" in content + assert "mj-column-per-100" in content diff --git a/tests/test_email_craft_upload.py b/tests/test_email_craft_upload.py new file mode 100644 index 0000000..998f7f4 --- /dev/null +++ b/tests/test_email_craft_upload.py @@ -0,0 +1,144 @@ +"""Tests for `tools.email_craft`'s live-call surface: image upload (via +`api.files.upload_file`, `source="public_image"`) and `api.files.delete_file`. + +Everything here is respx-mocked, same three-legged S3 dance +`test_smart_connectors_authoring.py` already exercises for +`SMART_CONNECTOR_IMPORT` — this pins the email-specific `source` constant +and the resulting `Image.src` shape instead. +""" + +from __future__ import annotations + +import struct +import zlib + +import httpx +import pytest +import respx + +from kizen_builder.api import files as files_api +from kizen_builder.api.client import KizenClient +from kizen_builder.models.spec.email_templates import EmailTemplateDef +from kizen_builder.tools import email_craft as ec +from tests.conftest import FAKE_BASE_URL + +S3_URL = "https://files.example.test/" + + +@pytest.fixture +def client(env_config): + with KizenClient(env_config) as c: + yield c + + +def _make_png(width: int, height: int) -> bytes: + sig = b"\x89PNG\r\n\x1a\n" + + def chunk(tag: bytes, data: bytes) -> bytes: + return ( + struct.pack(">I", len(data)) + + tag + + data + + struct.pack(">I", zlib.crc32(tag + data) & 0xFFFFFFFF) + ) + + ihdr = struct.pack(">IIBBBBB", width, height, 8, 2, 0, 0, 0) + raw = (b"\x00" + b"\xff\x00\x00" * width) * height + idat = zlib.compress(raw) + return sig + chunk(b"IHDR", ihdr) + chunk(b"IDAT", idat) + chunk(b"IEND", b"") + + +@respx.mock +def test_upload_email_image_uses_public_image_source_and_returns_natural_dims( + client, tmp_path +): + png_path = tmp_path / "logo.png" + png_path.write_bytes(_make_png(12, 34)) + + presign = respx.get(f"{FAKE_BASE_URL}/api/s3/presigned-post").mock( + return_value=httpx.Response( + 200, + json={ + "url": S3_URL, + "fields": {"key": "biz/public_image/logo.png"}, + "s3object_id": "s3-obj-9", + }, + ) + ) + respx.post(S3_URL).mock(return_value=httpx.Response(204, headers={"etag": '"e1"'})) + success = respx.post(f"{FAKE_BASE_URL}/api/s3/success").mock( + return_value=httpx.Response(200, json={"id": "file-9", "name": "logo.png"}) + ) + + info = ec.upload_email_image(client, FAKE_BASE_URL, png_path) + + assert presign.calls.last.request.url.params["source"] == files_api.PUBLIC_IMAGE + # is_public=true is what makes the emitted src reachable by an + # unauthenticated recipient — see api/files.py::upload_file. Without it + # every uploaded image 404s for anyone reading the email outside an + # authenticated session. + assert b"is_public=true" in success.calls.last.request.content + assert info["file_id"] == "file-9" + assert info["src"] == f"{FAKE_BASE_URL}/api/files/file-9/download" + assert info["natural_width"] == 12 + assert info["natural_height"] == 34 + + +@respx.mock +def test_resolve_spec_images_uploads_every_image_block(tmp_path): + png_path = tmp_path / "pic.png" + png_path.write_bytes(_make_png(5, 6)) + respx.get(f"{FAKE_BASE_URL}/api/s3/presigned-post").mock( + return_value=httpx.Response( + 200, + json={"url": S3_URL, "fields": {"key": "k"}, "s3object_id": "s3-obj-1"}, + ) + ) + respx.post(S3_URL).mock(return_value=httpx.Response(204, headers={"etag": '"e"'})) + respx.post(f"{FAKE_BASE_URL}/api/s3/success").mock( + return_value=httpx.Response(200, json={"id": "file-1", "name": "pic.png"}) + ) + + spec = EmailTemplateDef.model_validate( + { + "name": "t", + "sections": [ + { + "rows": [ + { + "layout": "1 Column", + "cells": [ + {"blocks": [{"kind": "image", "file": str(png_path)}]} + ], + } + ] + } + ], + } + ) + resolved = ec.resolve_spec_images(spec) + block = resolved[0]["rows"][0]["cells"][0]["blocks"][0] + assert block["file_id"] == "file-1" + assert block["natural_width"] == 5 + assert block["natural_height"] == 6 + + +@respx.mock +def test_delete_file_is_a_real_delete(client): + route = respx.delete(f"{FAKE_BASE_URL}/api/files/file-1").mock( + return_value=httpx.Response(204) + ) + files_api.delete_file(client, "file-1") + assert route.called + + +@respx.mock +def test_upload_email_image_rejects_unsupported_format_before_any_network_call( + client, tmp_path +): + bogus = tmp_path / "picture.gif" + bogus.write_bytes(b"GIF89a" + b"\x00" * 20) + # No routes registered: any network call would 500 via respx's + # assert_all_mocked default, proving the format check runs first. + with pytest.raises(ValueError, match="GIF"): + ec.upload_email_image(client, FAKE_BASE_URL, bogus) diff --git a/tests/test_email_template_planners.py b/tests/test_email_template_planners.py new file mode 100644 index 0000000..63e01c9 --- /dev/null +++ b/tests/test_email_template_planners.py @@ -0,0 +1,177 @@ +"""Tests for `tools.planners.messages`'s spec-driven email-template planners. + +`plan_create_template_from_spec`/`plan_update_template(..., spec=...)` take +already-resolved sections (image uploads happened earlier, in the CLI layer +— see `tools.email_craft.resolve_spec_images`'s docstring for why that split +exists) and must build a Plan with no live calls of their own beyond what +`plan_update_template` already needs to resolve the target template. Per +`CLAUDE.md`, nothing under `tools/planners/` performs a POST/PUT/PATCH/DELETE +— asserted here indirectly: these tests mock only GET/list endpoints, never +a write, and the planners still succeed. +""" + +from __future__ import annotations + +import httpx +import respx + +from kizen_builder.api.client import KizenClient +from kizen_builder.models.spec.email_templates import EmailTemplateDef +from kizen_builder.tools.messages import craft_summary +from kizen_builder.tools.planners import messages as message_planners +from kizen_builder.tools.plans import PlanError +from tests.conftest import FAKE_BASE_URL + +TEMPLATE_ID = "7cb5ce29-bf20-4f0f-bdc9-412a8c777ff8" + + +def _resolved_sections(*, layout: str = "1 Column", n_cells: int = 1) -> list[dict]: + cells = [ + {"blocks": [{"kind": "text", "html": f"

cell {i}

"}]} + for i in range(n_cells) + ] + return [ + {"background_color": "#FFFFFF", "rows": [{"layout": layout, "cells": cells}]} + ] + + +def _spec(**overrides) -> EmailTemplateDef: + base = {"name": "Newsletter", "subject": "Hi", "sections": []} + base.update(overrides) + return EmailTemplateDef.model_validate(base) + + +# --------------------------------------------------------------------------- +# plan_create_template_from_spec +# --------------------------------------------------------------------------- + + +def test_plan_create_builds_one_create_op_with_coupled_content(): + spec = _spec() + plan = message_planners.plan_create_template_from_spec(spec, _resolved_sections()) + assert len(plan.operations) == 1 + op = plan.operations[0] + assert op.action == "create" + assert op.kind == "email_template" + assert op.key == "Newsletter" + assert op.payload["name"] == "Newsletter" + assert op.payload["subject"] == "Hi" + assert op.payload["type"] == "email" + assert op.payload["sender_type"] == "business" + assert op.payload["from_name_type"] == "default" + # No raw craft_json/content ever entered this function — both are + # derived from resolved_sections by the same one-pass emitter. + summary = craft_summary( + {"craft_json": op.payload["craft_json"], "content": op.payload["content"]} + ) + assert summary["coupled"] is True + + +@respx.mock +def test_plan_create_makes_no_live_calls(): + """A planner performs no POST/PUT/PATCH/DELETE (CLAUDE.md). No route is + registered here, so respx's default `assert_all_mocked` would raise on + any httpx call this function tries to make — its absence is the proof.""" + message_planners.plan_create_template_from_spec(_spec(), _resolved_sections()) + + +def test_plan_create_row_cell_count_mismatch_raises_plan_error_not_silent_reshape(): + resolved = [ + { + "background_color": "#FFFFFF", + "rows": [ + { + "layout": "2 Columns", + "cells": [{"blocks": []}], # needs 2, got 1 + } + ], + } + ] + try: + message_planners.plan_create_template_from_spec(_spec(), resolved) + raise AssertionError("expected PlanError") + except PlanError as e: + assert "2 cell" in str(e) + + +def test_plan_create_unsupported_block_kind_raises_plan_error(): + resolved = [ + { + "background_color": "#FFFFFF", + "rows": [ + {"layout": "1 Column", "cells": [{"blocks": [{"kind": "attachments"}]}]} + ], + } + ] + try: + message_planners.plan_create_template_from_spec(_spec(), resolved) + raise AssertionError("expected PlanError") + except PlanError as e: + assert "unsupported block kind" in str(e) + + +# --------------------------------------------------------------------------- +# plan_update_template(..., spec=..., resolved_sections=...) +# --------------------------------------------------------------------------- + + +def _client() -> KizenClient: + from kizen_builder.config import load_env_config + + return KizenClient(load_env_config()) + + +@respx.mock +def test_plan_update_from_spec_rebuilds_both_fields_together(): + existing = { + "id": TEMPLATE_ID, + "name": "Old Name", + "subject": "Old subject", + "craft_json": {"ROOT": {"type": {"resolvedName": "Root"}}}, + "content": "

old

", + } + respx.get(f"{FAKE_BASE_URL}/api/messages/templates/{TEMPLATE_ID}").mock( + return_value=httpx.Response(200, json=existing) + ) + spec = _spec(name="New Name", subject="New subject") + plan = message_planners.plan_update_template( + TEMPLATE_ID, spec=spec, resolved_sections=_resolved_sections() + ) + op = plan.operations[0] + assert op.action == "update" + assert op.existing_uuid == TEMPLATE_ID + assert op.payload["name"] == "New Name" + assert op.payload["subject"] == "New subject" + summary = craft_summary( + {"craft_json": op.payload["craft_json"], "content": op.payload["content"]} + ) + assert summary["coupled"] is True + + +def test_plan_update_raw_patch_path_is_unchanged(): + """The existing --craft-json-file/--content-file path (patch dict, no + spec) must keep working exactly as before this item.""" + with respx.mock: + respx.get(f"{FAKE_BASE_URL}/api/messages/templates/{TEMPLATE_ID}").mock( + return_value=httpx.Response( + 200, + json={"id": TEMPLATE_ID, "name": "t", "craft_json": {}, "content": ""}, + ) + ) + plan = message_planners.plan_update_template(TEMPLATE_ID, {"name": "renamed"}) + assert plan.operations[0].payload == {"name": "renamed"} + + +def test_plan_update_with_neither_patch_nor_spec_raises_plan_error(): + with respx.mock: + respx.get(f"{FAKE_BASE_URL}/api/messages/templates/{TEMPLATE_ID}").mock( + return_value=httpx.Response( + 200, + json={"id": TEMPLATE_ID, "name": "t", "craft_json": {}, "content": ""}, + ) + ) + try: + message_planners.plan_update_template(TEMPLATE_ID, {}) + raise AssertionError("expected PlanError") + except PlanError as e: + assert "nothing to update" in str(e) diff --git a/tests/test_email_template_spec.py b/tests/test_email_template_spec.py new file mode 100644 index 0000000..89dd41f --- /dev/null +++ b/tests/test_email_template_spec.py @@ -0,0 +1,114 @@ +"""Tests for `models.spec.email_templates.EmailTemplateDef` and friends. + +Pins the acceptance criteria the model alone is responsible for: the row +layout is a closed enum (an invalid preset is unrepresentable, not just a +validation error to catch after the fact), and `create`'s spec has no way to +smuggle a raw `craft_json`/`content` value past validation. +""" + +from __future__ import annotations + +import pytest +from pydantic import ValidationError + +from kizen_builder.models.spec.email_templates import EmailTemplateDef + + +def _spec(**overrides): + base = { + "name": "Newsletter", + "subject": "Hello", + "sections": [ + { + "rows": [ + { + "layout": "1 Column", + "cells": [{"blocks": [{"kind": "text", "html": "

hi

"}]}], + } + ] + } + ], + } + base.update(overrides) + return base + + +def test_minimal_valid_spec(): + spec = EmailTemplateDef.model_validate(_spec()) + assert spec.name == "Newsletter" + assert spec.sections[0].rows[0].layout == "1 Column" + + +def test_subject_defaults_to_empty_string(): + spec = EmailTemplateDef.model_validate({"name": "t", "sections": []}) + assert spec.subject == "" + + +@pytest.mark.parametrize( + "layout", + [ + "1 Column", + "2 Columns", + "2 Columns (1/3 and 2/3)", + "2 Columns (2/3 and 1/3)", + ], +) +def test_v1_layout_names_are_accepted(layout): + spec_dict = _spec() + spec_dict["sections"][0]["rows"][0]["layout"] = layout + EmailTemplateDef.model_validate(spec_dict) # must not raise + + +@pytest.mark.parametrize( + "layout", ["3 Columns", "4 Columns", "Two Columns", "50/50", "1column"] +) +def test_invalid_or_out_of_scope_layout_names_are_unrepresentable(layout): + spec_dict = _spec() + spec_dict["sections"][0]["rows"][0]["layout"] = layout + with pytest.raises(ValidationError): + EmailTemplateDef.model_validate(spec_dict) + + +@pytest.mark.parametrize("kind", ["text", "image", "button", "divider"]) +def test_v1_block_kinds_are_accepted(kind): + block = { + "text": {"kind": "text", "html": "

hi

"}, + "image": {"kind": "image", "file": "/tmp/x.png"}, + "button": {"kind": "button", "label": "Go", "url": "https://x"}, + "divider": {"kind": "divider"}, + }[kind] + spec_dict = _spec() + spec_dict["sections"][0]["rows"][0]["cells"][0]["blocks"] = [block] + EmailTemplateDef.model_validate(spec_dict) # must not raise + + +@pytest.mark.parametrize("kind", ["attachments", "html", "custom_field", "video"]) +def test_unsupported_block_kinds_are_unrepresentable(kind): + spec_dict = _spec() + spec_dict["sections"][0]["rows"][0]["cells"][0]["blocks"] = [{"kind": kind}] + with pytest.raises(ValidationError): + EmailTemplateDef.model_validate(spec_dict) + + +def test_no_raw_craft_json_key_anywhere_in_the_model(): + """The foot-gun this whole surface exists to close: create must not + accept a hand-authored craft_json that can drift from `content`.""" + with pytest.raises(ValidationError): + EmailTemplateDef.model_validate(_spec(craft_json={})) + + +def test_no_raw_content_key_anywhere_in_the_model(): + with pytest.raises(ValidationError): + EmailTemplateDef.model_validate(_spec(content="

hand-authored

")) + + +def test_no_sender_type_key_in_the_model(): + """sender_type is hard-coded "business" by the planner — not a spec key.""" + with pytest.raises(ValidationError): + EmailTemplateDef.model_validate(_spec(sender_type="business")) + + +def test_grep_confirms_neither_craft_json_nor_content_is_a_model_field(): + field_names = set(EmailTemplateDef.model_fields) + assert "craft_json" not in field_names + assert "content" not in field_names diff --git a/tests/test_smart_connectors_authoring.py b/tests/test_smart_connectors_authoring.py index 4061a27..8b4ed3a 100644 --- a/tests/test_smart_connectors_authoring.py +++ b/tests/test_smart_connectors_authoring.py @@ -184,6 +184,29 @@ def test_upload_file_walks_presign_s3_and_success(client, tmp_path): ) assert b"uuid=s3-obj-1" in ok_req.content assert b"etag=abc123" in ok_req.content + # is_public defaults to omitted (server default is false) — a smart + # connector's reference file has no reason to be world-readable. + assert b"is_public" not in ok_req.content + + +@respx.mock +def test_upload_file_sends_is_public_true_when_requested(client, tmp_path): + src = tmp_path / "sample.csv" + src.write_bytes(b"order_number\n1\n") + respx.get(f"{FAKE_BASE_URL}/api/s3/presigned-post").mock( + return_value=httpx.Response( + 200, + json={"url": S3_URL, "fields": {"key": "k"}, "s3object_id": "s3-obj-1"}, + ) + ) + respx.post(S3_URL).mock(return_value=httpx.Response(204, headers={"etag": '"e"'})) + success = respx.post(f"{FAKE_BASE_URL}/api/s3/success").mock( + return_value=httpx.Response(200, json={"id": "file-1", "name": "sample.csv"}) + ) + + files_api.upload_file(client, src, is_public=True) + + assert b"is_public=true" in success.calls.last.request.content @respx.mock From c66b8fd51da72c2bc1fe2783eba39debce97074e Mon Sep 17 00:00:00 2001 From: Jeremy Bedient Date: Wed, 26 Aug 2026 12:09:04 -0400 Subject: [PATCH 2/4] Expose email template layout props and fix content/craft_json drift MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Email template specs can now set the layout knobs a designed newsletter needs instead of landing at this emitter's fixed defaults: `Section`/`Row` `max_width`/`container_width`/`padding`, `Divider` `size`, and `Button` `border_radius`/`padding_left`/`padding_right`/ `alignment`. `padding` is a new `PaddingDef` with four independent top/right/bottom/left strings, matching the wire format's four independent `containerPadding*` keys rather than a lossy shorthand — the reference template shows real asymmetric padding. `form_ui.py` gains `section_props`/`row_props` hooks mirroring the existing `cell_props` shape; every new field defaults to today's exact hardcoded value, so an all-defaults spec's `craft_json` and `content` are unchanged. The harder part was `content`, the compiled HTML actually sent: it had never read any of these values. Row/section width was frozen at a module-level 880px constant regardless of what `craft_json` said; `Section`/`Row` padding never rendered at all, on any template; and `Button.alignment`/`Image.position` had no effect on the compiled markup. Each was the same failure mode — a field lands correctly in `craft_json` and never reaches `content` — so `craft_json` and `content` disagreed about what the template actually looks like, exactly the two-fields-must-agree failure this surface exists to prevent. `_row_content_width_px` now derives a row's real pixel width from its own `containerWidth` or its parent Section's `maxWidth` minus padding, scaled by `Row.width`; `_padding_css` renders each Section's and Row's own padding onto its wrapper div; `_render_button` and `_render_image` now read `alignment`/`position` instead of ignoring them. A systematic test walks every field this item added and asserts its effect on `content` directly, with named exemptions only for the props confirmed to have no rendered representation in Kizen's real compiler. docs/specs/email-templates.md and CHANGELOG.md are updated to match. --- CHANGELOG.md | 24 + .../docs/specs/email-templates.md | 110 ++- .../models/spec/email_templates.py | 50 +- src/kizen_builder/tools/email_craft.py | 357 ++++++-- src/kizen_builder/tools/form_ui.py | 53 +- tests/test_cli_email_templates.py | 78 ++ tests/test_email_craft.py | 816 +++++++++++++++++- tests/test_email_template_spec.py | 121 ++- 8 files changed, 1527 insertions(+), 82 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 14d68f5..ed2dd19 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,6 +61,30 @@ called out explicitly under **Changed** or **Removed**. A real test send opened in Outlook is still the only way to confirm actual rendering — nothing offline can substitute for that. +- **Email template specs can now set the layout knobs a designed newsletter + needs — `Section`/`Row` width and padding, `Divider` thickness, `Button` + corner radius/padding/alignment — instead of every template landing at + this emitter's fixed defaults.** `sections[].max_width`/`container_width`/ + `padding` and `sections[].rows[].width`/`container_width`/`padding` set + `Section`/`Row` props directly; `padding` is four independent + `{top, right, bottom, left}` strings, matching the wire format's four + independent `containerPadding*` keys rather than a lossy CSS-style + shorthand. `button` blocks gain `border_radius`/`padding_left`/ + `padding_right`/`alignment`; `divider` blocks gain `size`. Every new field + defaults to this emitter's exact pre-existing hardcoded value, so a spec + that sets none of them produces the same output as before this change. + The compiled `content` HTML's row widths now track these same values too + (previously frozen at a hardcoded 880px regardless of what the spec set — + a real `craft_json`/`content` divergence, the exact failure this whole + surface exists to prevent), `content` now carries `Section`/`Row` + padding at all (previously absent entirely, on every template — text + always rendered flush against the canvas edge regardless of what + `craft_json` said), `content`'s `Button` markup now carries `align` + (previously every button rendered left-aligned regardless of the spec's + `alignment`), and a centered `Image` (`position: "center"`, the only + value this surface sets) now actually renders centered in `content` + instead of flush left. + ### Fixed - **`kizen upgrade --check` can now find a release tag from a `uv tool diff --git a/src/kizen_builder/docs/specs/email-templates.md b/src/kizen_builder/docs/specs/email-templates.md index 8f0d116..68dd54b 100644 --- a/src/kizen_builder/docs/specs/email-templates.md +++ b/src/kizen_builder/docs/specs/email-templates.md @@ -33,9 +33,15 @@ produce, offline, with `messages templates craft-config --spec-file "sections": [ { "background_color": "#FFFFFF", + "max_width": "600", + "container_width": "900", + "padding": {"top": "10", "right": "10", "bottom": "10", "left": "10"}, "rows": [ { "layout": "2 Columns", + "width": "100", + "container_width": "580", + "padding": {"top": "10", "right": "10", "bottom": "10", "left": "10"}, "cells": [ {"blocks": [{"kind": "text", "html": "

Left column

"}]}, {"blocks": [{"kind": "image", "file": "/path/to/logo.png", "alt": "Logo"}]} @@ -67,7 +73,8 @@ produce, offline, with `messages templates craft-config --spec-file escape hatch (no `HTMLBlock` on this surface at all, confirmed live). - `text`: `{"kind": "text", "html": "

...

"}` — embedded verbatim in both outputs. - - `image`: `{"kind": "image", "file": "", "alt": "", "link": "", "width": 150}`. + - `image`: `{"kind": "image", "file": "", "alt": "", "link": "", "width": 150, + "container_width": null, "max_width": null, "max_height": null}`. `file` is a **local path**, not a `file_id` — there is no CLI surface to look one up afterward (`GET /api/files` is broken; see below), so the spec captures the upload at the point it happens. **PNG and JPEG only** @@ -79,9 +86,80 @@ produce, offline, with `messages templates craft-config --spec-file when a `create`/`update --spec-file` is actually applied; under `--dry-run` the CLI resolves images offline instead (a placeholder `fileId`/`src`, real dimensions still read from the file's own header - bytes) so a dry run never writes. - - `button`: `{"kind": "button", "label": "...", "url": "...", "color": null}`. - - `divider`: `{"kind": "divider", "color": null}`. + bytes) so a dry run never writes. `container_width`/`max_width`/ + `max_height` default to `null`, meaning the corresponding `Image.props` + key is omitted entirely, matching this emitter's output before these + fields existed. + - `button`: `{"kind": "button", "label": "...", "url": "...", "color": null, + "border_radius": "8", "padding_left": "20", "padding_right": "20", + "alignment": "center"}`. `alignment` is a closed enum + (`left`/`center`/`right`). + - `divider`: `{"kind": "divider", "color": null, "size": "3"}` — `size` is + the rule's thickness in px. +- `sections[].max_width`/`container_width`/`padding` and + `sections[].rows[].width`/`container_width`/`padding` set `Section`/`Row` + width and padding directly. `padding` is `{"top", "right", "bottom", + "left"}`, four independent strings — not a CSS-style shorthand, since the + wire format itself has four independent `containerPadding{Top,Right, + Bottom,Left}` keys (a shorthand would be a lossy abstraction over that). + **Confirmed live 2026-08-26 against the reference template + (`0bc71ca1-72c7-4f8d-934e-322d9fd40975`): `Row` layout props are not + uniform.** Most rows in that template share `containerWidth: '580'` and + uniform `10` padding, but the same template also has rows with + `containerWidth: '600'`, one with asymmetric padding + (`containerPaddingLeft/Right: '40'`, top/bottom still `10`), one with all + four paddings at `0`, and one with `Row.width: '75'` (not the usual + `100`) — `Row.containerWidth` does **not** cleanly derive from + `Section.max_width - 2*padding` across these observations. So + `width`/`container_width`/`padding` are independent, spec-settable fields + on both `SectionDef` and `RowDef`, never computed from one another — the + same "redundant-but-must-agree" trust model this surface already uses for + `Row.props.columns` vs. every `Cell.props.__width`. + + Defaults reproduce this emitter's pre-existing hardcoded output exactly + (`Section.max_width: "900"`, `Row.width: "100"`, all padding uniform + `"10"`, no `containerWidth` key on either node) — **not** the reference + template's own common values (`Section.max_width: "600"`, + `Row.containerWidth: "580"`), which a spec sets explicitly when targeting + that layout. + + **These values also drive the compiled `content`'s row width — not just + `craft_json`.** Each row's mso table width and its `.section- { + max-width:...px; }` rule use an explicit `Row.container_width` directly + when the spec set one; otherwise they derive it as `Section.max_width - + (containerPaddingLeft + containerPaddingRight)` from the row's parent + `Section`. That result is then scaled by `Row.width` (percent, default + `"100"`) — confirmed live against the reference template, where a + `width: '75'` row on a `containerWidth: '580'` row compiles to `435px` + (`580 * 0.75`), not `580px`. With every field at its default this + reduces to exactly `900 - 10 - 10 = 880`, this emitter's original + hardcoded value — so an all-defaults spec's compiled HTML is unchanged. A + spec that sets `Section.max_width` (or a row's own `container_width`/ + `width`) without this derivation would otherwise produce a `craft_json` + that renders at the new width in Kizen's builder while `content` — the + HTML actually sent — stayed at the old width: `craft_summary()`'s + `structure_coupled`/`text_in_sync` checks cannot see this class of + divergence, since node ids and text both still match. + + **Padding follows the same rule.** Each `Section`/`Row`'s own + `containerPadding{Top,Right,Bottom,Left}` — default uniform `"10"` or an + explicit `padding` override — is rendered as an inline + `padding:Tpx Rpx Bpx Lpx;` (top/right/bottom/left, matching + `_render_button`'s existing padding order) on that node's own wrapper + `
` in `content`. This applies to every + section and row, not only ones that explicitly set the new `padding` + field — before this fix, `content` carried **no** padding declaration + for `Section`/`Row` at all, on any template, so text always rendered + flush against the canvas edge regardless of what `craft_json` said. + + **`Section.container_width` is `craft_json`-only in this emitter today, + but that is a deferred gap, not a by-design exemption.** Kizen's real + compiled `content` does apply it — to an outer full-bleed background-table + wrapper (mso ``, a ``, an outer `max-width`) that + this emitter hasn't built yet. Wiring it in is BCLI-025's scope, which + already owns the related "dropped `` background" gap; this emitter's + `_render_section` goes straight from a section's own div to its rows, with + no such wrapper to hang the value on. - `sender_type` and `from_name_type` are not spec keys. They are hard-coded to `"business"`/`"default"`, the only values ever observed live — see "Other top-level fields" below. There is no `--sender-type` flag. @@ -225,15 +303,35 @@ Block props confirmed live 2026-08-25: schemes 200 unauthenticated once set, 404 on both when not). The spec-file emitter's upload path sets it; a raw `upload_file()` call elsewhere in this repo does not unless asked (see `api/files.py`). + `position: "center"` (the only value ever hardcoded — not spec-settable, + no observed alternative) compiles to `margin:0 auto;` on the `` + itself; a plain `display:block` fixed-width image with no margin renders + flush left, not centered. `container_width`/`max_width`/`max_height` are + spec-settable (`ImageBlockDef`) but are **`craft_json`-only** — `content`'s + `` always uses the emitter's own `width`/fixed `max-width:100%` + pair, confirmed by reading `_render_image`. - **`Button`** — `{url, label, action: "url", color, textColor, fontSize, fontFamily, alignment, borderSize, borderColor, borderRadius, padding{Top,Left,Right,Bottom}, textStyles: [], openLinkInNewTab}` plus the `container*` set. The emitter's compiled `content` markup for this node (`_render_button`) was checked byte-exact against a real Button in a - Kizen-authored template, read-only, 2026-08-26. + Kizen-authored template, read-only, 2026-08-26. `borderRadius`/ + `padding{Left,Right}`/`alignment` are spec-settable + (`ButtonBlockDef.border_radius`/`padding_left`/`padding_right`/ + `alignment`), defaulting to this emitter's pre-existing hardcoded values + (`"8"`/`"20"`/`"20"`/`"center"`); the reference template's own newsletter + button uses `"20"`/`"30"`/`"30"`/`"center"`, set explicitly, not inherited + as the default. `alignment` compiles to an `align="..."` attribute on the + button's own wrapping `` (which also carries `line-height:100%;` + in its `style` — both confirmed present on a real Kizen-compiled button by + independent comparison in this item's review); without it every button + renders left-aligned in its cell regardless of the spec. - **`Divider`** — `{size, color, width, alignment, borderStyle}` plus the `container*` set. Same verification: `_render_divider`'s output matches a - real captured Divider's compiled markup byte-exact. + real captured Divider's compiled markup byte-exact. `size` (thickness in + px) is spec-settable (`DividerBlockDef.size`), defaulting to this + emitter's pre-existing hardcoded `"3"`; the reference template's divider + uses `"1"`, set explicitly. - **`Attachments`** — `props.attachments` is a list of **full file records** (id, key, url, name, size_bytes, content_type, thumbnail_url, `is_public`, and an `employee` object naming the uploader), plus an diff --git a/src/kizen_builder/models/spec/email_templates.py b/src/kizen_builder/models/spec/email_templates.py index 49f2e37..d921f49 100644 --- a/src/kizen_builder/models/spec/email_templates.py +++ b/src/kizen_builder/models/spec/email_templates.py @@ -41,6 +41,21 @@ } +class PaddingDef(BaseModel): + """Four independent sides, matching the wire format 1:1 + (`containerPaddingTop`/`Right`/`Bottom`/`Left`) rather than a CSS-style + shorthand — the reference template shows asymmetric padding (e.g. `40` + left/right with `10` top/bottom on one row), so a shorthand would be a + lossy abstraction over four independently-set keys.""" + + model_config = ConfigDict(extra="forbid") + + top: str = "10" + right: str = "10" + bottom: str = "10" + left: str = "10" + + class TextBlockDef(BaseModel): """Rich-text copy. `html` is embedded verbatim in both `craft_json` (`custom.text`) and the compiled `content` — see the coupling rule in @@ -72,6 +87,9 @@ class ImageBlockDef(BaseModel): default=None, description="Display width in px. Defaults to 150 (form_ui's default).", ) + container_width: str | None = None + max_width: str | None = None + max_height: str | None = None class ButtonBlockDef(BaseModel): @@ -81,6 +99,10 @@ class ButtonBlockDef(BaseModel): label: str url: str color: str | None = None + border_radius: str = "8" + padding_left: str = "20" + padding_right: str = "20" + alignment: Literal["left", "center", "right"] = "center" class DividerBlockDef(BaseModel): @@ -88,6 +110,7 @@ class DividerBlockDef(BaseModel): kind: Literal["divider"] = "divider" color: str | None = None + size: str = "3" BlockDef = Annotated[ @@ -106,19 +129,44 @@ class RowDef(BaseModel): """One row. `layout` picks a closed-enum column preset; the emitter (not this model — see `tools/planners/messages.py`) rejects a row whose cell count doesn't match the preset with a `PlanError`, at plan time rather - than as a silent reshape.""" + than as a silent reshape. + + `width`/`container_width`/`padding` are independently spec-settable, not + derived from `SectionDef`'s equivalents — the reference template shows + `Row.containerWidth`/padding/`width` varying row-to-row with no clean + formula from `Section.max_width`/padding (some rows fit + `max_width - 2*padding`, others don't). Same "redundant-but-must-agree" + trust model this surface already uses for `Row.props.columns` vs. + `Cell.props.__width`. + """ model_config = ConfigDict(extra="forbid") layout: ColumnPreset = "1 Column" cells: list[CellDef] + width: str = "100" + container_width: str | None = None + padding: PaddingDef | None = None class SectionDef(BaseModel): + """`max_width`/`container_width`/`padding` default to the emitter's + pre-existing hardcoded values (`900`/absent/`10` uniform) so a spec that + sets none of them is byte-identical to the pre-this-item emitter — see + `tools/email_craft.py::_section_props`. The reference template's own + common value for `max_width` is `600` (a real newsletter's content + width); a spec targeting that layout sets it explicitly rather than + inheriting it as the default, which would break the regression + guarantee this item's acceptance criteria require. + """ + model_config = ConfigDict(extra="forbid") rows: list[RowDef] = Field(default_factory=list) background_color: str = "#FFFFFF" + max_width: str = "900" + container_width: str | None = None + padding: PaddingDef | None = None class EmailTemplateDef(BaseModel): diff --git a/src/kizen_builder/tools/email_craft.py b/src/kizen_builder/tools/email_craft.py index b01dbd3..8644563 100644 --- a/src/kizen_builder/tools/email_craft.py +++ b/src/kizen_builder/tools/email_craft.py @@ -36,6 +36,7 @@ from __future__ import annotations +import math from collections.abc import Callable from html import escape from pathlib import Path @@ -49,6 +50,7 @@ DividerBlockDef, EmailTemplateDef, ImageBlockDef, + PaddingDef, TextBlockDef, ) from kizen_builder.tools import form_ui @@ -132,7 +134,7 @@ class ColumnLayout: - __slots__ = ("preset", "columns", "classes", "media_widths", "mso_widths_px") + __slots__ = ("preset", "columns", "classes", "media_widths") def __init__( self, @@ -140,17 +142,20 @@ def __init__( columns: tuple[float, ...], classes: tuple[str, ...], media_widths: tuple[str, ...], - mso_widths_px: tuple[float, ...], ) -> None: self.preset = preset self.columns = columns self.classes = classes self.media_widths = media_widths - self.mso_widths_px = mso_widths_px -# 880px content width in every case observed (900 Root maxWidth - 20px padding). -CONTENT_WIDTH_PX = 880.0 +# 880px was the content width in every case observed pre-BCLI-024 (900 +# Section maxWidth - 20px padding, both hardcoded at the time). Now that +# `Section.max_width`/`Row.container_width`/padding are spec-settable +# (BCLI-024), the actual per-row pixel width is computed by +# `_row_content_width_px` below, not held as one constant — see that +# function's docstring for the fallback formula, which reduces to exactly +# 880.0 when nothing is overridden. COLUMN_LAYOUTS: dict[str, ColumnLayout] = { "1 Column": ColumnLayout( @@ -158,28 +163,24 @@ def __init__( (1,), ("mj-column-per-100",), ("100%",), - (880.0,), ), "2 Columns": ColumnLayout( "2 Columns", (0.5, 0.5), ("mj-column-per-50", "mj-column-per-50"), ("50%", "50%"), - (440.0, 440.0), ), "2 Columns (1/3 and 2/3)": ColumnLayout( "2 Columns (1/3 and 2/3)", (0.3333333333333333, 0.6666666666666666), ("mj-column-per-33-333332", "mj-column-per-66-666664"), ("33.333332%", "66.666664%"), - (293.3333, 586.6666), ), "2 Columns (2/3 and 1/3)": ColumnLayout( "2 Columns (2/3 and 1/3)", (0.6666666666666666, 0.3333333333333333), ("mj-column-per-66-666664", "mj-column-per-33-333332"), ("66.666664%", "33.333332%"), - (586.6666, 293.3333), ), } @@ -226,6 +227,9 @@ def image_block( width: int | None = None, natural_width: int | None = None, natural_height: int | None = None, + container_width: str | None = None, + max_width: str | None = None, + max_height: str | None = None, ) -> dict[str, Any]: return { "kind": "image", @@ -237,28 +241,62 @@ def image_block( "width": width, "natural_width": natural_width, "natural_height": natural_height, + "container_width": container_width, + "max_width": max_width, + "max_height": max_height, } -def button_block(label: str, url: str, *, color: str | None = None) -> dict[str, Any]: - return {"kind": "button", "label": label, "url": url, "color": color} +def button_block( + label: str, + url: str, + *, + color: str | None = None, + border_radius: str | None = None, + padding_left: str | None = None, + padding_right: str | None = None, + alignment: str | None = None, +) -> dict[str, Any]: + return { + "kind": "button", + "label": label, + "url": url, + "color": color, + "border_radius": border_radius, + "padding_left": padding_left, + "padding_right": padding_right, + "alignment": alignment, + } -def divider_block(color: str | None = None) -> dict[str, Any]: - return {"kind": "divider", "color": color} +def divider_block( + color: str | None = None, *, size: str | None = None +) -> dict[str, Any]: + return {"kind": "divider", "color": color, "size": size} def cell(blocks: list[dict[str, Any]]) -> dict[str, Any]: return {"blocks": blocks} -def row(cells: list[dict[str, Any]], layout: str = "1 Column") -> dict[str, Any]: +def row( + cells: list[dict[str, Any]], + layout: str = "1 Column", + *, + width: str | None = None, + container_width: str | None = None, + padding: dict[str, str] | None = None, +) -> dict[str, Any]: """One row using a v1 column preset by name. Raises ``ValueError`` — never a silent reshape — for an unknown preset name or a cell count that doesn't match it, naming the valid presets or the expected count. The planner (``tools.planners.messages``) catches this and re-raises as a ``PlanError``. + + ``width``/``container_width``/``padding`` default to ``None`` — no + override, reproducing today's exact hardcoded output — see + ``_row_props``. """ if layout in _OUT_OF_SCOPE_LAYOUTS: raise ValueError( @@ -275,13 +313,34 @@ def row(cells: list[dict[str, Any]], layout: str = "1 Column") -> dict[str, Any] raise ValueError( f"layout {layout!r} needs {len(preset.columns)} cell(s), got {len(cells)}" ) - return {"cells": cells, "columns": list(preset.columns), "layout": layout} + return { + "cells": cells, + "columns": list(preset.columns), + "layout": layout, + "width": width, + "container_width": container_width, + "padding": padding, + } def section( - rows: list[dict[str, Any]], *, background_color: str = "#FFFFFF" + rows: list[dict[str, Any]], + *, + background_color: str = "#FFFFFF", + max_width: str | None = None, + container_width: str | None = None, + padding: dict[str, str] | None = None, ) -> dict[str, Any]: - return {"rows": rows, "background_color": background_color} + """``max_width``/``container_width``/``padding`` default to ``None`` — no + override, reproducing today's exact hardcoded output — see + ``_section_props``.""" + return { + "rows": rows, + "background_color": background_color, + "max_width": max_width, + "container_width": container_width, + "padding": padding, + } # --------------------------------------------------------------------------- @@ -410,6 +469,49 @@ def _cell_props(width: float | None) -> dict[str, Any]: return {"__width": width} +def _padding_overrides(padding: dict[str, str] | None) -> dict[str, Any]: + if padding is None: + return {} + return { + "containerPaddingTop": padding["top"], + "containerPaddingRight": padding["right"], + "containerPaddingBottom": padding["bottom"], + "containerPaddingLeft": padding["left"], + } + + +def _section_props(section_spec: dict[str, Any]) -> dict[str, Any]: + """The ``section_props`` hook for ``form_ui.build_content_tree``: an + overrides dict merged over form_ui's own Section defaults. ``None`` + values from ``SectionDef.container_width``/``padding`` mean "no + override" — the containerWidth key stays absent and padding stays + form_ui's hardcoded uniform ``"10"``, exactly matching this emitter's + output before this hook existed.""" + overrides: dict[str, Any] = {} + if section_spec.get("max_width") is not None: + overrides["maxWidth"] = section_spec["max_width"] + if section_spec.get("container_width") is not None: + overrides["containerWidth"] = section_spec["container_width"] + overrides.update(_padding_overrides(section_spec.get("padding"))) + return overrides + + +def _row_props(row_spec: dict[str, Any]) -> dict[str, Any]: + """The ``row_props`` hook for ``form_ui.build_content_tree`` — see + ``_section_props``. `Row` layout props are not uniform across the + reference template (`containerWidth`, padding, and `width` itself vary + row-to-row with no clean derivation from `Section.max_width`/padding), + so these are independent overrides, never computed from the parent + Section.""" + overrides: dict[str, Any] = {} + if row_spec.get("width") is not None: + overrides["width"] = row_spec["width"] + if row_spec.get("container_width") is not None: + overrides["containerWidth"] = row_spec["container_width"] + overrides.update(_padding_overrides(row_spec.get("padding"))) + return overrides + + def _assemble_email_block( block: dict[str, Any], parent_id: str, content: dict[str, Any] ) -> str: @@ -429,26 +531,33 @@ def _assemble_email_block( "linkedNodes": {}, } elif kind == "image": + image_props: dict[str, Any] = { + **_CONTAINER_DEFAULTS, + "size": "dynamic", + "unit": "pixel", + "height": None, + "width": block.get("width") or 150, + "display": "flex", + "position": "center", + "alt": block.get("alt", ""), + "link": block.get("link", ""), + "src": block["src"], + "name": block["name"], + "fileId": block["file_id"], + "naturalHeight": block.get("natural_height"), + "naturalWidth": block.get("natural_width"), + "dimension": "width", + } + if block.get("container_width") is not None: + image_props["containerWidth"] = block["container_width"] + if block.get("max_width") is not None: + image_props["maxWidth"] = block["max_width"] + if block.get("max_height") is not None: + image_props["maxHeight"] = block["max_height"] node = { "type": {"resolvedName": "Image"}, "isCanvas": False, - "props": { - **_CONTAINER_DEFAULTS, - "size": "dynamic", - "unit": "pixel", - "height": None, - "width": block.get("width") or 150, - "display": "flex", - "position": "center", - "alt": block.get("alt", ""), - "link": block.get("link", ""), - "src": block["src"], - "name": block["name"], - "fileId": block["file_id"], - "naturalHeight": block.get("natural_height"), - "naturalWidth": block.get("natural_width"), - "dimension": "width", - }, + "props": image_props, "displayName": "Image", "custom": {}, "parent": parent_id, @@ -469,13 +578,13 @@ def _assemble_email_block( "textColor": "rgba(255,255,255,1)", "fontSize": "16", "fontFamily": "Arial", - "alignment": "center", + "alignment": block.get("alignment") or "center", "borderSize": "0", "borderColor": "rgba(0,0,0,1)", - "borderRadius": "8", + "borderRadius": block.get("border_radius") or "8", "paddingTop": "10", - "paddingLeft": "20", - "paddingRight": "20", + "paddingLeft": block.get("padding_left") or "20", + "paddingRight": block.get("padding_right") or "20", "paddingBottom": "10", "textStyles": [], "openLinkInNewTab": True, @@ -493,7 +602,7 @@ def _assemble_email_block( "isCanvas": False, "props": { **_CONTAINER_DEFAULTS, - "size": "3", + "size": block.get("size") or "3", "color": block.get("color") or "rgba(78,193,145,1)", "width": "100", "alignment": "center", @@ -537,8 +646,10 @@ def _layout_for_columns(columns: list[float]) -> ColumnLayout: def _render_button(node: dict[str, Any]) -> str: p = node["props"] return ( - '
' + '
' "
str: def _render_image(node: dict[str, Any]) -> str: p = node["props"] + # `position: "center"` (the only value this surface ever sets, hardcoded + # in `_assemble_email_block` — see `models.spec.email_templates`'s + # `ImageBlockDef` docstring) means "center this block-level, fixed-width + # image within its cell" — the standard `margin:0 auto` technique. Read + # from props rather than hardcoded here so the render side can't drift + # from craft_json's own value if a future spec ever makes this settable. + align_style = "margin:0 auto;" if p.get("position") == "center" else "" img = ( f'{escape(p.get(' ) @@ -596,20 +714,83 @@ def _render_cell(cell_id: str, craft_json: dict[str, Any]) -> str: return "".join(_render_block(craft_json[bid]) for bid in node["nodes"]) +def _truncate4(value: float) -> float: + """Truncate (not round) to 4 decimal places — matches the live-confirmed + column-width convention (`293.3333`, not `293.3333333333333` or the + rounded `293.3334`; `586.6666`, not the rounded `586.6667`).""" + return math.floor(value * 10000) / 10000 + + +def _row_content_width_px(row_id: str, craft_json: dict[str, Any]) -> float: + """The row's actual compiled pixel width — the value the mso table and + the `.section- { max-width:...px; }` rule must use. + + Trusts an explicit `Row.props.containerWidth` when the spec set one + directly (BCLI-024's independent, spec-settable field — the same + "explicit, not derived" trust model this surface already uses for + `Row.props.columns`/`Cell.props.__width`). Otherwise derives it from the + parent `Section`'s own `maxWidth` and padding: content width = maxWidth + - (paddingLeft + paddingRight) — the formula this module's `content + width` comment already stated, now actually applied instead of frozen + as one hardcoded constant. With every prop at its pre-BCLI-024 default + (`Section.maxWidth: "900"`, padding `"10"` each side), this reduces to + exactly `900 - 10 - 10 = 880.0`, the old hardcoded value — so unmodified + specs compile identically to before. + + Either way, the result is then scaled by `Row.props.width` (percent, + default `"100"`) — Kizen's real compiler applies this as a genuine + multiplier on top of `containerWidth`/the derived width, confirmed + against the reference template (`width: '75'` on a `containerWidth: + '580'` row compiles to `435px`, not `580px`). A default `"100"` makes + this a no-op. + """ + row_props = craft_json[row_id]["props"] + if "containerWidth" in row_props: + base_width = float(row_props["containerWidth"]) + else: + section_props = craft_json[craft_json[row_id]["parent"]]["props"] + max_width = float(section_props["maxWidth"]) + pad_left = float(section_props.get("containerPaddingLeft", 0)) + pad_right = float(section_props.get("containerPaddingRight", 0)) + base_width = max_width - pad_left - pad_right + return base_width * float(row_props.get("width", 100)) / 100 + + +def _padding_css(props: dict[str, Any]) -> str: + """CSS `padding` shorthand (top/right/bottom/left, no unit suffix on the + value) from a node's own `containerPadding{Top,Right,Bottom,Left}` + props — the same order `_render_button` already uses for its own + padding. Reads whatever `Section`/`Row` actually carries in + `craft_json`, default `"10"` or an explicit spec override alike, so + `content` cannot silently disagree with `craft_json` the way it did + before this fix (Section/Row padding never reached the compiled output + at all).""" + return ( + f"{props.get('containerPaddingTop', '0')}px " + f"{props.get('containerPaddingRight', '0')}px " + f"{props.get('containerPaddingBottom', '0')}px " + f"{props.get('containerPaddingLeft', '0')}px" + ) + + def _render_row(row_id: str, craft_json: dict[str, Any]) -> tuple[str, str]: """Return (body_html, style_rule) for one Row.""" node = craft_json[row_id] columns = node["props"]["columns"] layout = _layout_for_columns(columns) cell_ids = [node["linkedNodes"][f"column-{i + 1}"] for i in range(len(columns))] + content_width_px = _row_content_width_px(row_id, craft_json) + mso_widths_px = [_truncate4(content_width_px * frac) for frac in layout.columns] - parts = [f'
'] + parts = [ + f'
' + ] parts.append( '") parts.append("
") - style_rule = f".section-{row_id} {{ max-width:{CONTENT_WIDTH_PX}px; }}" + style_rule = f".section-{row_id} {{ max-width:{content_width_px}px; }}" return "".join(parts), style_rule @@ -645,7 +826,12 @@ def _render_section( row_html, row_style = _render_row(row_id, craft_json) rows_html.append(row_html) style_rules.append(row_style) - body = f'
' + "".join(rows_html) + "
" + section_padding = _padding_css(node["props"]) + body = ( + f'
' + + "".join(rows_html) + + "
" + ) return body, style_rules @@ -744,6 +930,8 @@ def build_email_content(sections: list[dict[str, Any]]) -> tuple[dict[str, Any], root_props=EMAIL_ROOT_PROPS, cell_props=_cell_props, block_assembler=_assemble_email_block, + section_props=_section_props, + row_props=_row_props, ) content = _compile_html(craft_json) return craft_json, content @@ -783,23 +971,48 @@ def assemble_sections(resolved_sections: list[dict[str, Any]]) -> list[dict[str, width=b.get("width"), natural_width=b.get("natural_width"), natural_height=b.get("natural_height"), + container_width=b.get("container_width"), + max_width=b.get("max_width"), + max_height=b.get("max_height"), ) ) elif kind == "button": blocks.append( - button_block(b["label"], b["url"], color=b.get("color")) + button_block( + b["label"], + b["url"], + color=b.get("color"), + border_radius=b.get("border_radius"), + padding_left=b.get("padding_left"), + padding_right=b.get("padding_right"), + alignment=b.get("alignment"), + ) ) elif kind == "divider": - blocks.append(divider_block(b.get("color"))) + blocks.append(divider_block(b.get("color"), size=b.get("size"))) else: raise ValueError( f"unsupported block kind: {kind!r}. Supported: " f"{', '.join(known_block_kinds())}" ) cells.append(cell(blocks)) - rows.append(row(cells, layout=r["layout"])) + rows.append( + row( + cells, + layout=r["layout"], + width=r.get("width"), + container_width=r.get("container_width"), + padding=r.get("padding"), + ) + ) sections.append( - section(rows, background_color=s.get("background_color", "#FFFFFF")) + section( + rows, + background_color=s.get("background_color", "#FFFFFF"), + max_width=s.get("max_width"), + container_width=s.get("container_width"), + padding=s.get("padding"), + ) ) return sections @@ -817,6 +1030,10 @@ def assemble_sections(resolved_sections: list[dict[str, Any]]) -> list[dict[str, OFFLINE_HOST_PLACEHOLDER = "" +def _padding_dict(padding: PaddingDef | None) -> dict[str, str] | None: + return padding.model_dump() if padding is not None else None + + def _walk_blocks( spec: EmailTemplateDef, resolve_image: Callable[[ImageBlockDef], dict[str, Any]], @@ -838,17 +1055,47 @@ def _walk_blocks( "label": b.label, "url": b.url, "color": b.color, + "border_radius": b.border_radius, + "padding_left": b.padding_left, + "padding_right": b.padding_right, + "alignment": b.alignment, } ) elif isinstance(b, DividerBlockDef): - blocks.append({"kind": "divider", "color": b.color}) + blocks.append( + {"kind": "divider", "color": b.color, "size": b.size} + ) elif isinstance(b, ImageBlockDef): - blocks.append({"kind": "image", **resolve_image(b)}) + blocks.append( + { + "kind": "image", + **resolve_image(b), + "container_width": b.container_width, + "max_width": b.max_width, + "max_height": b.max_height, + } + ) else: # pragma: no cover - the discriminated union rejects this raise ValueError(f"unsupported block: {b!r}") cells.append({"blocks": blocks}) - rows.append({"layout": r.layout, "cells": cells}) - sections.append({"rows": rows, "background_color": s.background_color}) + rows.append( + { + "layout": r.layout, + "cells": cells, + "width": r.width, + "container_width": r.container_width, + "padding": _padding_dict(r.padding), + } + ) + sections.append( + { + "rows": rows, + "background_color": s.background_color, + "max_width": s.max_width, + "container_width": s.container_width, + "padding": _padding_dict(s.padding), + } + ) return sections diff --git a/src/kizen_builder/tools/form_ui.py b/src/kizen_builder/tools/form_ui.py index ebaf35a..68df1ed 100644 --- a/src/kizen_builder/tools/form_ui.py +++ b/src/kizen_builder/tools/form_ui.py @@ -535,6 +535,7 @@ def _assemble_row( *, cell_props: Callable[[float | None], dict[str, Any]] | None = None, block_assembler: Callable[[dict[str, Any], str, dict[str, Any]], str] | None = None, + row_props: Callable[[dict[str, Any]], dict[str, Any]] | None = None, ) -> str: row_id = _new_id() cells = row_spec["cells"] @@ -551,16 +552,19 @@ def _assemble_row( ) for i, c in enumerate(cells) ] + props = { + "columns": columns, + **_CONTAINER_DEFAULTS, + "maxWidth": "900", + "width": "100", + "alignment": "center", + } + if row_props: + props.update(row_props(row_spec)) content[row_id] = { "type": {"resolvedName": "Row"}, "isCanvas": False, - "props": { - "columns": columns, - **_CONTAINER_DEFAULTS, - "maxWidth": "900", - "width": "100", - "alignment": "center", - }, + "props": props, "displayName": "Row", "custom": {}, "parent": parent_id, @@ -578,6 +582,8 @@ def _assemble_section( *, cell_props: Callable[[float | None], dict[str, Any]] | None = None, block_assembler: Callable[[dict[str, Any], str, dict[str, Any]], str] | None = None, + section_props: Callable[[dict[str, Any]], dict[str, Any]] | None = None, + row_props: Callable[[dict[str, Any]], dict[str, Any]] | None = None, ) -> str: section_id = _new_id() row_ids = [ @@ -587,19 +593,23 @@ def _assemble_section( content, cell_props=cell_props, block_assembler=block_assembler, + row_props=row_props, ) for r in section_spec["rows"] ] + props = { + **_CONTAINER_DEFAULTS, + "containerBackgroundColor": section_spec.get("background_color", "#FFFFFF"), + "maxWidth": "900", + "width": "100", + "alignment": "center", + } + if section_props: + props.update(section_props(section_spec)) content[section_id] = { "type": {"resolvedName": "Section"}, "isCanvas": True, - "props": { - **_CONTAINER_DEFAULTS, - "containerBackgroundColor": section_spec.get("background_color", "#FFFFFF"), - "maxWidth": "900", - "width": "100", - "alignment": "center", - }, + "props": props, "displayName": "Section", "custom": {}, "parent": parent_id, @@ -616,6 +626,8 @@ def build_content_tree( root_props: dict[str, Any] | None = None, cell_props: Callable[[float | None], dict[str, Any]] | None = None, block_assembler: Callable[[dict[str, Any], str, dict[str, Any]], str] | None = None, + section_props: Callable[[dict[str, Any]], dict[str, Any]] | None = None, + row_props: Callable[[dict[str, Any]], dict[str, Any]] | None = None, ) -> dict[str, Any]: """Assemble a ``Root`` → ``Section`` → ``Row`` → ``Cell`` → block camelCase craft.js tree from :func:`section`/:func:`row`/:func:`cell`/ @@ -639,13 +651,22 @@ def build_content_tree( ``Cell.props`` or leaf-block prop shapes differ from this module's forms defaults — see ``tools.email_craft``, which needs ``Cell.props`` to carry ``{"__width": }`` and its own Button/Divider prop construction. - Both default to ``None``, which reproduces today's exact output — the + ``section_props``/``row_props`` are the same shape of hook for + ``Section``/``Row`` props — each takes the section/row spec dict and + returns a props-override dict merged over this module's defaults. All + four default to ``None``, which reproduces today's exact output — the call sites in this file and in ``tools/layouts.py`` are unaffected. """ content: dict[str, Any] = {} section_ids = [ _assemble_section( - s, "ROOT", content, cell_props=cell_props, block_assembler=block_assembler + s, + "ROOT", + content, + cell_props=cell_props, + block_assembler=block_assembler, + section_props=section_props, + row_props=row_props, ) for s in sections ] diff --git a/tests/test_cli_email_templates.py b/tests/test_cli_email_templates.py index 0298b55..8746dcd 100644 --- a/tests/test_cli_email_templates.py +++ b/tests/test_cli_email_templates.py @@ -187,6 +187,84 @@ def test_create_dry_run_with_an_image_block_uploads_nothing(tmp_path): assert image_node["props"]["fileId"] == ec.OFFLINE_FILE_PLACEHOLDER +@respx.mock +def test_craft_config_reflects_layout_props_from_the_spec(tmp_path): + """BCLI-024's acceptance criterion: `craft-config` reflects every new + prop in its output, exercised through the actual CLI command — not + just the model accepting the field.""" + spec = { + "name": "Newsletter", + "sections": [ + { + "max_width": "600", + "container_width": "900", + "padding": {"top": "0", "right": "0", "bottom": "0", "left": "0"}, + "rows": [ + { + "layout": "1 Column", + "width": "75", + "container_width": "580", + "padding": { + "top": "10", + "right": "40", + "bottom": "10", + "left": "40", + }, + "cells": [ + { + "blocks": [ + { + "kind": "button", + "label": "Go", + "url": "https://x", + "border_radius": "20", + "padding_left": "30", + "padding_right": "30", + "alignment": "left", + }, + {"kind": "divider", "size": "1"}, + ] + } + ], + } + ], + } + ], + } + spec_file = tmp_path / "spec.json" + spec_file.write_text(json.dumps(spec)) + result = runner.invoke( + cli.app, + ["messages", "templates", "craft-config", "--spec-file", str(spec_file)], + ) + assert result.exit_code == 0, result.output + payload = json.loads(result.output) + craft_json = payload["craft_json"] + + def props_of(kind): + return next( + n["props"] + for n in craft_json.values() + if isinstance(n, dict) and n.get("type", {}).get("resolvedName") == kind + ) + + section_props = props_of("Section") + assert section_props["maxWidth"] == "600" + assert section_props["containerWidth"] == "900" + + row_props = props_of("Row") + assert row_props["width"] == "75" + assert row_props["containerWidth"] == "580" + assert row_props["containerPaddingRight"] == "40" + + button_props = props_of("Button") + assert button_props["borderRadius"] == "20" + assert button_props["alignment"] == "left" + + divider_props = props_of("Divider") + assert divider_props["size"] == "1" + + def test_create_has_no_craft_json_or_content_flag(): result = runner.invoke(cli.app, ["messages", "templates", "create", "--help"]) assert result.exit_code == 0 diff --git a/tests/test_email_craft.py b/tests/test_email_craft.py index 96a1c73..ca74182 100644 --- a/tests/test_email_craft.py +++ b/tests/test_email_craft.py @@ -194,7 +194,14 @@ def test_button_and_divider_compiled_markup_matches_a_real_captured_template(): is byte-exact against that template's actual compiled `content` for a Button/Divider node carrying the same props. The real template's own copy/URL/color never enter this repo (personal-data rule) — the values - below are synthetic, chosen to exercise the same code path.""" + below are synthetic, chosen to exercise the same code path. + + **Corrected 2026-08-26, same-day, in BCLI-024 review**: the original + assertion here was missing the button table's own `align="center"` and + `line-height:100%;` — both confirmed present in Kizen's real compile of + the same button by independent comparison, and both now load-bearing + for `Button.alignment` actually reaching `content` (see + `test_content_reflects_every_layout_prop_this_item_added` below).""" sections = [ ec.section( [ @@ -216,8 +223,9 @@ def test_button_and_divider_compiled_markup_matches_a_real_captured_template(): ] _craft_json, content = ec.build_email_content(sections) assert ( - '
' + "
`margin:0 auto` being a real fix (see + BCLI-024's "Third and fourth blocking defects") — exactly the "verified + once by hand, never pinned" pattern this item was built to catch + elsewhere. Verified 2026-08-26, read-only, against a real Kizen-authored + template on `cli-testing`: byte-exact for a plain, non-linked Image + node.""" + sections = [ + ec.section( + [ + ec.row( + [ + ec.cell( + [ + ec.image_block( + file_id="f1", + src="https://example.com/logo.png", + name="logo.png", + alt="Logo", + natural_width=200, + natural_height=100, + width=150, + ) + ] + ) + ], + layout="1 Column", + ) + ] + ) + ] + _craft_json, content = ec.build_email_content(sections) + assert ( + 'Logo' + ) in content + + +def test_render_image_non_centered_position_omits_margin_auto(): + """`Image.position` isn't spec-settable today — `_assemble_email_block` + hardcodes it to `"center"` (see `ImageBlockDef`'s docstring) — but + `_render_image` reads the value rather than assuming it, so a future + spec that makes `position` settable can't silently drift from what it + renders. There's no way to reach this branch through a spec today, so + it's exercised directly against `_render_image`.""" + node = { + "props": { + "src": "https://example.com/logo.png", + "alt": "Logo", + "width": 150, + "position": "left", + } + } + img = ec._render_image(node) + assert "margin:0 auto" not in img + assert ( + 'style="display:block;width:150px;max-width:100%;height:auto;border:0;"' in img + ) + + def test_exactly_one_tr_per_row_regardless_of_column_count(): sections = [ ec.section( @@ -460,6 +530,746 @@ def test_offline_resolve_spec_images_reads_local_dims_without_uploading(tmp_path # --------------------------------------------------------------------------- +def _props_of(craft_json: dict, kind: str) -> dict: + return next(n["props"] for n in craft_json.values() if _resolved_name(n) == kind) + + +def _all_props_of(craft_json: dict, kind: str) -> list[dict]: + return [n["props"] for n in craft_json.values() if _resolved_name(n) == kind] + + +# --------------------------------------------------------------------------- +# Layout props (BCLI-024) — actual emitted prop values and placement, not +# merely "the key exists somewhere". The regression test pins the +# all-defaults case byte-identical to the pre-this-item emitter's exact +# key set — see the model's own defaults for why (SectionDef.max_width is +# "900", not the reference's "600"). +# --------------------------------------------------------------------------- + + +def test_all_defaults_spec_is_byte_identical_to_the_pre_item_section_and_row_props(): + """Pins BCLI-024's own regression-safety acceptance criterion: a spec + that sets none of the new layout fields must reproduce the *exact* key + set `form_ui._assemble_section`/`_assemble_row` produced before this + item — in particular, no `containerWidth` key at all (it was never + written before this item added the field).""" + sections = [ + ec.section([ec.row([ec.cell([ec.text_block("

x

")])], layout="1 Column")]) + ] + craft_json, _content = ec.build_email_content(sections) + + section_props = _props_of(craft_json, "Section") + assert section_props["maxWidth"] == "900" + assert section_props["width"] == "100" + assert "containerWidth" not in section_props + assert section_props["containerPaddingTop"] == "10" + assert section_props["containerPaddingRight"] == "10" + assert section_props["containerPaddingBottom"] == "10" + assert section_props["containerPaddingLeft"] == "10" + + row_props = _props_of(craft_json, "Row") + assert row_props["maxWidth"] == "900" + assert row_props["width"] == "100" + assert "containerWidth" not in row_props + assert row_props["containerPaddingTop"] == "10" + assert row_props["containerPaddingRight"] == "10" + assert row_props["containerPaddingBottom"] == "10" + assert row_props["containerPaddingLeft"] == "10" + + +def test_section_layout_props_are_emitted_when_set_matching_the_reference_pattern(): + """Values chosen to match the pattern independently confirmed live + against the reference template's `Section` nodes (BCLI-024 Context): + `maxWidth: '600'`, `containerWidth: '900'`, uniform padding `10` (or + `0` on the one full-bleed section observed).""" + sections = [ + ec.section( + [ec.row([ec.cell([ec.text_block("

x

")])], layout="1 Column")], + max_width="600", + container_width="900", + padding={"top": "0", "right": "0", "bottom": "0", "left": "0"}, + ) + ] + craft_json, _content = ec.build_email_content(sections) + props = _props_of(craft_json, "Section") + assert props["maxWidth"] == "600" + assert props["containerWidth"] == "900" + assert props["containerPaddingTop"] == "0" + assert props["containerPaddingRight"] == "0" + assert props["containerPaddingBottom"] == "0" + assert props["containerPaddingLeft"] == "0" + + +def test_row_layout_props_are_emitted_when_set_including_asymmetric_padding(): + """Values chosen to match the reference's non-uniform rows (BCLI-024 + Context): one row at `width: '75'` (not `'100'`), one row with + asymmetric `containerPaddingLeft/Right: '40'` against `'10'` + top/bottom — `Row` props are independent, not derived from `Section`.""" + sections = [ + ec.section( + [ + ec.row( + [ec.cell([ec.text_block("

x

")])], + layout="1 Column", + width="75", + container_width="600", + padding={"top": "10", "right": "40", "bottom": "10", "left": "40"}, + ) + ] + ) + ] + craft_json, _content = ec.build_email_content(sections) + props = _props_of(craft_json, "Row") + assert props["width"] == "75" + assert props["containerWidth"] == "600" + assert props["containerPaddingTop"] == "10" + assert props["containerPaddingRight"] == "40" + assert props["containerPaddingBottom"] == "10" + assert props["containerPaddingLeft"] == "40" + + +def test_row_layout_props_are_independent_per_row_not_uniform_across_a_section(): + """The load-bearing design fact this item's Context established: `Row` + layout props don't follow a clean formula from the parent `Section`, so + two rows in the same section can carry different values.""" + sections = [ + ec.section( + [ + ec.row( + [ec.cell([ec.text_block("

a

")])], + layout="1 Column", + container_width="580", + ), + ec.row( + [ec.cell([ec.text_block("

b

")])], + layout="1 Column", + container_width="600", + ), + ] + ) + ] + craft_json, _content = ec.build_email_content(sections) + widths = sorted(p["containerWidth"] for p in _all_props_of(craft_json, "Row")) + assert widths == ["580", "600"] + + +# --------------------------------------------------------------------------- +# Compiled `content` must track the SAME width `craft_json` carries — a +# blocking defect found in review: `content`'s mso table/media widths were +# still pinned to a module-level 880px constant, never reacting to +# `Section.max_width`/`Row.container_width`/padding, so a spec with +# `max_width: "600"` produced a `craft_json` that renders at 600px in +# Kizen's builder and a `content` that still renders at 880px — the exact +# two-fields-disagree failure this whole surface exists to prevent +# (`craft_summary()` can't see it: node ids and text both still match). +# --------------------------------------------------------------------------- + + +def _row_style_max_width_px(content: str, row_id: str) -> str: + m = re.search(rf"\.section-{row_id} \{{ max-width:([0-9.]+)px; \}}", content) + assert m, f"no max-width rule found for row {row_id}" + return m.group(1) + + +def _mso_table_width_px(content: str, row_id: str) -> str: + # The row's own opening
is + # immediately followed by its mso table; scope the search to that row's + # own fragment so a multi-row template can't match a sibling row's + # table. Matches on the class prefix only (not a full tag), since the + # div also carries a `style="padding:...;"` attribute. + start = content.index(f'
str: + m = re.search(rf'
str: + m = re.search( + rf'
x

")])], layout="1 Column")], + max_width="600", + ) + ] + craft_json, content = ec.build_email_content(sections) + row_id = next(nid for nid, n in craft_json.items() if _resolved_name(n) == "Row") + assert _row_style_max_width_px(content, row_id) == "580.0" + assert _mso_table_width_px(content, row_id) == "580.0" + + +def test_compiled_content_row_width_tracks_section_max_width_with_zero_padding(): + """Same section `max_width`, but full-bleed (`padding: 0/0/0/0`) — must + compile to 600px, not 580px and not 880px.""" + sections = [ + ec.section( + [ec.row([ec.cell([ec.text_block("

x

")])], layout="1 Column")], + max_width="600", + padding={"top": "0", "right": "0", "bottom": "0", "left": "0"}, + ) + ] + craft_json, content = ec.build_email_content(sections) + row_id = next(nid for nid, n in craft_json.items() if _resolved_name(n) == "Row") + assert _row_style_max_width_px(content, row_id) == "600.0" + assert _mso_table_width_px(content, row_id) == "600.0" + + +def test_compiled_content_row_width_defaults_to_880_matching_pre_bcli_024_output(): + """No overrides at all: must still compile to 880px — today's exact + pre-existing hardcoded value, now *derived* (900 Section maxWidth - 10 - + 10 padding) rather than frozen as a constant.""" + sections = [ + ec.section([ec.row([ec.cell([ec.text_block("

x

")])], layout="1 Column")]) + ] + craft_json, content = ec.build_email_content(sections) + row_id = next(nid for nid, n in craft_json.items() if _resolved_name(n) == "Row") + assert _row_style_max_width_px(content, row_id) == "880.0" + assert _mso_table_width_px(content, row_id) == "880.0" + + +def test_compiled_content_row_width_prefers_explicit_row_container_width_over_section(): + """An explicit `Row.container_width` is trusted directly, even when it + disagrees with what the Section-derived formula would produce — + matching BCLI-024's "independent, explicit fields" design, the same + trust model as `Row.props.columns`/`Cell.props.__width`.""" + sections = [ + ec.section( + [ + ec.row( + [ec.cell([ec.text_block("

x

")])], + layout="1 Column", + container_width="450", + ) + ], + max_width="600", + ) + ] + craft_json, content = ec.build_email_content(sections) + row_id = next(nid for nid, n in craft_json.items() if _resolved_name(n) == "Row") + assert craft_json[row_id]["props"]["containerWidth"] == "450" + assert _row_style_max_width_px(content, row_id) == "450.0" + assert _mso_table_width_px(content, row_id) == "450.0" + + +def test_compiled_content_two_rows_in_one_template_get_independent_widths(): + """Two sections with different `max_width`/padding in the SAME + template compile to two DIFFERENT row widths in `content` — proving + the width is computed per row, not held as one module-level value that + the last section computed would silently apply to every row.""" + sections = [ + ec.section( + [ec.row([ec.cell([ec.text_block("

a

")])], layout="1 Column")], + max_width="600", + ), + ec.section( + [ec.row([ec.cell([ec.text_block("

b

")])], layout="1 Column")], + max_width="600", + padding={"top": "0", "right": "0", "bottom": "0", "left": "0"}, + ), + ] + craft_json, content = ec.build_email_content(sections) + row_ids = [nid for nid, n in craft_json.items() if _resolved_name(n) == "Row"] + assert len(row_ids) == 2 + widths = sorted(_row_style_max_width_px(content, rid) for rid in row_ids) + assert widths == ["580.0", "600.0"] + + +def test_compiled_content_column_split_truncates_to_four_decimals_at_a_non_default_width(): + """The mso per-column pixel split must use the SAME truncate-to-4- + decimals convention the live-confirmed 880px defaults already used + (`293.3333`, not the rounded `293.3334` or the full-precision + `293.3333333333333`), generalized to a non-880 row width.""" + sections = [ + ec.section( + [ + ec.row( + [ + ec.cell([ec.text_block("

a

")]), + ec.cell([ec.text_block("

b

")]), + ], + layout="2 Columns (1/3 and 2/3)", + container_width="580", + ) + ] + ) + ] + _craft_json, content = ec.build_email_content(sections) + assert "width:193.3333px;" in content + assert "width:386.6666px;" in content + + +# --------------------------------------------------------------------------- +# Compiled `content` must also carry `Section`/`Row` padding — a second +# blocking defect found in review, same root cause as the width bug: +# `_render_row`/`_render_section` built their markup without ever reading +# `containerPadding{Top,Right,Bottom,Left}`, so `content` had NO padding +# declaration for `Section`/`Row` at all (text rendered flush against the +# canvas edge), regardless of what `craft_json` said. +# --------------------------------------------------------------------------- + + +def test_compiled_content_section_padding_matches_craft_json_when_set(): + sections = [ + ec.section( + [ec.row([ec.cell([ec.text_block("

x

")])], layout="1 Column")], + padding={"top": "0", "right": "0", "bottom": "0", "left": "0"}, + ) + ] + craft_json, content = ec.build_email_content(sections) + section_id = next( + nid for nid, n in craft_json.items() if _resolved_name(n) == "Section" + ) + assert _section_padding_css(content, section_id) == "0px 0px 0px 0px" + + +def test_compiled_content_row_padding_matches_craft_json_when_set(): + sections = [ + ec.section( + [ + ec.row( + [ec.cell([ec.text_block("

x

")])], + layout="1 Column", + padding={"top": "20", "right": "20", "bottom": "20", "left": "20"}, + ) + ] + ) + ] + craft_json, content = ec.build_email_content(sections) + row_id = next(nid for nid, n in craft_json.items() if _resolved_name(n) == "Row") + assert _row_padding_css(content, row_id) == "20px 20px 20px 20px" + + +def test_compiled_content_padding_defaults_to_uniform_10_matching_craft_json(): + """No overrides at all: `content` must still carry the SAME uniform + `'10'` padding `craft_json` has always defaulted to — this is not a new + field's default, it's a pre-existing prop that `content` simply never + rendered before this fix, on every section and row, not just ones that + explicitly set the new `padding` field.""" + sections = [ + ec.section([ec.row([ec.cell([ec.text_block("

x

")])], layout="1 Column")]) + ] + craft_json, content = ec.build_email_content(sections) + section_id = next( + nid for nid, n in craft_json.items() if _resolved_name(n) == "Section" + ) + row_id = next(nid for nid, n in craft_json.items() if _resolved_name(n) == "Row") + assert _section_padding_css(content, section_id) == "10px 10px 10px 10px" + assert _row_padding_css(content, row_id) == "10px 10px 10px 10px" + + +def test_compiled_content_padding_matches_craft_json_for_the_asymmetric_case(): + """The coordinator's own repro shape: left/right differ from top/bottom + (`20/30/20/30`, not a uniform value) — a fix that only handles the + uniform default would pass the two tests above and still fail this + one.""" + sections = [ + ec.section( + [ + ec.row( + [ec.cell([ec.text_block("

x

")])], + layout="1 Column", + padding={"top": "20", "right": "30", "bottom": "20", "left": "30"}, + ) + ], + padding={"top": "10", "right": "40", "bottom": "10", "left": "40"}, + ) + ] + craft_json, content = ec.build_email_content(sections) + section_id = next( + nid for nid, n in craft_json.items() if _resolved_name(n) == "Section" + ) + row_id = next(nid for nid, n in craft_json.items() if _resolved_name(n) == "Row") + assert _section_padding_css(content, section_id) == "10px 40px 10px 40px" + assert _row_padding_css(content, row_id) == "20px 30px 20px 30px" + + +def test_compiled_content_padding_is_independent_per_row_and_section(): + """Two sections, each with its own row, all four carrying different + padding — proving padding is read per node, not one value leaking + across the template (the same class of bug the width fix already + guarded against).""" + sections = [ + ec.section( + [ + ec.row( + [ec.cell([ec.text_block("

a

")])], + layout="1 Column", + padding={"top": "20", "right": "20", "bottom": "20", "left": "20"}, + ) + ], + padding={"top": "10", "right": "10", "bottom": "10", "left": "10"}, + ), + ec.section( + [ + ec.row( + [ec.cell([ec.text_block("

b

")])], + layout="1 Column", + padding={"top": "30", "right": "30", "bottom": "30", "left": "30"}, + ) + ], + padding={"top": "0", "right": "0", "bottom": "0", "left": "0"}, + ), + ] + craft_json, content = ec.build_email_content(sections) + section_ids = [ + nid for nid, n in craft_json.items() if _resolved_name(n) == "Section" + ] + row_ids = [nid for nid, n in craft_json.items() if _resolved_name(n) == "Row"] + section_paddings = sorted(_section_padding_css(content, sid) for sid in section_ids) + row_paddings = sorted(_row_padding_css(content, rid) for rid in row_ids) + assert section_paddings == ["0px 0px 0px 0px", "10px 10px 10px 10px"] + assert row_paddings == ["20px 20px 20px 20px", "30px 30px 30px 30px"] + + +def test_divider_size_is_emitted_when_set(): + sections = [ + ec.section([ec.row([ec.cell([ec.divider_block(size="1")])], layout="1 Column")]) + ] + craft_json, content = ec.build_email_content(sections) + props = _props_of(craft_json, "Divider") + assert props["size"] == "1" + assert "border-top:1px solid" in content + + +def test_divider_size_defaults_to_3_when_unset(): + sections = [ + ec.section([ec.row([ec.cell([ec.divider_block()])], layout="1 Column")]) + ] + craft_json, _content = ec.build_email_content(sections) + assert _props_of(craft_json, "Divider")["size"] == "3" + + +def test_button_layout_props_are_emitted_when_set_matching_the_reference_pattern(): + """Values chosen to match the pattern independently confirmed live + against the reference template's `Button` node (BCLI-024 Context): + `borderRadius: '20'`, `padding{Left,Right}: '30'`, `alignment: 'left'`.""" + sections = [ + ec.section( + [ + ec.row( + [ + ec.cell( + [ + ec.button_block( + "Go", + "https://example.com", + border_radius="20", + padding_left="30", + padding_right="30", + alignment="left", + ) + ] + ) + ], + layout="1 Column", + ) + ] + ) + ] + craft_json, content = ec.build_email_content(sections) + props = _props_of(craft_json, "Button") + assert props["borderRadius"] == "20" + assert props["paddingLeft"] == "30" + assert props["paddingRight"] == "30" + assert props["alignment"] == "left" + assert "border-radius:20px;" in content + assert "padding:10px 30px 10px 30px;" in content + + +def test_button_layout_props_default_to_todays_hardcoded_values_when_unset(): + sections = [ + ec.section( + [ + ec.row( + [ec.cell([ec.button_block("Go", "https://example.com")])], + layout="1 Column", + ) + ] + ) + ] + craft_json, _content = ec.build_email_content(sections) + props = _props_of(craft_json, "Button") + assert props["borderRadius"] == "8" + assert props["paddingLeft"] == "20" + assert props["paddingRight"] == "20" + assert props["alignment"] == "center" + + +def test_image_layout_props_are_emitted_only_when_set(): + sections = [ + ec.section( + [ + ec.row( + [ + ec.cell( + [ + ec.image_block( + file_id="f1", + src="https://host/api/files/f1/download", + name="logo.png", + container_width="580", + max_width="300", + max_height="200", + ) + ] + ) + ], + layout="1 Column", + ) + ] + ) + ] + craft_json, _content = ec.build_email_content(sections) + props = _props_of(craft_json, "Image") + assert props["containerWidth"] == "580" + assert props["maxWidth"] == "300" + assert props["maxHeight"] == "200" + + +def test_image_layout_props_are_absent_by_default_matching_todays_output(): + sections = [ + ec.section( + [ + ec.row( + [ + ec.cell( + [ + ec.image_block( + file_id="f1", + src="https://host/api/files/f1/download", + name="logo.png", + ) + ] + ) + ], + layout="1 Column", + ) + ] + ) + ] + craft_json, _content = ec.build_email_content(sections) + props = _props_of(craft_json, "Image") + assert "containerWidth" not in props + assert "maxWidth" not in props + assert "maxHeight" not in props + + +def test_spec_driven_layout_props_flow_end_to_end_from_email_template_def(): + """The full path this item wires: `EmailTemplateDef` -> `_walk_blocks` + -> `assemble_sections` -> `build_email_content`, for one section/row + carrying every new field at once.""" + spec = EmailTemplateDef.model_validate( + { + "name": "Newsletter", + "sections": [ + { + "max_width": "600", + "container_width": "900", + "padding": {"top": "0", "right": "0", "bottom": "0", "left": "0"}, + "rows": [ + { + "layout": "1 Column", + "width": "75", + "container_width": "580", + "padding": { + "top": "10", + "right": "40", + "bottom": "10", + "left": "40", + }, + "cells": [ + { + "blocks": [ + { + "kind": "button", + "label": "Go", + "url": "https://x", + "border_radius": "20", + "padding_left": "30", + "padding_right": "30", + "alignment": "left", + } + ] + } + ], + } + ], + } + ], + } + ) + resolved = ec.offline_resolve_spec_images(spec) + sections = ec.assemble_sections(resolved) + craft_json, _content = ec.build_email_content(sections) + + section_props = _props_of(craft_json, "Section") + assert section_props["maxWidth"] == "600" + assert section_props["containerWidth"] == "900" + + row_props = _props_of(craft_json, "Row") + assert row_props["width"] == "75" + assert row_props["containerWidth"] == "580" + assert row_props["containerPaddingRight"] == "40" + + button_props = _props_of(craft_json, "Button") + assert button_props["borderRadius"] == "20" + assert button_props["alignment"] == "left" + + +# --------------------------------------------------------------------------- +# Systematic content-coverage test — added in review after THREE separate +# craft_json-vs-content divergences were found by hand (width, then padding, +# then Button.alignment/Image.position): a new field landing correctly in +# craft_json and silently never reaching content is this item's recurring +# failure mode, and it was invisible to every acceptance test up to this +# point because the original reference-diff only ever compared craft_json. +# This test walks every field BCLI-024 added and asserts its value reaches +# content ON THE RIGHT NODE — not merely somewhere in the document — with a +# named, commented exemption for any field confirmed to be craft_json-only +# by design, never a silent pass. +# --------------------------------------------------------------------------- + + +def test_content_reflects_every_layout_prop_this_item_added(): + """One fixture, every new field set to a distinctive, individually + identifiable value, so a bug that reads the wrong node's prop (not just + "no prop at all") would also be caught.""" + + def resolved(n: dict) -> str: + t = n.get("type") + return t.get("resolvedName") if isinstance(t, dict) else t + + sections = [ + ec.section( + [ + # Row A: no explicit container_width, so Section.max_width's + # effect on the compiled row width is directly observable + # (isolated from Row B's own explicit override below). + ec.row( + [ + ec.cell( + [ + ec.divider_block(size="7"), + ec.button_block( + "Go", + "https://example.com", + border_radius="31", + padding_left="33", + padding_right="37", + alignment="right", + ), + ec.image_block( + file_id="f1", + src="https://host/x", + name="x.png", + container_width="401", + max_width="403", + max_height="407", + ), + ] + ) + ], + layout="1 Column", + width="77", # exempt, see below + padding={"top": "21", "right": "23", "bottom": "27", "left": "29"}, + ), + # Row B: explicit container_width, proving it's trusted + # directly rather than only ever derived from the Section. + ec.row( + [ec.cell([ec.text_block("

b

")])], + layout="1 Column", + container_width="271", + ), + ], + max_width="543", + container_width="919", # exempt, see below + padding={"top": "11", "right": "13", "bottom": "17", "left": "19"}, + ) + ] + craft_json, content = ec.build_email_content(sections) + + section_id = next(nid for nid, n in craft_json.items() if resolved(n) == "Section") + row_ids = [nid for nid, n in craft_json.items() if resolved(n) == "Row"] + row_a = next( + rid for rid in row_ids if craft_json[rid]["props"].get("width") == "77" + ) + row_b = next( + rid + for rid in row_ids + if craft_json[rid]["props"].get("containerWidth") == "271" + ) + image_id = next(nid for nid, n in craft_json.items() if resolved(n) == "Image") + + # --- SectionDef.max_width: INDIRECT — with no Row.container_width + # override, Row A's derived width is 543 - 19 - 13 = 511.0 (Section's + # own left/right padding, not Row A's). + # --- RowDef.width: DIRECT — a genuine multiplier on top of that derived + # width, confirmed against the reference template (a `width: '75'` row + # there compiles narrower than its `containerWidth`, not equal to it). + # 511.0 * 0.77 = 393.47. + assert _row_style_max_width_px(content, row_a) == "393.47" + + # --- SectionDef.padding: DIRECT, on the Section's own wrapper div. + assert _section_padding_css(content, section_id) == "11px 13px 17px 19px" + + # --- RowDef.container_width (Row B): DIRECT, trusted over the + # Section-derived formula Row A exercises above. + assert _row_style_max_width_px(content, row_b) == "271.0" + + # --- RowDef.padding (Row A): DIRECT, on the Row's own wrapper div — + # independent of Section's own padding, asserted above as a different + # value (21/23/27/29 vs. 11/13/17/19). + assert _row_padding_css(content, row_a) == "21px 23px 27px 29px" + + # --- DividerBlockDef.size: DIRECT, in the compiled border-top rule. + assert "border-top:7px" in content + + # --- ButtonBlockDef.border_radius/padding_left/padding_right/alignment: + # DIRECT, all on the Button's own compiled table/anchor markup. Scoped + # to this button's own fragment (not "somewhere in content") by + # locating its distinctive align="right". + button_start = content.index('` always uses the emitter's own `width`/fixed + # `max-width:100%` pair — these three new props have no consumer in + # `_render_image`, and this one *is* confirmed absent from the + # reference's compiled content too). + assert craft_json[section_id]["props"]["containerWidth"] == "919" + assert craft_json[row_a]["props"]["width"] == "77" + assert craft_json[image_id]["props"]["containerWidth"] == "401" + assert craft_json[image_id]["props"]["maxWidth"] == "403" + assert craft_json[image_id]["props"]["maxHeight"] == "407" + + def test_golden_output_for_a_small_synthetic_template(monkeypatch: pytest.MonkeyPatch): counter = iter(f"{i:024x}" for i in range(1, 50)) monkeypatch.setattr(form_ui, "_new_id", lambda: next(counter)) diff --git a/tests/test_email_template_spec.py b/tests/test_email_template_spec.py index 89dd41f..0d93144 100644 --- a/tests/test_email_template_spec.py +++ b/tests/test_email_template_spec.py @@ -11,7 +11,15 @@ import pytest from pydantic import ValidationError -from kizen_builder.models.spec.email_templates import EmailTemplateDef +from kizen_builder.models.spec.email_templates import ( + ButtonBlockDef, + DividerBlockDef, + EmailTemplateDef, + ImageBlockDef, + PaddingDef, + RowDef, + SectionDef, +) def _spec(**overrides): @@ -112,3 +120,114 @@ def test_grep_confirms_neither_craft_json_nor_content_is_a_model_field(): field_names = set(EmailTemplateDef.model_fields) assert "craft_json" not in field_names assert "content" not in field_names + + +# --------------------------------------------------------------------------- +# Layout props (BCLI-024) — defaults reproduce today's hardcoded emitter +# output; explicit values round-trip through the model unchanged. +# --------------------------------------------------------------------------- + + +def test_padding_def_defaults_to_uniform_10_on_all_four_sides(): + p = PaddingDef() + assert (p.top, p.right, p.bottom, p.left) == ("10", "10", "10", "10") + + +def test_padding_def_sides_are_independently_settable(): + p = PaddingDef.model_validate( + {"top": "10", "right": "40", "bottom": "10", "left": "40"} + ) + assert (p.top, p.right, p.bottom, p.left) == ("10", "40", "10", "40") + + +def test_section_def_layout_defaults_reproduce_todays_hardcoded_emitter_output(): + """`max_width` defaults to `900` (today's `form_ui._assemble_section` + hardcode), not the reference template's `600` — see this item's + Implementation notes for why the acceptance criteria's literal default + was corrected. `container_width`/`padding` default to `None`, meaning + "no override", matching that `containerWidth` is absent and padding is + the uniform `10` today.""" + s = SectionDef.model_validate({"rows": []}) + assert s.max_width == "900" + assert s.container_width is None + assert s.padding is None + + +def test_section_def_layout_props_are_independently_settable(): + s = SectionDef.model_validate( + { + "rows": [], + "max_width": "600", + "container_width": "900", + "padding": {"top": "0", "right": "0", "bottom": "0", "left": "0"}, + } + ) + assert s.max_width == "600" + assert s.container_width == "900" + assert s.padding == PaddingDef(top="0", right="0", bottom="0", left="0") + + +def test_row_def_layout_defaults_reproduce_todays_hardcoded_emitter_output(): + r = RowDef.model_validate({"cells": []}) + assert r.width == "100" + assert r.container_width is None + assert r.padding is None + + +def test_row_def_layout_props_are_independently_settable(): + """`Row.width`/`container_width`/`padding` are not derived from the + parent `Section` — the reference template shows them varying + row-to-row with no clean formula (BCLI-024 Context).""" + r = RowDef.model_validate( + { + "cells": [], + "width": "75", + "container_width": "580", + "padding": {"top": "10", "right": "40", "bottom": "10", "left": "40"}, + } + ) + assert r.width == "75" + assert r.container_width == "580" + assert r.padding == PaddingDef(top="10", right="40", bottom="10", left="40") + + +def test_divider_block_def_size_defaults_to_todays_hardcoded_3(): + d = DividerBlockDef() + assert d.size == "3" + + +def test_button_block_def_layout_props_default_to_todays_hardcoded_values(): + b = ButtonBlockDef(label="Go", url="https://x") + assert (b.border_radius, b.padding_left, b.padding_right, b.alignment) == ( + "8", + "20", + "20", + "center", + ) + + +@pytest.mark.parametrize("alignment", ["left", "center", "right"]) +def test_button_block_def_alignment_accepts_the_closed_enum(alignment): + b = ButtonBlockDef(label="Go", url="https://x", alignment=alignment) + assert b.alignment == alignment + + +def test_button_block_def_alignment_rejects_values_outside_the_closed_enum(): + with pytest.raises(ValidationError): + ButtonBlockDef(label="Go", url="https://x", alignment="justify") + + +def test_image_block_def_layout_props_default_to_none_matching_absent_keys_today(): + img = ImageBlockDef(file="/tmp/x.png") + assert img.container_width is None + assert img.max_width is None + assert img.max_height is None + + +def test_image_block_def_layout_props_are_independently_settable(): + img = ImageBlockDef( + file="/tmp/x.png", container_width="580", max_width="300", max_height="200" + ) + assert img.container_width == "580" + assert img.max_width == "300" + assert img.max_height == "200" From 7d3be1e5e8c7890d2a5bf21401a1fc72bbb8c6a4 Mon Sep 17 00:00:00 2001 From: Jeremy Bedient Date: Wed, 26 Aug 2026 13:10:08 -0400 Subject: [PATCH 3/4] Fix email content drift against Kizen's own compiled output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Compiled `content` (the HTML actually sent) now matches Kizen's own compiler on every point measured against a real captured reference template: text blocks carry a real `font-family` via the same `kizen-text-styles` wrapper Kizen uses (previously `content` had none at all, so any template without hand-inlined font styles rendered in the client's serif fallback); the `.moz-text-html` rule Gecko clients key column-stacking off; the MJML reset block; the mobile breakpoint now reads `Root.props.mobileBreak` (414, not the hardcoded 480); ``'s background colour, plus an outer background-table wrapper for `Section.container_width` (deferred here from the layout-props change); and `Image` blocks gain a genuine auto-sizing mode — omitting `width` now fills the parent Section's `containerWidth` instead of silently defaulting to 150px — with markup matching Kizen's own attribute/style set exactly. A float-formatting artifact that printed `880.0px`-style widths, including the mso `
` per-column widths, now prints `880px`. Auto-mode image sizing is a real default-behaviour change: an omitted `width` used to mean a fixed 150px image; it now means fill-to-container. Fixed-width images are unaffected. Deliberately left open: the outer Section wrapper skips the VML ``/`` fallback Kizen always emits alongside a solid background colour, not only for background images — this emitter has no background-image concept at all, so the risk deferred here is Outlook's Word rendering engine handling `background-color` unreliably in general, not just "no background-image support." That can't be closed by any offline check; it needs a real test send opened in Outlook desktop, a gap open since the first commit on this surface and still unmet. Linked-image markup is pinned by a new test but unverified against a real captured template — the one worked reference example has no link. The drift test's stale hardcoded 480px breakpoint is also fixed, but it's `@pytest.mark.drift` and deselected by default, so the fix itself hasn't been run. --- CHANGELOG.md | 27 + .../docs/specs/email-templates.md | 82 ++- .../models/spec/email_templates.py | 8 +- src/kizen_builder/tools/email_craft.py | 394 +++++++++++-- tests/drift/test_email_template_roundtrip.py | 13 +- tests/test_email_craft.py | 529 ++++++++++++++++-- 6 files changed, 918 insertions(+), 135 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ed2dd19..1c5b727 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -87,6 +87,33 @@ called out explicitly under **Changed** or **Removed**. ### Fixed +- **Compiled email `content` no longer diverges from what Kizen's own + builder produces for the same layout.** Every recipient's email now + carries a real `font-family` for body text (`Root.props.fontFamily`, + via the same `kizen-text-styles` wrapper class/`" +) + +# `.kizen-text-styles` — the class Kizen's own compiler uses to scope text +# typography and rich-text element styling (links, paragraphs, code/pre). +# The rule text itself carries no per-template data except `linkColor` +# (interpolated below), so it's kept as one structural block, confirmed +# against the reference template's compiled `content`. BCLI-023's text model +# (paragraph/list/code rendering) is untouched by this — these rules only +# apply Kizen's own styling to whatever HTML a `Text` block already embeds. +_KIZEN_TEXT_STYLES_TEMPLATE = ( + ".kizen-text-styles a {{ color: {link_color}; text-decoration: none; }}" + ".kizen-text-styles a *, .kizen-text-styles span * " + "{{ color: inherit; font-size: inherit; }}" + ".kizen-text-styles a:hover, .kizen-text-styles a:focus, " + ".kizen-text-styles a:hover *, .kizen-text-styles a:focus * " + "{{ text-decoration: underline; }}" + ".kizen-text-styles a:hover s, .kizen-text-styles a:focus s " + "{{ text-decoration: underline line-through; }}" + ".kizen-text-styles p {{ margin: 0; line-height: 1.5em; min-height: 1em; }}" + ".kizen-text-styles p * " + "{{ font-family: inherit; font-size: inherit; line-height: inherit; }}" + ".kizen-text-styles ul, .kizen-text-styles ol " + "{{ margin-top: 0; margin-bottom: 10px; }}" + ".kizen-text-styles code {{ font-family: 'Courier New', Monospace; " + "font-size: inherit; font-weight: 400; background-color: #F5F6F7; " + "padding: 5px; color: #4A5660; border-radius: 4px; }}" + ".kizen-text-styles pre {{ padding: 10px; background-color: #F5F6F7; " + "border-radius: 4px; border: 1px solid #D8DDE1; }}" + ".kizen-text-styles pre code {{ padding: 0; background-color: unset; " + "border-radius: 0; white-space: pre-wrap; word-break: break-all; }}" +) + + +def _section_class(node_id: str) -> str: + """`section-` — the existing Section/Row coupling class every + `_render_section`/`_render_row` call site uses. A tiny shared helper so + it and `_image_auto_class` below format node ids identically rather than + as two independently hand-rolled f-strings — see + `test_image_and_section_class_conventions_share_id_formatting`.""" + return f"section-{node_id}" + + +def _image_auto_class(node_id: str) -> str: + """`image--auto` — the auto-mode image sizing class, same + `-[-suffix]` shape as `_section_class` above. Confirmed + against the reference template's compiled `content` + (`.image--auto > table td { ... }`, one real occurrence + inspected structurally, node id not reproduced here).""" + return f"image-{node_id}-auto" + + # --------------------------------------------------------------------------- # v1 column presets — byte-exact, confirmed live 2026-08-25. Do not round or # recompute; see the work item's "Live probe findings". @@ -531,12 +625,20 @@ def _assemble_email_block( "linkedNodes": {}, } elif kind == "image": + # An omitted `width` used to collapse to a fixed `150` right here, + # before the node ever reached `craft_json` — so "omit width" never + # actually meant "auto mode", just a silent 150px default. Kizen's + # own auto mode (confirmed live against the reference: the one Image + # node with no `width` set at all) drops the `width` key entirely + # and sets `size: "auto"` instead of `"dynamic"` — both reproduced + # here. See `_render_image` for how `content` resolves the omitted + # width from the parent Section's `containerWidth`. + width = block.get("width") image_props: dict[str, Any] = { **_CONTAINER_DEFAULTS, - "size": "dynamic", + "size": "dynamic" if width is not None else "auto", "unit": "pixel", "height": None, - "width": block.get("width") or 150, "display": "flex", "position": "center", "alt": block.get("alt", ""), @@ -548,6 +650,8 @@ def _assemble_email_block( "naturalWidth": block.get("natural_width"), "dimension": "width", } + if width is not None: + image_props["width"] = width if block.get("container_width") is not None: image_props["containerWidth"] = block["container_width"] if block.get("max_width") is not None: @@ -672,46 +776,130 @@ def _render_divider(node: dict[str, Any]) -> str: ) -def _render_image(node: dict[str, Any]) -> str: +def _ancestor_section_props(node_id: str, craft_json: dict[str, Any]) -> dict[str, Any]: + """Walk a leaf block's fixed 3-hop ancestry — block -> Cell -> Row -> + Section (`_assemble_cell` always sets a block's own `parent` to the + enclosing Cell's id, per `tools.form_ui`) — and return the enclosing + `Section`'s own `props`. Used by auto-mode image sizing below to read + the Section's `containerWidth`.""" + cell_id = craft_json[node_id]["parent"] + row_id = craft_json[cell_id]["parent"] + section_id = craft_json[row_id]["parent"] + return craft_json[section_id]["props"] + + +def _render_image(node_id: str, craft_json: dict[str, Any]) -> tuple[str, list[str]]: + """Return `(html, extra_style_rules)`. Kizen wraps every Image block in + the same two-level table Kizen's own compiler uses (confirmed against + the reference's compiled `content` for both a fixed-width and an + auto-mode image) — an outer `` carrying block padding and, in auto + mode, the `.image--auto` coupling class, then a nested + `' in content + assert " { max-width:880px; }" in content + assert 'width:880px;">' in content # the 1-Column row's own mso
` around the `` itself. That inner + `` is what the auto-mode CSS rule's `> table td` selector actually + targets — the rule is meaningless without this wrapper, so the two are + implemented together, not the rule alone. + + Auto mode (`Image.props.width` absent — see `_assemble_email_block`): + the ``'s `width` attribute becomes the parent Section's own + `containerWidth`, falling back to `Root.props.maxWidth` when the + Section doesn't set one (inferred, not observed live — see the work + item's Open questions), and a `.image--auto > table td` rule + caps it at the image's own `naturalWidth`. + """ + node = craft_json[node_id] p = node["props"] - # `position: "center"` (the only value this surface ever sets, hardcoded - # in `_assemble_email_block` — see `models.spec.email_templates`'s - # `ImageBlockDef` docstring) means "center this block-level, fixed-width - # image within its cell" — the standard `margin:0 auto` technique. Read - # from props rather than hardcoded here so the render side can't drift - # from craft_json's own value if a future spec ever makes this settable. - align_style = "margin:0 auto;" if p.get("position") == "center" else "" + style_rules: list[str] = [] + explicit_width = p.get("width") + if explicit_width is not None: + img_width: Any = explicit_width + td_class = "" + else: + section_props = _ancestor_section_props(node_id, craft_json) + container_width = section_props.get("containerWidth") + if container_width is None: + container_width = craft_json["ROOT"]["props"]["maxWidth"] + img_width = container_width + td_class = _image_auto_class(node_id) + natural_width = p.get("naturalWidth") + if natural_width is not None: + style_rules.append( + f".{td_class} > table td {{ width: 100% !important; " + f"max-width: {natural_width}px; }}" + ) + img = ( - f'{escape(p.get(' + f'{escape(p.get(' + ) + wrapped = ( + '' + f'
' + '' + f'' + "
{img}
" + "
" ) link = p.get("link") if link: - return f'{img}' - return img + wrapped = f'{wrapped}' + return wrapped, style_rules + + +def _render_text(node: dict[str, Any], craft_json: dict[str, Any]) -> str: + """The wrapper Kizen's own compiler puts around every Text block's copy + — the `kizen-text-styles` class plus its typography, sourced from + `Root.props` (confirmed against the reference's compiled `content`): + `font-family`/`font-size`/`color` (rgba->hex converted) come from + `Root.props`; `line-height:1`/`text-align:left` are fixed literals with + no controlling Root prop in the reference. `custom.text` is still + embedded verbatim inside it — this never touches BCLI-023's text model, + only the wrapper *around* it.""" + root_props = craft_json["ROOT"]["props"] + font_family = root_props.get("fontFamily", "Arial") + font_size = root_props.get("fontSize", "14") + color = _rgba_to_hex(root_props.get("color", "rgba(74,86,96,1)")) + return ( + '
{node["custom"]["text"]}
' + ) -def _render_block(node: dict[str, Any]) -> str: +def _render_block(node_id: str, craft_json: dict[str, Any]) -> tuple[str, list[str]]: + node = craft_json[node_id] name = _resolved_name(node) if name == "Text": # Embedded verbatim, not stripped — see craft_summary()'s _plain_text, # which tag-strips both sides before comparing. - return f'
{node["custom"]["text"]}
' + return _render_text(node, craft_json), [] if name == "Image": - return _render_image(node) + return _render_image(node_id, craft_json) if name == "Button": - return _render_button(node) + return _render_button(node), [] if name == "Divider": - return _render_divider(node) + return _render_divider(node), [] raise ValueError(f"cannot compile HTML for unsupported node type {name!r}") -def _render_cell(cell_id: str, craft_json: dict[str, Any]) -> str: +def _render_cell(cell_id: str, craft_json: dict[str, Any]) -> tuple[str, list[str]]: node = craft_json[cell_id] - return "".join(_render_block(craft_json[bid]) for bid in node["nodes"]) + html_parts: list[str] = [] + style_rules: list[str] = [] + for block_id in node["nodes"]: + html, rules = _render_block(block_id, craft_json) + html_parts.append(html) + style_rules.extend(rules) + return "".join(html_parts), style_rules def _truncate4(value: float) -> float: @@ -773,23 +961,42 @@ def _padding_css(props: dict[str, Any]) -> str: ) -def _render_row(row_id: str, craft_json: dict[str, Any]) -> tuple[str, str]: - """Return (body_html, style_rule) for one Row.""" +def _fmt_px(value: float) -> str: + """Format a computed pixel length for the compiled CSS without a + spurious trailing `.0` (`880.0px` -> `880px`) while leaving a genuine + fractional value untouched (`293.3333px` stays `293.3333px`). Used at + `_render_row`'s width call sites: `_row_content_width_px`'s two sites + and the per-column `mso_widths_px` (`_truncate4`'s output).""" + if value == int(value): + return str(int(value)) + return str(value) + + +def _render_row(row_id: str, craft_json: dict[str, Any]) -> tuple[str, list[str]]: + """Return (body_html, style_rules) for one Row — `style_rules` is the + row's own `max-width` rule followed by any rules its cells' blocks + contributed (currently only an auto-mode Image's `.image--auto` + rule, see `_render_image`).""" node = craft_json[row_id] columns = node["props"]["columns"] layout = _layout_for_columns(columns) cell_ids = [node["linkedNodes"][f"column-{i + 1}"] for i in range(len(columns))] content_width_px = _row_content_width_px(row_id, craft_json) - mso_widths_px = [_truncate4(content_width_px * frac) for frac in layout.columns] + content_width_str = _fmt_px(content_width_px) + mso_widths_px = [ + _fmt_px(_truncate4(content_width_px * frac)) for frac in layout.columns + ] parts = [ - f'
' + f'
' ] parts.append( '") parts.append("
") - style_rule = f".section-{row_id} {{ max-width:{content_width_px}px; }}" - return "".join(parts), style_rule + style_rule = f".{_section_class(row_id)} {{ max-width:{content_width_str}px; }}" + return "".join(parts), [style_rule, *extra_style_rules] def _render_section( section_id: str, craft_json: dict[str, Any] ) -> tuple[str, list[str]]: node = craft_json[section_id] - bg = node["props"].get("containerBackgroundColor", "#FFFFFF") + props = node["props"] + bg = props.get("containerBackgroundColor", "#FFFFFF") rows_html: list[str] = [] - style_rules = [f".section-{section_id} {{ background-color:{bg}; }}"] + style_rules = [f".{_section_class(section_id)} {{ background-color:{bg}; }}"] for row_id in node["nodes"]: - row_html, row_style = _render_row(row_id, craft_json) + row_html, row_rules = _render_row(row_id, craft_json) rows_html.append(row_html) - style_rules.append(row_style) - section_padding = _padding_css(node["props"]) + style_rules.extend(row_rules) + section_padding = _padding_css(props) body = ( - f'
' + f'
' + "".join(rows_html) + "
" ) + + # The outer background-table wrapper `Section.props.containerWidth` + # needs, deferred here from BCLI-024 (see that item's Outcome). Only + # added when the spec set an explicit container_width — matching this + # surface's existing "None means no override, byte-identical to + # pre-this-prop output" convention (see `_section_props`), and the only + # case this module has real reference evidence for. Simplified relative + # to Kizen's own VML/background-image fallback markup (this emitter has + # no background-image concept at all, only `background_color`) — see the + # work item's report for the scoping call. + container_width = props.get("containerWidth") + if container_width is not None: + style_rules.append( + f".{_section_class(section_id)} {{ max-width:{container_width}px; }}" + ) + body = ( + '" + body + "" + ) return body, style_rules +def _distinct_column_widths(craft_json: dict[str, Any]) -> dict[str, str]: + """Every distinct `mj-column-per-N` class in use across this template's + `Row` nodes, mapped to its media-query width percentage. Shared by + `_column_base_width_rules`, `_media_query_rules`, and + `_moz_text_html_style_block` so the three rule sets can't independently + drift on which classes exist.""" + seen: dict[str, str] = {} + for node in craft_json.values(): + if _resolved_name(node) != "Row": + continue + columns = node["props"]["columns"] + layout = _layout_for_columns(columns) + for cls, media_w in zip(layout.classes, layout.media_widths, strict=True): + seen[cls] = media_w + return seen + + def _column_base_width_rules(craft_json: dict[str, Any]) -> list[str]: """Unconditional `.mj-column-per-N` width rules — MJML's own convention. @@ -845,36 +1093,42 @@ def _column_base_width_rules(craft_json: dict[str, Any]) -> list[str]: render side by side by default; `_media_query_rules` below is what collapses them back to full width on narrow viewports. """ - seen: dict[str, str] = {} - for node in craft_json.values(): - if _resolved_name(node) != "Row": - continue - columns = node["props"]["columns"] - layout = _layout_for_columns(columns) - for cls, media_w in zip(layout.classes, layout.media_widths, strict=True): - seen[cls] = media_w return [ f".{cls} {{ width:{w} !important; max-width:{w}; }}" - for cls, w in sorted(seen.items()) + for cls, w in sorted(_distinct_column_widths(craft_json).items()) ] def _media_query_rules(craft_json: dict[str, Any]) -> list[str]: - """`max-width:480px` rules that collapse every column to full width, so - a narrow viewport stacks instead of staying multi-column.""" - classes: set[str] = set() - for node in craft_json.values(): - if _resolved_name(node) != "Row": - continue - classes.update(_layout_for_columns(node["props"]["columns"]).classes) + """`max-width:px` rules that collapse every column to full + width, so a narrow viewport stacks instead of staying multi-column.""" return [ f".{cls} {{ width:100% !important; max-width:100%; }}" - for cls in sorted(classes) + for cls in sorted(_distinct_column_widths(craft_json)) ] +def _moz_text_html_style_block(craft_json: dict[str, Any], mobile_break: str) -> str: + """Gecko-based mail clients (Thunderbird and others) key column-stacking + behaviour off a `.moz-text-html`-prefixed selector rather than the plain + `.mj-column-per-N` rule `_column_base_width_rules` emits — its absence + is real and recipient-visible, scoped to that client family. Same + class/width pairs, wrapped in Kizen's own `min-width` media-attribute + convention (confirmed against the reference template's compiled + `content`) so it only applies above the same `mobileBreak` breakpoint.""" + widths = _distinct_column_widths(craft_json) + if not widths: + return "" + rules = "".join( + f".moz-text-html .{cls} {{ width:{w} !important; max-width:{w}; }} " + for cls, w in sorted(widths.items()) + ) + return f'' + + def _compile_html(craft_json: dict[str, Any]) -> str: root = craft_json["ROOT"] + root_props = root["props"] bodies: list[str] = [] style_rules: list[str] = [] for section_id in root["nodes"]: @@ -882,6 +1136,11 @@ def _compile_html(craft_json: dict[str, Any]) -> str: bodies.append(body) style_rules.extend(rules) + # Confirmed live 2026-08-26: `Root.props.mobileBreak` (`"414"` by + # default), not the hardcoded `480` this module used before. The `480` + # fallback below only applies if a caller's `craft_json` predates this + # prop entirely — every tree this module itself builds always has it. + mobile_break = str(root_props.get("mobileBreak", "480")) column_rules = _column_base_width_rules(craft_json) media_rules = _media_query_rules(craft_json) style_block = ( @@ -890,12 +1149,22 @@ def _compile_html(craft_json: dict[str, Any]) -> str: + "".join(style_rules) + "".join(column_rules) + ( - "@media only screen and (max-width:480px){" + "".join(media_rules) + "}" + f"@media only screen and (max-width:{mobile_break}px){{" + + "".join(media_rules) + + "}" if media_rules else "" ) + "" + + _moz_text_html_style_block(craft_json, mobile_break) + ) + link_color = _rgba_to_hex(root_props.get("linkColor", "rgba(82,142,249,1)")) + kizen_text_styles_block = ( + "" ) + body_bg = root_props.get("backgroundColor", "#F8FAFF") return ( "" ' str: '' '' '' - "" + style_block + "" - '' + "".join(bodies) + "" + "" + + style_block + + kizen_text_styles_block + + "" + f'' + f'
' + + "".join(bodies) + + "
" ) diff --git a/tests/drift/test_email_template_roundtrip.py b/tests/drift/test_email_template_roundtrip.py index 94c8eba..09be8b4 100644 --- a/tests/drift/test_email_template_roundtrip.py +++ b/tests/drift/test_email_template_roundtrip.py @@ -142,9 +142,16 @@ def test_create_template_from_spec_roundtrips_live( # The stored content's column-width rule must be a BASE rule, not only # inside the mobile @media query — confirms the fix against the real - # stored payload, not just the offline emitter. - style_start = live["content"].index('", 1)[0] + """(base_css, media_css) — split this module's own main `", start)] if "@media" not in style: return style, "" - base, media = style.split("@media only screen and (max-width:480px){", 1) + base, media = style.split("@media only screen and (max-width:", 1) + _breakpoint, media = media.split("px){", 1) # Strip exactly the one closing brace that ends the @media block itself # (not a rule's own closing brace). return base, media[:-1] if media.endswith("}") else media @@ -178,14 +185,28 @@ def test_one_column_and_two_column_compiled_markup_matches_live_probe(): _craft_json, content = ec.build_email_content(sections) base_css, media_css = _split_style_block(content) assert "mj-column-per-100" in content - assert "width:880.0px;" in content - # 2 Columns: one base rule + one media-collapse rule + one div per - # column = 4 occurrences of the class name. - assert content.count("mj-column-per-50") == 4 + # 880px, not 880.0px, at every width call site in `_render_row` — + # `_row_content_width_px`'s two sites and the per-column `mso_widths_px` + # (`_truncate4`'s output) alike. See the float-formatting fix (BCLI-025 + # item 7). + assert 'role="presentation" style="width:880px;">
+ assert "width:880.0px;" not in content + # 2 Columns: one base rule + one media-collapse rule + one + # `.moz-text-html`-prefixed rule (BCLI-025 item 2) + one div per column + # = 5 occurrences of the class name. + assert content.count("mj-column-per-50") == 5 assert ".mj-column-per-50 { width:50% !important; max-width:50%; }" in base_css assert ".mj-column-per-50 { width:100% !important; max-width:100%; }" in media_css assert "width:50%" not in media_css - assert content.count("width:440.0px;") == 2 + # The 2-Columns row's own two `content_width_px` call sites are unchanged + # from the 1-Column row above (same Section, so still 880/880px, not + # split). "440px" here is the per-column `mso_widths_px` (`_truncate4`, + # 0.5 * 880), once per column. + assert content.count("width:440px;") == 2 + assert "width:440.0px;" not in content + assert not re.search(r"\.0px", content) def test_button_and_divider_compiled_markup_matches_a_real_captured_template(): @@ -238,14 +259,17 @@ def test_button_and_divider_compiled_markup_matches_a_real_captured_template(): ) in content -def test_render_image_compiled_markup_matches_a_real_captured_template(): - """`_render_image` had zero automated coverage before this review round, - despite `position: "center"` -> `margin:0 auto` being a real fix (see - BCLI-024's "Third and fourth blocking defects") — exactly the "verified - once by hand, never pinned" pattern this item was built to catch - elsewhere. Verified 2026-08-26, read-only, against a real Kizen-authored - template on `cli-testing`: byte-exact for a plain, non-linked Image - node.""" +def test_render_image_fixed_width_compiled_markup_matches_the_reference_shape(): + """`_render_image`'s output shape, re-derived from the reference + template's real compiled `content` (read-only `GET`, 2026-08-26) — see + the work item's report for the full trace. Kizen wraps every Image in a + two-level table (`` carrying block padding, a nested + `
` around the ``), not a bare `` + tag as this emitter produced before BCLI-025 — the ``'s own + attributes/style are byte-exact against the reference; the wrapper is + asserted structurally (tag/attribute placement), not as one giant + pinned string, so a future formatting-only tweak to the wrapper doesn't + make this test as brittle as re-deriving the whole thing by hand.""" sections = [ ec.section( [ @@ -270,34 +294,185 @@ def test_render_image_compiled_markup_matches_a_real_captured_template(): ] ) ] - _craft_json, content = ec.build_email_content(sections) + craft_json, content = ec.build_email_content(sections) assert ( - 'Logo' + 'Logo' ) in content + assert "data-natural-width" not in content + assert "data-natural-height" not in content + image_id = next( + nid for nid, n in craft_json.items() if _resolved_name(n) == "Image" + ) + # Fixed mode: no `-auto` class, and no auto-mode CSS rule at all. + assert f'class="{ec._image_auto_class(image_id)}"' not in content + assert f".{ec._image_auto_class(image_id)} > table td" not in content + # The outer block-level carries the image's own containerPadding. + outer_td = re.search( + r'', + content, + ) + assert outer_td, "no block-level wrapper found around the Image" + assert outer_td.groups() == ("10", "10", "10", "10") + # The inner nested table's own carries the img's pixel width. + assert '' in content + + +def test_render_image_with_link_wraps_the_whole_table_in_an_anchor(): + """Pins current output shape; not verified against a real captured + template. The reference template's one worked Image example (the hero) + has no `link`, so this case has never been checked against Kizen's own + compiled `content` — this test only guards against `_render_image`'s + ``-wrapping silently changing shape, e.g. reverting to wrapping just + the bare `` instead of the whole two-level table.""" + sections = [ + ec.section( + [ + ec.row( + [ + ec.cell( + [ + ec.image_block( + file_id="f1", + src="https://example.com/logo.png", + name="logo.png", + alt="Logo", + link="https://example.com/landing", + natural_width=200, + natural_height=100, + width=150, + ) + ] + ) + ], + layout="1 Column", + ) + ] + ) + ] + _craft_json, content = ec.build_email_content(sections) + anchor_open = '' + assert anchor_open in content + start = content.index(anchor_open) + len(anchor_open) + # The anchor wraps the ENTIRE image table, not just the tag — + # everything between the anchor open and its matching close is the + # two-level table this fix is pinning, ending right where the 's + # own markup does. + assert content[start : start + len('", start) + len("/>") + assert content[ + img_end : img_end + len("
") + ] == ("
") + + +def test_render_image_auto_mode_uses_section_container_width_and_natural_width_rule(): + """Auto mode (`width` omitted in the spec): the ``'s `width` + attribute becomes the parent Section's own `containerWidth`, and a + `.image--auto > table td` rule caps it at the image's own + `naturalWidth` — both confirmed against the reference template's one + real auto-mode Image node (`containerWidth: 600`, `naturalWidth: 1200` + -> `width="600"` and `max-width: 1200px` in that exact rule shape).""" + sections = [ + ec.section( + [ + ec.row( + [ + ec.cell( + [ + ec.image_block( + file_id="f1", + src="https://example.com/hero.png", + name="hero.png", + alt="Hero", + natural_width=1200, + natural_height=630, + ) + ] + ) + ], + layout="1 Column", + container_width="600", + ) + ], + container_width="900", + ) + ] + craft_json, content = ec.build_email_content(sections) + image_id = next( + nid for nid, n in craft_json.items() if _resolved_name(n) == "Image" + ) + assert "width" not in craft_json[image_id]["props"] + assert craft_json[image_id]["props"]["size"] == "auto" -def test_render_image_non_centered_position_omits_margin_auto(): - """`Image.position` isn't spec-settable today — `_assemble_email_block` - hardcodes it to `"center"` (see `ImageBlockDef`'s docstring) — but - `_render_image` reads the value rather than assuming it, so a future - spec that makes `position` settable can't silently drift from what it - renders. There's no way to reach this branch through a spec today, so - it's exercised directly against `_render_image`.""" - node = { - "props": { - "src": "https://example.com/logo.png", - "alt": "Logo", - "width": 150, - "position": "left", - } - } - img = ec._render_image(node) - assert "margin:0 auto" not in img + auto_class = ec._image_auto_class(image_id) + assert f'class="{auto_class}"' in content assert ( - 'style="display:block;width:150px;max-width:100%;height:auto;border:0;"' in img + f".{auto_class} > table td {{ width: 100% !important; max-width: 1200px; }}" + in content + ) + # Auto width comes from the Row's parent SECTION's own containerWidth + # (900), not the Row's own containerWidth (600) and not the image's own + # naturalWidth (1200) — the fallback chain's first, most-specific rung. + assert ( + 'Hero' in content + + +def test_render_image_auto_mode_falls_back_to_root_max_width_when_section_unset(): + """When the parent Section has no explicit `container_width` at all + (the common case for an unmodified spec), auto mode falls back to + `Root.props.maxWidth`. Inferred, not observed live — the work item's + Open questions flags this as the fallback with the weakest evidence in + this item, since the reference template's one auto-mode image had an + explicit `Section.containerWidth` set.""" + sections = [ + ec.section( + [ + ec.row( + [ + ec.cell( + [ + ec.image_block( + file_id="f1", + src="https://example.com/hero.png", + name="hero.png", + natural_width=1200, + natural_height=630, + ) + ] + ) + ], + layout="1 Column", + ) + ] + ) + ] + craft_json, content = ec.build_email_content(sections) + section_id = next( + nid for nid, n in craft_json.items() if _resolved_name(n) == "Section" ) + assert "containerWidth" not in craft_json[section_id]["props"] + # EMAIL_ROOT_PROPS["maxWidth"] == "900". + assert 'width="900"' in content + assert '
' in content + + +def test_image_and_section_class_conventions_share_id_formatting(): + """The `.image--auto` and `.section-` conventions must + not drift apart from each other — pinned here, together, per the work + item's explicit constraint, rather than trusting two independently + hand-rolled f-strings to stay in sync. Both wrap the SAME node id in + the SAME `-[-suffix]` shape.""" + for node_id in ["abc123", "0" * 24, "a-node-with-dashes"]: + assert ec._section_class(node_id) == f"section-{node_id}" + assert ec._image_auto_class(node_id) == f"image-{node_id}-auto" def test_exactly_one_tr_per_row_regardless_of_column_count(): @@ -710,8 +885,8 @@ def test_compiled_content_row_width_tracks_section_max_width_with_default_paddin ] craft_json, content = ec.build_email_content(sections) row_id = next(nid for nid, n in craft_json.items() if _resolved_name(n) == "Row") - assert _row_style_max_width_px(content, row_id) == "580.0" - assert _mso_table_width_px(content, row_id) == "580.0" + assert _row_style_max_width_px(content, row_id) == "580" + assert _mso_table_width_px(content, row_id) == "580" def test_compiled_content_row_width_tracks_section_max_width_with_zero_padding(): @@ -726,8 +901,8 @@ def test_compiled_content_row_width_tracks_section_max_width_with_zero_padding() ] craft_json, content = ec.build_email_content(sections) row_id = next(nid for nid, n in craft_json.items() if _resolved_name(n) == "Row") - assert _row_style_max_width_px(content, row_id) == "600.0" - assert _mso_table_width_px(content, row_id) == "600.0" + assert _row_style_max_width_px(content, row_id) == "600" + assert _mso_table_width_px(content, row_id) == "600" def test_compiled_content_row_width_defaults_to_880_matching_pre_bcli_024_output(): @@ -739,8 +914,8 @@ def test_compiled_content_row_width_defaults_to_880_matching_pre_bcli_024_output ] craft_json, content = ec.build_email_content(sections) row_id = next(nid for nid, n in craft_json.items() if _resolved_name(n) == "Row") - assert _row_style_max_width_px(content, row_id) == "880.0" - assert _mso_table_width_px(content, row_id) == "880.0" + assert _row_style_max_width_px(content, row_id) == "880" + assert _mso_table_width_px(content, row_id) == "880" def test_compiled_content_row_width_prefers_explicit_row_container_width_over_section(): @@ -763,8 +938,8 @@ def test_compiled_content_row_width_prefers_explicit_row_container_width_over_se craft_json, content = ec.build_email_content(sections) row_id = next(nid for nid, n in craft_json.items() if _resolved_name(n) == "Row") assert craft_json[row_id]["props"]["containerWidth"] == "450" - assert _row_style_max_width_px(content, row_id) == "450.0" - assert _mso_table_width_px(content, row_id) == "450.0" + assert _row_style_max_width_px(content, row_id) == "450" + assert _mso_table_width_px(content, row_id) == "450" def test_compiled_content_two_rows_in_one_template_get_independent_widths(): @@ -787,7 +962,7 @@ def test_compiled_content_two_rows_in_one_template_get_independent_widths(): row_ids = [nid for nid, n in craft_json.items() if _resolved_name(n) == "Row"] assert len(row_ids) == 2 widths = sorted(_row_style_max_width_px(content, rid) for rid in row_ids) - assert widths == ["580.0", "600.0"] + assert widths == ["580", "600"] def test_compiled_content_column_split_truncates_to_four_decimals_at_a_non_default_width(): @@ -1230,7 +1405,7 @@ def resolved(n: dict) -> str: # --- RowDef.container_width (Row B): DIRECT, trusted over the # Section-derived formula Row A exercises above. - assert _row_style_max_width_px(content, row_b) == "271.0" + assert _row_style_max_width_px(content, row_b) == "271" # --- RowDef.padding (Row A): DIRECT, on the Row's own wrapper div — # independent of Section's own padding, asserted above as a different @@ -1251,25 +1426,265 @@ def resolved(n: dict) -> str: # left/right were added by ButtonBlockDef; order is top/right/bottom/left. assert "padding:10px 37px 10px 33px;" in button_fragment + # --- SectionDef.container_width: DIRECT as of BCLI-025 (was + # craft_json-only through BCLI-024 — see that item's Outcome). Kizen's + # real `content` applies it to an outer background-table wrapper; + # `_render_section` now builds a simplified version of that wrapper + # (mso-conditional table only, no VML background-image fallback, since + # this emitter has no background-image concept — see the work item's + # report for that scoping call). + assert 'width="919" style="width:919px;"' in content + assert f".{ec._section_class(section_id)} {{ max-width:919px; }}" in content + # --- Fields confirmed craft_json-only by checking Kizen's own compiled # output for the reference template (not merely "no consumer in # `_render_*`" — see the process note in BCLI-024's work item for why # that alone isn't sufficient): - # * SectionDef.container_width — deferred to BCLI-025, not exempt by - # design. Kizen's real content DOES apply it, to an outer - # background-table wrapper this emitter hasn't built yet. # * ImageBlockDef.container_width/max_width/max_height (content's # `` always uses the emitter's own `width`/fixed # `max-width:100%` pair — these three new props have no consumer in # `_render_image`, and this one *is* confirmed absent from the # reference's compiled content too). - assert craft_json[section_id]["props"]["containerWidth"] == "919" assert craft_json[row_a]["props"]["width"] == "77" assert craft_json[image_id]["props"]["containerWidth"] == "401" assert craft_json[image_id]["props"]["maxWidth"] == "403" assert craft_json[image_id]["props"]["maxHeight"] == "407" +# --------------------------------------------------------------------------- +# BCLI-025 — compiled `content` fidelity against Kizen's own compiler. +# Every fix below was checked against the reference template's REAL compiled +# `content` (read-only `GET`, `cli-testing`, 2026-08-26), not inferred from +# `craft_json` or from "no consumer exists in `_render_*`" — see that item's +# report for the trace. In binding priority order. +# --------------------------------------------------------------------------- + + +def test_font_family_reaches_every_text_blocks_wrapper(monkeypatch: pytest.MonkeyPatch): + """Divergence 1 (highest priority): compiled `content` carried NO + `font-family` for text at all before this fix — every recipient + rendered in the client's serif fallback. Fixed via the + `kizen-text-styles` wrapper div Kizen's own compiler uses (confirmed + structurally against the reference), sourced from `Root.props`, never + `TextBlockDef`/`_render_paragraphs` (BCLI-023's scope — untouched).""" + monkeypatch.setitem(ec.EMAIL_ROOT_PROPS, "fontFamily", "Georgia") + monkeypatch.setitem(ec.EMAIL_ROOT_PROPS, "color", "rgba(10,20,30,1)") + sections = [ + ec.section([ec.row([ec.cell([ec.text_block("

x

")])], layout="1 Column")]) + ] + _craft_json, content = ec.build_email_content(sections) + assert ( + '
' + "

x

" + ) in content + + +def test_font_family_default_matches_todays_arial_value(): + sections = [ + ec.section([ec.row([ec.cell([ec.text_block("

x

")])], layout="1 Column")]) + ] + _craft_json, content = ec.build_email_content(sections) + assert "font-family:Arial;font-size:14px;line-height:1;" in content + + +def test_rgba_to_hex_matches_the_two_conversions_confirmed_live(): + """`Root.props.color: rgba(74,86,96,1)` -> `#4a5660` and + `Root.props.linkColor: rgba(82,142,249,1)` -> `#528ef9` — both read + directly off the reference template's compiled `content`.""" + assert ec._rgba_to_hex("rgba(74,86,96,1)") == "#4a5660" + assert ec._rgba_to_hex("rgba(82,142,249,1)") == "#528ef9" + assert ec._rgba_to_hex("#FFFFFF") == "#FFFFFF" # passes through non-rgba unchanged + + +def test_moz_text_html_rule_exists_for_every_column_class_in_a_multi_column_layout(): + """Divergence 2: Gecko-based mail clients (Thunderbird and others) key + column-stacking behaviour off `.moz-text-html`-prefixed rules — real, + recipient-visible, but scoped to that client family. Parses the + compiled `', content + ) + assert moz_style, "no .moz-text-html style block found" + moz_css = moz_style.group(1) + assert ( + ".moz-text-html .mj-column-per-33-333332 " + "{ width:33.333332% !important; max-width:33.333332%; }" + ) in moz_css + assert ( + ".moz-text-html .mj-column-per-66-666664 " + "{ width:66.666664% !important; max-width:66.666664%; }" + ) in moz_css + + +def test_no_moz_text_html_style_block_when_template_has_no_rows(): + """`_moz_text_html_style_block` returns nothing when there are no `Row` + nodes to derive column classes from, rather than an empty/broken rule.""" + assert ec._moz_text_html_style_block({}, "414") == "" + + +def test_mjml_reset_block_is_present_and_byte_exact(): + """Divergence 3: the static MJML reset block (`#outlook a`, + `body{margin:0}`, `table,td{border-collapse}`, `img{...}`, + `p{display:block;margin:13px 0}`) — confirmed byte-exact against the + reference template's compiled `content`. No per-template data, so + byte-exact is the right bar here (same allowance the work item gives + the `.moz-text-html` rule's structural shape).""" + sections = [ + ec.section([ec.row([ec.cell([ec.text_block("

x

")])], layout="1 Column")]) + ] + _craft_json, content = ec.build_email_content(sections) + assert ( + '" + ) in content + + +def test_mobile_break_breakpoint_reads_root_props_not_hardcoded_480( + monkeypatch: pytest.MonkeyPatch, +): + """Divergence 4: the mobile-collapse media query's breakpoint must come + from `craft_json["ROOT"]["props"]["mobileBreak"]`, not a hardcoded + `480`. `mobileBreak` isn't spec-settable today (no `EmailTemplateDef` + field for it), so this monkeypatches `EMAIL_ROOT_PROPS` directly, per + the work item's own suggested approach for this test.""" + monkeypatch.setitem(ec.EMAIL_ROOT_PROPS, "mobileBreak", "600") + sections = [ + ec.section( + [ + ec.row( + [ec.cell([]), ec.cell([])], + layout="2 Columns", + ) + ] + ) + ] + _craft_json, content = ec.build_email_content(sections) + assert "@media only screen and (max-width:600px){" in content + assert "@media only screen and (max-width:480px)" not in content + # The .moz-text-html block's min-width also tracks the same breakpoint. + assert 'media="screen and (min-width:600px)"' in content + + +def test_mobile_break_default_is_414_not_the_old_hardcoded_480(): + sections = [ec.section([ec.row([ec.cell([]), ec.cell([])], layout="2 Columns")])] + _craft_json, content = ec.build_email_content(sections) + assert "@media only screen and (max-width:414px){" in content + assert "@media only screen and (max-width:480px)" not in content + + +def test_body_background_color_reads_root_props(monkeypatch: pytest.MonkeyPatch): + """Divergence 5 (body-background half): `Root.props.backgroundColor` + must reach ``'s inline style — confirmed against the reference + that it also reaches an outer wrapping `
`, not just ``.""" + monkeypatch.setitem(ec.EMAIL_ROOT_PROPS, "backgroundColor", "#112233") + sections = [ + ec.section([ec.row([ec.cell([ec.text_block("

x

")])], layout="1 Column")]) + ] + _craft_json, content = ec.build_email_content(sections) + assert '' in content + assert '
' in content + + +def test_body_background_color_default_matches_todays_value(): + sections = [ + ec.section([ec.row([ec.cell([ec.text_block("

x

")])], layout="1 Column")]) + ] + _craft_json, content = ec.build_email_content(sections) + assert '' in content + + +def test_section_container_width_gets_no_outer_wrapper_when_unset(): + """Byte-identical-defaults guarantee: a Section that doesn't set + `container_width` gets no outer mso wrapper table at all — matching + this surface's existing "None means no override" convention for every + other layout prop `_section_props` handles.""" + sections = [ + ec.section([ec.row([ec.cell([ec.text_block("

x

")])], layout="1 Column")]) + ] + craft_json, content = ec.build_email_content(sections) + section_id = next( + nid for nid, n in craft_json.items() if _resolved_name(n) == "Section" + ) + assert f'class="{ec._section_class(section_id)}"' in content + assert 'role="presentation" align="center" width=' not in content + + +def test_section_container_width_wrapper_carries_the_containerWidth_attribute(): + """Divergence 5 (outer wrapper half): a Section with an explicit + `container_width` gets an mso-conditional table hosting that value as + both a `width` attribute and an inline `width:...px` style — matching + Kizen's own reference-confirmed pattern of applying `containerWidth` to + an outer background-table wrapper (`_render_section`'s simplified + version of it — see the work item's report for the scoping call on how + far this emitter reproduces Kizen's full VML markup).""" + sections = [ + ec.section( + [ec.row([ec.cell([ec.text_block("

x

")])], layout="1 Column")], + container_width="900", + ) + ] + craft_json, content = ec.build_email_content(sections) + section_id = next( + nid for nid, n in craft_json.items() if _resolved_name(n) == "Section" + ) + assert ( + '" + f'
` width alike.""" + sections = [ + ec.section( + [ + ec.row( + [ec.cell([ec.text_block("

x

")])], + layout="1 Column", + container_width="700", + ) + ] + ) + ] + craft_json, content = ec.build_email_content(sections) + row_id = next(nid for nid, n in craft_json.items() if _resolved_name(n) == "Row") + assert _row_style_max_width_px(content, row_id) == "700" + assert _mso_table_width_px(content, row_id) == "700" + assert not re.search(r"\.0px", content) + + def test_golden_output_for_a_small_synthetic_template(monkeypatch: pytest.MonkeyPatch): counter = iter(f"{i:024x}" for i in range(1, 50)) monkeypatch.setattr(form_ui, "_new_id", lambda: next(counter)) From e9dd790a25ac03a23c1203225934d070434a832c Mon Sep 17 00:00:00 2001 From: Jeremy Bedient Date: Wed, 26 Aug 2026 13:46:51 -0400 Subject: [PATCH 4/4] Split email_craft.py into email_craft, email_html, and email_images MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pure code motion, no behaviour change: `email_craft.py` (1421 lines) becomes three modules — `email_craft.py` (craft-tree assembly, prop shapes, spec resolution, the public entry point), `email_html.py` (everything that compiles `content`, including `ColumnLayout`/ `COLUMN_LAYOUTS`), and `email_images.py` (upload + header-byte dimension reading). Done now, ahead of the structured-text model that lands on the same file next, so that change is one clean diff instead of a refactor tangled with new behaviour. `ColumnLayout`/`COLUMN_LAYOUTS` live in `email_html.py` even though `email_craft.py`'s `row()` also reads `.columns` off them: the reverse placement would create a cycle, since `email_craft.py` already needs `_compile_html` from `email_html.py` for `build_email_content`. `email_craft.py` imports `COLUMN_LAYOUTS`/`_compile_html`/ `upload_email_image`/`read_image_dimensions` back by name, so callers of `email_craft.COLUMN_LAYOUTS` etc. keep resolving with no re-export shim. `email_html.py` and `email_images.py` are genuine leaves — no import of `form_ui`, `email_craft`, or each other — so id minting stays exactly where it already was, `form_ui._new_id()` called from one place in `email_craft.py`. `_assemble_email_block`, the largest function in the file at 121 lines, is reduced to a thin dispatcher over four new per-kind helpers (`_assemble_text_block`/`_assemble_image_block`/`_assemble_button_block`/ `_assemble_divider_block`), each just the extracted branch body with no logic change. Gives the structured-text change a clean seam to add a fifth branch instead of growing the dispatcher past 150 lines under the pressure of also shipping new behaviour. Only `tests/test_email_craft.py` changes outside the split itself: four private compile-side symbols it reaches via `ec.` moved to `email_html.py`, so those 13 call sites now import and use `eh.` instead. No assertion changed. Verified with two independent passes: a before/after byte comparison of compiled `content` across all four block kinds and layout variants (identical once node ids are normalized out), and an AST-level diff of every top-level function/constant between the pre-split file and the union of the three post-split files, which found only the `_assemble_email_block` extraction and two docstring wording changes differ — everything else is byte-identical. Full suite matches baseline exactly: 1336 passed, 4 skipped, 77 deselected, lint/format/ typecheck/extra_checks all clean. --- src/kizen_builder/tools/email_craft.py | 998 ++++-------------------- src/kizen_builder/tools/email_html.py | 618 +++++++++++++++ src/kizen_builder/tools/email_images.py | 126 +++ tests/test_email_craft.py | 27 +- 4 files changed, 913 insertions(+), 856 deletions(-) create mode 100644 src/kizen_builder/tools/email_html.py create mode 100644 src/kizen_builder/tools/email_images.py diff --git a/src/kizen_builder/tools/email_craft.py b/src/kizen_builder/tools/email_craft.py index 83bb5cc..ceb9d51 100644 --- a/src/kizen_builder/tools/email_craft.py +++ b/src/kizen_builder/tools/email_craft.py @@ -13,20 +13,35 @@ template whose builder view and real output silently disagree — the exact failure this module exists to make impossible. -`build_email_content()` is the one entry point that upholds that invariant. -Everything else here is either structural reuse of `tools.form_ui` (the -`Root`/`Section`/`Row`/`Cell` assembly is identical topology, threaded -through the `cell_props`/`block_assembler` hooks added there for this -module) or email-specific: this surface's own `Text`/`Image`/`Button`/ -`Divider` prop shapes (email's `Button`/`Divider` props differ from the -forms surface's — see `docs/specs/email-templates.md`), the v1 column-preset -table (byte-exact `columns`/`__width` fractions and compiled-HTML markup, -confirmed live 2026-08-25), and image upload (`api/files.py::upload_file` -with `source="public_image"`, plus reading `naturalWidth`/`naturalHeight` -straight from the uploaded file's own header bytes). +`build_email_content()` is the one entry point that upholds that invariant: +it calls `tools.form_ui.build_content_tree` once (the single id-minting pass +for `Root`/`Section`/`Row`/`Cell` nodes, threaded through the +`cell_props`/`block_assembler`/`section_props`/`row_props` hooks added there +for this module) and then compiles `content` from that exact tree via +`email_html._compile_html` — never a second tree-walk that could mint ids of +its own. This module owns id minting end-to-end: `form_ui.build_content_tree` +for the container nodes, `_assemble_email_block` for leaf blocks below. +Split across three modules for size — `email_html.py` (compiles `content` +from an existing tree; never mints an id) and `email_images.py` (upload + +header-byte dimension reading) — but the one-pass invariant this docstring +describes is unchanged: neither of those modules imports `form_ui` or calls +`_new_id()`. + +This module keeps this surface's own `Text`/`Image`/`Button`/`Divider` prop +shapes (email's `Button`/`Divider` props differ from the forms surface's — +see `docs/specs/email-templates.md`), craft_json assembly, and spec +resolution. `upload_email_image`/`read_image_dimensions` are imported back +from `email_images.py` by name, so `email_craft.upload_email_image` etc. +keep resolving with no re-export shim. The v1 column-preset table +(`ColumnLayout`/`COLUMN_LAYOUTS`, byte-exact `columns`/`__width` fractions +and compiled-HTML markup, confirmed live 2026-08-25) lives in `email_html.py` +instead of here — `row()` below imports `COLUMN_LAYOUTS` back from there to +validate cell counts, since `email_html.py` can't import from this module +(this module already needs `_compile_html` from it, and a circular import +isn't an option). v1 scope only: `Text`, `Image`, `Button`, `Divider` leaf blocks, and the 4 -column presets in `COLUMN_LAYOUT` below (`1 Column`, `2 Columns`, `2 Columns +column presets in `COLUMN_LAYOUTS` (`1 Column`, `2 Columns`, `2 Columns (1/3 and 2/3)`, `2 Columns (2/3 and 1/3)`). `Attachments` and the other 5 presets (`3`/`4`/`5`/`6 Columns`, `3 Columns (gutters)`) are confirmed live but out of scope — anything using them fails loudly rather than silently @@ -36,14 +51,10 @@ from __future__ import annotations -import math -import re from collections.abc import Callable -from html import escape from pathlib import Path from typing import Any -from kizen_builder.api import files as files_api from kizen_builder.api.client import KizenClient from kizen_builder.config import load_env_config from kizen_builder.models.spec.email_templates import ( @@ -55,6 +66,8 @@ TextBlockDef, ) from kizen_builder.tools import form_ui +from kizen_builder.tools.email_html import COLUMN_LAYOUTS, _compile_html +from kizen_builder.tools.email_images import read_image_dimensions, upload_email_image # --------------------------------------------------------------------------- # Root/container prop shapes @@ -129,155 +142,11 @@ } # --------------------------------------------------------------------------- -# Compiled-HTML fidelity fixes (BCLI-025) — static blocks and small -# conversions shared by several `_render_*`/`_compile_html` call sites below. +# v1 column presets — the byte-exact layout table (`ColumnLayout`, +# `COLUMN_LAYOUTS`) lives in `email_html.py` and is imported above; this +# module keeps the out-of-scope guard and the public enumeration API. # --------------------------------------------------------------------------- -_RGBA_RE = re.compile(r"rgba?\(\s*(\d+)\s*,\s*(\d+)\s*,\s*(\d+)\s*(?:,\s*[\d.]+\s*)?\)") - - -def _rgba_to_hex(value: str) -> str: - """Convert an `rgba(r,g,b,a)`/`rgb(r,g,b)` colour string to the lowercase - `#rrggbb` hex Kizen's own compiler emits for the same value — confirmed - against the reference template's compiled `content`: `Root.props.color` - (`rgba(74,86,96,1)`) compiles to `#4a5660`, `Root.props.linkColor` - (`rgba(82,142,249,1)`) compiles to `#528ef9`. Alpha is dropped, matching - both observed conversions (both alpha `1`) — this surface has no - confirmed case of a translucent text/link colour reaching `content`. - Anything that isn't `rgba(...)`/`rgb(...)` passes through unchanged - (most `container*` colour props on this surface are already hex or a - literal like `"transparent"`, not every colour prop here uses the - rgba wire format).""" - m = _RGBA_RE.fullmatch(value.strip()) - if not m: - return value - r, g, b = (int(x) for x in m.groups()) - return f"#{r:02x}{g:02x}{b:02x}" - - -# The MJML boilerplate reset block — Outlook/webkit/Gecko normalization with -# no per-template data, confirmed byte-exact against the reference template's -# compiled `content` (read-only `GET`, 2026-08-26). Kept as one literal -# constant rather than built up piecewise, since every byte here is fixed. -_MJML_RESET_STYLE = ( - '" -) - -# `.kizen-text-styles` — the class Kizen's own compiler uses to scope text -# typography and rich-text element styling (links, paragraphs, code/pre). -# The rule text itself carries no per-template data except `linkColor` -# (interpolated below), so it's kept as one structural block, confirmed -# against the reference template's compiled `content`. BCLI-023's text model -# (paragraph/list/code rendering) is untouched by this — these rules only -# apply Kizen's own styling to whatever HTML a `Text` block already embeds. -_KIZEN_TEXT_STYLES_TEMPLATE = ( - ".kizen-text-styles a {{ color: {link_color}; text-decoration: none; }}" - ".kizen-text-styles a *, .kizen-text-styles span * " - "{{ color: inherit; font-size: inherit; }}" - ".kizen-text-styles a:hover, .kizen-text-styles a:focus, " - ".kizen-text-styles a:hover *, .kizen-text-styles a:focus * " - "{{ text-decoration: underline; }}" - ".kizen-text-styles a:hover s, .kizen-text-styles a:focus s " - "{{ text-decoration: underline line-through; }}" - ".kizen-text-styles p {{ margin: 0; line-height: 1.5em; min-height: 1em; }}" - ".kizen-text-styles p * " - "{{ font-family: inherit; font-size: inherit; line-height: inherit; }}" - ".kizen-text-styles ul, .kizen-text-styles ol " - "{{ margin-top: 0; margin-bottom: 10px; }}" - ".kizen-text-styles code {{ font-family: 'Courier New', Monospace; " - "font-size: inherit; font-weight: 400; background-color: #F5F6F7; " - "padding: 5px; color: #4A5660; border-radius: 4px; }}" - ".kizen-text-styles pre {{ padding: 10px; background-color: #F5F6F7; " - "border-radius: 4px; border: 1px solid #D8DDE1; }}" - ".kizen-text-styles pre code {{ padding: 0; background-color: unset; " - "border-radius: 0; white-space: pre-wrap; word-break: break-all; }}" -) - - -def _section_class(node_id: str) -> str: - """`section-` — the existing Section/Row coupling class every - `_render_section`/`_render_row` call site uses. A tiny shared helper so - it and `_image_auto_class` below format node ids identically rather than - as two independently hand-rolled f-strings — see - `test_image_and_section_class_conventions_share_id_formatting`.""" - return f"section-{node_id}" - - -def _image_auto_class(node_id: str) -> str: - """`image--auto` — the auto-mode image sizing class, same - `-[-suffix]` shape as `_section_class` above. Confirmed - against the reference template's compiled `content` - (`.image--auto > table td { ... }`, one real occurrence - inspected structurally, node id not reproduced here).""" - return f"image-{node_id}-auto" - - -# --------------------------------------------------------------------------- -# v1 column presets — byte-exact, confirmed live 2026-08-25. Do not round or -# recompute; see the work item's "Live probe findings". -# --------------------------------------------------------------------------- - - -class ColumnLayout: - __slots__ = ("preset", "columns", "classes", "media_widths") - - def __init__( - self, - preset: str, - columns: tuple[float, ...], - classes: tuple[str, ...], - media_widths: tuple[str, ...], - ) -> None: - self.preset = preset - self.columns = columns - self.classes = classes - self.media_widths = media_widths - - -# 880px was the content width in every case observed pre-BCLI-024 (900 -# Section maxWidth - 20px padding, both hardcoded at the time). Now that -# `Section.max_width`/`Row.container_width`/padding are spec-settable -# (BCLI-024), the actual per-row pixel width is computed by -# `_row_content_width_px` below, not held as one constant — see that -# function's docstring for the fallback formula, which reduces to exactly -# 880.0 when nothing is overridden. - -COLUMN_LAYOUTS: dict[str, ColumnLayout] = { - "1 Column": ColumnLayout( - "1 Column", - (1,), - ("mj-column-per-100",), - ("100%",), - ), - "2 Columns": ColumnLayout( - "2 Columns", - (0.5, 0.5), - ("mj-column-per-50", "mj-column-per-50"), - ("50%", "50%"), - ), - "2 Columns (1/3 and 2/3)": ColumnLayout( - "2 Columns (1/3 and 2/3)", - (0.3333333333333333, 0.6666666666666666), - ("mj-column-per-33-333332", "mj-column-per-66-666664"), - ("33.333332%", "66.666664%"), - ), - "2 Columns (2/3 and 1/3)": ColumnLayout( - "2 Columns (2/3 and 1/3)", - (0.6666666666666666, 0.3333333333333333), - ("mj-column-per-66-666664", "mj-column-per-33-333332"), - ("66.666664%", "33.333332%"), - ), -} - # The other 5 presets are pre-captured groundwork for a follow-on item, not # built here. Naming one is a clear, immediate error, not a silent skip. _OUT_OF_SCOPE_LAYOUTS = ( @@ -437,120 +306,6 @@ def section( } -# --------------------------------------------------------------------------- -# Image upload + header-byte pixel dimensions -# --------------------------------------------------------------------------- - - -def _png_dimensions(data: bytes) -> tuple[int, int]: - # Signature (8 bytes) + IHDR chunk: length(4) type(4) width(4) height(4). - if len(data) < 24 or data[:8] != b"\x89PNG\r\n\x1a\n": - raise ValueError("not a valid PNG (bad signature)") - width = int.from_bytes(data[16:20], "big") - height = int.from_bytes(data[20:24], "big") - return width, height - - -# JPEG SOF (start-of-frame) markers that carry dimensions. Excludes DHT -# (0xC4), JPG (0xC8), DAC (0xCC) — same-range bytes that are NOT SOF markers. -_JPEG_SOF_MARKERS = frozenset( - {0xC0, 0xC1, 0xC2, 0xC3, 0xC5, 0xC6, 0xC7, 0xC9, 0xCA, 0xCB, 0xCD, 0xCE, 0xCF} -) -# Markers with no following length/payload — skip straight past them. -_JPEG_STANDALONE_MARKERS = frozenset({0x01, 0xD8, 0xD9} | set(range(0xD0, 0xD8))) - - -def _jpeg_dimensions(data: bytes) -> tuple[int, int]: - if len(data) < 4 or data[0:2] != b"\xff\xd8": - raise ValueError("not a valid JPEG (bad SOI marker)") - pos = 2 - n = len(data) - while pos < n - 1: - if data[pos] != 0xFF: - raise ValueError("malformed JPEG: expected a marker") - marker = data[pos + 1] - pos += 2 - while marker == 0xFF and pos < n: # padding fill bytes between markers - marker = data[pos] - pos += 1 - if marker in _JPEG_STANDALONE_MARKERS: - continue - if pos + 2 > n: - break - seg_len = int.from_bytes(data[pos : pos + 2], "big") - if marker in _JPEG_SOF_MARKERS: - if pos + 7 > n: - break - height = int.from_bytes(data[pos + 3 : pos + 5], "big") - width = int.from_bytes(data[pos + 5 : pos + 7], "big") - return width, height - pos += seg_len - raise ValueError("no SOF0/SOF2 segment found in JPEG") - - -def read_image_dimensions(data: bytes) -> tuple[int, int, str]: - """Return ``(width, height, content_type)`` read from the file's own - header bytes. PNG and JPEG only — both are real cases on this surface - (every image already stored in the target environment is PNG, but the - browser trace that settled the ``source`` question was a JPEG upload). - GIF/WebP/SVG fail loudly as unsupported rather than being silently - mis-parsed; SVG especially has no pixel dimensions to read this way at - all. - """ - if data[:8] == b"\x89PNG\r\n\x1a\n": - w, h = _png_dimensions(data) - return w, h, "image/png" - if data[:3] == b"\xff\xd8\xff": - w, h = _jpeg_dimensions(data) - return w, h, "image/jpeg" - if data[:6] in (b"GIF87a", b"GIF89a"): - raise ValueError("GIF is not supported on this surface — PNG or JPEG only") - if data[:4] == b"RIFF" and data[8:12] == b"WEBP": - raise ValueError("WebP is not supported on this surface — PNG or JPEG only") - if data[:5] == b" dict[str, Any]: - """Upload a local PNG/JPEG for use in an Image block and return the - resolved block fields (``file_id``, ``src``, ``name``, ``natural_width``, - ``natural_height``). - - A real write — reuses ``api.files.upload_file`` with - ``source=files_api.PUBLIC_IMAGE`` and ``is_public=True``, confirmed live - 2026-08-25 (without ``is_public``, the upload defaults to non-public and - the resulting `src` 404s for any recipient without an authenticated - session — see `docs/specs/email-templates.md`). Callers outside - ``tools/planners/`` only (planners never write — see ``CLAUDE.md``); the - CLI only calls this for a real apply — under ``--dry-run`` it calls - :func:`offline_resolve_spec_images` instead, which uploads nothing. - ``base_url`` is the target env's own base URL (``EnvConfig.base_url``) — - ``Image.src`` is host-absolute, confirmed live, so a template is - environment-bound. - """ - src_path = Path(path) - data = src_path.read_bytes() - width, height, _content_type = read_image_dimensions(data) - registered = files_api.upload_file( - client, src_path, source=files_api.PUBLIC_IMAGE, is_public=True - ) - file_id = registered["id"] - src = f"{base_url}/api/files/{file_id}/download" - return { - "file_id": file_id, - "src": src, - "name": src_path.name, - "natural_width": width, - "natural_height": height, - } - - # --------------------------------------------------------------------------- # craft_json assembly — reuses tools.form_ui's Root/Section/Row/Cell shell # via the cell_props/block_assembler hooks added there for this module. @@ -606,6 +361,121 @@ def _row_props(row_spec: dict[str, Any]) -> dict[str, Any]: return overrides +def _assemble_text_block(block: dict[str, Any], parent_id: str) -> dict[str, Any]: + return { + "type": {"resolvedName": "Text"}, + "isCanvas": False, + "props": dict(_CONTAINER_DEFAULTS), + "displayName": "Text", + "custom": {"text": block["html"]}, + "parent": parent_id, + "hidden": False, + "nodes": [], + "linkedNodes": {}, + } + + +def _assemble_image_block(block: dict[str, Any], parent_id: str) -> dict[str, Any]: + # An omitted `width` used to collapse to a fixed `150` right here, + # before the node ever reached `craft_json` — so "omit width" never + # actually meant "auto mode", just a silent 150px default. Kizen's + # own auto mode (confirmed live against the reference: the one Image + # node with no `width` set at all) drops the `width` key entirely + # and sets `size: "auto"` instead of `"dynamic"` — both reproduced + # here. See `email_html._render_image` for how `content` resolves the + # omitted width from the parent Section's `containerWidth`. + width = block.get("width") + image_props: dict[str, Any] = { + **_CONTAINER_DEFAULTS, + "size": "dynamic" if width is not None else "auto", + "unit": "pixel", + "height": None, + "display": "flex", + "position": "center", + "alt": block.get("alt", ""), + "link": block.get("link", ""), + "src": block["src"], + "name": block["name"], + "fileId": block["file_id"], + "naturalHeight": block.get("natural_height"), + "naturalWidth": block.get("natural_width"), + "dimension": "width", + } + if width is not None: + image_props["width"] = width + if block.get("container_width") is not None: + image_props["containerWidth"] = block["container_width"] + if block.get("max_width") is not None: + image_props["maxWidth"] = block["max_width"] + if block.get("max_height") is not None: + image_props["maxHeight"] = block["max_height"] + return { + "type": {"resolvedName": "Image"}, + "isCanvas": False, + "props": image_props, + "displayName": "Image", + "custom": {}, + "parent": parent_id, + "hidden": False, + "nodes": [], + "linkedNodes": {}, + } + + +def _assemble_button_block(block: dict[str, Any], parent_id: str) -> dict[str, Any]: + return { + "type": {"resolvedName": "Button"}, + "isCanvas": False, + "props": { + **_CONTAINER_DEFAULTS, + "url": block.get("url", ""), + "label": block["label"], + "action": "url", + "color": block.get("color") or "rgba(0,51,160,1)", + "textColor": "rgba(255,255,255,1)", + "fontSize": "16", + "fontFamily": "Arial", + "alignment": block.get("alignment") or "center", + "borderSize": "0", + "borderColor": "rgba(0,0,0,1)", + "borderRadius": block.get("border_radius") or "8", + "paddingTop": "10", + "paddingLeft": block.get("padding_left") or "20", + "paddingRight": block.get("padding_right") or "20", + "paddingBottom": "10", + "textStyles": [], + "openLinkInNewTab": True, + }, + "displayName": "Button", + "custom": {}, + "parent": parent_id, + "hidden": False, + "nodes": [], + "linkedNodes": {}, + } + + +def _assemble_divider_block(block: dict[str, Any], parent_id: str) -> dict[str, Any]: + return { + "type": {"resolvedName": "Divider"}, + "isCanvas": False, + "props": { + **_CONTAINER_DEFAULTS, + "size": block.get("size") or "3", + "color": block.get("color") or "rgba(78,193,145,1)", + "width": "100", + "alignment": "center", + "borderStyle": "solid", + }, + "displayName": "Divider", + "custom": {}, + "parent": parent_id, + "hidden": False, + "nodes": [], + "linkedNodes": {}, + } + + def _assemble_email_block( block: dict[str, Any], parent_id: str, content: dict[str, Any] ) -> str: @@ -613,112 +483,13 @@ def _assemble_email_block( node_id = form_ui._new_id() if kind == "text": - node = { - "type": {"resolvedName": "Text"}, - "isCanvas": False, - "props": dict(_CONTAINER_DEFAULTS), - "displayName": "Text", - "custom": {"text": block["html"]}, - "parent": parent_id, - "hidden": False, - "nodes": [], - "linkedNodes": {}, - } + node = _assemble_text_block(block, parent_id) elif kind == "image": - # An omitted `width` used to collapse to a fixed `150` right here, - # before the node ever reached `craft_json` — so "omit width" never - # actually meant "auto mode", just a silent 150px default. Kizen's - # own auto mode (confirmed live against the reference: the one Image - # node with no `width` set at all) drops the `width` key entirely - # and sets `size: "auto"` instead of `"dynamic"` — both reproduced - # here. See `_render_image` for how `content` resolves the omitted - # width from the parent Section's `containerWidth`. - width = block.get("width") - image_props: dict[str, Any] = { - **_CONTAINER_DEFAULTS, - "size": "dynamic" if width is not None else "auto", - "unit": "pixel", - "height": None, - "display": "flex", - "position": "center", - "alt": block.get("alt", ""), - "link": block.get("link", ""), - "src": block["src"], - "name": block["name"], - "fileId": block["file_id"], - "naturalHeight": block.get("natural_height"), - "naturalWidth": block.get("natural_width"), - "dimension": "width", - } - if width is not None: - image_props["width"] = width - if block.get("container_width") is not None: - image_props["containerWidth"] = block["container_width"] - if block.get("max_width") is not None: - image_props["maxWidth"] = block["max_width"] - if block.get("max_height") is not None: - image_props["maxHeight"] = block["max_height"] - node = { - "type": {"resolvedName": "Image"}, - "isCanvas": False, - "props": image_props, - "displayName": "Image", - "custom": {}, - "parent": parent_id, - "hidden": False, - "nodes": [], - "linkedNodes": {}, - } + node = _assemble_image_block(block, parent_id) elif kind == "button": - node = { - "type": {"resolvedName": "Button"}, - "isCanvas": False, - "props": { - **_CONTAINER_DEFAULTS, - "url": block.get("url", ""), - "label": block["label"], - "action": "url", - "color": block.get("color") or "rgba(0,51,160,1)", - "textColor": "rgba(255,255,255,1)", - "fontSize": "16", - "fontFamily": "Arial", - "alignment": block.get("alignment") or "center", - "borderSize": "0", - "borderColor": "rgba(0,0,0,1)", - "borderRadius": block.get("border_radius") or "8", - "paddingTop": "10", - "paddingLeft": block.get("padding_left") or "20", - "paddingRight": block.get("padding_right") or "20", - "paddingBottom": "10", - "textStyles": [], - "openLinkInNewTab": True, - }, - "displayName": "Button", - "custom": {}, - "parent": parent_id, - "hidden": False, - "nodes": [], - "linkedNodes": {}, - } + node = _assemble_button_block(block, parent_id) elif kind == "divider": - node = { - "type": {"resolvedName": "Divider"}, - "isCanvas": False, - "props": { - **_CONTAINER_DEFAULTS, - "size": block.get("size") or "3", - "color": block.get("color") or "rgba(78,193,145,1)", - "width": "100", - "alignment": "center", - "borderStyle": "solid", - }, - "displayName": "Divider", - "custom": {}, - "parent": parent_id, - "hidden": False, - "nodes": [], - "linkedNodes": {}, - } + node = _assemble_divider_block(block, parent_id) else: raise ValueError( f"unsupported email block kind: {kind!r}. Supported: " @@ -729,465 +500,6 @@ def _assemble_email_block( return node_id -# --------------------------------------------------------------------------- -# content (compiled HTML) — walks the SAME craft_json dict build_content_tree -# just returned, using its dict keys as node ids. No second id-minting pass. -# --------------------------------------------------------------------------- - - -def _resolved_name(node: dict[str, Any]) -> str: - t = node.get("type") - return str(t.get("resolvedName")) if isinstance(t, dict) else str(t) - - -def _layout_for_columns(columns: list[float]) -> ColumnLayout: - for layout in COLUMN_LAYOUTS.values(): - if list(layout.columns) == list(columns): - return layout - raise ValueError(f"no v1 column layout matches columns={columns!r}") - - -def _render_button(node: dict[str, Any]) -> str: - p = node["props"] - return ( - '' - "
' - f'' - f"{escape(p['label'])}
" - ) - - -def _render_divider(node: dict[str, Any]) -> str: - p = node["props"] - return ( - f'

' - ) - - -def _ancestor_section_props(node_id: str, craft_json: dict[str, Any]) -> dict[str, Any]: - """Walk a leaf block's fixed 3-hop ancestry — block -> Cell -> Row -> - Section (`_assemble_cell` always sets a block's own `parent` to the - enclosing Cell's id, per `tools.form_ui`) — and return the enclosing - `Section`'s own `props`. Used by auto-mode image sizing below to read - the Section's `containerWidth`.""" - cell_id = craft_json[node_id]["parent"] - row_id = craft_json[cell_id]["parent"] - section_id = craft_json[row_id]["parent"] - return craft_json[section_id]["props"] - - -def _render_image(node_id: str, craft_json: dict[str, Any]) -> tuple[str, list[str]]: - """Return `(html, extra_style_rules)`. Kizen wraps every Image block in - the same two-level table Kizen's own compiler uses (confirmed against - the reference's compiled `content` for both a fixed-width and an - auto-mode image) — an outer `
` carrying block padding and, in auto - mode, the `.image--auto` coupling class, then a nested - `
` around the `` itself. That inner - `` is what the auto-mode CSS rule's `> table td` selector actually - targets — the rule is meaningless without this wrapper, so the two are - implemented together, not the rule alone. - - Auto mode (`Image.props.width` absent — see `_assemble_email_block`): - the ``'s `width` attribute becomes the parent Section's own - `containerWidth`, falling back to `Root.props.maxWidth` when the - Section doesn't set one (inferred, not observed live — see the work - item's Open questions), and a `.image--auto > table td` rule - caps it at the image's own `naturalWidth`. - """ - node = craft_json[node_id] - p = node["props"] - style_rules: list[str] = [] - explicit_width = p.get("width") - if explicit_width is not None: - img_width: Any = explicit_width - td_class = "" - else: - section_props = _ancestor_section_props(node_id, craft_json) - container_width = section_props.get("containerWidth") - if container_width is None: - container_width = craft_json["ROOT"]["props"]["maxWidth"] - img_width = container_width - td_class = _image_auto_class(node_id) - natural_width = p.get("naturalWidth") - if natural_width is not None: - style_rules.append( - f".{td_class} > table td {{ width: 100% !important; " - f"max-width: {natural_width}px; }}" - ) - - img = ( - f'{escape(p.get(' - ) - wrapped = ( - '' - f'
' - '' - f'' - "
{img}
" - "
" - ) - link = p.get("link") - if link: - wrapped = f'{wrapped}' - return wrapped, style_rules - - -def _render_text(node: dict[str, Any], craft_json: dict[str, Any]) -> str: - """The wrapper Kizen's own compiler puts around every Text block's copy - — the `kizen-text-styles` class plus its typography, sourced from - `Root.props` (confirmed against the reference's compiled `content`): - `font-family`/`font-size`/`color` (rgba->hex converted) come from - `Root.props`; `line-height:1`/`text-align:left` are fixed literals with - no controlling Root prop in the reference. `custom.text` is still - embedded verbatim inside it — this never touches BCLI-023's text model, - only the wrapper *around* it.""" - root_props = craft_json["ROOT"]["props"] - font_family = root_props.get("fontFamily", "Arial") - font_size = root_props.get("fontSize", "14") - color = _rgba_to_hex(root_props.get("color", "rgba(74,86,96,1)")) - return ( - '
{node["custom"]["text"]}
' - ) - - -def _render_block(node_id: str, craft_json: dict[str, Any]) -> tuple[str, list[str]]: - node = craft_json[node_id] - name = _resolved_name(node) - if name == "Text": - # Embedded verbatim, not stripped — see craft_summary()'s _plain_text, - # which tag-strips both sides before comparing. - return _render_text(node, craft_json), [] - if name == "Image": - return _render_image(node_id, craft_json) - if name == "Button": - return _render_button(node), [] - if name == "Divider": - return _render_divider(node), [] - raise ValueError(f"cannot compile HTML for unsupported node type {name!r}") - - -def _render_cell(cell_id: str, craft_json: dict[str, Any]) -> tuple[str, list[str]]: - node = craft_json[cell_id] - html_parts: list[str] = [] - style_rules: list[str] = [] - for block_id in node["nodes"]: - html, rules = _render_block(block_id, craft_json) - html_parts.append(html) - style_rules.extend(rules) - return "".join(html_parts), style_rules - - -def _truncate4(value: float) -> float: - """Truncate (not round) to 4 decimal places — matches the live-confirmed - column-width convention (`293.3333`, not `293.3333333333333` or the - rounded `293.3334`; `586.6666`, not the rounded `586.6667`).""" - return math.floor(value * 10000) / 10000 - - -def _row_content_width_px(row_id: str, craft_json: dict[str, Any]) -> float: - """The row's actual compiled pixel width — the value the mso table and - the `.section- { max-width:...px; }` rule must use. - - Trusts an explicit `Row.props.containerWidth` when the spec set one - directly (BCLI-024's independent, spec-settable field — the same - "explicit, not derived" trust model this surface already uses for - `Row.props.columns`/`Cell.props.__width`). Otherwise derives it from the - parent `Section`'s own `maxWidth` and padding: content width = maxWidth - - (paddingLeft + paddingRight) — the formula this module's `content - width` comment already stated, now actually applied instead of frozen - as one hardcoded constant. With every prop at its pre-BCLI-024 default - (`Section.maxWidth: "900"`, padding `"10"` each side), this reduces to - exactly `900 - 10 - 10 = 880.0`, the old hardcoded value — so unmodified - specs compile identically to before. - - Either way, the result is then scaled by `Row.props.width` (percent, - default `"100"`) — Kizen's real compiler applies this as a genuine - multiplier on top of `containerWidth`/the derived width, confirmed - against the reference template (`width: '75'` on a `containerWidth: - '580'` row compiles to `435px`, not `580px`). A default `"100"` makes - this a no-op. - """ - row_props = craft_json[row_id]["props"] - if "containerWidth" in row_props: - base_width = float(row_props["containerWidth"]) - else: - section_props = craft_json[craft_json[row_id]["parent"]]["props"] - max_width = float(section_props["maxWidth"]) - pad_left = float(section_props.get("containerPaddingLeft", 0)) - pad_right = float(section_props.get("containerPaddingRight", 0)) - base_width = max_width - pad_left - pad_right - return base_width * float(row_props.get("width", 100)) / 100 - - -def _padding_css(props: dict[str, Any]) -> str: - """CSS `padding` shorthand (top/right/bottom/left, no unit suffix on the - value) from a node's own `containerPadding{Top,Right,Bottom,Left}` - props — the same order `_render_button` already uses for its own - padding. Reads whatever `Section`/`Row` actually carries in - `craft_json`, default `"10"` or an explicit spec override alike, so - `content` cannot silently disagree with `craft_json` the way it did - before this fix (Section/Row padding never reached the compiled output - at all).""" - return ( - f"{props.get('containerPaddingTop', '0')}px " - f"{props.get('containerPaddingRight', '0')}px " - f"{props.get('containerPaddingBottom', '0')}px " - f"{props.get('containerPaddingLeft', '0')}px" - ) - - -def _fmt_px(value: float) -> str: - """Format a computed pixel length for the compiled CSS without a - spurious trailing `.0` (`880.0px` -> `880px`) while leaving a genuine - fractional value untouched (`293.3333px` stays `293.3333px`). Used at - `_render_row`'s width call sites: `_row_content_width_px`'s two sites - and the per-column `mso_widths_px` (`_truncate4`'s output).""" - if value == int(value): - return str(int(value)) - return str(value) - - -def _render_row(row_id: str, craft_json: dict[str, Any]) -> tuple[str, list[str]]: - """Return (body_html, style_rules) for one Row — `style_rules` is the - row's own `max-width` rule followed by any rules its cells' blocks - contributed (currently only an auto-mode Image's `.image--auto` - rule, see `_render_image`).""" - node = craft_json[row_id] - columns = node["props"]["columns"] - layout = _layout_for_columns(columns) - cell_ids = [node["linkedNodes"][f"column-{i + 1}"] for i in range(len(columns))] - content_width_px = _row_content_width_px(row_id, craft_json) - content_width_str = _fmt_px(content_width_px) - mso_widths_px = [ - _fmt_px(_truncate4(content_width_px * frac)) for frac in layout.columns - ] - - parts = [ - f'
' - ] - parts.append( - '' - ) - else: - parts.append( - f'" - ) - parts.append( - f'
' - ) - cell_html, cell_style_rules = _render_cell(cid, craft_json) - parts.append(cell_html) - extra_style_rules.extend(cell_style_rules) - parts.append("
") - parts.append("") - parts.append("
") - - style_rule = f".{_section_class(row_id)} {{ max-width:{content_width_str}px; }}" - return "".join(parts), [style_rule, *extra_style_rules] - - -def _render_section( - section_id: str, craft_json: dict[str, Any] -) -> tuple[str, list[str]]: - node = craft_json[section_id] - props = node["props"] - bg = props.get("containerBackgroundColor", "#FFFFFF") - rows_html: list[str] = [] - style_rules = [f".{_section_class(section_id)} {{ background-color:{bg}; }}"] - for row_id in node["nodes"]: - row_html, row_rules = _render_row(row_id, craft_json) - rows_html.append(row_html) - style_rules.extend(row_rules) - section_padding = _padding_css(props) - body = ( - f'
' - + "".join(rows_html) - + "
" - ) - - # The outer background-table wrapper `Section.props.containerWidth` - # needs, deferred here from BCLI-024 (see that item's Outcome). Only - # added when the spec set an explicit container_width — matching this - # surface's existing "None means no override, byte-identical to - # pre-this-prop output" convention (see `_section_props`), and the only - # case this module has real reference evidence for. Simplified relative - # to Kizen's own VML/background-image fallback markup (this emitter has - # no background-image concept at all, only `background_color`) — see the - # work item's report for the scoping call. - container_width = props.get("containerWidth") - if container_width is not None: - style_rules.append( - f".{_section_class(section_id)} {{ max-width:{container_width}px; }}" - ) - body = ( - '" + body + "" - ) - return body, style_rules - - -def _distinct_column_widths(craft_json: dict[str, Any]) -> dict[str, str]: - """Every distinct `mj-column-per-N` class in use across this template's - `Row` nodes, mapped to its media-query width percentage. Shared by - `_column_base_width_rules`, `_media_query_rules`, and - `_moz_text_html_style_block` so the three rule sets can't independently - drift on which classes exist.""" - seen: dict[str, str] = {} - for node in craft_json.values(): - if _resolved_name(node) != "Row": - continue - columns = node["props"]["columns"] - layout = _layout_for_columns(columns) - for cls, media_w in zip(layout.classes, layout.media_widths, strict=True): - seen[cls] = media_w - return seen - - -def _column_base_width_rules(craft_json: dict[str, Any]) -> list[str]: - """Unconditional `.mj-column-per-N` width rules — MJML's own convention. - - Each column `
` also carries a hardcoded inline `width:100%` (see - `_render_row`), which is the fallback for clients that ignore `' - - -def _compile_html(craft_json: dict[str, Any]) -> str: - root = craft_json["ROOT"] - root_props = root["props"] - bodies: list[str] = [] - style_rules: list[str] = [] - for section_id in root["nodes"]: - body, rules = _render_section(section_id, craft_json) - bodies.append(body) - style_rules.extend(rules) - - # Confirmed live 2026-08-26: `Root.props.mobileBreak` (`"414"` by - # default), not the hardcoded `480` this module used before. The `480` - # fallback below only applies if a caller's `craft_json` predates this - # prop entirely — every tree this module itself builds always has it. - mobile_break = str(root_props.get("mobileBreak", "480")) - column_rules = _column_base_width_rules(craft_json) - media_rules = _media_query_rules(craft_json) - style_block = ( - '" - + _moz_text_html_style_block(craft_json, mobile_break) - ) - link_color = _rgba_to_hex(root_props.get("linkColor", "rgba(82,142,249,1)")) - kizen_text_styles_block = ( - "" - ) - body_bg = root_props.get("backgroundColor", "#F8FAFF") - return ( - "" - '' - "" - '' - '' - '' - + _MJML_RESET_STYLE - + "" - + style_block - + kizen_text_styles_block - + "" - f'' - f'
' - + "".join(bodies) - + "
" - ) - - # --------------------------------------------------------------------------- # The entry point # --------------------------------------------------------------------------- @@ -1380,10 +692,10 @@ def resolve_spec_images(spec: EmailTemplateDef) -> list[dict[str, Any]]: reference and return ``spec.sections`` as the plain nested dicts ``tools.planners.messages`` turns into a plan. - A real write (see :func:`upload_email_image`) — call this from the CLI - layer for a real apply, never from ``tools/planners/``. Under - ``--dry-run`` the CLI calls :func:`offline_resolve_spec_images` instead, - so a dry run uploads nothing — see that function. + A real write (see :func:`email_images.upload_email_image`) — call this + from the CLI layer for a real apply, never from ``tools/planners/``. + Under ``--dry-run`` the CLI calls :func:`offline_resolve_spec_images` + instead, so a dry run uploads nothing — see that function. """ config = load_env_config() with KizenClient(config) as client: diff --git a/src/kizen_builder/tools/email_html.py b/src/kizen_builder/tools/email_html.py new file mode 100644 index 0000000..b5eee75 --- /dev/null +++ b/src/kizen_builder/tools/email_html.py @@ -0,0 +1,618 @@ +"""Compile an email template's craft_json tree into the Outlook-safe +`content` HTML. + +Split out of `tools/email_craft.py` — see that module's docstring for the +full "why `craft_json` and `content` must come from one pass over one tree" +reasoning; this module is the compile side of that pass. Every function here +takes an existing `node_id`/`craft_json` and never mints one — id minting +stays entirely in `email_craft.py` (`form_ui.build_content_tree` + +`_assemble_email_block`). `email_craft.build_email_content()` calls +`_compile_html` below once, on the exact tree `form_ui.build_content_tree` +just returned, and that's the only entry point into this module's compile +path. + +`ColumnLayout`/`COLUMN_LAYOUTS` also live here even though `email_craft.py`'s +`row()` reads `.columns` off them too (`email_craft.py` imports +`COLUMN_LAYOUTS` back by name) — they can't live in `email_craft.py` instead, +since `email_craft.py` already needs `_compile_html` from this module, and a +module can't import back from a module that imports it. +""" + +from __future__ import annotations + +import math +import re +from html import escape +from typing import Any + +_RGBA_RE = re.compile(r"rgba?\(\s*(\d+)\s*,\s*(\d+)\s*,\s*(\d+)\s*(?:,\s*[\d.]+\s*)?\)") + + +def _rgba_to_hex(value: str) -> str: + """Convert an `rgba(r,g,b,a)`/`rgb(r,g,b)` colour string to the lowercase + `#rrggbb` hex Kizen's own compiler emits for the same value — confirmed + against the reference template's compiled `content`: `Root.props.color` + (`rgba(74,86,96,1)`) compiles to `#4a5660`, `Root.props.linkColor` + (`rgba(82,142,249,1)`) compiles to `#528ef9`. Alpha is dropped, matching + both observed conversions (both alpha `1`) — this surface has no + confirmed case of a translucent text/link colour reaching `content`. + Anything that isn't `rgba(...)`/`rgb(...)` passes through unchanged + (most `container*` colour props on this surface are already hex or a + literal like `"transparent"`, not every colour prop here uses the + rgba wire format).""" + m = _RGBA_RE.fullmatch(value.strip()) + if not m: + return value + r, g, b = (int(x) for x in m.groups()) + return f"#{r:02x}{g:02x}{b:02x}" + + +# The MJML boilerplate reset block — Outlook/webkit/Gecko normalization with +# no per-template data, confirmed byte-exact against the reference template's +# compiled `content` (read-only `GET`, 2026-08-26). Kept as one literal +# constant rather than built up piecewise, since every byte here is fixed. +_MJML_RESET_STYLE = ( + '" +) + +# `.kizen-text-styles` — the class Kizen's own compiler uses to scope text +# typography and rich-text element styling (links, paragraphs, code/pre). +# The rule text itself carries no per-template data except `linkColor` +# (interpolated below), so it's kept as one structural block, confirmed +# against the reference template's compiled `content`. BCLI-023's text model +# (paragraph/list/code rendering) is untouched by this — these rules only +# apply Kizen's own styling to whatever HTML a `Text` block already embeds. +_KIZEN_TEXT_STYLES_TEMPLATE = ( + ".kizen-text-styles a {{ color: {link_color}; text-decoration: none; }}" + ".kizen-text-styles a *, .kizen-text-styles span * " + "{{ color: inherit; font-size: inherit; }}" + ".kizen-text-styles a:hover, .kizen-text-styles a:focus, " + ".kizen-text-styles a:hover *, .kizen-text-styles a:focus * " + "{{ text-decoration: underline; }}" + ".kizen-text-styles a:hover s, .kizen-text-styles a:focus s " + "{{ text-decoration: underline line-through; }}" + ".kizen-text-styles p {{ margin: 0; line-height: 1.5em; min-height: 1em; }}" + ".kizen-text-styles p * " + "{{ font-family: inherit; font-size: inherit; line-height: inherit; }}" + ".kizen-text-styles ul, .kizen-text-styles ol " + "{{ margin-top: 0; margin-bottom: 10px; }}" + ".kizen-text-styles code {{ font-family: 'Courier New', Monospace; " + "font-size: inherit; font-weight: 400; background-color: #F5F6F7; " + "padding: 5px; color: #4A5660; border-radius: 4px; }}" + ".kizen-text-styles pre {{ padding: 10px; background-color: #F5F6F7; " + "border-radius: 4px; border: 1px solid #D8DDE1; }}" + ".kizen-text-styles pre code {{ padding: 0; background-color: unset; " + "border-radius: 0; white-space: pre-wrap; word-break: break-all; }}" +) + + +def _section_class(node_id: str) -> str: + """`section-` — the existing Section/Row coupling class every + `_render_section`/`_render_row` call site uses. A tiny shared helper so + it and `_image_auto_class` below format node ids identically rather than + as two independently hand-rolled f-strings — see + `test_image_and_section_class_conventions_share_id_formatting`.""" + return f"section-{node_id}" + + +def _image_auto_class(node_id: str) -> str: + """`image--auto` — the auto-mode image sizing class, same + `-[-suffix]` shape as `_section_class` above. Confirmed + against the reference template's compiled `content` + (`.image--auto > table td { ... }`, one real occurrence + inspected structurally, node id not reproduced here).""" + return f"image-{node_id}-auto" + + +class ColumnLayout: + __slots__ = ("preset", "columns", "classes", "media_widths") + + def __init__( + self, + preset: str, + columns: tuple[float, ...], + classes: tuple[str, ...], + media_widths: tuple[str, ...], + ) -> None: + self.preset = preset + self.columns = columns + self.classes = classes + self.media_widths = media_widths + + +# 880px was the content width in every case observed pre-BCLI-024 (900 +# Section maxWidth - 20px padding, both hardcoded at the time). Now that +# `Section.max_width`/`Row.container_width`/padding are spec-settable +# (BCLI-024), the actual per-row pixel width is computed by +# `_row_content_width_px` below, not held as one constant — see that +# function's docstring for the fallback formula, which reduces to exactly +# 880.0 when nothing is overridden. + +COLUMN_LAYOUTS: dict[str, ColumnLayout] = { + "1 Column": ColumnLayout( + "1 Column", + (1,), + ("mj-column-per-100",), + ("100%",), + ), + "2 Columns": ColumnLayout( + "2 Columns", + (0.5, 0.5), + ("mj-column-per-50", "mj-column-per-50"), + ("50%", "50%"), + ), + "2 Columns (1/3 and 2/3)": ColumnLayout( + "2 Columns (1/3 and 2/3)", + (0.3333333333333333, 0.6666666666666666), + ("mj-column-per-33-333332", "mj-column-per-66-666664"), + ("33.333332%", "66.666664%"), + ), + "2 Columns (2/3 and 1/3)": ColumnLayout( + "2 Columns (2/3 and 1/3)", + (0.6666666666666666, 0.3333333333333333), + ("mj-column-per-66-666664", "mj-column-per-33-333332"), + ("66.666664%", "33.333332%"), + ), +} + + +def _resolved_name(node: dict[str, Any]) -> str: + t = node.get("type") + return str(t.get("resolvedName")) if isinstance(t, dict) else str(t) + + +def _layout_for_columns(columns: list[float]) -> ColumnLayout: + for layout in COLUMN_LAYOUTS.values(): + if list(layout.columns) == list(columns): + return layout + raise ValueError(f"no v1 column layout matches columns={columns!r}") + + +def _render_button(node: dict[str, Any]) -> str: + p = node["props"] + return ( + '' + "
' + f'' + f"{escape(p['label'])}
" + ) + + +def _render_divider(node: dict[str, Any]) -> str: + p = node["props"] + return ( + f'

' + ) + + +def _ancestor_section_props(node_id: str, craft_json: dict[str, Any]) -> dict[str, Any]: + """Walk a leaf block's fixed 3-hop ancestry — block -> Cell -> Row -> + Section (`_assemble_cell` always sets a block's own `parent` to the + enclosing Cell's id, per `tools.form_ui`) — and return the enclosing + `Section`'s own `props`. Used by auto-mode image sizing below to read + the Section's `containerWidth`.""" + cell_id = craft_json[node_id]["parent"] + row_id = craft_json[cell_id]["parent"] + section_id = craft_json[row_id]["parent"] + return craft_json[section_id]["props"] + + +def _render_image(node_id: str, craft_json: dict[str, Any]) -> tuple[str, list[str]]: + """Return `(html, extra_style_rules)`. Kizen wraps every Image block in + the same two-level table Kizen's own compiler uses (confirmed against + the reference's compiled `content` for both a fixed-width and an + auto-mode image) — an outer `
` carrying block padding and, in auto + mode, the `.image--auto` coupling class, then a nested + `
` around the `` itself. That inner + `` is what the auto-mode CSS rule's `> table td` selector actually + targets — the rule is meaningless without this wrapper, so the two are + implemented together, not the rule alone. + + Auto mode (`Image.props.width` absent — see `_assemble_email_block`): + the ``'s `width` attribute becomes the parent Section's own + `containerWidth`, falling back to `Root.props.maxWidth` when the + Section doesn't set one (inferred, not observed live — see the work + item's Open questions), and a `.image--auto > table td` rule + caps it at the image's own `naturalWidth`. + """ + node = craft_json[node_id] + p = node["props"] + style_rules: list[str] = [] + explicit_width = p.get("width") + if explicit_width is not None: + img_width: Any = explicit_width + td_class = "" + else: + section_props = _ancestor_section_props(node_id, craft_json) + container_width = section_props.get("containerWidth") + if container_width is None: + container_width = craft_json["ROOT"]["props"]["maxWidth"] + img_width = container_width + td_class = _image_auto_class(node_id) + natural_width = p.get("naturalWidth") + if natural_width is not None: + style_rules.append( + f".{td_class} > table td {{ width: 100% !important; " + f"max-width: {natural_width}px; }}" + ) + + img = ( + f'{escape(p.get(' + ) + wrapped = ( + '' + f'
' + '' + f'' + "
{img}
" + "
" + ) + link = p.get("link") + if link: + wrapped = f'{wrapped}' + return wrapped, style_rules + + +def _render_text(node: dict[str, Any], craft_json: dict[str, Any]) -> str: + """The wrapper Kizen's own compiler puts around every Text block's copy + — the `kizen-text-styles` class plus its typography, sourced from + `Root.props` (confirmed against the reference's compiled `content`): + `font-family`/`font-size`/`color` (rgba->hex converted) come from + `Root.props`; `line-height:1`/`text-align:left` are fixed literals with + no controlling Root prop in the reference. `custom.text` is still + embedded verbatim inside it — this never touches BCLI-023's text model, + only the wrapper *around* it.""" + root_props = craft_json["ROOT"]["props"] + font_family = root_props.get("fontFamily", "Arial") + font_size = root_props.get("fontSize", "14") + color = _rgba_to_hex(root_props.get("color", "rgba(74,86,96,1)")) + return ( + '
{node["custom"]["text"]}
' + ) + + +def _render_block(node_id: str, craft_json: dict[str, Any]) -> tuple[str, list[str]]: + node = craft_json[node_id] + name = _resolved_name(node) + if name == "Text": + # Embedded verbatim, not stripped — see craft_summary()'s _plain_text, + # which tag-strips both sides before comparing. + return _render_text(node, craft_json), [] + if name == "Image": + return _render_image(node_id, craft_json) + if name == "Button": + return _render_button(node), [] + if name == "Divider": + return _render_divider(node), [] + raise ValueError(f"cannot compile HTML for unsupported node type {name!r}") + + +def _render_cell(cell_id: str, craft_json: dict[str, Any]) -> tuple[str, list[str]]: + node = craft_json[cell_id] + html_parts: list[str] = [] + style_rules: list[str] = [] + for block_id in node["nodes"]: + html, rules = _render_block(block_id, craft_json) + html_parts.append(html) + style_rules.extend(rules) + return "".join(html_parts), style_rules + + +def _truncate4(value: float) -> float: + """Truncate (not round) to 4 decimal places — matches the live-confirmed + column-width convention (`293.3333`, not `293.3333333333333` or the + rounded `293.3334`; `586.6666`, not the rounded `586.6667`).""" + return math.floor(value * 10000) / 10000 + + +def _row_content_width_px(row_id: str, craft_json: dict[str, Any]) -> float: + """The row's actual compiled pixel width — the value the mso table and + the `.section- { max-width:...px; }` rule must use. + + Trusts an explicit `Row.props.containerWidth` when the spec set one + directly (BCLI-024's independent, spec-settable field — the same + "explicit, not derived" trust model this surface already uses for + `Row.props.columns`/`Cell.props.__width`). Otherwise derives it from the + parent `Section`'s own `maxWidth` and padding: content width = maxWidth + - (paddingLeft + paddingRight) — the formula this module's `content + width` comment already stated, now actually applied instead of frozen + as one hardcoded constant. With every prop at its pre-BCLI-024 default + (`Section.maxWidth: "900"`, padding `"10"` each side), this reduces to + exactly `900 - 10 - 10 = 880.0`, the old hardcoded value — so unmodified + specs compile identically to before. + + Either way, the result is then scaled by `Row.props.width` (percent, + default `"100"`) — Kizen's real compiler applies this as a genuine + multiplier on top of `containerWidth`/the derived width, confirmed + against the reference template (`width: '75'` on a `containerWidth: + '580'` row compiles to `435px`, not `580px`). A default `"100"` makes + this a no-op. + """ + row_props = craft_json[row_id]["props"] + if "containerWidth" in row_props: + base_width = float(row_props["containerWidth"]) + else: + section_props = craft_json[craft_json[row_id]["parent"]]["props"] + max_width = float(section_props["maxWidth"]) + pad_left = float(section_props.get("containerPaddingLeft", 0)) + pad_right = float(section_props.get("containerPaddingRight", 0)) + base_width = max_width - pad_left - pad_right + return base_width * float(row_props.get("width", 100)) / 100 + + +def _padding_css(props: dict[str, Any]) -> str: + """CSS `padding` shorthand (top/right/bottom/left, no unit suffix on the + value) from a node's own `containerPadding{Top,Right,Bottom,Left}` + props — the same order `_render_button` already uses for its own + padding. Reads whatever `Section`/`Row` actually carries in + `craft_json`, default `"10"` or an explicit spec override alike, so + `content` cannot silently disagree with `craft_json` the way it did + before this fix (Section/Row padding never reached the compiled output + at all).""" + return ( + f"{props.get('containerPaddingTop', '0')}px " + f"{props.get('containerPaddingRight', '0')}px " + f"{props.get('containerPaddingBottom', '0')}px " + f"{props.get('containerPaddingLeft', '0')}px" + ) + + +def _fmt_px(value: float) -> str: + """Format a computed pixel length for the compiled CSS without a + spurious trailing `.0` (`880.0px` -> `880px`) while leaving a genuine + fractional value untouched (`293.3333px` stays `293.3333px`). Used at + `_render_row`'s width call sites: `_row_content_width_px`'s two sites + and the per-column `mso_widths_px` (`_truncate4`'s output).""" + if value == int(value): + return str(int(value)) + return str(value) + + +def _render_row(row_id: str, craft_json: dict[str, Any]) -> tuple[str, list[str]]: + """Return (body_html, style_rules) for one Row — `style_rules` is the + row's own `max-width` rule followed by any rules its cells' blocks + contributed (currently only an auto-mode Image's `.image--auto` + rule, see `_render_image`).""" + node = craft_json[row_id] + columns = node["props"]["columns"] + layout = _layout_for_columns(columns) + cell_ids = [node["linkedNodes"][f"column-{i + 1}"] for i in range(len(columns))] + content_width_px = _row_content_width_px(row_id, craft_json) + content_width_str = _fmt_px(content_width_px) + mso_widths_px = [ + _fmt_px(_truncate4(content_width_px * frac)) for frac in layout.columns + ] + + parts = [ + f'
' + ] + parts.append( + '' + ) + else: + parts.append( + f'" + ) + parts.append( + f'
' + ) + cell_html, cell_style_rules = _render_cell(cid, craft_json) + parts.append(cell_html) + extra_style_rules.extend(cell_style_rules) + parts.append("
") + parts.append("") + parts.append("
") + + style_rule = f".{_section_class(row_id)} {{ max-width:{content_width_str}px; }}" + return "".join(parts), [style_rule, *extra_style_rules] + + +def _render_section( + section_id: str, craft_json: dict[str, Any] +) -> tuple[str, list[str]]: + node = craft_json[section_id] + props = node["props"] + bg = props.get("containerBackgroundColor", "#FFFFFF") + rows_html: list[str] = [] + style_rules = [f".{_section_class(section_id)} {{ background-color:{bg}; }}"] + for row_id in node["nodes"]: + row_html, row_rules = _render_row(row_id, craft_json) + rows_html.append(row_html) + style_rules.extend(row_rules) + section_padding = _padding_css(props) + body = ( + f'
' + + "".join(rows_html) + + "
" + ) + + # The outer background-table wrapper `Section.props.containerWidth` + # needs, deferred here from BCLI-024 (see that item's Outcome). Only + # added when the spec set an explicit container_width — matching this + # surface's existing "None means no override, byte-identical to + # pre-this-prop output" convention (see `_section_props`), and the only + # case this module has real reference evidence for. Simplified relative + # to Kizen's own VML/background-image fallback markup (this emitter has + # no background-image concept at all, only `background_color`) — see the + # work item's report for the scoping call. + container_width = props.get("containerWidth") + if container_width is not None: + style_rules.append( + f".{_section_class(section_id)} {{ max-width:{container_width}px; }}" + ) + body = ( + '" + body + "" + ) + return body, style_rules + + +def _distinct_column_widths(craft_json: dict[str, Any]) -> dict[str, str]: + """Every distinct `mj-column-per-N` class in use across this template's + `Row` nodes, mapped to its media-query width percentage. Shared by + `_column_base_width_rules`, `_media_query_rules`, and + `_moz_text_html_style_block` so the three rule sets can't independently + drift on which classes exist.""" + seen: dict[str, str] = {} + for node in craft_json.values(): + if _resolved_name(node) != "Row": + continue + columns = node["props"]["columns"] + layout = _layout_for_columns(columns) + for cls, media_w in zip(layout.classes, layout.media_widths, strict=True): + seen[cls] = media_w + return seen + + +def _column_base_width_rules(craft_json: dict[str, Any]) -> list[str]: + """Unconditional `.mj-column-per-N` width rules — MJML's own convention. + + Each column `
` also carries a hardcoded inline `width:100%` (see + `_render_row`), which is the fallback for clients that ignore `' + + +def _compile_html(craft_json: dict[str, Any]) -> str: + root = craft_json["ROOT"] + root_props = root["props"] + bodies: list[str] = [] + style_rules: list[str] = [] + for section_id in root["nodes"]: + body, rules = _render_section(section_id, craft_json) + bodies.append(body) + style_rules.extend(rules) + + # Confirmed live 2026-08-26: `Root.props.mobileBreak` (`"414"` by + # default), not the hardcoded `480` this module used before. The `480` + # fallback below only applies if a caller's `craft_json` predates this + # prop entirely — every tree this module itself builds always has it. + mobile_break = str(root_props.get("mobileBreak", "480")) + column_rules = _column_base_width_rules(craft_json) + media_rules = _media_query_rules(craft_json) + style_block = ( + '" + + _moz_text_html_style_block(craft_json, mobile_break) + ) + link_color = _rgba_to_hex(root_props.get("linkColor", "rgba(82,142,249,1)")) + kizen_text_styles_block = ( + "" + ) + body_bg = root_props.get("backgroundColor", "#F8FAFF") + return ( + "" + '' + "" + '' + '' + '' + + _MJML_RESET_STYLE + + "" + + style_block + + kizen_text_styles_block + + "" + f'' + f'
' + + "".join(bodies) + + "
" + ) diff --git a/src/kizen_builder/tools/email_images.py b/src/kizen_builder/tools/email_images.py new file mode 100644 index 0000000..a4db24f --- /dev/null +++ b/src/kizen_builder/tools/email_images.py @@ -0,0 +1,126 @@ +"""Image upload + header-byte pixel dimensions for email `Image` blocks. + +Split out of `tools/email_craft.py` (see that module's docstring for the +craft_json/content coupling invariant this surface exists around) — nothing +here mints a node id or touches `craft_json`/`content`. `email_craft.py` +imports `upload_email_image`/`read_image_dimensions` by name so its own +`resolve_spec_images`/`offline_resolve_spec_images` keep calling them as if +they were still local. +""" + +from __future__ import annotations + +from pathlib import Path +from typing import Any + +from kizen_builder.api import files as files_api +from kizen_builder.api.client import KizenClient + + +def _png_dimensions(data: bytes) -> tuple[int, int]: + # Signature (8 bytes) + IHDR chunk: length(4) type(4) width(4) height(4). + if len(data) < 24 or data[:8] != b"\x89PNG\r\n\x1a\n": + raise ValueError("not a valid PNG (bad signature)") + width = int.from_bytes(data[16:20], "big") + height = int.from_bytes(data[20:24], "big") + return width, height + + +# JPEG SOF (start-of-frame) markers that carry dimensions. Excludes DHT +# (0xC4), JPG (0xC8), DAC (0xCC) — same-range bytes that are NOT SOF markers. +_JPEG_SOF_MARKERS = frozenset( + {0xC0, 0xC1, 0xC2, 0xC3, 0xC5, 0xC6, 0xC7, 0xC9, 0xCA, 0xCB, 0xCD, 0xCE, 0xCF} +) +# Markers with no following length/payload — skip straight past them. +_JPEG_STANDALONE_MARKERS = frozenset({0x01, 0xD8, 0xD9} | set(range(0xD0, 0xD8))) + + +def _jpeg_dimensions(data: bytes) -> tuple[int, int]: + if len(data) < 4 or data[0:2] != b"\xff\xd8": + raise ValueError("not a valid JPEG (bad SOI marker)") + pos = 2 + n = len(data) + while pos < n - 1: + if data[pos] != 0xFF: + raise ValueError("malformed JPEG: expected a marker") + marker = data[pos + 1] + pos += 2 + while marker == 0xFF and pos < n: # padding fill bytes between markers + marker = data[pos] + pos += 1 + if marker in _JPEG_STANDALONE_MARKERS: + continue + if pos + 2 > n: + break + seg_len = int.from_bytes(data[pos : pos + 2], "big") + if marker in _JPEG_SOF_MARKERS: + if pos + 7 > n: + break + height = int.from_bytes(data[pos + 3 : pos + 5], "big") + width = int.from_bytes(data[pos + 5 : pos + 7], "big") + return width, height + pos += seg_len + raise ValueError("no SOF0/SOF2 segment found in JPEG") + + +def read_image_dimensions(data: bytes) -> tuple[int, int, str]: + """Return ``(width, height, content_type)`` read from the file's own + header bytes. PNG and JPEG only — both are real cases on this surface + (every image already stored in the target environment is PNG, but the + browser trace that settled the ``source`` question was a JPEG upload). + GIF/WebP/SVG fail loudly as unsupported rather than being silently + mis-parsed; SVG especially has no pixel dimensions to read this way at + all. + """ + if data[:8] == b"\x89PNG\r\n\x1a\n": + w, h = _png_dimensions(data) + return w, h, "image/png" + if data[:3] == b"\xff\xd8\xff": + w, h = _jpeg_dimensions(data) + return w, h, "image/jpeg" + if data[:6] in (b"GIF87a", b"GIF89a"): + raise ValueError("GIF is not supported on this surface — PNG or JPEG only") + if data[:4] == b"RIFF" and data[8:12] == b"WEBP": + raise ValueError("WebP is not supported on this surface — PNG or JPEG only") + if data[:5] == b" dict[str, Any]: + """Upload a local PNG/JPEG for use in an Image block and return the + resolved block fields (``file_id``, ``src``, ``name``, ``natural_width``, + ``natural_height``). + + A real write — reuses ``api.files.upload_file`` with + ``source=files_api.PUBLIC_IMAGE`` and ``is_public=True``, confirmed live + 2026-08-25 (without ``is_public``, the upload defaults to non-public and + the resulting `src` 404s for any recipient without an authenticated + session — see `docs/specs/email-templates.md`). Callers outside + ``tools/planners/`` only (planners never write — see ``CLAUDE.md``); the + CLI only calls this for a real apply — under ``--dry-run`` it calls + :func:`email_craft.offline_resolve_spec_images` instead, which uploads + nothing. ``base_url`` is the target env's own base URL + (``EnvConfig.base_url``) — ``Image.src`` is host-absolute, confirmed + live, so a template is environment-bound. + """ + src_path = Path(path) + data = src_path.read_bytes() + width, height, _content_type = read_image_dimensions(data) + registered = files_api.upload_file( + client, src_path, source=files_api.PUBLIC_IMAGE, is_public=True + ) + file_id = registered["id"] + src = f"{base_url}/api/files/{file_id}/download" + return { + "file_id": file_id, + "src": src, + "name": src_path.name, + "natural_width": width, + "natural_height": height, + } diff --git a/tests/test_email_craft.py b/tests/test_email_craft.py index 4f8e8a7..66c09c7 100644 --- a/tests/test_email_craft.py +++ b/tests/test_email_craft.py @@ -27,6 +27,7 @@ from kizen_builder.models.spec.email_templates import COLUMN_FRACTIONS, EmailTemplateDef from kizen_builder.tools import email_craft as ec +from kizen_builder.tools import email_html as eh from kizen_builder.tools import form_ui from kizen_builder.tools.messages import craft_summary @@ -307,8 +308,8 @@ def test_render_image_fixed_width_compiled_markup_matches_the_reference_shape(): nid for nid, n in craft_json.items() if _resolved_name(n) == "Image" ) # Fixed mode: no `-auto` class, and no auto-mode CSS rule at all. - assert f'class="{ec._image_auto_class(image_id)}"' not in content - assert f".{ec._image_auto_class(image_id)} > table td" not in content + assert f'class="{eh._image_auto_class(image_id)}"' not in content + assert f".{eh._image_auto_class(image_id)} > table td" not in content # The outer block-level
carries the image's own containerPadding. outer_td = re.search( r' table td {{ width: 100% !important; max-width: 1200px; }}" @@ -471,8 +472,8 @@ def test_image_and_section_class_conventions_share_id_formatting(): hand-rolled f-strings to stay in sync. Both wrap the SAME node id in the SAME `-[-suffix]` shape.""" for node_id in ["abc123", "0" * 24, "a-node-with-dashes"]: - assert ec._section_class(node_id) == f"section-{node_id}" - assert ec._image_auto_class(node_id) == f"image-{node_id}-auto" + assert eh._section_class(node_id) == f"section-{node_id}" + assert eh._image_auto_class(node_id) == f"image-{node_id}-auto" def test_exactly_one_tr_per_row_regardless_of_column_count(): @@ -1434,7 +1435,7 @@ def resolved(n: dict) -> str: # this emitter has no background-image concept — see the work item's # report for that scoping call). assert 'width="919" style="width:919px;"' in content - assert f".{ec._section_class(section_id)} {{ max-width:919px; }}" in content + assert f".{eh._section_class(section_id)} {{ max-width:919px; }}" in content # --- Fields confirmed craft_json-only by checking Kizen's own compiled # output for the reference template (not merely "no consumer in @@ -1492,9 +1493,9 @@ def test_rgba_to_hex_matches_the_two_conversions_confirmed_live(): """`Root.props.color: rgba(74,86,96,1)` -> `#4a5660` and `Root.props.linkColor: rgba(82,142,249,1)` -> `#528ef9` — both read directly off the reference template's compiled `content`.""" - assert ec._rgba_to_hex("rgba(74,86,96,1)") == "#4a5660" - assert ec._rgba_to_hex("rgba(82,142,249,1)") == "#528ef9" - assert ec._rgba_to_hex("#FFFFFF") == "#FFFFFF" # passes through non-rgba unchanged + assert eh._rgba_to_hex("rgba(74,86,96,1)") == "#4a5660" + assert eh._rgba_to_hex("rgba(82,142,249,1)") == "#528ef9" + assert eh._rgba_to_hex("#FFFFFF") == "#FFFFFF" # passes through non-rgba unchanged def test_moz_text_html_rule_exists_for_every_column_class_in_a_multi_column_layout(): @@ -1535,7 +1536,7 @@ def test_moz_text_html_rule_exists_for_every_column_class_in_a_multi_column_layo def test_no_moz_text_html_style_block_when_template_has_no_rows(): """`_moz_text_html_style_block` returns nothing when there are no `Row` nodes to derive column classes from, rather than an empty/broken rule.""" - assert ec._moz_text_html_style_block({}, "414") == "" + assert eh._moz_text_html_style_block({}, "414") == "" def test_mjml_reset_block_is_present_and_byte_exact(): @@ -1629,7 +1630,7 @@ def test_section_container_width_gets_no_outer_wrapper_when_unset(): section_id = next( nid for nid, n in craft_json.items() if _resolved_name(n) == "Section" ) - assert f'class="{ec._section_class(section_id)}"' in content + assert f'class="{eh._section_class(section_id)}"' in content assert 'role="presentation" align="center" width=' not in content @@ -1655,9 +1656,9 @@ def test_section_container_width_wrapper_carries_the_containerWidth_attribute(): '" - f'