Skip to content

Commit 96e8ea0

Browse files
fix+docs: pre-release review fixes (OOXML lvl ordering, stale docs, verb tables)
Multi-agent pre-release review of the v0.7.0..HEAD wave (4 dimensions, every HIGH/MEDIUM finding adversarially verified): 10 confirmed, 7 fixed here, 3 correctly deferred to the release commit (plugin/marketplace version bumps and the [Unreleased] -> 0.8.0 conversion). - REAL BUG (docx numbering, D3 defensive path): creating a missing w:numFmt / w:lvlText on a cloned w:lvl used append(), which violates the ECMA-376 CT_Lvl child sequence when w:pPr/w:rPr already exist (Word rejects out-of-order children). Added _LVL_CHILD_ORDER + _insert_lvl_child_ordered (the established _insert_ppr_child_ordered pattern) and used it at both sites. - Stale docs from the 6-axis evolution: common/appearance.py module + apply_role_appearance docstrings, formats/docx/typography.py module docstring, tests/test_typography.py module docstring now describe all six axes, their differing pathways and the E3 parity ledger. - documentation/USAGE.md: the IntermediateDocument section now documents the cover named-slot shape ({"cover": {"title", "subtitle"}}) and the toc block - the exact authoring gap the e2e validation tripped over. - skills/*/SKILL.md (all three, kept structurally aligned): "The seven verbs" table - the deterministic core plus comprehend / learn / propose-overrides / refine, each marked fail-closed and advisory-until-accept. Gate: ruff clean; suite 848; anchor + reference-sync 17; real-render 70. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent b57e0dc commit 96e8ea0

8 files changed

Lines changed: 185 additions & 81 deletions

File tree

documentation/USAGE.md

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,13 @@ the profile resolves all of that.
2323

2424
```json
2525
{
26-
"cover": { "title": "Quarterly Review", "fields": { "doc_id": "RPT-001" } },
26+
"cover": {
27+
"title": "Quarterly Review",
28+
"subtitle": "Q2 - Revenue and delivery",
29+
"fields": { "doc_id": "RPT-001", "date": "2026-06-10" }
30+
},
2731
"blocks": [
32+
{ "type": "toc", "title": "Contents", "max_level": 3 },
2833
{ "type": "heading", "level": 1, "text": "Highlights" },
2934
{ "type": "paragraph", "text": "This paragraph resolves to the brand body style." },
3035
{ "type": "callout", "intent": "info", "text": "The profile chooses the callout style." },
@@ -34,6 +39,20 @@ the profile resolves all of that.
3439
}
3540
```
3641

42+
Structural notes:
43+
44+
- The optional top-level **`cover`** object carries named semantic slots only:
45+
`title`, an optional `subtitle`, and an optional `fields` mapping of
46+
key-to-value pairs (document id, date, author, ...). The profile decides where
47+
and how each slot renders on the template's own cover.
48+
- A **`toc`** block is a table-of-contents placeholder, not content: it carries an
49+
optional `title` and a `max_level` (default 3), and the engine emits a native
50+
field or defers to a preserved template outline.
51+
- Block order in `blocks` is the author's reading order; every block resolves
52+
through semantic roles, so nothing in the JSON names a style, color or font.
53+
54+
A comprehensive worked example lives at
55+
[`skills/brand-docx/examples/intermediate-document.example.json`](../skills/brand-docx/examples/intermediate-document.example.json).
3756
PowerPoint uses the same `IntermediateDocument`; Excel uses a `GridDocument`
3857
(named-region fills, formulas preserved).
3958

scripts/brandkit/common/appearance.py

Lines changed: 38 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,40 @@
11
# SPDX-License-Identifier: MIT
2-
"""Format-neutral brand APPEARANCE apply orchestration (font / size / color).
2+
"""Format-neutral brand APPEARANCE apply orchestration (font / size / color /
3+
geometry / table / numbering).
34
45
This is the shared control flow the per-format generators (docx and pptx today;
5-
xlsx in a later PR) delegate to so the "read the three axes off the resolver op,
6-
then brand each run only when its axis is unset" logic has exactly ONE writer
7-
across kinds.
6+
xlsx in a later PR) delegate to so the "read the appearance axes off the resolver
7+
op, then brand each run/paragraph only when its axis is unset" logic has exactly
8+
ONE writer across kinds.
89
910
It is lxml / python-docx / pptx / openpyxl-FREE at import (like
10-
:mod:`brandkit.common.text`): the per-axis run mutations and the set-only-when-unset
11-
probes live behind a small BACKEND object the format adapter supplies (e.g. docx's
12-
``DOCX_BACKEND`` wrapping ``run.font.name``/``.size``/``.color``; pptx's
13-
``PPTX_BACKEND``). This module only:
11+
:mod:`brandkit.common.text`): the per-axis run/paragraph mutations and the
12+
set-only-when-unset probes live behind a small BACKEND object the format adapter
13+
supplies (e.g. docx's ``DOCX_BACKEND`` wrapping ``run.font.name``/``.size``/
14+
``.color`` and the paragraph's ``w:pPr`` geometry; pptx's ``PPTX_BACKEND``). This
15+
module only:
1416
1517
1. reads the captured brand axes off the resolver op (:func:`op_latin` /
16-
:func:`op_size_hp` / :func:`op_color`) - STRICTLY from ``op.appearance``, never
17-
a literal in the engine, so off-brand output stays impossible by construction;
18+
:func:`op_size_hp` / :func:`op_color` / :func:`op_geometry` / :func:`op_table` /
19+
:func:`op_numbering`) - STRICTLY from ``op.appearance``, never a literal in the
20+
engine, so off-brand output stays impossible by construction. The axes ride
21+
different pathways: font/size/color are run axes applied here through the
22+
backend; geometry is a paragraph axis applied here via the backend's
23+
``set_geometry`` hook (docx-only today); table and numbering are docx-only and
24+
realized by dedicated writers OUTSIDE this orchestration, but declared in
25+
:data:`APPEARANCE_AXES` so the parity ledger measures them;
1826
2. resolves a run's ``color`` palette TOKEN to its captured ref
1927
(:func:`resolve_run_color`), recording a graceful INFO finding for an unknown
2028
token (the writer never fabricates a color);
21-
3. drives the backend to apply those axes (:func:`apply_role_appearance` over a
22-
paragraph's runs; :func:`apply_run_color` for a single run), gating each write
23-
on the backend's ``*_unset`` probe so an inherited-but-correct value is never
24-
clobbered and re-runs stay byte-identical.
29+
3. drives the backend to apply the run/paragraph axes
30+
(:func:`apply_role_appearance` over a paragraph's runs and geometry;
31+
:func:`apply_run_color` for a single run), gating each write on the backend's
32+
``*_unset`` probe so an inherited-but-correct value is never clobbered and
33+
re-runs stay byte-identical;
34+
4. keeps the parity ledger (Cluster E3): :func:`_record_degraded_axes` emits one
35+
INFO ``appearance_apply_degraded`` finding per captured axis the format backend
36+
does not declare it realizes, so an unmaterialized axis surfaces gracefully
37+
instead of silently dropping.
2538
2639
The brand guarantee is preserved end to end: every applied value comes only from
2740
``op.appearance`` / the resolved palette ref, the set-only-when-unset guard is
@@ -283,15 +296,19 @@ def resolve_run_color(
283296
def apply_role_appearance(
284297
backend: AppearanceBackend, target, op, findings: list[Finding]
285298
) -> None:
286-
"""Apply captured brand typography (font, size, color) from the resolved op as
287-
direct run formatting on ``target``'s runs (hyperlink runs included for docx).
299+
"""Apply captured brand typography (font, size, color) and geometry from the
300+
resolved op as direct run/paragraph formatting on ``target`` (hyperlink runs
301+
included for docx).
288302
289-
The three axes are INDEPENDENT: each is applied only when the run's corresponding
303+
The run axes are INDEPENDENT: each is applied only when the run's corresponding
290304
``*_unset`` probe is true, so a role carrying a size but no font (or a color but
291-
no font) still applies the axes it has. A target that exposes no runs (a docx
292-
table here) yields nothing and is skipped. An empty appearance (a pre-capture
293-
profile) returns before touching any run, so output stays byte-identical to
294-
today."""
305+
no font) still applies the axes it has. Geometry is a separate PARAGRAPH-level
306+
axis, applied per paragraph via the backend's ``set_geometry`` hook (its
307+
set-only-when-unset guard lives per PROPERTY inside the backend). Table and
308+
numbering are MEASURED here by the parity ledger but realized by dedicated
309+
format-specific writers elsewhere. A target that exposes no runs (a docx table
310+
here) yields nothing and is skipped. An empty appearance (a pre-capture profile)
311+
returns before touching any run, so output stays byte-identical to today."""
295312
# Parity ledger (Cluster E3): surface any captured axis this backend cannot
296313
# realize BEFORE the early return, so a table/numbering-only op on a format
297314
# without those writers is still measured. Appends findings only; it never

scripts/brandkit/formats/docx/generate.py

Lines changed: 48 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1099,6 +1099,46 @@ def _ensure_numbering_def_present(doc, shell_doc, abstract_num_id: str) -> None:
10991099
out_root.append(clone)
11001100

11011101

1102+
# The WordprocessingML ``CT_Lvl`` child sequence (spec-fixed order, ECMA-376
1103+
# ``w:lvl``). A re-asserted level fact must be inserted BEFORE the first existing
1104+
# child that follows it in this order (e.g. ``w:lvlText`` precedes ``w:pPr``), or a
1105+
# strict OOXML reader rejects the cloned definition. The peer of
1106+
# :data:`_PPR_CHILD_ORDER` / :data:`_TBLPR_CHILD_ORDER` for the numbering part.
1107+
_LVL_CHILD_ORDER: tuple[str, ...] = (
1108+
"w:start",
1109+
"w:numFmt",
1110+
"w:lvlRestart",
1111+
"w:pStyle",
1112+
"w:isLgl",
1113+
"w:suff",
1114+
"w:lvlText",
1115+
"w:lvlPicBulletId",
1116+
"w:legacy",
1117+
"w:lvlJc",
1118+
"w:pPr",
1119+
"w:rPr",
1120+
)
1121+
1122+
1123+
def _insert_lvl_child_ordered(lvl, tag: str):
1124+
"""Get-or-create ``lvl/<tag>`` at the SPEC-CORRECT position in the ``CT_Lvl`` child
1125+
sequence (so Word accepts the cloned numbering definition). Returns the existing
1126+
element when present, else a new one inserted before the first existing successor
1127+
in :data:`_LVL_CHILD_ORDER` (appended only when no successor exists)."""
1128+
existing = lvl.find(qn(tag))
1129+
if existing is not None:
1130+
return existing
1131+
el = OxmlElement(tag)
1132+
successors = _LVL_CHILD_ORDER[_LVL_CHILD_ORDER.index(tag) + 1 :]
1133+
for child in lvl:
1134+
for succ in successors:
1135+
if child.tag == qn(succ):
1136+
child.addprevious(el)
1137+
return el
1138+
lvl.append(el)
1139+
return el
1140+
1141+
11021142
def _reassert_level_facts(lvl, facts: dict) -> None:
11031143
"""Re-assert one level's captured facts (numFmt / lvlText / indent) onto its
11041144
``w:lvl`` element, SET-ONLY-WHEN-UNSET (Cluster D3).
@@ -1107,32 +1147,22 @@ def _reassert_level_facts(lvl, facts: dict) -> None:
11071147
authored value (the shell's own, or a manual edit) is never clobbered and re-runs
11081148
stay byte-identical. The engine writes only VALUES the profile captured from the
11091149
shell (never synthesized): ``w:numFmt@w:val`` / ``w:lvlText@w:val`` are set on the
1110-
existing or a freshly-created child; each ``w:ind`` attribute is set on the level's
1111-
``w:pPr/w:ind`` set-only-when-unset."""
1150+
existing or a freshly-created child (created at its SPEC-CORRECT ``CT_Lvl``
1151+
position via :func:`_insert_lvl_child_ordered`); each ``w:ind`` attribute is set
1152+
on the level's ``w:pPr/w:ind`` set-only-when-unset."""
11121153
numfmt = facts.get("numFmt")
11131154
if numfmt is not None:
1114-
el = lvl.find(qn("w:numFmt"))
1115-
if el is None:
1116-
el = OxmlElement("w:numFmt")
1117-
lvl.insert(0, el)
1118-
el.set(qn("w:val"), str(numfmt))
1119-
elif el.get(qn("w:val")) is None:
1155+
el = _insert_lvl_child_ordered(lvl, "w:numFmt")
1156+
if el.get(qn("w:val")) is None:
11201157
el.set(qn("w:val"), str(numfmt))
11211158
lvltext = facts.get("lvlText")
11221159
if lvltext is not None:
1123-
el = lvl.find(qn("w:lvlText"))
1124-
if el is None:
1125-
el = OxmlElement("w:lvlText")
1126-
lvl.append(el)
1127-
el.set(qn("w:val"), str(lvltext))
1128-
elif el.get(qn("w:val")) is None:
1160+
el = _insert_lvl_child_ordered(lvl, "w:lvlText")
1161+
if el.get(qn("w:val")) is None:
11291162
el.set(qn("w:val"), str(lvltext))
11301163
indent = facts.get("indent") or {}
11311164
if indent:
1132-
ppr = lvl.find(qn("w:pPr"))
1133-
if ppr is None:
1134-
ppr = OxmlElement("w:pPr")
1135-
lvl.append(ppr)
1165+
ppr = _insert_lvl_child_ordered(lvl, "w:pPr")
11361166
ind = ppr.find(qn("w:ind"))
11371167
if ind is None:
11381168
ind = OxmlElement("w:ind")

scripts/brandkit/formats/docx/typography.py

Lines changed: 36 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,44 @@
11
# SPDX-License-Identifier: MIT
2-
"""DOCX brand typography capture (font family, size, and color).
3-
4-
The brand's REAL visible typography often lives as DIRECT run-level formatting
5-
(``w:rPr/w:rFonts`` / ``w:sz`` / ``w:color``) on the template's content rather than
6-
in the named styles or the theme: a designed template may put everything in
7-
``Normal`` with a direct Roboto / Montserrat override at 22 half-points in accent1.
8-
Role inference (``roles.py``) and theme extraction read only named styles and
9-
``theme1.xml``, so those direct values are never captured and a generated document
10-
falls back to the ``docDefaults`` font/size/color.
11-
12-
This module captures the DOMINANT direct run typography, deterministically, as
13-
THREE INDEPENDENT axes (font family, size, color) sampled in a SINGLE pass:
14-
15-
- per role: the dominant explicit value among the runs that use the role's style
16-
-> ``role['appearance']['font'] = {'latin': <name>}`` /
17-
``role['appearance']['size_hp'] = <int>`` /
18-
``role['appearance']['color'] = {'kind': ...}``;
19-
- the document's effective body typography: the dominant explicit value across all
20-
body runs -> ``theme['fonts']['body']['latin'/'size_hp']`` and
2+
"""DOCX brand appearance capture (run typography and structural appearance).
3+
4+
The brand's REAL visible appearance often lives as DIRECT formatting on the
5+
template's content rather than in the named styles or the theme: a designed
6+
template may put everything in ``Normal`` with a direct Roboto / Montserrat
7+
override at 22 half-points in accent1, hand-tuned paragraph spacing, and an
8+
explicit ``w:tblLook`` on every table. Role inference (``roles.py``) and theme
9+
extraction read only named styles and ``theme1.xml``, so those direct values are
10+
never captured and a generated document falls back to the ``docDefaults``.
11+
12+
This module captures the DOMINANT direct values, deterministically, as INDEPENDENT
13+
appearance axes:
14+
15+
- run typography (:func:`capture_fonts`, a SINGLE pass over the runs): font
16+
family, size, and color, per role (the dominant explicit value among the runs
17+
that use the role's style -> ``role['appearance']['font'] = {'latin': <name>}``
18+
/ ``role['appearance']['size_hp'] = <int>`` / ``role['appearance']['color'] =
19+
{'kind': ...}``) and as the document's effective body typography
20+
(``theme['fonts']['body']['latin'/'size_hp']`` and
2121
``theme['text']['body']['color']`` - the fallbacks the generator applies to a
22-
paragraph whose role carries no captured value.
22+
paragraph whose role carries no captured value);
23+
- paragraph GEOMETRY (:func:`capture_geometry`, Cluster D1, docx-only): the
24+
dominant explicit ``w:pPr`` spacing / indentation / paragraph borders / shading,
25+
per role (``role['appearance']['geometry']``) and as the body default
26+
(``theme['geometry']['body']``);
27+
- TABLE conditional-format facts (:func:`capture_table_appearance`, Cluster D2,
28+
docx-only): the dominant explicit ``w:tblLook`` bitmask, referenced table-style
29+
id, and ``w:tblCellMar`` margins, per ``table.*`` role
30+
(``role['appearance']['table']``) and as ``theme['table']['body']``.
31+
32+
(The sixth axis, list NUMBERING / Cluster D3, is captured in ``roles.py``, not
33+
here.)
2334
2435
Each axis is independent: a role may carry a captured size but no captured font
2536
(or vice versa). Only a clear DOMINANT is recorded per axis (at least
26-
:data:`_MIN_RUNS` explicit values and a winner covering at least
27-
:data:`_MIN_DOMINANCE` of them), with its dominance stored as a per-axis confidence
28-
(``confidence`` for font, ``size_confidence`` for size, ``color_confidence`` for
29-
color). Capture is deterministic (model-free).
37+
:data:`~brandkit.common.typography.MIN_RUNS` explicit values and a winner covering
38+
at least :data:`~brandkit.common.typography.MIN_DOMINANCE` of them), with its
39+
dominance stored as a per-axis confidence (``confidence`` for font,
40+
``size_confidence`` for size, ``color_confidence`` for color). Capture is
41+
deterministic (model-free).
3042
3143
The brand guarantee is preserved: every captured value is a FACT observed in the
3244
template, stored in the profile, applied only via the resolver, and re-validated

skills/brand-docx/SKILL.md

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,17 +19,23 @@ write JSON or run shell commands. The agent converts the user's content into an
1919
IntermediateDocument, invokes the internal engine, verifies the output, and
2020
returns the generated `.docx`.
2121

22-
## The four verbs
22+
## The seven verbs: three deterministic + four model-assisted
2323

2424
Every brand skill (`brand-docx`, `brand-pptx`, `brand-xlsx`) implements the same
25-
contract: **extract / comprehend / verify / generate**.
25+
contract. The deterministic core is **extract / verify / generate**; on top of it
26+
sit the optional learning verbs **comprehend / learn / propose-overrides /
27+
refine**, each fail-closed (the engine validates every proposal and authors every
28+
value).
2629

2730
| Verb | Input | Output |
2831
|---|---|---|
2932
| **extract** | a company `.docx` template | a reusable Brand Profile |
3033
| **comprehend** *(optional, model-driven)* | a saved profile + a model-authored `comprehension.json` | the profile with a validated, cached `comprehension` block |
3134
| **verify** | a saved Brand Profile | QA findings + a verdict |
3235
| **generate** | content (an IntermediateDocument) + a profile | a new on-brand `.docx` |
36+
| **learn** *(deterministic distillation)* | the profile's cross-run generation history | recurring QA findings distilled into shell-frozen overrides, advisory until `--accept` |
37+
| **propose-overrides** *(model-driven)* | the recurring remainder `learn` could not bind + a model-authored proposal | shell-backed corrections through the same fail-closed sink, advisory until `--accept` |
38+
| **refine** | end-of-generation user feedback (text or a screenshot) as a `refinement.json` delta | the existing comprehension overlaid for FUTURE generations, advisory until `--accept` |
3339

3440
`comprehend` is **optional**: `generate` works on the deterministic profile alone.
3541
When a current comprehension is present, `generate` additionally reconciles the

skills/brand-pptx/SKILL.md

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,17 +19,23 @@ the deck they want; the agent converts that request into an IntermediateDocument
1919
uses the internal Python engine, verifies the output, and returns the generated
2020
`.pptx`.
2121

22-
## The four verbs
22+
## The seven verbs: three deterministic + four model-assisted
2323

2424
Every brand skill (`brand-docx`, `brand-pptx`, `brand-xlsx`) implements the same
25-
contract: **extract / comprehend / verify / generate**.
25+
contract. The deterministic core is **extract / verify / generate**; on top of it
26+
sit the optional learning verbs **comprehend / learn / propose-overrides /
27+
refine**, each fail-closed (the engine validates every proposal and authors every
28+
value).
2629

2730
| Verb | Input | Output |
2831
|---|---|---|
2932
| **extract** | a company `.pptx` template | a reusable Brand Profile |
3033
| **comprehend** *(optional, model-driven)* | a saved profile + a model-authored `comprehension.json` | the profile with a validated, cached `comprehension` block |
3134
| **verify** | a saved Brand Profile | QA findings + a verdict |
3235
| **generate** | content (an IntermediateDocument) + a profile | a new on-brand `.pptx` |
36+
| **learn** *(deterministic distillation)* | the profile's cross-run generation history | recurring QA findings distilled into shell-frozen overrides, advisory until `--accept` |
37+
| **propose-overrides** *(model-driven)* | the recurring remainder `learn` could not bind + a model-authored proposal | shell-backed corrections through the same fail-closed sink, advisory until `--accept` |
38+
| **refine** | end-of-generation user feedback (text or a screenshot) as a `refinement.json` delta | the existing comprehension overlaid for FUTURE generations, advisory until `--accept` |
3339

3440
`comprehend` is **optional**: `generate` works on the deterministic profile alone.
3541
See [reference/comprehension.md](reference/comprehension.md) for the full step.

0 commit comments

Comments
 (0)