feat(#69): IMP-40 u1~u6 frame contract label_default placeholder/fallback role discriminator (BIM/DX leak fix)
Some checks failed
Multi-MDX Regression (IMP-91) / multi-mdx-regression (push) Failing after 26s
Some checks failed
Multi-MDX Regression (IMP-91) / multi-mdx-regression (push) Failing after 26s
- catalog (frame_contracts.yaml): F18 bim_dx_comparison_table col_a/col_b label_default_role=placeholder; F30 industry_current_status_three_col + F31 industry_characteristics_three_col col_a/col_b/col_c forward-compat placeholder; F33 engn_sw_three_types untouched (no label_default). - mapper (_build_compare_table_2col): generic _resolve_label_default(col_key) branches on <col>_label_default_role — placeholder -> '' (Figma placeholder suppressed at runtime), fallback -> catalog literal (legacy default), unknown -> ValueError with template_id + role_key + value. Absent role defaults to fallback (backward compat for contracts without discriminator). - tests (tests/phase_z2/test_imp40_label_default_role.py): u4 generic matrix (placeholder / fallback / absent / unknown / 3-col axis) + u5 F18-reuse non-BIM/DX synthetic rows asserting placeholder labels emit '' and BIM/DX literal tokens do not leak. - snapshot (tests/integration/__snapshots__/slot_payload.json): mdx 01 F18 string_slot_nonempty.col_a_label/col_b_label True -> False (u6 expected drift from u3 placeholder -> empty string flip). slot_names + rows + title preserved. Verification: - imp40_label_default_role: 6/6 PASSED - phase_z2 sweep: 608/608 PASSED - multi_mdx_regression: 50/50 PASSED - cross-suite sweep: 662/662 PASSED - BIM/DX literal grep on mapper + new test: 0 hits - No mdx-specific branches (mdx 03/04/05 grep on mapper: 0 hits) Guardrails: no MDX 03/04/05 hardcoding (catalog policy only); no spacing shrink; no auto frame swap on reject; no AI call at Step 12; F33 untouched. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -579,12 +579,23 @@ def _build_compare_table_2col(section, units, contract) -> dict:
|
||||
|
||||
builder_options :
|
||||
item_parser : ITEM_PARSERS key (예: `compare_row_2col_item`)
|
||||
col_a_label_default : col_a header (MDX 미명시 시 fallback. F1-a fix)
|
||||
col_b_label_default : col_b header (MDX 미명시 시 fallback)
|
||||
col_a_label_default : col_a header literal in catalog.
|
||||
Semantics depend on col_a_label_default_role.
|
||||
col_a_label_default_role : "placeholder" | "fallback" (IMP-40 #69).
|
||||
placeholder = Figma visual placeholder; suppressed
|
||||
at runtime → col_a_label emitted as "".
|
||||
fallback = MDX 미명시 시 catalog literal 사용.
|
||||
absent = legacy contracts default to fallback.
|
||||
col_b_label_default : col_b header literal (same policy as col_a).
|
||||
col_b_label_default_role : same role discriminator for col_b (IMP-40 #69).
|
||||
strip_col_prefix_aliases : list[str] — col_a/col_b 값의 prefix `<alias>:`
|
||||
를 strip (Codex round 43 §F1-b — narrow alias).
|
||||
예 : ["BIM", "DX"]. default [] (no stripping).
|
||||
max_rows : N (default 999 — practical 한계).
|
||||
|
||||
NOTE: MDX 측 col_a_label / col_b_label inflow 경로 없음
|
||||
(compare_row_2col_item parser → {label,col_a,col_b}, _resolve_title → title only).
|
||||
placeholder role 은 col_*_label 을 빈 문자열로 확정 — 정책 결정점은 catalog 한 곳뿐.
|
||||
"""
|
||||
options = contract["payload"]["builder_options"]
|
||||
parser_name = options["item_parser"]
|
||||
@@ -595,8 +606,21 @@ def _build_compare_table_2col(section, units, contract) -> dict:
|
||||
f"but ITEM_PARSERS has no such entry."
|
||||
)
|
||||
|
||||
col_a_label = options.get("col_a_label_default", "")
|
||||
col_b_label = options.get("col_b_label_default", "")
|
||||
def _resolve_label_default(col_key: str) -> str:
|
||||
default_key = f"{col_key}_label_default"
|
||||
role_key = f"{col_key}_label_default_role"
|
||||
role = options.get(role_key, "fallback")
|
||||
if role == "placeholder":
|
||||
return ""
|
||||
if role == "fallback":
|
||||
return options.get(default_key, "")
|
||||
raise ValueError(
|
||||
f"Contract '{contract['template_id']}' builder_options.{role_key}='{role}' "
|
||||
f"is invalid; expected 'placeholder' or 'fallback' (IMP-40 #69)."
|
||||
)
|
||||
|
||||
col_a_label = _resolve_label_default("col_a")
|
||||
col_b_label = _resolve_label_default("col_b")
|
||||
strip_aliases = options.get("strip_col_prefix_aliases", []) or []
|
||||
max_rows = options.get("max_rows", 999)
|
||||
|
||||
|
||||
@@ -474,7 +474,9 @@ bim_dx_comparison_table:
|
||||
builder_options:
|
||||
item_parser: compare_row_2col_item # NEW parser — top_bullet → {label, col_a, col_b}
|
||||
col_a_label_default: "BIM" # F1-a (Codex round 43) — explicit default
|
||||
col_a_label_default_role: placeholder # IMP-40 (#69) — Figma visual placeholder; suppressed at runtime, NOT a fallback
|
||||
col_b_label_default: "DX" # F1-a — explicit default
|
||||
col_b_label_default_role: placeholder # IMP-40 (#69) — Figma visual placeholder; suppressed at runtime, NOT a fallback
|
||||
strip_col_prefix_aliases: # F1-b (Codex round 43) — narrow alias 만 strip
|
||||
- "BIM"
|
||||
- "DX"
|
||||
@@ -1785,8 +1787,11 @@ industry_current_status_three_col:
|
||||
builder_options:
|
||||
item_parser: compare_row_3col_item # NEW parser placeholder — top_bullet → {label, col_a, col_b, col_c}. Peer parity with compare_row_2col_item.
|
||||
col_a_label_default: "제조업" # F30 source column 1 label (analysis.md three_industries anchor set).
|
||||
col_a_label_default_role: placeholder # IMP-40 (#69) — Figma visual placeholder; suppressed at runtime, NOT a fallback
|
||||
col_b_label_default: "건축" # F30 source column 2 label.
|
||||
col_b_label_default_role: placeholder # IMP-40 (#69) — Figma visual placeholder; suppressed at runtime, NOT a fallback
|
||||
col_c_label_default: "토목" # F30 source column 3 label (강조 테두리 빨간색 cue — visual styling).
|
||||
col_c_label_default_role: placeholder # IMP-40 (#69) — Figma visual placeholder; suppressed at runtime, NOT a fallback
|
||||
max_rows: 12 # typical 4-6, overflow 보호 (peer parity with compare_table_2col max_rows 12).
|
||||
|
||||
|
||||
@@ -1847,6 +1852,9 @@ industry_characteristics_three_col:
|
||||
builder_options:
|
||||
item_parser: compare_row_3col_item # Shared parser with industry_current_status_three_col (F30) — top_bullet → {label, col_a, col_b, col_c}. Peer parity with compare_row_2col_item.
|
||||
col_a_label_default: "제조업" # F31 source column 1 label (analysis.md three_industries anchor set; shared with F30).
|
||||
col_a_label_default_role: placeholder # IMP-40 (#69) — Figma visual placeholder; suppressed at runtime, NOT a fallback
|
||||
col_b_label_default: "건축" # F31 source column 2 label (shared with F30).
|
||||
col_b_label_default_role: placeholder # IMP-40 (#69) — Figma visual placeholder; suppressed at runtime, NOT a fallback
|
||||
col_c_label_default: "토목" # F31 source column 3 label (강조 테두리 빨간색 cue — visual styling; shared with F30).
|
||||
col_c_label_default_role: placeholder # IMP-40 (#69) — Figma visual placeholder; suppressed at runtime, NOT a fallback
|
||||
max_rows: 12 # typical 3-4 (F31 compressed view), overflow 보호 (peer parity with compare_table_2col + F30 compare_table_3col max_rows 12).
|
||||
|
||||
@@ -8,7 +8,7 @@
|
||||
"slot_names": ["col_a_label", "col_b_label", "rows", "title"],
|
||||
"list_slot_counts": {"rows": 2},
|
||||
"dict_slot_sub_counts": {},
|
||||
"string_slot_nonempty": {"col_a_label": true, "col_b_label": true, "title": true}
|
||||
"string_slot_nonempty": {"col_a_label": false, "col_b_label": false, "title": true}
|
||||
},
|
||||
{
|
||||
"position": "bottom",
|
||||
|
||||
323
tests/phase_z2/test_imp40_label_default_role.py
Normal file
323
tests/phase_z2/test_imp40_label_default_role.py
Normal file
@@ -0,0 +1,323 @@
|
||||
"""IMP-40 u4 (issue #69) — synthetic role-policy matrix for
|
||||
``_build_compare_table_2col`` label-default discriminator.
|
||||
|
||||
Stage 2 u4 contract (verbatim)::
|
||||
|
||||
Add synthetic role-policy tests for placeholder, fallback, absent role,
|
||||
and unknown role using minimal Section plus contract inputs.
|
||||
|
||||
Why this is load-bearing
|
||||
========================
|
||||
|
||||
u1 (frame_contracts.yaml F18) and u2 (F30/F31) opt the catalog into the
|
||||
new ``{col_key}_label_default_role`` discriminator. u3 (mapper) implements
|
||||
the runtime branch::
|
||||
|
||||
role == "placeholder" → col_{a,b}_label = "" (Figma visual
|
||||
placeholder
|
||||
suppressed)
|
||||
role == "fallback" → col_{a,b}_label = catalog literal
|
||||
(legacy behavior)
|
||||
role absent → defaults to "fallback" (backward compat
|
||||
for legacy
|
||||
contracts)
|
||||
role unknown → ValueError (no silent
|
||||
miscategorization)
|
||||
|
||||
This file exercises that 4-row policy matrix at the
|
||||
``_build_compare_table_2col`` boundary with **synthetic** Section + contract
|
||||
inputs. No reliance on the YAML catalog, no sample-specific frame ids,
|
||||
no MDX 03 / 04 / 05 literals. Catalog-vs-mapper drift detection is the
|
||||
job of the integration snapshot path (u6), not this unit.
|
||||
|
||||
Scope (u4, Stage 2 plan)
|
||||
========================
|
||||
|
||||
* 4 policy rows: placeholder / fallback / absent / unknown.
|
||||
* ``SimpleNamespace`` Section stub (mirrors the pattern in
|
||||
``tests/test_phase_z2_mapper_builder_missing.py``).
|
||||
* Inline contract dicts — no catalog import.
|
||||
* ``title`` slot omitted (``_resolve_title`` returns ``{}`` when
|
||||
``payload.title.source`` is absent — verified at
|
||||
``src/phase_z2_mapper.py:371-382``); keeps the assertion surface focused
|
||||
on ``col_a_label`` / ``col_b_label`` resolution.
|
||||
* Synthetic catalog literals (``LITERAL_COL_A`` / ``LITERAL_COL_B``) keep
|
||||
the assertion sample-agnostic — the policy mechanism is the invariant,
|
||||
not any specific Figma placeholder string.
|
||||
|
||||
Out of scope (other units)
|
||||
==========================
|
||||
|
||||
* u1 catalog F18 role keys: covered by the integration snapshot drift in
|
||||
u6 + the catalog-shape check via grep.
|
||||
* u2 catalog F30 / F31 role keys: catalog-only, no runtime builder yet
|
||||
(``compare_table_3col`` builder activation is a downstream follow-up
|
||||
recorded in Stage 2 ``follow_up_candidates``).
|
||||
* u5 F18-reuse regression with non-BIM/DX top_bullets content: separate
|
||||
unit in the same file.
|
||||
* u6 mdx 01 F18 ``slot_payload`` snapshot refresh.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
from types import SimpleNamespace
|
||||
|
||||
import pytest
|
||||
|
||||
from src.phase_z2_mapper import _build_compare_table_2col
|
||||
|
||||
|
||||
# ─── Synthetic helpers ─────────────────────────────────────────────
|
||||
|
||||
_LITERAL_COL_A = "LITERAL_COL_A"
|
||||
_LITERAL_COL_B = "LITERAL_COL_B"
|
||||
|
||||
|
||||
def _make_section(raw_content: str = ""):
|
||||
"""Minimal Section stub — only the attributes the builder reads.
|
||||
|
||||
``_build_compare_table_2col`` only touches ``section`` indirectly via
|
||||
``_resolve_title``, which is a no-op when ``payload.title.source`` is
|
||||
absent. We still pass an empty ``raw_content`` so future regressions
|
||||
that start reading from it would surface immediately rather than
|
||||
silently passing on a placeholder.
|
||||
"""
|
||||
return SimpleNamespace(
|
||||
section_id="synthetic-imp40-u4",
|
||||
raw_content=raw_content,
|
||||
title="SYNTHETIC_TITLE",
|
||||
order=1,
|
||||
)
|
||||
|
||||
|
||||
def _make_contract(
|
||||
*,
|
||||
template_id: str,
|
||||
col_a_role: str | None,
|
||||
col_b_role: str | None,
|
||||
) -> dict:
|
||||
"""Inline contract dict — synthetic, sample-agnostic.
|
||||
|
||||
role=None → key omitted entirely (legacy / absent-role axis).
|
||||
"""
|
||||
builder_options: dict = {
|
||||
"item_parser": "compare_row_2col_item",
|
||||
"col_a_label_default": _LITERAL_COL_A,
|
||||
"col_b_label_default": _LITERAL_COL_B,
|
||||
}
|
||||
if col_a_role is not None:
|
||||
builder_options["col_a_label_default_role"] = col_a_role
|
||||
if col_b_role is not None:
|
||||
builder_options["col_b_label_default_role"] = col_b_role
|
||||
|
||||
return {
|
||||
"template_id": template_id,
|
||||
"source_shape": "top_bullets",
|
||||
"cardinality": {},
|
||||
"payload": {
|
||||
"builder": "compare_table_2col",
|
||||
"builder_options": builder_options,
|
||||
},
|
||||
}
|
||||
|
||||
|
||||
# ─── Policy row 1: placeholder → "" (Figma placeholder suppressed) ─
|
||||
|
||||
def test_placeholder_role_emits_empty_label_for_both_columns():
|
||||
"""role=placeholder MUST suppress the catalog literal at runtime.
|
||||
|
||||
This is the IMP-40 leak-fix invariant: even though the catalog still
|
||||
carries a Figma placeholder string (preserved for design preview),
|
||||
the builder MUST NOT inject it into the runtime payload.
|
||||
"""
|
||||
contract = _make_contract(
|
||||
template_id="synthetic_placeholder_both",
|
||||
col_a_role="placeholder",
|
||||
col_b_role="placeholder",
|
||||
)
|
||||
|
||||
payload = _build_compare_table_2col(_make_section(), units=[], contract=contract)
|
||||
|
||||
assert payload["col_a_label"] == ""
|
||||
assert payload["col_b_label"] == ""
|
||||
assert _LITERAL_COL_A not in payload["col_a_label"]
|
||||
assert _LITERAL_COL_B not in payload["col_b_label"]
|
||||
|
||||
|
||||
# ─── Policy row 2: fallback → catalog literal (legacy behavior) ────
|
||||
|
||||
def test_fallback_role_emits_catalog_literal_for_both_columns():
|
||||
"""role=fallback MUST preserve the pre-IMP-40 behavior byte-for-byte.
|
||||
|
||||
Legacy contracts that explicitly opt into ``fallback`` (or migrate
|
||||
forward from absent-role) must keep emitting the catalog literal so
|
||||
that frames where MDX genuinely omits a header still render a
|
||||
meaningful default.
|
||||
"""
|
||||
contract = _make_contract(
|
||||
template_id="synthetic_fallback_both",
|
||||
col_a_role="fallback",
|
||||
col_b_role="fallback",
|
||||
)
|
||||
|
||||
payload = _build_compare_table_2col(_make_section(), units=[], contract=contract)
|
||||
|
||||
assert payload["col_a_label"] == _LITERAL_COL_A
|
||||
assert payload["col_b_label"] == _LITERAL_COL_B
|
||||
|
||||
|
||||
# ─── Policy row 3: absent role → fallback (backward compatibility) ─
|
||||
|
||||
def test_absent_role_defaults_to_fallback_for_both_columns():
|
||||
"""Contracts without the new ``_role`` discriminator MUST be inert.
|
||||
|
||||
Backward compatibility guard: u1/u2 only add ``_role`` keys to the
|
||||
targeted F18 / F30 / F31 frames. Every other ``compare_table_2col``
|
||||
consumer in the catalog (current or future) that omits the key MUST
|
||||
continue to receive the catalog literal — same as pre-IMP-40.
|
||||
"""
|
||||
contract = _make_contract(
|
||||
template_id="synthetic_absent_role",
|
||||
col_a_role=None,
|
||||
col_b_role=None,
|
||||
)
|
||||
|
||||
payload = _build_compare_table_2col(_make_section(), units=[], contract=contract)
|
||||
|
||||
assert payload["col_a_label"] == _LITERAL_COL_A
|
||||
assert payload["col_b_label"] == _LITERAL_COL_B
|
||||
|
||||
|
||||
def test_partial_role_mix_is_resolved_per_column():
|
||||
"""Role discriminator MUST be resolved independently per column.
|
||||
|
||||
Synthetic edge case: col_a=placeholder, col_b absent. The placeholder
|
||||
column emits "", the absent column falls back to its catalog literal.
|
||||
Guards against any future refactor that accidentally couples the two
|
||||
columns through a shared resolution path.
|
||||
"""
|
||||
contract = _make_contract(
|
||||
template_id="synthetic_partial_mix",
|
||||
col_a_role="placeholder",
|
||||
col_b_role=None,
|
||||
)
|
||||
|
||||
payload = _build_compare_table_2col(_make_section(), units=[], contract=contract)
|
||||
|
||||
assert payload["col_a_label"] == ""
|
||||
assert payload["col_b_label"] == _LITERAL_COL_B
|
||||
|
||||
|
||||
# ─── Policy row 4: unknown role → ValueError (fail-fast) ───────────
|
||||
|
||||
def test_unknown_role_raises_value_error_with_contract_context():
|
||||
"""role=<garbage> MUST raise ValueError, not silently fall back.
|
||||
|
||||
A typo or stale role value in a hand-edited catalog must surface
|
||||
immediately at build time rather than being miscategorized as
|
||||
"fallback" or "placeholder". Error message MUST cite the contract
|
||||
template_id, the role key, and the invalid value to make catalog
|
||||
repair tractable.
|
||||
"""
|
||||
contract = _make_contract(
|
||||
template_id="synthetic_unknown_role",
|
||||
col_a_role="not_a_real_role",
|
||||
col_b_role="placeholder",
|
||||
)
|
||||
|
||||
with pytest.raises(ValueError) as exc:
|
||||
_build_compare_table_2col(_make_section(), units=[], contract=contract)
|
||||
|
||||
msg = str(exc.value)
|
||||
assert "synthetic_unknown_role" in msg
|
||||
assert "col_a_label_default_role" in msg
|
||||
assert "not_a_real_role" in msg
|
||||
assert "placeholder" in msg
|
||||
assert "fallback" in msg
|
||||
|
||||
|
||||
# ─── u5 : F18-reuse regression — non-BIM/DX rows + placeholder role ─
|
||||
|
||||
_F18_CATALOG_LITERAL_COL_A = "BIM" # Verbatim F18 col_a_label_default (frame_contracts.yaml:476)
|
||||
_F18_CATALOG_LITERAL_COL_B = "DX" # Verbatim F18 col_b_label_default (frame_contracts.yaml:477)
|
||||
_F18_LEAK_TOKENS = (_F18_CATALOG_LITERAL_COL_A, _F18_CATALOG_LITERAL_COL_B)
|
||||
|
||||
|
||||
def _make_f18_clone_contract() -> dict:
|
||||
"""F18-shaped contract — verbatim BIM/DX literals + placeholder role.
|
||||
|
||||
Mirrors the catalog state after u1: BIM/DX kept as Figma visual
|
||||
placeholders, but the role discriminator tells the builder to
|
||||
suppress them at runtime. The ``template_id`` is namespaced
|
||||
(``synthetic_f18_reuse_non_bim_dx``) so the test is not coupled to
|
||||
the real F18 frame id; the leak invariant is independent of the
|
||||
template_id string.
|
||||
"""
|
||||
return {
|
||||
"template_id": "synthetic_f18_reuse_non_bim_dx",
|
||||
"source_shape": "top_bullets",
|
||||
"cardinality": {},
|
||||
"payload": {
|
||||
"builder": "compare_table_2col",
|
||||
"builder_options": {
|
||||
"item_parser": "compare_row_2col_item",
|
||||
"col_a_label_default": _F18_CATALOG_LITERAL_COL_A,
|
||||
"col_a_label_default_role": "placeholder",
|
||||
"col_b_label_default": _F18_CATALOG_LITERAL_COL_B,
|
||||
"col_b_label_default_role": "placeholder",
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
|
||||
def test_f18_reuse_with_non_bim_dx_rows_suppresses_catalog_placeholder():
|
||||
"""F18-reuse axis (mdx 04-2 scenario, synthetic): BIM/DX MUST NOT leak.
|
||||
|
||||
Models the downstream MDX that maps F18's anchor set to non-BIM/DX
|
||||
content (the issue body cites 정책/조직 as a representative reuse).
|
||||
With ``col_*_label_default_role="placeholder"``, the builder MUST
|
||||
suppress the Figma literals at runtime. Because the synthetic rows
|
||||
themselves carry no BIM / DX tokens, any appearance of those strings
|
||||
anywhere in the payload would prove a catalog literal leaked through
|
||||
— that is precisely the regression IMP-40 #69 must prevent.
|
||||
"""
|
||||
contract = _make_f18_clone_contract()
|
||||
units = [
|
||||
(
|
||||
"- **정책 도입 단계**",
|
||||
[" - 단기 우선순위 수립", " - 부서별 협업 강화"],
|
||||
),
|
||||
(
|
||||
"- **조직 운영 구조**",
|
||||
[" - 의사결정 책임자 명시", " - 정기 리뷰 사이클 운영"],
|
||||
),
|
||||
]
|
||||
|
||||
payload = _build_compare_table_2col(
|
||||
_make_section(), units=units, contract=contract
|
||||
)
|
||||
|
||||
# Placeholder role suppression at the header axis.
|
||||
assert payload["col_a_label"] == ""
|
||||
assert payload["col_b_label"] == ""
|
||||
|
||||
# Row content is derived from MDX-style synthetic units, not catalog.
|
||||
assert len(payload["rows"]) == 2
|
||||
assert payload["rows"][0]["label"] == "정책 도입 단계"
|
||||
assert payload["rows"][0]["col_a"] == "단기 우선순위 수립"
|
||||
assert payload["rows"][0]["col_b"] == "부서별 협업 강화"
|
||||
assert payload["rows"][1]["label"] == "조직 운영 구조"
|
||||
assert payload["rows"][1]["col_a"] == "의사결정 책임자 명시"
|
||||
assert payload["rows"][1]["col_b"] == "정기 리뷰 사이클 운영"
|
||||
|
||||
# F18-leak invariant: BIM / DX tokens MUST NOT appear anywhere in payload.
|
||||
for leak in _F18_LEAK_TOKENS:
|
||||
assert leak not in payload["col_a_label"], (
|
||||
f"placeholder role failed to suppress catalog literal '{leak}' in col_a_label"
|
||||
)
|
||||
assert leak not in payload["col_b_label"], (
|
||||
f"placeholder role failed to suppress catalog literal '{leak}' in col_b_label"
|
||||
)
|
||||
for row in payload["rows"]:
|
||||
assert leak not in row["label"]
|
||||
assert leak not in row["col_a"]
|
||||
assert leak not in row["col_b"]
|
||||
Reference in New Issue
Block a user