refactor(ingest): adopt shared llm-ingestion-okf v0.3.1 behind a thin adapter
Door A (manifest -> connector -> deterministic materialization -> index) is no longer implemented here. src/portfolio_optimiser/ingest.py becomes a thin consumer seam over the shared library, git-pinned to v0.3.1 on the same Forgejo channel portfolio-optimiser-claude uses. Net -626/+385; ingest.py 599 -> 145 lines. shared/ingest-spec.md remains the normative spec: the library implements it, it does not replace it. Spec changes continue to go via commons. Acceptance criterion met and proven: all three golden bundles (file/sql/http) are byte-exact before and after, including the idempotence re-run. examples/ and shared/ carry ZERO modifications -- the fasit was not adjusted to fit. The rejection set was verified equivalent, not assumed: all 22 malformations the repo's pydantic models refused are refused by the library, with typed codes (okf_type_reserved, credential_embedded, extraction_id_duplicate, ...). Test rebinding (invariants preserved, vehicle changed): the library has zero runtime dependencies by design, so pydantic is unavailable to it. ManifestV1.model_validate(dict) -> load_manifest_bytes(bytes); ValidationError -> ManifestError; model_fields -> dataclasses.fields; PathSecurityError -> SourceError(path_escape); ValueError -> MaterializationError(ingested_at_invalid). Tests now also pin the refusal `code`, the library's documented stability contract -- a sharper assertion than "some validation error was raised". Two accepted behavioural deltas, recorded rather than silently dropped: - Title whitespace is stored verbatim instead of collapsed at validation, so the frontmatter title and the index label are no longer guaranteed identical for irregular whitespace. Both behaviours are spec-conformant (the spec is SILENT; the old one was a repo-local pinned decision). Queued as a commons-amendment candidate so both stacks pin the same answer. Goldens unaffected. - The section 8 audit log moves to logger llm_ingestion_okf.materialize. Nothing in the repo consumed the old channel. Also: the `type` discriminator is no longer a dataclass field, so the spec cross-check asserts it explicitly -- without that line the swap would have silently narrowed the test. New tests/test_ingest_library_seam.py pins the seam itself: the restated section 5 stamp formula against the stamp the library actually writes (the one place the adapter does not purely delegate, since v0.3.1 exposes no stamp helper), the local-only allow_network default, the list[Path] unwrapping, and a guard that the adapter never regrows local Door A machinery. All four verified RED when detached, as were both golden regressions under a byte-level render mutation. Door A is UNGATED: it calls no guard before writing to disk. Gating untrusted content remains the caller's responsibility (guard wiring still planned). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B4jNN186eVqfe1x5DnTU6r
This commit is contained in:
parent
7ec60618b0
commit
0a11af74a4
9 changed files with 385 additions and 626 deletions
|
|
@ -4,8 +4,16 @@ Every malformed manifest raises at validation, BEFORE any source access (the sta
|
|||
discipline of method spec §10 / ingest spec §4 and §9's technical gate). The verdict-layer
|
||||
reservation (okf_type != verdict, case-insensitive) is enforced here — at the contract, never
|
||||
downstream — closing session-plan key assumption 2 (fail-fast manifest validation without
|
||||
network). Pattern: tests/test_contracts.py (inline dict constants + pytest.raises per
|
||||
malformation; fail-fast ordering proof mirrors test_no_chat_client_call_on_malformed_contract).
|
||||
network).
|
||||
|
||||
Since the ``llm-ingestion-okf`` adoption (2026-07-20) the contract is enforced by the shared
|
||||
library rather than by repo-local pydantic models. The INVARIANTS are unchanged — every
|
||||
malformation rejected before was verified to still be rejected — but the assertion vehicle
|
||||
moved: ``ManifestV1.model_validate(dict)`` → ``load_manifest_bytes(bytes)``, and
|
||||
``pydantic.ValidationError`` → ``ManifestError``. The library has zero runtime dependencies
|
||||
BY DESIGN, so pydantic is not available to it. These tests now also pin the refusal ``code``,
|
||||
which is the library's documented stability contract (the message text explicitly is not) —
|
||||
a strictly sharper assertion than "some validation error was raised".
|
||||
"""
|
||||
|
||||
import copy
|
||||
|
|
@ -15,7 +23,8 @@ from pathlib import Path
|
|||
from typing import Any
|
||||
|
||||
import pytest
|
||||
from pydantic import ValidationError
|
||||
from llm_ingestion_okf import FileSource, HttpSource, ManifestError, SqlSource
|
||||
from llm_ingestion_okf.manifest import load_manifest_bytes
|
||||
|
||||
from portfolio_optimiser.ingest import ManifestV1, load_manifest
|
||||
|
||||
|
|
@ -54,11 +63,19 @@ def _variant(**overrides: Any) -> dict[str, Any]:
|
|||
return data
|
||||
|
||||
|
||||
def _validate(data: dict[str, Any]) -> ManifestV1:
|
||||
"""In-memory validation without touching disk — the library's equivalent of the pydantic
|
||||
``model_validate`` these tests used before the adoption."""
|
||||
return load_manifest_bytes(json.dumps(data).encode("utf-8"))
|
||||
|
||||
|
||||
def test_valid_file_manifest_loads_with_stamp(tmp_path: Path) -> None:
|
||||
path = _write(tmp_path, _MANIFEST)
|
||||
manifest, stamp = load_manifest(path)
|
||||
assert isinstance(manifest, ManifestV1)
|
||||
assert manifest.source.type == "file"
|
||||
# The library's source models drop the `type` discriminator as a FIELD (it is consumed by
|
||||
# validation dispatch), so the variant is identified by class rather than by `.type`.
|
||||
assert isinstance(manifest.source, FileSource)
|
||||
assert [e.id for e in manifest.extractions] == ["costs", "meta"]
|
||||
# §5: stamp = {stem}@{first 16 hex of SHA-256 over the manifest file's RAW bytes}.
|
||||
expected = "manifest@" + hashlib.sha256(path.read_bytes()).hexdigest()[:16]
|
||||
|
|
@ -69,59 +86,62 @@ def test_each_missing_top_level_field_raises(tmp_path: Path) -> None:
|
|||
for field in ("manifest_version", "source", "bundle_summary", "extractions"):
|
||||
data = copy.deepcopy(_MANIFEST)
|
||||
del data[field]
|
||||
with pytest.raises(ValidationError):
|
||||
with pytest.raises(ManifestError):
|
||||
load_manifest(_write(tmp_path, data, name=f"missing-{field}.json"))
|
||||
|
||||
|
||||
def test_manifest_version_other_than_1_rejected(tmp_path: Path) -> None:
|
||||
with pytest.raises(ValidationError):
|
||||
with pytest.raises(ManifestError) as exc:
|
||||
load_manifest(_write(tmp_path, _variant(manifest_version=2)))
|
||||
assert exc.value.code == "manifest_version_unsupported"
|
||||
|
||||
|
||||
def test_empty_extractions_rejected(tmp_path: Path) -> None:
|
||||
with pytest.raises(ValidationError):
|
||||
with pytest.raises(ManifestError):
|
||||
load_manifest(_write(tmp_path, _variant(extractions=[])))
|
||||
|
||||
|
||||
def test_bad_id_grammar_rejected(tmp_path: Path) -> None:
|
||||
def test_bad_id_grammar_rejected() -> None:
|
||||
# §4 grammar for source.id and extraction.id: ^[a-z0-9][a-z0-9-]*$
|
||||
for bad in ("Upper", "-leading", "", "space id", "æøå"):
|
||||
data = copy.deepcopy(_MANIFEST)
|
||||
data["source"]["id"] = bad
|
||||
with pytest.raises(ValidationError):
|
||||
ManifestV1.model_validate(data)
|
||||
with pytest.raises(ManifestError):
|
||||
_validate(data)
|
||||
data = copy.deepcopy(_MANIFEST)
|
||||
data["extractions"][0]["id"] = bad
|
||||
with pytest.raises(ValidationError):
|
||||
ManifestV1.model_validate(data)
|
||||
with pytest.raises(ManifestError):
|
||||
_validate(data)
|
||||
|
||||
|
||||
def test_duplicate_extraction_ids_rejected(tmp_path: Path) -> None:
|
||||
data = copy.deepcopy(_MANIFEST)
|
||||
data["extractions"][1]["id"] = data["extractions"][0]["id"]
|
||||
with pytest.raises(ValidationError):
|
||||
with pytest.raises(ManifestError) as exc:
|
||||
load_manifest(_write(tmp_path, data))
|
||||
assert exc.value.code == "extraction_id_duplicate"
|
||||
|
||||
|
||||
def test_nonpositive_max_rows_rejected(tmp_path: Path) -> None:
|
||||
for bad in (0, -1):
|
||||
data = copy.deepcopy(_MANIFEST)
|
||||
data["extractions"][0]["max_rows"] = bad
|
||||
with pytest.raises(ValidationError):
|
||||
with pytest.raises(ManifestError):
|
||||
load_manifest(_write(tmp_path, data, name=f"rows-{bad}.json"))
|
||||
|
||||
|
||||
def test_unknown_source_type_rejected(tmp_path: Path) -> None:
|
||||
data = copy.deepcopy(_MANIFEST)
|
||||
data["source"] = {"type": "ftp", "id": "x", "root": "fixture"}
|
||||
with pytest.raises(ValidationError):
|
||||
with pytest.raises(ManifestError) as exc:
|
||||
load_manifest(_write(tmp_path, data))
|
||||
assert exc.value.code == "source_type_unknown"
|
||||
|
||||
|
||||
def test_file_source_missing_root_rejected(tmp_path: Path) -> None:
|
||||
data = copy.deepcopy(_MANIFEST)
|
||||
del data["source"]["root"]
|
||||
with pytest.raises(ValidationError):
|
||||
with pytest.raises(ManifestError):
|
||||
load_manifest(_write(tmp_path, data))
|
||||
|
||||
|
||||
|
|
@ -132,46 +152,57 @@ def test_verdict_okf_type_rejected_case_insensitively(tmp_path: Path) -> None:
|
|||
for spelling in ("verdict", "Verdict", "VERDICT"):
|
||||
data = copy.deepcopy(_MANIFEST)
|
||||
data["extractions"][0]["okf_type"] = spelling
|
||||
with pytest.raises(ValidationError):
|
||||
with pytest.raises(ManifestError) as exc:
|
||||
load_manifest(_write(tmp_path, data, name=f"verdict-{spelling}.json"))
|
||||
assert exc.value.code == "okf_type_reserved"
|
||||
|
||||
|
||||
def test_multiline_title_rejected(tmp_path: Path) -> None:
|
||||
def test_multiline_title_rejected() -> None:
|
||||
for bad in ("line1\nline2", "line1\rline2", ""):
|
||||
data = copy.deepcopy(_MANIFEST)
|
||||
data["extractions"][0]["title"] = bad
|
||||
with pytest.raises(ValidationError):
|
||||
ManifestV1.model_validate(data)
|
||||
with pytest.raises(ManifestError):
|
||||
_validate(data)
|
||||
|
||||
|
||||
def test_title_whitespace_normalized() -> None:
|
||||
# Pinned decision: title runs are collapsed at validation so the frontmatter rendering
|
||||
# (okf.render_frontmatter collapses runs) and the index label are guaranteed identical.
|
||||
def test_title_whitespace_is_preserved_verbatim() -> None:
|
||||
"""ACCEPTED DIVERGENCE (2026-07-20), recorded rather than silently dropped.
|
||||
|
||||
The repo previously COLLAPSED title whitespace runs at validation, so the frontmatter
|
||||
title and the index label were guaranteed byte-identical. The library stores the title
|
||||
verbatim instead: ``_render_frontmatter`` still collapses runs when writing frontmatter,
|
||||
but the §6 index label is written raw — so for a title with irregular internal whitespace
|
||||
the two now differ. Both behaviours are spec-conformant: ``shared/ingest-spec.md`` is
|
||||
SILENT on normalization, and the old behaviour was a repo-local pinned decision, not a
|
||||
spec requirement.
|
||||
|
||||
This test pins the CURRENT behaviour so the divergence cannot drift unnoticed. It goes RED
|
||||
if the library starts normalizing — which is the desired end state, and is why the point is
|
||||
queued as a commons-amendment candidate so both stacks pin the same answer in the spec.
|
||||
The golden bundles are unaffected (their titles carry no irregular whitespace)."""
|
||||
data = copy.deepcopy(_MANIFEST)
|
||||
data["extractions"][0]["title"] = " Project costs "
|
||||
manifest = ManifestV1.model_validate(data)
|
||||
assert manifest.extractions[0].title == "Project costs"
|
||||
manifest = _validate(data)
|
||||
assert manifest.extractions[0].title == " Project costs "
|
||||
|
||||
|
||||
def test_base_url_embedded_credentials_rejected() -> None:
|
||||
# §4: base_url MUST NOT embed credentials (userinfo is the URL credential mechanism).
|
||||
data = copy.deepcopy(_MANIFEST)
|
||||
data["source"] = {"type": "http", "id": "api", "base_url": "https://user:pw@host/api"}
|
||||
with pytest.raises(ValidationError):
|
||||
ManifestV1.model_validate(data)
|
||||
with pytest.raises(ManifestError) as exc:
|
||||
_validate(data)
|
||||
assert exc.value.code == "credential_embedded"
|
||||
|
||||
|
||||
def test_sql_and_http_variants_validate() -> None:
|
||||
# Schema breadth (brief assumption 1): the polymorphic §4 schema validates all three
|
||||
# source variants; the `file` and `sql` connectors EXECUTE (I2, I4), http → I6.
|
||||
sql = ManifestV1.model_validate(
|
||||
_variant(source={"type": "sql", "id": "db", "connection_ref": "PROJ_DB"})
|
||||
)
|
||||
assert sql.source.type == "sql"
|
||||
http = ManifestV1.model_validate(
|
||||
_variant(source={"type": "http", "id": "api", "base_url": "https://host/api"})
|
||||
)
|
||||
assert http.source.type == "http"
|
||||
sql = _validate(_variant(source={"type": "sql", "id": "db", "connection_ref": "PROJ_DB"}))
|
||||
assert isinstance(sql.source, SqlSource)
|
||||
assert sql.source.connection_ref == "PROJ_DB"
|
||||
http = _validate(_variant(source={"type": "http", "id": "api", "base_url": "https://host/api"}))
|
||||
assert isinstance(http.source, HttpSource)
|
||||
assert http.source.credential_ref is None
|
||||
|
||||
|
||||
|
|
@ -183,13 +214,16 @@ def test_failfast_before_source_access(tmp_path: Path) -> None:
|
|||
data = copy.deepcopy(_MANIFEST)
|
||||
data["source"]["root"] = str(tmp_path / "does-not-exist")
|
||||
data["extractions"][0]["max_rows"] = 0
|
||||
with pytest.raises(ValidationError):
|
||||
with pytest.raises(ManifestError):
|
||||
load_manifest(_write(tmp_path, data))
|
||||
assert not (tmp_path / "does-not-exist").exists()
|
||||
|
||||
|
||||
def test_malformed_json_raises(tmp_path: Path) -> None:
|
||||
# The library wraps json decoding so EVERY manifest problem surfaces as one typed family
|
||||
# (previously this leaked a raw json.JSONDecodeError to the caller).
|
||||
path = tmp_path / "broken.json"
|
||||
path.write_text("{not json", encoding="utf-8")
|
||||
with pytest.raises(json.JSONDecodeError):
|
||||
with pytest.raises(ManifestError) as exc:
|
||||
load_manifest(path)
|
||||
assert exc.value.code == "manifest_invalid_json"
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue