Uh oh!
There was an error while loading. Please reload this page.
Re-land the email-template authoring surface - #24
Merged
Conversation
`kizen messages templates create --spec-file <f>` builds a complete
email template — `craft_json` and the compiled, Outlook-safe `content`
HTML — from one declarative spec; `update <tmpl> --spec-file <f>`
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.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.
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); `<body>`'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 `<td>` 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 `<v:rect>`/`<v:fill>` 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.
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.
…xt-blocks # Conflicts: # CHANGELOG.md
BREAKING: `messages templates create/update --spec-file`'s `text`
block no longer accepts `html`. A spec with a lingering `html` key
now fails validation, naming `paragraphs` as the expected field.
Anyone with an existing spec must migrate.
This was deliberate, not a stopgap: raw HTML did not survive a human
opening and saving the template in Kizen's builder — 7 of 9 text
blocks in a real captured template were silently rewritten by the
builder's own save, the exact "renders once, breaks on touch" bug
this item exists to fix. `TextBlockDef.paragraphs` is a closed
vocabulary of {text, size, bold, color, link, align} that the new
`_paragraphs_to_html` renders into the canonical markup Kizen's own
editor normalises to, confirmed live against a real template — so a
template built from a spec now round-trips through a builder save
unchanged in the cases this model covers.
It does not cover every case. Two live paragraph shapes fall outside
the model — a bare `<a>` with no `<span>` (4 of 31 `<p>` blocks in
the reference template), and partial-run bold within one span (2 of
them — bold on part of a sentence, not the whole paragraph). With
`html` gone there is no escape hatch, so neither shape can be
authored from the CLI at all. Filed as a follow-on:
00-inbox/paragraph-model-cannot-express-partial-run-formatting.md.
A paragraph's `text` may also carry inline `{{ namespace.field }}`
merge-field tokens, rendered via the shared `tools/merge_fields.py`
(landed separately, merged into this branch) rather than
reimplemented here. `automation_variable.*` tokens are rejected at
spec-validation time, since a library template isn't scoped to one
automation. `craft-config`/`--dry-run` never makes a live call, so
`data-merge-field-objectname` is omitted for custom-object fields in
that offline preview — the same class of divergence already
documented for Image blocks.
Also fixes `_render_button`'s compiled `<a>` tag, which diverged from
Kizen's real output (rgba() color instead of hex, five missing style
declarations), and splits a test that falsely claimed byte-exactness
against a real captured template into a genuinely live-verified
button assertion and an honest "pins current shape" Divider
assertion. Two other byte-exact test claims (image fixed-width, MJML
reset) were independently re-verified live and held up.
Two gaps are owner-only and still open: a real test send opened in
Outlook desktop, and opening a CLI-built template in Kizen's builder
to confirm the paragraph markup round-trips unchanged — this item's
own core premise, unverified by anything offline.PRs #21 and #22 were merged into feat/email-template-commands-and-drift-checks rather than main. That base branch was deleted after its own PR merged, so both were recorded as merged into a branch that no longer exists and their content never reached main. BCLI-024, 025 and 026 were committed and never pushed at all. This brings all five back: the email-template spec models and emitter (email_craft, email_html, email_images), the messages CLI additions, and their tests.
Merged
annaliu-kizen
approved these changes
Sep 1, 2026
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This adds no new work. It lands code that was already written, reviewed, and marked done, but which is absent from
main.What happened
Three merged PRs never reached
main. All were stacked PRs targeting another feature branch; when the base branch's own PR merged and GitHub deleted it, the stacked PR was recorded as MERGED into a branch that no longer exists, and the content never propagated.main?feat/email-template-commands-and-drift-checksfeat/email-template-commands-and-drift-checksVerified with
git merge-base --is-ancestor— neither merge commit is an ancestor ofmain. Two further items were committed and never pushed at all.So
maintoday has notools/email_craft.py,email_html.py,email_images.py, ormodels/spec/email_templates.py. This PR restores all five items' work:Merged rather than rebased, deliberately:
edd7a12already resolvesfeat/merge-fields-moduleagainst this stack, and replaying that resolution risks getting it wrong. The branch tips are byte-identical to PRs #21 and #22'sheadRefOid, so what lands here is exactly what was reviewed.Verification
git merge-treepredicted a clean merge, and it was — no conflicts.bin/check.sh builder-cli --dir <worktree>— lint, format, typecheck, test, extra_checks all pass:That matches the pass/skip counts recorded when this work was originally verified, which is the strongest available signal that nothing was lost in the re-land. Toolchain isolation was confirmed first (
pytestandkizen_builderboth resolving inside the worktree).dump_cli_tree.pydiffs clean against the baseline.Confirmed no regression to what already landed on
main:tools/merge_fields.py(PR #20) and thecli/messages.pyemail-template commands (PR #19) are intact, andtests/drift/contracts.pyandschema_snapshot.jsonare byte-identical tomain— the merge correctly keptmain's newer 2026-08-13 drift tier rather than the branch's older one.One thing this PR does not prove
The branch adds
tests/drift/test_email_template_roundtrip.py, written against the older drift tier (snapshot 2026-08-06, acontracts.pywithout the bare-enum expansion logicmainnow has). Drift tests are deselected by default —--collect-onlyon that file reports1 deselected— so the green run above says nothing about it.A clean textual merge does not make those compatible. Someone should run
KIZEN_DRIFT_PROFILE=<profile> uv run pytest -m driftagainst a disposable environment before trusting that file. If it fails, fix the test againstmain's tier — do not regresstests/drift/to the branch's version.Follow-up worth doing separately
Nothing detects this failure mode. A merged PR whose commits are not ancestors of
mainis invisible to every existing check, and it happened three times before anyone noticed. Detection is one command per item:The narrower structural fix is to stop merging PRs into feature branches, or to verify the base is
mainat merge time.