feat: official-document frames, templates and table styling (Phase 2.4, GONG-03) - #133
Merged
Conversation
Adds crates/hwp-convert/src/frames.rs (FrameFields, parse_field, parse_frame_fields, leading_frames) as a sibling module to from_markdown.rs (D-10), building the 두문 block as one Table control in an anchor paragraph (D-02), the same shape table_paragraph() uses for GFM tables. Splices it in front of the body before inject_section_controls() runs, proving the architectural bet this phase depends on: a 두문 table can legally become paragraph 0 and still receive secd/cold/pgnp. Wires all five frame flags (--doc-head/--doc-foot/--notice-head/ --notice-foot/--press-head) onto `hwp new`, threaded through NewOptions::with_frames() so the CLI and (later) MCP share one validator. Only doc_head/doc_foot have real key allowlists in this plan; the other three refuse every key until plan 02 wires them, which keeps "unknown key" fail-closed rather than accept-and-ignore. crates/hwp-cli/tests/frames.rs mirrors tests/skill_templates.rs's Command-driven harness and adds the block-text-sequence helper later tasks in this plan build on (D-03).
… guard Adds trailing_frames() to frames.rs: the optional 끝. guard paragraph (idempotent by construction -- it inspects the last non-empty body paragraph rather than relying on a marker) followed by the 결문 table. Row order is 발신명의 -> 결재 -> 협조 -> 시행/접수 -> 주소/홈페이지 -> 연락처; 결재/협조 are always-emitted placeholder rows on separate lines, never a filled multi-column approval grid (D-04, per korean-official-format.md 5 part 8 -- the approval system renders that grid, templates hold placeholders only). 발신명의 gets its own centered ParaShape and 22pt bold CharShape, cloned from the existing base shapes (index 0) to keep the shade/ shadow defaults that avoid the historical "black bar" bug, and appended via format.rs's find_or_insert/find_or_insert_para (widened to pub(crate)) so re-applying the same --doc-foot never grows either shape table. Ordering fix found while wiring this in: official::apply_profile() overwrites every char shape's base_size by table position, so the 22pt bold shape has to be allocated AFTER apply_profile runs or it gets clobbered back to the profile's body size. Moved the trailing-frames splice below the apply_profile call in from_markdown_inner (caught by doc_foot_last_block_is_table_with_centered_bold_발신명의 failing with base_size=1200 instead of 2200). Extends crates/hwp-cli/tests/frames.rs with the plan's <behavior> cases, including the two-run byte-equality check for D-08-style idempotence.
Adds criterion_1_full_block_order_두문_to_연락처, driving `hwp new` with a complete --doc-head/--doc-foot field set plus a real markdown body and asserting the full 두문(기관명, 수신, 경유) -> 본문 -> 끝. -> 발신명의 -> 결재 -> 협조 -> 시행/접수 -> 연락처 sequence as strictly increasing block indices (D-03: block order over paragraphs and table controls together, not paragraph order or adjacency). Also asserts the two regression guards this suite is responsible for on the framed output: `hwp lint` stays silent and `hwp validate` exits 0. Confirmed separately (not part of this test file): `cargo test -p hwp-cli --test official_presets` stays green, proving Phase 2.2's pgnp/eight-level-numbering/apply_profile assertions survive a table at paragraph 0; `cargo test -p hwp5 --test identity` and `cargo test -p hwp-cli --test cli_reference` also green.
Wires the notice_head/notice_foot/press_head key sets in frames.rs, extending leading_frames/trailing_frames to build the 공고 head (기관명, 제NNNN호 with double-wrap guard), the 공고 foot (공고일자, 발신명의 reusing the centered/22pt/ bold shape), and the 보도자료 head box (title, 기관명, 보도시점/배포일, 담당 contact line). Every frame stays a single table control (D-02), row order follows the shipped templates, and unknown keys still fail closed. Extends crates/hwp-cli/tests/frames.rs with the <behavior> integration cases and updates the English/Korean CLI help text for the three flags now that their key sets are real.
Adds hwp_convert::compatibility_warnings(fields, preset), surfaced through the existing import_warnings channel in `hwp new`. Warns once per offending frame flag (not per k=v pair) when the resolved --preset doesn't belong to that flag's document class, or when no --preset was given at all. The document is still written either way, and the message text never contains the "계약 위반" substring the --strict HTML-contract filter selects on, so a mismatched combination cannot trip that unrelated gate.
- New crates/hwp-convert/src/style.rs module, exported from lib.rs - display_width counts a Hangul syllable (and other wide-range chars) as 2 half-width columns, everything else as 1; total function, no panic path - Covers Hangul Jamo/Compat Jamo, CJK punctuation/ideographs/Ext-A, enclosed CJK letters, Hiragana/Katakana, fullwidth forms - Unit tests for every behavior bullet in 02.4-03-PLAN.md task 1
…D-08) - New style::style_table(): header-row shading (#F2F2F2) + centering, narrow- column (display width <= 8) centering, and content-proportional column widths with a 3400-HWPUNIT floor and a 1:4 label:value special case for 2-column tables. Integer arithmetic only, value-deduped shape allocation (D-08 purity/idempotence). - from_markdown.rs::table_paragraph no longer computes BODY_WIDTH / cols; it delegates to style_table for every GFM table (D-07: every table under a preset, header detection free since GFM tables always have row 0). - Builder gains extra_border_fills (staged like extra_para_shapes) and BASE_BORDER_FILLS, merged into header.border_fills alongside the existing para/char-shape merge. - Extracted from_markdown.rs::table_cell_para_shape() (the base para shape index 0 already used for table cells) so style_table can clone it as the base for the centered ParaShape without needing the fully-assembled header, which does not exist yet while table_paragraph runs. - csv.rs::cell_text made pub(crate) for reuse (avoids a second cell-text extractor). - Updated a pre-existing test (개조식_기호_예외_경로) whose 1-column table now legitimately gets D-07 centering; the assertion now checks cells never pick up the □/○ outdent shape instead of asserting para_shape==0. cargo test -p hwp-convert (267 passed), cargo test -p hwp5 --test identity, cargo test -p hwp-cli --test frames/official_presets/cli_reference/skill_templates all green.
- Header row shaded + centered, body row untouched when wide - Column widths proportional to per-column display-width weight - Min column width floor (3400 HWPUNIT) for a single-character column - Rounding remainder lands on the last column, sum stays exact - Two-column label:value special case lands on the fixed 1:4 ratio - Narrow-column (<=8) centering boundary: 8 centers, 9 does not - Equal weights keep an even split - Calling style_table twice changes neither cell values nor shape/fill vector lengths (the cheap half of D-08's idempotence guard) - All six OfficialPreset variants get header styling via from_markdown_report cargo test -p hwp-convert style_table (9 passed), full crate suite (276 passed), clippy -D warnings clean, cargo fmt --check clean.
Adds hwp_convert::style::style_tables, a document-wide walker that styles every eligible table already in a document through the SAME style_table function markdown import uses, so hwp edit --style-tables and import-time styling can never drift. - D-11: single-column tables (frame blocks) are skipped, so 두문/결문/공고/ 보도 frame rows never get mistaken for a header row. - style_table gains a char_shapes parameter so header runs get the bold variant of their CURRENT CharShape on the edit path (import-time bold is already free via tb.in_head). - find_or_insert_border_fill/find_or_insert_para_ignoring_tail match ignoring a hwp5 round trip's writer-materialized tail bytes, so re-styling an already-styled .hwp document recognizes the existing entry instead of appending a duplicate on every application (D-08). - style_tables clears line_segs only on cells whose styled value actually changed, not unconditionally - an unconditional clear made an otherwise no-op reapplication still look "changed" to the hwp5 patch writer and churn the CFB container's sector layout. - New hwp edit --style-tables <preset> flag (KO overlay row added, manual regenerated).
crates/hwp-cli/tests/style_tables_idempotence.rs drives the real hwp binary end to end: write once, style into a second file, style that into a third, assert the second and third are byte-identical, for both the .hwp and the .hwpx writer. Also covers an already-preset-styled document, a table nested inside a table cell, and a framed document where the frame tables must stay untouched while the body table is still a considered target. Verified the byte-comparison actually catches a regression, not just that it passes: temporarily perturbed style_tables' para_shape_base/border_fill_base from the fixed 0 to a length-derived value (simulating a shifting-base bug), confirmed 3 of the 5 tests went red, then reverted.
…l tree hwp new --template <slug|alias> resolves against the existing SKILL_FILES table (no filesystem path built from the argument, no second embedding) and feeds the same markdown-import path --from already uses. --list-templates prints all eight slugs with their Korean aliases and needs no -o. --template is refused together with --from or any frame flag, per D-05: templates already carry their own 두문/결문 (Phase 2.1 D-19), so combining them with generate-from-data frame flags would double the frames. output on `hwp new` becomes Option<PathBuf> with required_unless_present = "list_templates" so listing works without -o.
…gh --template Four of the eight CASES rows bound plan/minutes to `report` and notice/press to the deprecated `gian` alias, so criterion 3 and the lint-silence gate were vacuous for four of the six canonical profiles. Rebind per skills/hwp/official-documents.md: gian-internal/gian-external/gongmun-basic -> official, report -> report, plan -> plan, minutes -> minutes, notice -> notice, press -> press. Switch the harness from `--from <path>` to `--template <slug>` so the smoke gate exercises the flag this phase ships. Template source is still read from the repo for the source_slots comparison.
Extends tests/skill_templates.rs with the rest of GONG-03/TMPL-01 criterion 3: --list-templates names exactly eight slugs matching the smoke table, roman numerals in a template source survive as U+2160 (never an ASCII "I." substitute), minutes carries all nine 공공기록물 관리에 관한 법률 시행령 제18조 statutory elements, and hwp lint stays silent on every embedded template --template resolves to (assert zero findings, not a specific message, so a new rule fails loudly here rather than being tolerated).
- tool_new gains doc_head/doc_foot/notice_head/notice_foot/press_head (arrays of
{key, value}) and template, decoded through the SAME parse_frame_fields validator
and skill::template_file lookup the CLI uses, so errors are identical on both
surfaces. D-05 (template mutually exclusive with markdown/json and every frame
argument) is enforced at the MCP boundary too.
- tool_edit gains style_tables, parsed via OfficialPreset::parse and routed to the
same TypedEditOperation::StyleTables the CLI --style-tables flag already builds;
stays silent on stdout like every other MCP mutation path.
- Both tools/list input schemas describe every new argument (D-09: the schema is
the discovery surface).
- Tests: new-key schema exposure, frame-argument parity with the CLI byte for
byte, unknown frame key produces the identical Korean error as the CLI, D-05
refusal for template+frame and template+markdown, and style_tables producing
the same styling change the CLI operation does.
…guide Phase 2.4 built native --doc-head/--doc-foot/--notice-head/--notice-foot/ --press-head frames; the guide still told authors to splice an HTML fragment through hwp fill --set to fake a centered 22pt bold 발신명의. Replace that recipe with the native --doc-foot path in both official-documents.md and its Korean mirror (same commit, matching H2/H3 structure). Also updates the Korean alias table intro (--template ships now, so "will accept" / the --from interim workaround are both false) and the §2 recipe intro (two authoring paths exist: --template direct, or --from + hwp fill). Ticks GN-4/GN-5/GN-6 as resolved in phase 2.4 in 12-feature-gaps.md and its Korean mirror.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fad4e89ab5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The note read `gian은 gongmun의 별칭입니다`, steering a caller off one alias and
onto another. `official` is the canonical key: it is what
`OfficialPreset::name()` returns, what the `--preset` help lists, and what
`docs/manual/cli-reference{,.ko}.md` advertise. `gongmun` is a second alias
beside `gian`.
Phase 2.3 D-03 asserted "gongmun is the official-document preset name" while
the same decision also recorded "no preset key rename", so `official` stayed
canonical and the premise the wording rested on never became true. Raised on
#132 and parked in Phase 2.4's CONTEXT open questions; this closes it by
correcting the note rather than renaming the key.
The binary-level test that pins the string moves with it.
…ailure Codex found two defects on #133. Both are real, both are fixed here, and verifying them turned up two more problems that are filed rather than fixed. **Publish gate.** Re-applying `--style-tables` to an already-styled document failed outright ("적용되지 않은 편집 요청이 있습니다"). Zero edits was indistinguishable from "no table matched", so the second of two identical runs errored instead of being the no-op the flag's own help advertises. `style_tables` now returns `(eligible, changed)`: only zero eligible tables is an unapplied request; zero changes among eligible tables publishes unmodified. **Change reporting.** `style_table` returned `true` for every table with at least one column rather than only for tables it changed. The walker gates cache invalidation on that value, including clearing a raw-backed generic's `hwpx_raw_xml` — the only lossless serialization source for an `hp:container`, without which the HWPX writer reports OpaqueControlUnrepresentable. It now snapshots the cells and compares, so a no-op re-application destroys nothing. **Merged cell width.** A horizontally merged cell was assigned only its starting column's width while the unmerged body row still occupied every column it spanned. Width is now the sum over `col..col + col_span`. Each fix carries a regression that was confirmed to fail when that fix alone is reverted. The first attempt at this was a single integration test, which turned out to exercise only the publish gate — reverting either of the other two left it green — so the other two got their own unit tests. **Two problems found while verifying, filed not fixed.** On the real HWPX used for the container regression, `--verify` fails an edit/re-read semantic-hash comparison for reasons unrelated to styling (#135; its ten containers hold no tables at all), and the HWPX writer is not byte-deterministic on that foreign package — two runs over the *same* input differ in ZIP container metadata while every extracted entry is identical (#136). Because of #136, the container regression asserts entry-by-entry content equality rather than byte equality. Byte equality across re-application is still asserted for generated documents, where the writer is deterministic. Conflating the two would have either weakened the strong claim or made the weak one permanently flaky.
`hwp edit --style-tables` promises byte stability across re-application (D-08), but the patch writer stamped the wall clock onto every entry it rewrote. Two identical edits a second apart therefore produced different bytes. Untouched entries are raw-copied and keep the source package's timestamps; only the transformed ones were affected, and `zip`'s default options record "now". `write/mod.rs` already pins the DOS ZIP epoch for exactly this reason and its comment says the patch path is independent because it raw-copies. That is true of copied entries and false of rewritten ones. This is the root cause behind three symptoms seen while reviewing #133: - Windows CI failed `style_tables_byte_stable_on_second_application_both_writers` where macOS passed. The two runs simply landed in different seconds on the slower runner; the assertion was right and the writer was wrong. - The HWPX writer looked non-deterministic on a foreign package (two runs over the same input differing in ZIP container metadata). Same clock, same cause. Closes #136. - The opaque-container regression had been weakened to compare entry contents instead of bytes to work around that. It compares bytes again, and now also asserts the FIRST output is already a fixed point. Verified with a deliberate two-second gap between runs, on both a generated document and the real HWPX with ten `hp:container` elements: identical bytes in every case.
Merged
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Phase 2.4 (GONG-03 = TMPL-01 + STYL-01), closing GN-4, GN-5 and GN-6 of
docs/design/12-feature-gaps.md§14. Fifteen commits across six plans, executed wave by wave withthe orchestrator re-verifying each wave before dispatching the next.
What ships
Frames (GN-4).
hwp new --doc-head k=v --doc-foot k=v --notice-head --notice-foot --press-head,all repeatable, building 두문/결문 and the 공고/보도 frames. New sibling module
crates/hwp-convert/src/frames.rs.Templates (GN-5).
hwp new --template <slug|한국어 별칭>and--list-templatesover the eightinclude_str!-embedded skeletons.--templateand frame flags are mutually exclusive.Table styling (GN-6). Header row shaded
#F2F2F2, bold and centred, with content-proportionalcolumn widths, applied at markdown import and to existing files via
hwp edit --style-tables <preset>.New
crates/hwp-convert/src/style.rswithdisplay_width,style_tableand thestyle_tableswalker.MCP.
hwp_newandhwp_editinput schemas extended. Tool count stays 17 — no tool was added,and
tests/cli_surface.rsstill asserts 17.Three defects found during execution
None of these were reasoned out; a failing test surfaced each one first.
apply_profile()overwrites char-shapebase_sizeby table position. The 22pt bold 발신명의silently became 1200 when its shape was allocated before
apply_profileran. Fixed by reorderingthe splice in
from_markdown_inner.BorderFill/ParaShapetail bytes defeat strict-equality dedup on a hwp5round-trip, so a second
--style-tablespass allocated duplicates. Fixed with tail-tolerantfind_or_insert_*.line_segs.clear()made a true no-op look changed to the hwp5 patch writer,churning the CFB container layout.
Defects 2 and 3 are invisible to
hwp5 --test identity, which operates at the record-stream level.Only running the real binary twice and comparing raw file bytes finds them. This is what D-08
bought: idempotence by construction, not by detection. Had
--style-tablesused an"already styled" marker, both bugs would have hidden behind it permanently.
Decisions worth knowing (full text in the phase CONTEXT)
hold placeholders; 결재/협조 are emitted as placeholder rows with 협조 on its own row.
--style-tablesis byte-stable by construction.style_tablereads only the table's owncells, header-row count and total width — no clock, no counter, no global, no document state.
without the skip
--style-tableswould shade 발신명의 grey on top of its 22pt bold centred shape.tests/skill_templates.rsrebound: 기안문→official, 보고서→report, 계획서→plan,회의록→
minutes, 공고문→notice, 보도자료→press, and the deprecatedgianalias dropped.Before this, four of the six profiles were never exercised through the template path and the
lint-silence gate was vacuous for them.
Verification
Gates run individually (a single
scripts/check.shinvocation kept getting interrupted on thishost, so each stage was run and captured separately, all under the pinned 1.93.0 toolchain):
cargo +1.93.0 fmt --all --checkcargo +1.93.0 clippy --workspace --all-targets -- -D warningscargo +1.93.0 test --workspacepython3 -m unittest tools/test_pdf_parity.pyscripts/check-structured-corpus.shThe corpus gate passed with its frozen hashes untouched — no corpus file and no
FROZEN_MANIFEST_SHA256was modified on this branch, confirmed by diff.Hancom acceptance: pass on 20/20. Twenty artifacts (frames both extensions, 공고, 보도, six
preset tables, styled/unstyled pair, all eight templates) opened in genuine Hancom Office
12.30.0 build 6446 on macOS 26.6.2 build 25G83; twenty windows, zero modal sheets. Observed
directly, not delegated: 두문 renders as a table at paragraph 0; the criterion-1 block order holds;
발신명의 is centred, larger and bold; 결재 and 협조 are separate rows; and the E1/E2 pair shows the
styling difference unmistakably (E1 unshaded, left-aligned, even widths; E2 shaded, bold, centred,
content-proportional). Twenty
hancom-verification-receipt-v1receipts written and schema-validated(20/20 pairs); private evidence, never committed.
Follow-up after the first review pass
All three items originally recorded as "not fixed" have been handled.
Fixed here (
ceb0fae): thegiandeprecation note now names the canonical key. It readgian은 gongmun의 별칭입니다, steering a caller off one alias and onto another.officialis whatOfficialPreset::name()returns, what the--presethelp lists, and what the generated manualadvertises. Phase 2.3 D-03 asserted "gongmun is the official-document preset name" while the same
decision declined the rename, so the premise never became true. Closes the question raised on #132.
The binary-level test pinning the string moves with it.
Fixed in the planning tree:
REQUIREMENTS.mdcheckboxes. Two causes, not one. The milestone-2requirements had no checkboxes, and the Requirement/Phase/Status table omitted milestone 2
entirely while the coverage line claimed "20 total (12 milestone 1 + 8 milestone 2)". Checkboxes
added and nine rows filled in;
requirements mark-completenow resolves GONG-01/02/03, LINT-01,TMPL-01, STYL-01 and SKILL-01. SKILL-01 had a checkbox all along and showed the same symptom, so
this dated from Phase 2.1. (
.planning/is gitignored, so this is not in the diff.)Fixed: the A1/A2 fixtures were regenerated with a plain body and re-verified in Hancom;
all twenty receipts refreshed and re-validated (20/20 pairs).
Two findings that came out of regenerating those fixtures
hwp lintearned its keep. My first replacement body used hand-typed가./나.marks. ThePhase 2.3 lint caught them as
struct-item-markand flagged a keyboard~as well. Rewritten as anested ordered list, the source lints clean and the marks render as engine-assigned
가./나.onseparate lines.
A real defect, filed as #134 rather than fixed here.
hwp cat --format markdownemits alevel-2 item as
- 가. 대상— a GFM bullet plus the engine-assigned mark as literal text. Readingthat markdown back hardens
가.into body text and the engine adds its own mark on top:markdown.rs:721-726routes every non-digit marker down this path, which is the whole Hangul halfof the statutory ladder (
가.,가),(가),㉮). The tool's own output fails the tool's ownrule; the lint is right and the exporter is wrong. It belongs to the markdown export path, not to
GN-4/5/6, so it is out of scope for this PR.
Worth knowing:
hwp lintis silent on all eight embedded templates only because none of them nestsan ordered list deep enough to produce a Hangul mark. That silence is luck, not coverage, and it
breaks the moment a template gains a level-2 ordered item.
Second review pass: two Codex findings, and the root cause behind them
Both P1 and P2 were real. Verifying them uncovered a third problem that turned out to explain
several unrelated-looking symptoms at once.
Publish gate. Re-applying
--style-tablesto an already-styled document failed outright:zero edits was indistinguishable from "no table matched", so the second of two identical runs
errored instead of being the no-op the flag advertises.
style_tablesnow returns(eligible, changed).Change reporting (P1).
style_tablereturnedtruefor every table with a column rather thanonly for tables it changed. The walker gates cache invalidation on that, including clearing a
raw-backed generic's
hwpx_raw_xml— the only lossless serialization source for anhp:container. It now snapshots the cells and compares.Merged cell width (P2). A horizontally merged cell got only its starting column's width while
the unmerged body row occupied every column it spanned. Width is now the sum over
col..col + col_span.Root cause: the patch writer stamped the wall clock. Untouched entries are raw-copied and keep
the source package's timestamps, but a rewritten entry is written fresh and
zip's defaultsrecord "now", so two identical edits a second apart produced different bytes.
write/mod.rsalready pins the DOS ZIP epoch for exactly this reason; its comment says the patch path is
independent because it raw-copies, which is true of copied entries and false of rewritten ones.
That one clock explains three symptoms: Windows CI failing the byte-stability test where macOS
passed (different seconds on a slower runner), an apparent writer nondeterminism on foreign
packages (filed and now closed as #136), and the reason I had temporarily weakened the
opaque-container regression to compare entry contents instead of bytes. It compares bytes again
and now asserts the first output is already a fixed point.
A correction to my own work. My first regression for this was a single integration test.
Negative-controlling it showed it exercised only the publish gate — reverting either other fix
left it green — so the change-reporting and merged-cell fixes each got their own unit test. All
now fail when that fix alone is reverted.
Still open, filed not fixed. #135:
--verifyfails an edit/re-read semantic-hash comparisonon the private parity fixture for reasons unrelated to styling (its ten containers hold no tables
at all). The opaque-container regression drives that fixture without
--verifyso the two are notcoupled.
Refs #121
🤖 Generated with Claude Code
https://claude.ai/code/session_011NP1tBbckiBLFFBGBa4bWr