feat(profiles): materialize_bundle takes a keyword-only profile (req 6)
`OKF_V0_2` landed in D2 but was unreachable from outside: no door took a profile. This threads one through, keyword-only behind the `*` the signature already carried, so every three-positional call site stays source-compatible — which is what po-claude asked for, and what makes additivity a property of the signature rather than something a consumer measures. Nine sites, not the ~6 STATE claimed. The load-bearing one is the call at materialize.py:378: the CONTENT phase has accepted `profile` since D2, but the call site never passed one, so A-E3/A-E4/A-E5 were all unreachable. The other eight are the disk phase (ownership glob, index name, index maintenance, concept filenames) plus `generated_filename` in manifest.py. `link_in_index` is public and called from all three doors, so it gets `*, profile=DEFAULT` rather than having the lookup moved to the call site: doors B and C keep exactly the behaviour they had, and which profile THEY own stays an open question instead of being decided silently by a signature change. Byte-neutrality is proven, not asserted: `OKF_V0_2.paths is DEFAULT.paths` and `.index is DEFAULT.index`, and the golden suite is green. That identity is also why six of the nine sites cannot be proven reachable by any shipped-profile test — no assertion distinguishes two names for one object. A synthetic test-only profile renaming the index and the concept files closes that gap, so a site left on `DEFAULT` fails by name rather than passing quietly. Scope stated rather than glossed: the profile does NOT reach manifest type validation (`manifest.py:198` still reads `DEFAULT.types`; measured equal to `OKF_V0_2.types`, so nothing is hidden today), and `STRICT_V1` is not supported here — its index policy sets three judging fields the materializer does not honour. Both are named in the docstring. No `okf_version` anywhere: that lands once, at D5, when the §12 placement question closes. 550 tests pass (was 542). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tf2BbC8uSRVU4ApQ9NL7QR
This commit is contained in:
parent
ddc8f76446
commit
ed08ac15e9
3 changed files with 270 additions and 18 deletions
|
|
@ -15,7 +15,7 @@ from pathlib import Path
|
|||
from typing import Any, Union
|
||||
|
||||
from .errors import ManifestError
|
||||
from .profiles import DEFAULT
|
||||
from .profiles import DEFAULT, BundleProfile
|
||||
|
||||
_ID_PATTERN = re.compile(r"[a-z0-9][a-z0-9-]*\Z")
|
||||
|
||||
|
|
@ -59,13 +59,17 @@ class Manifest:
|
|||
extractions: tuple[Extraction, ...]
|
||||
|
||||
|
||||
def generated_filename(extraction_id: str) -> str:
|
||||
def generated_filename(extraction_id: str, *, profile: BundleProfile = DEFAULT) -> str:
|
||||
"""The concept filename for an extraction (spec §5).
|
||||
|
||||
The `ingest-` prefix keeps the namespace disjoint from `index.md` and
|
||||
`promoted-verdict-*` (spec §3) for every id the §4 grammar admits.
|
||||
|
||||
The prefix and suffix are the profile's, because the ownership scan globs
|
||||
on the same two values: a name built from one profile and scanned for under
|
||||
another is a file the library cannot recognise as its own.
|
||||
"""
|
||||
return f"{DEFAULT.paths.ingest_prefix}{extraction_id}{DEFAULT.paths.concept_suffix}"
|
||||
return f"{profile.paths.ingest_prefix}{extraction_id}{profile.paths.concept_suffix}"
|
||||
|
||||
|
||||
def load_manifest(path: Path) -> Manifest:
|
||||
|
|
|
|||
|
|
@ -251,7 +251,11 @@ def write_bytes(bundle_dir: Path, name: str, content: str) -> Path:
|
|||
|
||||
|
||||
def _update_index_lines(
|
||||
index_path: Path, removed_targets: set[str], labels_by_target: dict[str, str]
|
||||
index_path: Path,
|
||||
removed_targets: set[str],
|
||||
labels_by_target: dict[str, str],
|
||||
*,
|
||||
profile: BundleProfile = DEFAULT,
|
||||
) -> None:
|
||||
"""§6 maintenance on an EXISTING index: drop managed lines whose target is
|
||||
an ingest file removed in this run; refresh in place a managed label that
|
||||
|
|
@ -265,7 +269,7 @@ def _update_index_lines(
|
|||
for line in lines:
|
||||
content = line.rstrip("\r\n")
|
||||
ending = line[len(content) :]
|
||||
match = DEFAULT.index.link_pattern.match(content)
|
||||
match = profile.index.link_pattern.match(content)
|
||||
if match is not None:
|
||||
target = match.group("target")
|
||||
if target in removed_targets:
|
||||
|
|
@ -273,17 +277,25 @@ def _update_index_lines(
|
|||
continue
|
||||
new_label = labels_by_target.get(target)
|
||||
if new_label is not None and match.group("label") != new_label:
|
||||
line = DEFAULT.index.render_link(new_label, target) + ending
|
||||
line = profile.index.render_link(new_label, target) + ending
|
||||
changed = True
|
||||
updated.append(line)
|
||||
if changed:
|
||||
index_path.write_bytes("".join(updated).encode("utf-8"))
|
||||
|
||||
|
||||
def link_in_index(bundle_dir: Path, target_name: str, label: str) -> None:
|
||||
def link_in_index(
|
||||
bundle_dir: Path, target_name: str, label: str, *, profile: BundleProfile = DEFAULT
|
||||
) -> None:
|
||||
# §6: idempotent by target — a link whose target is already present in
|
||||
# the index is never added twice.
|
||||
index_path = safe_resolve(bundle_dir, DEFAULT.index.name)
|
||||
#
|
||||
# `profile` is keyword-only with a default because this function is public
|
||||
# and called from all three doors (A here, B in inbox.py, C in importer.py).
|
||||
# Doors B and C keep the default, which is the behaviour they already had;
|
||||
# which profile THEY should own is a separate question, and answering it by
|
||||
# changing this signature would have decided it silently.
|
||||
index_path = safe_resolve(bundle_dir, profile.index.name)
|
||||
body = index_path.read_bytes().decode("utf-8")
|
||||
if f"]({target_name})" in body:
|
||||
return
|
||||
|
|
@ -291,7 +303,7 @@ def link_in_index(bundle_dir: Path, target_name: str, label: str) -> None:
|
|||
# bundle_summary first, but Door B has no summary to invent, so its index
|
||||
# starts empty and must not open with a blank line.
|
||||
prefix = body if (body == "" or body.endswith("\n")) else body + "\n"
|
||||
line = DEFAULT.index.render_link(label, target_name)
|
||||
line = profile.index.render_link(label, target_name)
|
||||
index_path.write_bytes(f"{prefix}{line}\n".encode())
|
||||
|
||||
|
||||
|
|
@ -302,6 +314,7 @@ def materialize_bundle(
|
|||
*,
|
||||
allow_network: bool = False,
|
||||
http_get: HttpGet | None = None,
|
||||
profile: BundleProfile = DEFAULT,
|
||||
) -> IngestResult:
|
||||
"""Materialize a manifest's extractions into an OKF bundle (§5).
|
||||
|
||||
|
|
@ -313,6 +326,17 @@ def materialize_bundle(
|
|||
seam (default urllib_get, the only socket path; ignored for `file`/`sql`)
|
||||
so tests run socket-free (§11). Source calls are logged per §8 (which
|
||||
source, when, row count) — never cell contents, never secrets.
|
||||
|
||||
`profile` selects the bundle contract: `DEFAULT` (commons' ingest-spec §5)
|
||||
or `OKF_V0_2`. It is keyword-only behind the `*` the signature already
|
||||
carried, so every three-positional call site stays source-compatible —
|
||||
support for a new upstream version is additive, never a migration. The
|
||||
profile governs the emitted frontmatter, the ownership stamp the collision
|
||||
gate recognises, the concept filenames, and the index; it does NOT reach
|
||||
manifest type validation, which runs against `DEFAULT` (the two policies
|
||||
compare equal today). `STRICT_V1` is not supported here: its index policy
|
||||
sets `per_directory`, `entries_match_directory` and `root_frontmatter`,
|
||||
none of which this materializer honours.
|
||||
"""
|
||||
validate_ingested_at(ingested_at)
|
||||
manifest_file = Path(manifest_path)
|
||||
|
|
@ -376,9 +400,9 @@ def materialize_bundle(
|
|||
"source call: source=%s ingested_at=%s rows=%d", source.id, ingested_at, row_count
|
||||
)
|
||||
content = _render_concept_file(
|
||||
manifest, extraction, body, ingested_at=ingested_at, stamp=stamp
|
||||
manifest, extraction, body, ingested_at=ingested_at, stamp=stamp, profile=profile
|
||||
)
|
||||
staged.append((generated_filename(extraction.id), content))
|
||||
staged.append((generated_filename(extraction.id, profile=profile), content))
|
||||
|
||||
# Disk phase.
|
||||
bundle = Path(bundle_dir)
|
||||
|
|
@ -389,8 +413,9 @@ def materialize_bundle(
|
|||
# ingest stamp are ours to replace.
|
||||
owned = {
|
||||
path.name
|
||||
for path in sorted(bundle.glob(f"*{DEFAULT.paths.concept_suffix}"))
|
||||
if path.name != DEFAULT.index.name and _is_ingest_owned(path, manifest_file.stem)
|
||||
for path in sorted(bundle.glob(f"*{profile.paths.concept_suffix}"))
|
||||
if path.name != profile.index.name
|
||||
and _is_ingest_owned(path, manifest_file.stem, profile=profile)
|
||||
}
|
||||
# §3 collision gate — BEFORE any mutation: a staged filename occupied by
|
||||
# a file WITHOUT the stamp is curated content; never overwrite it.
|
||||
|
|
@ -409,16 +434,22 @@ def materialize_bundle(
|
|||
|
||||
# §6 index generation — the last disk mutation. A fresh index gets
|
||||
# bundle_summary as its body; links are appended in extraction order.
|
||||
index_path = bundle / DEFAULT.index.name
|
||||
index_path = bundle / profile.index.name
|
||||
labels_by_target = {
|
||||
generated_filename(extraction.id): extraction.title for extraction in manifest.extractions
|
||||
generated_filename(extraction.id, profile=profile): extraction.title
|
||||
for extraction in manifest.extractions
|
||||
}
|
||||
if not index_path.is_file():
|
||||
write_bytes(bundle, DEFAULT.index.name, manifest.bundle_summary + "\n")
|
||||
write_bytes(bundle, profile.index.name, manifest.bundle_summary + "\n")
|
||||
else:
|
||||
# Links whose target is an ingest-owned file removed this run MUST be
|
||||
# removed; all other links — curated and promoted — are preserved.
|
||||
_update_index_lines(index_path, owned - staged_names, labels_by_target)
|
||||
_update_index_lines(index_path, owned - staged_names, labels_by_target, profile=profile)
|
||||
for extraction in manifest.extractions:
|
||||
link_in_index(bundle, generated_filename(extraction.id), extraction.title)
|
||||
link_in_index(
|
||||
bundle,
|
||||
generated_filename(extraction.id, profile=profile),
|
||||
extraction.title,
|
||||
profile=profile,
|
||||
)
|
||||
return IngestResult(written=written, stamp=stamp)
|
||||
|
|
|
|||
217
tests/test_profile_threading.py
Normal file
217
tests/test_profile_threading.py
Normal file
|
|
@ -0,0 +1,217 @@
|
|||
"""Requirement 6 — `profile` threaded through `materialize_bundle`'s disk phase.
|
||||
|
||||
`OKF_V0_2` landed in D2, but nothing outside the library could ask for it: the
|
||||
content phase already accepted `profile`, while `materialize_bundle` neither
|
||||
took one nor passed one down. The parameter is keyword-only behind the `*` the
|
||||
signature already carried, so every three-positional call site stays
|
||||
source-compatible — which is what the consumer who asked for it needs.
|
||||
|
||||
Two of these tests would be untestable by construction if written only against
|
||||
the shipped profiles: `OKF_V0_2.paths is DEFAULT.paths` and
|
||||
`.index is DEFAULT.index`, so no assertion can tell `profile.index.name` apart
|
||||
from `DEFAULT.index.name` when they are the same object. That identity is what
|
||||
makes the change byte-neutral, and it is also what would let six of the nine
|
||||
threading sites stay hard-coded with every shipped-profile test still green.
|
||||
`_SYNTHETIC` exists to close exactly that gap: it renames the index and the
|
||||
concept files, so a site still reading `DEFAULT` writes to the wrong path and
|
||||
fails. It is a test instrument, never a supported profile.
|
||||
|
||||
What this does NOT claim: the materializer honours `profile.types` (validation
|
||||
still runs against `DEFAULT` in `manifest.py`; both policies compare equal
|
||||
today, so nothing is hidden — but the parameter does not reach it), nor
|
||||
`IndexPolicy`'s judging fields (`per_directory`, `entries_match_directory`,
|
||||
`root_frontmatter`). `STRICT_V1` sets all three, so passing it here produces a
|
||||
bundle that does not meet its own profile. Supported here: `DEFAULT` and
|
||||
`OKF_V0_2`.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
from dataclasses import replace
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
import pytest
|
||||
|
||||
from llm_ingestion_okf.materialize import materialize_bundle
|
||||
from llm_ingestion_okf.profiles import DEFAULT, OKF_V0_2, BundleProfile
|
||||
|
||||
INGESTED_AT = "2026-07-16T12:00:00Z"
|
||||
|
||||
# Renames both halves the disk phase reads: a threading site left on `DEFAULT`
|
||||
# writes `ingest-orders.md` / `index.md` instead, and the assertions below say
|
||||
# so by name rather than by a byte diff that cannot point at the cause.
|
||||
_SYNTHETIC = replace(
|
||||
OKF_V0_2,
|
||||
paths=replace(DEFAULT.paths, ingest_prefix="x-", concept_suffix=".mdx"),
|
||||
index=replace(DEFAULT.index, name="contents.md"),
|
||||
)
|
||||
|
||||
|
||||
def _manifest_data() -> dict[str, Any]:
|
||||
return {
|
||||
"manifest_version": 1,
|
||||
"source": {"type": "file", "id": "catalogue-1", "root": "data"},
|
||||
"bundle_summary": "A test bundle.",
|
||||
"extractions": [
|
||||
{
|
||||
"id": "orders",
|
||||
"title": "Orders",
|
||||
"query": "orders.csv",
|
||||
"okf_type": "dataset",
|
||||
"max_rows": 100,
|
||||
}
|
||||
],
|
||||
}
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def file_setup(tmp_path: Path) -> tuple[Path, Path]:
|
||||
"""A manifest + CSV catalogue in tmp/src, and an empty bundle target path."""
|
||||
src = tmp_path / "src"
|
||||
src.mkdir(parents=True)
|
||||
manifest_path = src / "manifest.json"
|
||||
manifest_path.write_text(json.dumps(_manifest_data()), encoding="utf-8")
|
||||
(src / "data").mkdir()
|
||||
(src / "data" / "orders.csv").write_text("a,b\n1,x\n2,y\n", encoding="utf-8", newline="")
|
||||
return manifest_path, tmp_path / "bundle"
|
||||
|
||||
|
||||
# --- the profile reaches the content phase ---
|
||||
|
||||
|
||||
def test_v0_2_profile_reaches_the_emitted_frontmatter(file_setup: tuple[Path, Path]) -> None:
|
||||
"""`_render_concept_file` took `profile` since D2; the call site did not pass
|
||||
one, so `OKF_V0_2` was unreachable through the only public door.
|
||||
"""
|
||||
manifest_path, bundle = file_setup
|
||||
materialize_bundle(manifest_path, bundle, INGESTED_AT, profile=OKF_V0_2)
|
||||
text = (bundle / "ingest-orders.md").read_text(encoding="utf-8")
|
||||
assert f"generated: {{ by: process:llm-ingestion-okf, at: {INGESTED_AT} }}" in text
|
||||
assert "sources: [{ id: catalogue-1, resource: data }]" in text
|
||||
assert "generated: true" not in text
|
||||
|
||||
|
||||
def test_default_run_is_unchanged_by_the_new_parameter(file_setup: tuple[Path, Path]) -> None:
|
||||
"""Byte-neutrality at the signature: passing `DEFAULT` explicitly and passing
|
||||
nothing must produce the same bundle, or the default is not the default.
|
||||
"""
|
||||
manifest_path, bundle = file_setup
|
||||
materialize_bundle(manifest_path, bundle, INGESTED_AT)
|
||||
implicit = (bundle / "ingest-orders.md").read_bytes()
|
||||
index_implicit = (bundle / "index.md").read_bytes()
|
||||
|
||||
explicit_dir = bundle.parent / "bundle-explicit"
|
||||
materialize_bundle(manifest_path, explicit_dir, INGESTED_AT, profile=DEFAULT)
|
||||
assert (explicit_dir / "ingest-orders.md").read_bytes() == implicit
|
||||
assert (explicit_dir / "index.md").read_bytes() == index_implicit
|
||||
assert b"generated: true" in implicit
|
||||
|
||||
|
||||
def test_profile_is_keyword_only(file_setup: tuple[Path, Path]) -> None:
|
||||
"""The parameter sits behind the `*` the signature already had. A positional
|
||||
fourth argument must not be accepted, or a later reordering would silently
|
||||
rebind existing call sites.
|
||||
"""
|
||||
manifest_path, bundle = file_setup
|
||||
with pytest.raises(TypeError):
|
||||
materialize_bundle(manifest_path, bundle, INGESTED_AT, DEFAULT) # type: ignore[misc]
|
||||
|
||||
|
||||
# --- A-E5: two runs into the SAME directory ---
|
||||
|
||||
|
||||
def test_second_v0_2_run_into_the_same_directory_succeeds(
|
||||
file_setup: tuple[Path, Path],
|
||||
) -> None:
|
||||
"""A-E5, end to end. The emitter and the ownership predicate are coupled
|
||||
through the stamp value; `OKF_V0_2` changes that value. If the predicate is
|
||||
not given the same profile the emitter used, the library stops recognising
|
||||
its own output and the §3 collision gate fires `collision_unstamped` on the
|
||||
files the previous run wrote. One run compared byte for byte cannot see it.
|
||||
"""
|
||||
manifest_path, bundle = file_setup
|
||||
materialize_bundle(manifest_path, bundle, INGESTED_AT, profile=OKF_V0_2)
|
||||
first = (bundle / "ingest-orders.md").read_bytes()
|
||||
first_index = (bundle / "index.md").read_bytes()
|
||||
|
||||
materialize_bundle(manifest_path, bundle, INGESTED_AT, profile=OKF_V0_2)
|
||||
|
||||
assert (bundle / "ingest-orders.md").read_bytes() == first
|
||||
assert (bundle / "index.md").read_bytes() == first_index
|
||||
|
||||
|
||||
# --- the paths/index half of the profile reaches disk ---
|
||||
|
||||
|
||||
def test_profile_paths_and_index_name_reach_disk(file_setup: tuple[Path, Path]) -> None:
|
||||
"""Six of the nine threading sites read `paths`/`index`, which are the same
|
||||
objects on every shipped profile. `_SYNTHETIC` renames both so the sites are
|
||||
provable rather than merely regression-covered.
|
||||
"""
|
||||
manifest_path, bundle = file_setup
|
||||
result = materialize_bundle(manifest_path, bundle, INGESTED_AT, profile=_SYNTHETIC)
|
||||
|
||||
assert (bundle / "x-orders.mdx").is_file()
|
||||
assert not (bundle / "ingest-orders.md").exists()
|
||||
assert (bundle / "contents.md").is_file()
|
||||
assert not (bundle / "index.md").exists()
|
||||
assert [path.name for path in result.written] == ["x-orders.mdx"]
|
||||
assert "- [Orders](x-orders.mdx)" in (bundle / "contents.md").read_text(encoding="utf-8")
|
||||
|
||||
|
||||
def test_second_synthetic_run_replaces_rather_than_colliding(
|
||||
file_setup: tuple[Path, Path],
|
||||
) -> None:
|
||||
"""The §3 ownership scan globs `*{concept_suffix}` and excludes `index.name`.
|
||||
Left on `DEFAULT`, the glob finds nothing under a renamed suffix, `owned` is
|
||||
empty, and the gate refuses to overwrite the file this same code just wrote.
|
||||
The index must also not gain a second link for the same target.
|
||||
"""
|
||||
manifest_path, bundle = file_setup
|
||||
materialize_bundle(manifest_path, bundle, INGESTED_AT, profile=_SYNTHETIC)
|
||||
first = (bundle / "x-orders.mdx").read_bytes()
|
||||
|
||||
materialize_bundle(manifest_path, bundle, INGESTED_AT, profile=_SYNTHETIC)
|
||||
|
||||
assert (bundle / "x-orders.mdx").read_bytes() == first
|
||||
index_text = (bundle / "contents.md").read_text(encoding="utf-8")
|
||||
assert index_text.count("- [Orders](x-orders.mdx)") == 1
|
||||
|
||||
|
||||
def test_removed_extraction_is_unlinked_under_a_renamed_index(
|
||||
file_setup: tuple[Path, Path], tmp_path: Path
|
||||
) -> None:
|
||||
"""`_update_index_lines` runs only on an EXISTING index, so it is reached
|
||||
solely by a second run — and only its `link_pattern`/`render_link` are
|
||||
profile-owned. Dropping an extraction is what forces the removal branch.
|
||||
"""
|
||||
manifest_path, bundle = file_setup
|
||||
materialize_bundle(manifest_path, bundle, INGESTED_AT, profile=_SYNTHETIC)
|
||||
|
||||
data = _manifest_data()
|
||||
data["extractions"][0]["id"] = "invoices"
|
||||
data["extractions"][0]["title"] = "Invoices"
|
||||
manifest_path.write_text(json.dumps(data), encoding="utf-8")
|
||||
(tmp_path / "src" / "data" / "invoices.csv").write_text(
|
||||
"a,b\n1,x\n", encoding="utf-8", newline=""
|
||||
)
|
||||
|
||||
materialize_bundle(manifest_path, bundle, INGESTED_AT, profile=_SYNTHETIC)
|
||||
|
||||
index_text = (bundle / "contents.md").read_text(encoding="utf-8")
|
||||
assert "- [Invoices](x-invoices.mdx)" in index_text
|
||||
assert "x-orders.mdx" not in index_text
|
||||
assert not (bundle / "x-orders.mdx").exists()
|
||||
|
||||
|
||||
def test_synthetic_profile_is_not_a_shipped_profile() -> None:
|
||||
"""Guards the instrument: if a future profile ever shares these overrides,
|
||||
the two tests above stop proving anything and would go quietly green.
|
||||
"""
|
||||
assert isinstance(_SYNTHETIC, BundleProfile)
|
||||
assert _SYNTHETIC.paths is not DEFAULT.paths
|
||||
assert _SYNTHETIC.index is not DEFAULT.index
|
||||
assert OKF_V0_2.paths is DEFAULT.paths
|
||||
assert OKF_V0_2.index is DEFAULT.index
|
||||
Loading…
Add table
Add a link
Reference in a new issue