fix(frontmatter): a nested key must not substitute for a top-level one
The line-oriented frontmatter grammar exists in three copies, each with the duplication documented at its site: `materialize` reads a path, `structure` needs a character offset, `profiles` returns body lines. All three keyed on `key.strip()`, which discards the indentation that is the only thing telling a nested key from a top-level one. An indented `title:` under a `sources:` block therefore landed in the same flat namespace as the document's own `title:` and, arriving later, won. The failure is substitution, not omission. A dropped value is visible to whoever reads the concept; a substituted one is not -- the document carries a title that looks entirely right and belongs to something else. Because `number` derives from `title` and `parent` derives from `number`, one substitution walks the hierarchy. Measured, not inferred: a document titled `N100.2` with a nested source titled `N200.7` came back as N200.7 with parent N200 instead of N100.2 with parent N100. Measured incidence across the two corpora, denominators stated: `_okf-canonical` @ ad30107, 54 documents with parsable frontmatter, 49 carry a nested key colliding with a top-level name (90.7%); `_okf-upstream` @ 9a15b13, 66 documents, 58 collide (87.9%). The colliding key is `title`, and often `resource` with it -- in `acme_retail/tables/orders.md` the concept's own BigQuery resource pointer was replaced by a nested one. This is a fix that clears observed damage, not a hardening without a witness. The fix refuses indented lines; it does not read them. Block form stays unreadable -- `sources` and `verified` still come back empty -- so D4's flow-form emission rule is untouched and the structured reader is still D1b. Two characterization tests that pinned the old behaviour now pin the new: the block-list family still DROPS its value, and only the key-space pollution is gone. That family is not otherwise addressed here. Test first, red before the code was touched, with known-positive controls for all three parsers so that a parser returning nothing could not pass. Order: 20260830T000740Z-4733930312-from-.claude Co-Authored-By: Claude <claude-opus-5>
This commit is contained in:
parent
087be0bc87
commit
2337a328d9
6 changed files with 239 additions and 20 deletions
|
|
@ -160,20 +160,26 @@ def test_the_contract_fields_round_trip_in_their_flow_form(tmp_path: Path) -> No
|
|||
assert parse_frontmatter(path) == values
|
||||
|
||||
|
||||
def test_two_nested_block_mappings_sharing_a_key_collide_in_the_scalar_parser(
|
||||
def test_two_nested_block_mappings_sharing_a_key_are_refused_not_flattened(
|
||||
tmp_path: Path,
|
||||
) -> None:
|
||||
"""Measured 2026-07-31, and the reason D4 stops at the flow form.
|
||||
"""Measured 2026-07-31, re-measured 2026-08-31, and still the reason D4
|
||||
stops at the flow form.
|
||||
|
||||
§10.2 presents `executor` and `attester` as nested BLOCK mappings, and both
|
||||
carry a `resource`. The line-oriented parser has no indentation model, so it
|
||||
flattens them into one namespace where the second `resource` overwrites the
|
||||
first: `executor.resource` is lost and `attester.resource` surfaces as a
|
||||
top-level key. No error is raised.
|
||||
carry a `resource`. Neither is READABLE here -- both come back empty, which
|
||||
is exactly why D4 pins emission to the flow form and why reading this shape
|
||||
is D1b's work, not this parser's.
|
||||
|
||||
Pinned rather than fixed. Reading this form needs the structured reader
|
||||
(D1b), and a half-reader that silently drops half a contract is worse than
|
||||
one that never claimed to read it.
|
||||
What the parser no longer does is FLATTEN them. Before order
|
||||
`...4733930312` it had no indentation model at all: both `resource` lines
|
||||
landed in the document's own namespace, the second overwrote the first, and
|
||||
`attester.resource` surfaced as a top-level `resource` that no document
|
||||
declared -- silently, with no error. A dropped value is visible to whoever
|
||||
reads the concept; a substituted one is not.
|
||||
|
||||
So the contract is still half-read, and that is deliberate. It is now
|
||||
half-read by REFUSAL rather than by substitution.
|
||||
"""
|
||||
path = tmp_path / "block-form.md"
|
||||
path.write_text(
|
||||
|
|
@ -192,8 +198,13 @@ def test_two_nested_block_mappings_sharing_a_key_collide_in_the_scalar_parser(
|
|||
|
||||
assert parsed["executor"] == ""
|
||||
assert parsed["attester"] == ""
|
||||
assert parsed["resource"] == "attesters/revenue.py"
|
||||
# The substitution is gone: neither nested `resource` reaches the namespace.
|
||||
assert "resource" not in parsed
|
||||
assert "receipt" not in parsed
|
||||
assert set(parsed) == {"type", "runtime", "executor", "attester"}
|
||||
# Still unreadable, as D4 requires -- neither value survives anywhere.
|
||||
assert "skills/run-on-bq.md" not in parsed.values()
|
||||
assert "attesters/revenue.py" not in parsed.values()
|
||||
|
||||
|
||||
# --- door C surfaces the §10 pointers it imports ---------------------------
|
||||
|
|
|
|||
172
tests/test_frontmatter_nesting.py
Normal file
172
tests/test_frontmatter_nesting.py
Normal file
|
|
@ -0,0 +1,172 @@
|
|||
"""A nested frontmatter key must never substitute for a top-level one.
|
||||
|
||||
This library's line-oriented frontmatter grammar exists in three copies, by
|
||||
design and with the duplication documented at each site: `materialize`
|
||||
reads a path, `structure` needs a character offset, `profiles` returns body
|
||||
lines. All three split a line on its first colon and key the result on
|
||||
`key.strip()` — which discards the indentation that is the ONLY thing
|
||||
distinguishing a nested key from a top-level one. An indented `title:` under
|
||||
a `sources:` block therefore lands in the same flat namespace as the
|
||||
document's own `title:` and, arriving later, wins.
|
||||
|
||||
The failure this closes is not a dropped value but a SUBSTITUTED one. A
|
||||
missing title is visible to whoever reads the concept; a substituted title
|
||||
is not — the document carries a title that looks entirely right and belongs
|
||||
to something else. And because `number` derives from `title` and `parent`
|
||||
derives from `number`, one substitution walks the whole hierarchy. That is
|
||||
`P4` (a concept ID is a path, and a path is a promise) broken in the one
|
||||
property no later run can repair.
|
||||
|
||||
Scope: this is the SUBSTITUTION defect only. The block-list family — a
|
||||
`- item` line carrying no colon and being dropped — is the same family and a
|
||||
different defect, and is deliberately not addressed here. Nor is this a
|
||||
structured YAML reader; that is D1b. Flow form stays the emission rule, and
|
||||
the round-trip tests below pin that the fix does not disturb it.
|
||||
"""
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
from llm_ingestion_okf.materialize import parse_frontmatter
|
||||
from llm_ingestion_okf.profiles import _split_frontmatter as _profiles_split
|
||||
from llm_ingestion_okf.structure import _split_frontmatter as _structure_split
|
||||
from llm_ingestion_okf.structure import derive_document_structure
|
||||
|
||||
# A `sources:` block whose entry carries its own `title:`. The top-level title
|
||||
# and the nested one are both well-formed document titles, and both bear a
|
||||
# number — which is what lets the test measure `number` and `parent` moving
|
||||
# rather than only asserting that they could.
|
||||
NESTED = """\
|
||||
---
|
||||
title: N100.2 Kryss og avkjoersler
|
||||
generated: true
|
||||
source_file: vegnormal.md
|
||||
sources:
|
||||
- resource: https://example.test/bruprosjektering.pdf
|
||||
title: N200.7 Bruprosjektering
|
||||
---
|
||||
|
||||
# Kryss og avkjoersler
|
||||
|
||||
Body text.
|
||||
"""
|
||||
|
||||
# The same document with the nested block removed. Nothing else differs.
|
||||
FLAT = """\
|
||||
---
|
||||
title: N100.2 Kryss og avkjoersler
|
||||
generated: true
|
||||
source_file: vegnormal.md
|
||||
---
|
||||
|
||||
# Kryss og avkjoersler
|
||||
|
||||
Body text.
|
||||
"""
|
||||
|
||||
|
||||
# --- known-positive controls ---------------------------------------------
|
||||
#
|
||||
# Without these, a parser that returned nothing at all would pass every
|
||||
# assertion below: "the nested title did not win" is satisfied by a parser
|
||||
# that reads no title whatsoever. Each control proves the SAME parser, on the
|
||||
# SAME shape, actually finds the top-level key when no nested key competes.
|
||||
|
||||
|
||||
def test_control_materialize_reads_top_level_title(tmp_path: Path) -> None:
|
||||
path = tmp_path / "vegnormal.md"
|
||||
path.write_text(FLAT, encoding="utf-8")
|
||||
assert parse_frontmatter(path)["title"] == "N100.2 Kryss og avkjoersler"
|
||||
|
||||
|
||||
def test_control_structure_reads_top_level_title() -> None:
|
||||
declared, offset = _structure_split(FLAT)
|
||||
assert declared["title"] == "N100.2 Kryss og avkjoersler"
|
||||
assert offset > 0
|
||||
|
||||
|
||||
def test_control_profiles_reads_top_level_title() -> None:
|
||||
head, _body = _profiles_split(FLAT)
|
||||
assert head["title"] == "N100.2 Kryss og avkjoersler"
|
||||
|
||||
|
||||
def test_control_derivation_reads_top_level_title() -> None:
|
||||
structure = derive_document_structure(FLAT, source_file="vegnormal.md")
|
||||
assert structure.title == "N100.2 Kryss og avkjoersler"
|
||||
assert structure.number == "N100.2"
|
||||
assert structure.parent_number == "N100"
|
||||
|
||||
|
||||
# --- the defect, once per parser copy ------------------------------------
|
||||
|
||||
|
||||
def test_nested_title_does_not_substitute_in_materialize(tmp_path: Path) -> None:
|
||||
path = tmp_path / "vegnormal.md"
|
||||
path.write_text(NESTED, encoding="utf-8")
|
||||
assert parse_frontmatter(path)["title"] == "N100.2 Kryss og avkjoersler"
|
||||
|
||||
|
||||
def test_nested_title_does_not_substitute_in_structure() -> None:
|
||||
declared, _offset = _structure_split(NESTED)
|
||||
assert declared["title"] == "N100.2 Kryss og avkjoersler"
|
||||
|
||||
|
||||
def test_nested_title_does_not_substitute_in_profiles() -> None:
|
||||
head, _body = _profiles_split(NESTED)
|
||||
assert head["title"] == "N100.2 Kryss og avkjoersler"
|
||||
|
||||
|
||||
# --- the propagation the order asks to be MEASURED, not assumed ----------
|
||||
|
||||
|
||||
def test_substituted_title_moves_number_and_parent() -> None:
|
||||
"""`title` -> `number` -> `parent`, all three, on a real derivation.
|
||||
|
||||
The source filename carries no number, so `number` falls through to the
|
||||
title — which is precisely the path a substituted title travels. This
|
||||
asserts the whole chain rather than the first link, because the order's
|
||||
question was whether `number` and `parent` move in PRACTICE or only in
|
||||
theory.
|
||||
"""
|
||||
structure = derive_document_structure(NESTED, source_file="vegnormal.md")
|
||||
assert structure.title == "N100.2 Kryss og avkjoersler"
|
||||
assert structure.number == "N100.2"
|
||||
assert structure.parent_number == "N100"
|
||||
|
||||
|
||||
def test_nested_key_does_not_invent_a_top_level_field() -> None:
|
||||
"""A nested `resource:` must not appear as a top-level `resource`.
|
||||
|
||||
The substitution has a quieter twin: a nested key with NO top-level
|
||||
counterpart is not overwriting anything, it is fabricating a field the
|
||||
document never declared. `derive_document_structure` exposes `declared`
|
||||
directly, so this pins the namespace itself and not one lucky key.
|
||||
"""
|
||||
structure = derive_document_structure(NESTED, source_file="vegnormal.md")
|
||||
assert "resource" not in structure.declared
|
||||
assert set(structure.declared) == {"title", "generated", "source_file", "sources"}
|
||||
|
||||
|
||||
# --- the emission rule the fix must not disturb --------------------------
|
||||
|
||||
|
||||
def test_flow_form_still_round_trips(tmp_path: Path) -> None:
|
||||
"""Flow form is a single top-level line and must be untouched by the fix.
|
||||
|
||||
This library pins its own emission to flow form precisely because the
|
||||
line-oriented parser round-trips it as one opaque string. A fix that
|
||||
keyed on anything other than indentation could break that, so it is
|
||||
pinned here rather than assumed.
|
||||
"""
|
||||
flow = (
|
||||
"---\n"
|
||||
"title: N100.2 Kryss og avkjoersler\n"
|
||||
"generated: { by: process:okf-ingest, at: 2026-08-31T00:00:00Z }\n"
|
||||
"sources: [ a.pdf, b.pdf ]\n"
|
||||
"---\n\nBody.\n"
|
||||
)
|
||||
path = tmp_path / "flow.md"
|
||||
path.write_text(flow, encoding="utf-8")
|
||||
parsed = parse_frontmatter(path)
|
||||
assert parsed["generated"] == "{ by: process:okf-ingest, at: 2026-08-31T00:00:00Z }"
|
||||
assert parsed["sources"] == "[ a.pdf, b.pdf ]"
|
||||
assert parsed["title"] == "N100.2 Kryss og avkjoersler"
|
||||
|
|
@ -65,15 +65,23 @@ def test_an_inline_flow_mapping_survives_the_scalar_parser_verbatim(tmp_path: Pa
|
|||
assert parse_frontmatter(path) == values
|
||||
|
||||
|
||||
def test_a_block_list_pollutes_the_scalar_parsers_key_space(tmp_path: Path) -> None:
|
||||
def test_a_block_list_is_dropped_without_polluting_the_key_space(tmp_path: Path) -> None:
|
||||
"""The measured reason `sources` is emitted as an inline flow sequence.
|
||||
|
||||
Upstream's canonical `sources` is a block list of multi-key mappings. Read
|
||||
through this line-oriented parser, each item line becomes a KEY: the list
|
||||
disappears and `- id` / `resource` appear as frontmatter keys that no
|
||||
document declared. `_is_ingest_owned` reads through this same parser, so
|
||||
emitting the block form would have forced the gate and the parser to be
|
||||
hardened in one step.
|
||||
Upstream's canonical `sources` is a block list of multi-key mappings, and
|
||||
this line-oriented parser still cannot READ one: the list value comes back
|
||||
empty. That is unchanged, and it remains the whole reason this library
|
||||
emits the flow form -- a value it can write is a value it can read back.
|
||||
|
||||
What changed (2026-08-31, order `...4733930312`) is the second, quieter
|
||||
half. The item lines no longer become KEYS. `- id` and `resource` are
|
||||
indented, so they belong to the block above them and are refused rather
|
||||
than flattened into the document's namespace. The distinction matters
|
||||
because `_is_ingest_owned` reads through this same parser: a fabricated
|
||||
top-level key is a fact about the document that no document declared.
|
||||
|
||||
Refusing is not parsing. The block form stays unreadable; it is now
|
||||
unreadable LOUDLY rather than by substitution. Reading it is D1b.
|
||||
"""
|
||||
path = tmp_path / "concept.md"
|
||||
path.write_bytes(
|
||||
|
|
@ -84,8 +92,9 @@ def test_a_block_list_pollutes_the_scalar_parsers_key_space(tmp_path: Path) -> N
|
|||
parsed = parse_frontmatter(path)
|
||||
|
||||
assert parsed["sources"] == ""
|
||||
assert parsed["- id"] == "margin-standard"
|
||||
assert parsed["resource"] == "policies/margin-standard.md"
|
||||
assert "- id" not in parsed
|
||||
assert "resource" not in parsed
|
||||
assert set(parsed) == {"type", "sources"}
|
||||
|
||||
|
||||
def _stamped_concept(generated: str) -> bytes:
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue