docs(i2): brief + plan for ingest file/CSV session (I2)

This commit is contained in:
Kjell Tore Guttormsen 2026-07-03 18:44:37 +02:00
commit fdcbaffc88
2 changed files with 995 additions and 0 deletions

View file

@ -0,0 +1,263 @@
---
type: trekbrief
brief_version: "2.2"
created: 2026-07-03
task: "I2 — MAF reference implementation of the ingest layer: file catalogue/CSV (offline)"
slug: i2-ingest-csv-maf
project_dir: .claude/projects/2026-07-03-i2-ingest-csv-maf/
research_topics: 0
research_status: skipped
auto_research: false
interview_turns: 0
source: manual
framing: preserve
phase_signals:
- phase: research
effort: high
model: fable
- phase: plan
effort: high
model: fable
- phase: execute
effort: high
model: fable
- phase: review
effort: high
model: fable
---
# Task: I2 — MAF reference implementation: ingest file catalogue/CSV (offline)
> Generated by `/trekbrief` on 2026-07-03.
> This brief is the contract between requirements and planning. `/trekplan`
> reads it to produce the implementation plan. Every decision in the plan must
> trace back to content in this brief.
>
> **Provenance note (framing defense):** this brief was derived autonomously from
> two FROZEN, operator-approved sources — the I2 section of
> `docs/plan/2026-07-03-sesjonsplan-ingest.md` and the normative
> `shared/ingest-spec.md` — under the STATE.md operating model ("Les STATE.md og
> følg instruksjonene" = go-signal for the NEXT step; I2 is not gated). No
> interview questions were asked because every answer already exists in written,
> approved form; re-asking would violate the operator's recorded
> plan-over-menu preference. `framing: preserve` is documented, not guessed:
> I2 continues the frozen plan arc unchanged (STATE.md: "I2 [NESTE]").
## TL;DR
Implement the ingest layer's first source type on the MAF side, exactly as frozen in
`shared/ingest-spec.md`: fail-fast pydantic manifest contract, CSV/file-catalogue
connector, deterministic materialization into an OKF bundle with provenance
frontmatter + index generation — offline, zero model calls, TDD. Framing: **preserve**
— the "I2 [NESTE]" step of the operator-approved, frozen ingest session plan.
## Intent
The 8-step optimiser loop consumes hand-curated OKF bundles today; the frozen ingest
målbilde closes the gap to real data sources while preserving the architecture-defining
rule that **data reaches the model ONLY via OKF bundles** (no RAG, no query-time
retrieval — method spec §3). I2 is the first implementation session of that program:
it proves the frozen contract (`shared/ingest-spec.md`, authored and guarded in I1) is
actually implementable from the spec alone, on the reference (MAF) side, using the
offline `file` source type so the whole session stays at zero model calls and zero
network (cost discipline D6). It also closes the plan's key assumption 2 — that an
ingest manifest can be validated fail-fast without network — which is currently
marked RISK with only pattern-level evidence (`contracts.py` is in-process). Finally,
I2 turns the planning session's ephemeral ad-hoc proof (a synthetic generated bundle
passing `navigate_bundle`/`bundle_context`) into a durable load-bearing test, so the
seam can never silently detach.
## Goal
`uv run pytest` green (without network or credentials) on a repo where: a pydantic
manifest contract validates ingest manifests fail-fast per spec §4 (polymorphic on
`source.type`, verdict-layer reservation enforced before any source call); a
file-catalogue/CSV connector executes extractions per §4 (boundary-checked path
resolution, `max_rows` fail-fast); materialization per §5§7 writes
`ingest-{id}.md` concept files with the exact 7-key provenance frontmatter and
generates/updates `index.md` per §6 (idempotent by target, curated/promoted links
preserved); and a golden extraction case at `examples/ingest-golden-file/` (spec §11
layout) reproduces byte for byte from fixture + fixed `ingested_at`. The existing
loop consumes the generated bundle through UNCHANGED `okf.py` navigation code. Every
seam in the I2 load-bearing set has a test proven RED at detach.
## Non-Goals
- **SQL connector** — I4 (the pydantic manifest schema MAY validate the `sql`/`http`
variants per §4, but no `sql` connector execution in I2; see Open Questions).
- **HTTP/MCP connector and the network opt-in gate test** — I6 (gated: network
capability). The `file` source type requires no network flag (§5, §8).
- **Sibling repo (D7) work** — I3/I5 (gated: other repo). No files outside this repo.
- **Commons/spec changes** — any needed spec change means STOP and a separate gated
commons round (session-plan frame rule); never folded into I2. The golden case is
authored LOCALLY in this repo in I2; sharing it via commons is a later gated round.
- **Wiring ingest into `run_project`** — ingest is an explicit operator-run step
BEFORE the loop (spec §2, §9); the run path is untouched.
- **Extension points explicitly deferred by the spec:** per-row file splitting,
multiple manifests per bundle, incremental re-ingest, approval registry /
signed `approved_by` (spec §5, §9, §10).
- **S12 release work** — independently gated (LICENSE is an operator-owned choice).
Note on spec conformance: full §1 conformance requires both `file` and `sql`
connectors — I2 is deliberately a partial-conformance milestone; §1 conformance
completes at I4. Reviewers must not read §1 as an I2 failure.
## Constraints
- **The spec is normative and frozen.** Implement from `shared/ingest-spec.md` alone;
MUST/MUST NOT clauses are binding. On any discovered spec defect or needed change:
stop, report — separate gated commons round.
- **Offline, deterministic, zero model calls** (D6 + spec §1, §11): tests run without
credentials and without network; no live sources anywhere.
- **TDD Iron Law:** no production code without a failing test first. Load-bearing
tests must be proven RED at detach (detach → observe red → reattach), the method
spec §11 regime.
- **`okf.py` navigation code stays UNCHANGED** — the navigability criterion is
explicitly "consumable via UNCHANGED `okf.py`". Reusing its existing primitives
(e.g. the index-linking primitive, whose idempotent-by-target semantics §6
references) is expected; modifying them is not in scope.
- **Context-layer neutrality invariant:** `okf.py` and `shared/` remain pure stdlib,
MAF-free (`test_okf_is_maf_free`). The new ingest module makes zero model calls and
MUST NOT import `agent_framework`/`mcp` (it is a deterministic pre-loop step);
pydantic is fine (already a core dependency, `contracts.py` pattern).
- **Commons is PULL-ONLY**; nothing in I2 writes to commons or `shared/`.
- **One session = one closed delivery:** green suite, STATE.md overwritten,
commit + push to Forgejo; never a broken state across the session boundary.
- **Surgical scope:** new ingest module + tests + golden example; no refactoring of
adjacent code.
## Preferences
- Follow the existing `contracts.py` pydantic style for the manifest contract
(fail-fast, explicit models, type hints everywhere).
- Mirror the existing load-bearing test idiom (e.g.
`tests/test_step8_promotion_loadbearing.py`): each test names its seam and fails
when the seam is detached.
- Reuse `okf.py`'s existing index-linking primitive for §6 index updates rather than
reimplementing link semantics.
- Golden case directory per spec §11 convention: `examples/ingest-golden-file/`
with `manifest.json`, `fixture/`, `ingested-at.txt`, `expected-bundle/`.
- Small synthetic fixture data (cost discipline — no large test payloads).
- Code/comments/docs in English; Conventional Commits.
## Non-Functional Requirements
- **Bit-determinism:** same source content + manifest + `ingested_at` → byte-identical
bundle; repeated runs idempotent (spec §10, §11).
- **Zero new runtime dependencies** (pydantic and stdlib suffice; CSV via stdlib `csv`).
- **Fail-fast, never silent:** malformed manifest, verdict-typed mapping, path escape,
`max_rows` overflow, non-CSV-representable cell value, curated-file collision — all
ERRORS, never warnings/truncation/coercion (spec §3, §4, §5, §8).
- **File hygiene per spec §5:** LF-only, exactly one trailing newline, single-line
frontmatter values, whitespace-collapsed `source_query`.
- **Source calls are logged** (which source, when, row count — spec §8); test-visible
via stdlib `logging`, no new dependency.
- **Contract completeness:** the plan traces spec §12's cross-check table
field-by-field (`manifest_version``generated`, golden-case entries) so no
contract field is silently dropped — including `bundle_summary` (§4/§6) and the
`ingest_manifest` stamp format `{stem}@{hash16}` (§5).
## Success Criteria
All commands run from the repo root; "without network/credentials" is the standing
test condition (spec §11).
1. **Golden extraction, bit-deterministic:** a test materializes
`examples/ingest-golden-file/` from `manifest.json` + `fixture/` +
`ingested-at.txt` and compares against `expected-bundle/` file by file, byte for
byte — `uv run pytest tests/ -k ingest` exits 0. Running materialization twice
over the same target produces identical bytes (idempotence, §10).
2. **Load-bearing: provenance detach** — with the §7 stamping detached (e.g.
provenance keys not written), the suite goes RED. Proof recorded (detach → red →
reattach) in the session log.
3. **Load-bearing: navigability via UNCHANGED `okf.py`** — the generated bundle is
consumed by the existing navigation code (`navigate_bundle`/`bundle_context`
seam): generated files reachable via index cross-links, unknown provenance fields
preserved. Test goes RED if index generation/linking is detached. `git diff` for
the delivery shows no modification to `okf.py`.
4. **Load-bearing: verdict reservation** — a manifest with `okf_type: verdict`
(case-insensitive) or a generated-filename collision with `index.md`/the
`promoted-verdict-*` namespace is REJECTED at validation, before any source call;
test goes RED if the reservation is removed.
5. **Load-bearing: re-ingest layer safety** — re-materialization over a bundle
containing a promoted verdict (created via the real `promote_verdict`) preserves
the verdict file AND its index link; test goes RED if replacement semantics stop
honouring the ingest-stamp ownership rule (§3, §6).
6. **Fail-fast manifest validation without network (closes key assumption 2):**
malformed manifests (missing fields, bad `id` grammar, empty `extractions`,
non-positive `max_rows`, unknown `source.type`) raise before any source access —
asserted by tests that run with no network available.
7. **Full suite + gates green:** `uv run pytest` exits 0 — the pre-existing suite
passes unchanged (baseline captured at session start before any I2 change) plus
the new ingest tests; `uv run ruff check .` exits 0, `uv run ruff format --check .`
clean, `uv run mypy src` exits 0.
8. **Honesty of artefacts:** no artefact delivered in I2 claims SQL/HTTP support,
live-source integration, or commons sharing of the golden case. Concretely:
case-insensitive grep for `sql`, `http`, `live`, `commons` over the new module's
docstrings, new test files' docstrings, and any touched README/docs sections
yields no support-claiming hits (mentions that explicitly DEFER — "I4", "I6",
"extension point" — are fine); the plan fixes the exact grep invocation.
## Research Plan
No external research needed — the codebase and this brief contain sufficient context
for planning. Rationale (recorded in lieu of an interview): the task is fully
specified by the frozen `shared/ingest-spec.md`; the implementation surface is
stdlib + pydantic patterns already established in this repo (`contracts.py`,
`okf.py`, `verdicts.py`); the session is offline by rule, so there is no external
API/library/version decision to research. Key assumption 4 (D7 SDK hooks) belongs to
I3, not I2.
## Open Questions / Assumptions
- **[ASSUMPTION] Manifest schema breadth:** the pydantic contract is built
polymorphic per spec §4 (validating `file`, `sql`, `http` source variants), while
I2 implements ONLY the `file` connector execution path. This is the cheapest
spec-complete reading; the plan may narrow it to `file`-only validation with
justification (spec conformance for `sql` arrives in I4 either way).
- **[ASSUMPTION] Invocation surface:** spec §9 requires ingest to be an explicit
operator command. For I2 a public programmatic API (module function) satisfies the
procedural gate for tests; whether to also add a `python -m` CLI entry in I2 is a
plan decision (cheap, but not demanded by the I2 verification set).
- **[ASSUMPTION] Module placement:** new module `src/portfolio_optimiser/ingest.py`
(name/path is a plan decision; the constraint is only that `okf.py` stays unchanged
and the module is MAF-free).
- **[ASSUMPTION] Expected-bundle authoring:** `expected-bundle/` in the golden case is
generated ONCE by the implementation after review of its content (then frozen in
git), rather than hand-typed — acceptable because the golden test's job is
regression pinning (spec §11), while correctness is carried by the unit/load-bearing
tests. The plan should sequence this so the golden freeze happens LAST, after all
behaviour tests are green.
## Prior Attempts
None as implementation — this is the first ingest implementation session. Two prior
results are load-bearing context, not failures: (1) key assumption 1 was proven ad hoc
on 2026-07-03 (a synthetic bundle with provenance frontmatter passed
`navigate_bundle`/`bundle_context`; unknown fields preserved, context rendered) — I2
makes that proof durable; (2) I1 delivered the frozen spec into `shared/` with the
framework guard + structure test extended
(`tests/test_method_spec_loadbearing.py`, commits `4df2140` + `7ba0fae`), so the
contract I2 implements is already guarded in this repo.
## Metadata
- **Created:** 2026-07-03
- **Interview turns:** 0 (brief derived from frozen, operator-approved plan + spec;
see provenance note at top)
- **Auto-research opted in:** no (0 topics)
- **Source:** manual
---
## How to continue
Manual (default):
```bash
# No research topics — go straight to plan:
/trekplan --project .claude/projects/2026-07-03-i2-ingest-csv-maf/
# Then execute:
/trekexecute --project .claude/projects/2026-07-03-i2-ingest-csv-maf/
```

View file

@ -0,0 +1,732 @@
# I2 — MAF reference implementation: ingest file catalogue/CSV (offline)
> **Plan quality: B+** (88/100) — APPROVE_WITH_NOTES
>
> Generated by trekplan v5.9 on 2026-07-03 — `plan_version: 1.7`
## Context
The 8-step optimiser loop consumes hand-curated OKF bundles today; the frozen ingest
målbilde closes the gap to real data sources while preserving the architecture-defining
rule that data reaches the model ONLY via OKF bundles (no RAG, no query-time retrieval —
method spec §3). I2 is the first implementation session of that program: it proves the
frozen contract (`shared/ingest-spec.md`, authored and guarded in I1) is implementable
from the spec alone, on the reference (MAF) side, using the offline `file` source type so
the session stays at zero model calls and zero network (cost discipline D6). It closes the
session plan's key assumption 2 (fail-fast manifest validation without network, currently
RISK-marked) and turns the planning session's ephemeral ad-hoc navigability proof into a
durable load-bearing test. Source: brief Intent, verbatim intent trace.
Scope note (brief, Non-Goals): I2 is deliberately a **partial-conformance milestone**
spec §1 full conformance (`file` + `sql`) completes at I4. The manifest contract validates
all three source variants; only the `file` connector executes in I2.
## Architecture Diagram
```mermaid
graph TD
subgraph "New in this plan"
M[manifest.json] -->|"load_manifest() — pydantic fail-fast (§4), verdict reservation (§3)"| C[ingest.py contract models]
C -->|"file connector: safe_resolve boundary + csv.reader + max_rows cap"| X[extraction rows in memory]
X -->|"render: escaped markdown table + §5/§7 provenance frontmatter"| S[staged files in memory]
S -->|"replacement semantics: delete stamped only, collision fail (§3)"| B[OKF bundle dir]
S -->|"index create/update: bundle_summary body, managed-line removal, link_in_index append (§6)"| I[index.md]
end
subgraph "Existing, UNCHANGED"
B --> N["okf.navigate_bundle / bundle_context"]
I --> N
V["verdicts.promote_verdict"] -->|"promoted-verdict-*.md + index link — must survive re-ingest"| B
R["retrieval.safe_resolve / PathSecurityError"] -.reused by.-> C
L["okf.link_in_index / render_frontmatter / parse_frontmatter"] -.reused by.-> S
end
```
## Codebase Analysis
- **Tech stack:** Python ≥3.10, pydantic ≥2.11, `uv`, pytest ≥8 (`asyncio_mode=auto`; ingest
tests are plain sync), ruff (line-length 100), mypy (py3.10 target). No new runtime
dependencies needed: stdlib `csv`, `hashlib`, `logging`, `pathlib` + pydantic suffice.
- **Key patterns:** fail-fast pydantic startup contracts (`contracts.py:30-96`); frozen
dataclasses for value objects, `BaseModel` for validated external input; PEP 604 unions,
keyword-only options via bare `*`; condition-noun exceptions (`PromotionRefused`
`verdicts.py:447`, `PathSecurityError` `retrieval.py:24`, dominant base `RuntimeError`);
narrative module docstrings citing målbilde/spec sections; load-bearing tests with
detach-RED docstrings (`tests/test_step8_promotion_loadbearing.py`).
- **Relevant files:** `src/portfolio_optimiser/okf.py` (UNCHANGED — reuse
`parse_frontmatter:31`, `render_frontmatter:146`, `link_in_index:168`,
`navigate_bundle:113`, `bundle_context:133`), `src/portfolio_optimiser/retrieval.py`
(`safe_resolve:63`, `is_within_dir:48`, `PathSecurityError:24`),
`src/portfolio_optimiser/verdicts.py` (`promote_verdict:465` — structural analogue and
the SC5 test's real promotion; SHA-256 `[:16]` idiom at `verdicts.py:95,461`),
`tests/test_method_spec_loadbearing.py:156-174` (recorded I2 obligation: field-level
cross-check), `tests/conftest.py` (no ingest-relevant fixtures — none needed),
`shared/examples/bygg-energi-mikro/` (copytree fixture for the re-ingest safety test).
- **Reusable code:** `safe_resolve` for the §4 root boundary (fail-closed);
`render_frontmatter` for §5 key order + single-lining (collapses whitespace in ALL
values — acceptable: §5 requires single-line values everywhere; golden pins it);
`link_in_index` for §6 appends (idempotent by `](target)` — exactly the primitive §6
references); `parse_frontmatter` for stamp detection (`generated == "true"` string
compare — it returns strings, never booleans); `promote_verdict` as the shape template
(explicit required `timestamp`, no wall-clock default).
- **Deliberately NOT reused:** `okf.write_concept_file` for the final write — it delegates
to `Path.write_text` (platform newline translation risk) and does not guarantee exactly
one trailing newline (`okf.py:164`). The ingest module builds each file's full content as
a string and writes bytes (`encode("utf-8")`) for the §5 LF-only guarantee on CONCEPT
files. Honest limit: index APPENDS reuse the frozen `link_in_index` (`Path.write_text`,
`okf.py:180`), so `index.md` LF-ness is platform-scoped — nil on the LF-only
development/target platform; recorded as Assumption 6.
`contracts.py` is NOT imported: it imports `backends``agent_framework`
(`contracts.py:25`), which would break the ingest module's MAF-free guard. Manifest
models live in `ingest.py` itself.
- **External tech (researched):** none — offline task, frozen spec, established repo
patterns (brief Research Plan: 0 topics).
- **Recent git activity:** I1 landed the spec + guards 1h before planning (`4df2140`,
`7ba0fae`); `tests/test_method_spec_loadbearing.py` is freshly touched — extend, don't
conflict. All work is sequential on `main`, clean tree. Commons subtree is PULL-ONLY
(violation observed + cleaned 2026-07-03); **nothing in this plan writes under `shared/`**.
- **Codebase-analysis correction (honesty):** the brief's Research Plan rationale mentions
JSON-Schema validation; exploration verified `datasource.py` contains no schema
validation — the fail-fast validation pattern lives in `contracts.py` as pydantic. The
plan follows the pydantic pattern (which the brief's Preferences already mandate).
## Implementation Plan
Every step is TDD (Iron Law): the step's tests are written and observed RED before the
production code that turns them green. `okf.py` and everything under `shared/` are
untouchable in every step (see per-step `forbidden_paths`). All commands run offline.
### Step 1: Fail-fast manifest contract (pydantic, polymorphic, verdict reservation)
- **Files:** `src/portfolio_optimiser/ingest.py` (new), `tests/test_ingest_manifest.py` (new)
- **Changes:** Create the ingest module with its narrative docstring (purpose, spec §
references, MAF-free invariant, D7-portability) and the manifest contract per spec §4:
`FileSource` (`type: Literal["file"]`, `root: str`), `SqlSource` (`type: Literal["sql"]`,
`connection_ref: str`), `HttpSource` (`type: Literal["http"]`, `base_url: str` — reject
embedded credentials (userinfo `@`) via validator, optional `credential_ref: str`), all
with common `id: str` constrained to `^[a-z0-9][a-z0-9-]*$`; `source` as a discriminated
union on `type` (`Field(discriminator="type")`). `Extraction`: `id` (same grammar),
`title` (non-empty, single-line — reject `\n`/`\r`), `query: str`, `okf_type` (non-empty,
single-line, `@field_validator` rejecting `verdict` case-insensitively — spec §3, before
any source call), `max_rows: int = Field(gt=0)`. `ManifestV1`: `manifest_version:
Literal[1]`, `source`, `bundle_summary: str` (non-empty), `extractions` (`min_length=1`,
`@model_validator` rejecting duplicate extraction ids). Public loader
`load_manifest(path: str | Path) -> tuple[ManifestV1, str]` that reads raw bytes ONCE,
computes the `{stem}@{sha256(raw)[:16]}` stamp (spec §5), `json.loads`, validates —
raising `pydantic.ValidationError` / `json.JSONDecodeError` before ANY source access.
Define `IngestError(RuntimeError)` (condition-noun family; used by later steps for
materialization-time refusals). `title` is whitespace-normalized at validation
(`" ".join(title.split())` after the single-line check) so the frontmatter rendering
(which collapses runs, `okf.py:154`) and the index label are guaranteed identical.
The `HttpSource` credential validator is defined precisely: reject when
`urllib.parse.urlsplit(base_url)` yields a non-`None` `username` or `password`
(userinfo is THE credential-embedding URL mechanism §4 targets; query-param token
heuristics are out of scope — documented in the validator docstring).
**Baseline capture (SC7):** BEFORE writing anything in this step, run `uv run pytest -q`
on the clean tree and record the summary line in the session log — this is the
pre-existing-suite baseline SC7's "unchanged-green" is checked against. (new file)
- **Reuses:** pydantic contract idiom from `contracts.py:30-96` (`Field` constraints,
`Literal` enums, `@model_validator(mode="after")` returning `self`); SHA-256 `[:16]`
idiom from `verdicts.py:95`; docstring style from `okf.py:1-12`.
- **Test first:**
- File: `tests/test_ingest_manifest.py` (new)
- Verifies: valid `file` manifest loads with correct stamp (stem + 16-hex of raw bytes);
each missing top-level field raises `ValidationError`; `manifest_version != 1` rejected;
empty `extractions` rejected; bad `id` grammar (uppercase, leading `-`, empty) rejected
for both source and extraction ids; duplicate extraction ids rejected; `max_rows <= 0`
rejected; unknown `source.type` rejected; `file` source missing `root` rejected;
`okf_type` of `verdict`/`Verdict`/`VERDICT` rejected; multi-line `title` rejected;
`base_url` with embedded credentials rejected; **fail-fast ordering proof**: a manifest
that is malformed AND whose `source.root` does not exist raises `ValidationError`
without touching the missing root (the ingest analogue of
`test_no_chat_client_call_on_malformed_contract` in `tests/test_contracts.py`);
`sql`/`http` variants VALIDATE (schema breadth per brief assumption) though no
connector executes them in I2.
- Pattern: `tests/test_contracts.py` (inline dict constants, one `pytest.raises` per
malformation)
- **Verify:** `uv run pytest tests/test_ingest_manifest.py -q` → expected: all pass (after
observed RED on first run without implementation)
- **On failure:** revert — `rm -f src/portfolio_optimiser/ingest.py tests/test_ingest_manifest.py`
(both files are NEW and untracked until this step's checkpoint — `git checkout --` cannot
remove untracked files)
- **Checkpoint:** `git commit -m "feat(ingest): fail-fast manifest contract with verdict reservation (I2)"`
- **Manifest:**
```yaml
manifest:
expected_paths:
- src/portfolio_optimiser/ingest.py
- tests/test_ingest_manifest.py
min_file_count: 2
commit_message_pattern: "^feat\\(ingest\\): fail-fast manifest contract with verdict reservation \\(I2\\)$"
bash_syntax_check: []
forbidden_paths:
- src/portfolio_optimiser/okf.py
- shared/ingest-spec.md
must_contain:
- path: src/portfolio_optimiser/ingest.py
pattern: "load_manifest"
- path: src/portfolio_optimiser/ingest.py
pattern: "discriminator"
```
### Step 2: CSV connector — boundary check, streaming row cap, cell escaping, table body
- **Files:** `src/portfolio_optimiser/ingest.py`, `tests/test_ingest_materialize.py` (new)
- **Changes:** Add the `file` connector + renderer. Connector: resolve the manifest's
`root` against the **manifest file's parent directory** when relative (pinned decision —
extraction must not depend on cwd), then `safe_resolve(root, extraction.query)` for the
fail-closed §4 boundary check; open with `encoding="utf-8-sig"` (BOM never leaks into
the first header cell) and `newline=""`; parse with `csv.reader`. First row = header;
fail (`IngestError`) on empty file (no header). Stream data rows counting against
`max_rows` — raise `IngestError` the moment the cap is exceeded (§8: error, never silent
truncation; no fetch-all-then-count). Fail on ragged rows (data row width ≠ header
width) — silent padding/truncation is coercion. Renderer: cell text **verbatim** with
escaping in this exact order: `\``\\` FIRST, then `|``\|`, then any newline (CR,
LF, CRLF) → single space (pinned decision: on the `file`/CSV path every cell is a
string, so §5's integer/float/NULL clauses bite typed `sql` values in I4 — a spec-text
reading recorded as Assumption 1). Newline replacement treats CRLF as ONE unit: replace
`\r\n` first, then lone `\r`, then lone `\n` — each with a single space (a per-character
replace would turn CRLF into two spaces). A well-formed manifest whose `root` directory
or resolved `query` CSV does not exist raises `IngestError` with a clear message (never a
bare `FileNotFoundError` leaking from `open()`). Body = markdown table: `| h1 | h2 |`
header row, `| --- | --- |` separator, data rows in source order, LF line endings, ending
in exactly one `\n`. **Header cells are escaped identically to data cells** (spec §5
separates "column names" from "cell values" without giving headers their own rule —
pinned decision: same escaping/newline-collapse, or a header containing `|` or an
embedded newline breaks the table). The connector itself is PURE (returns header +
rows); the §8 source-call logging lives in `materialize` (Step 3), which owns
`ingested_at`.
- **Reuses:** `retrieval.safe_resolve`/`PathSecurityError` (`retrieval.py:63,24`) —
identical fail-closed semantics `okf.py` itself relies on.
- **Test first:**
- File: `tests/test_ingest_materialize.py` (new)
- Verifies: table rendering (header/separator/rows, source order); escaping of `\`,
`|`, embedded quoted newline (one assertion each, plus one cell containing BOTH `\`
and `|` proving escape order, plus an explicit embedded-CRLF cell → exactly ONE
space); a header cell containing `|` and one containing an embedded newline →
escaped/collapsed identically to data cells; header-only CSV → header + separator,
zero data rows; empty CSV file → `IngestError`; ragged row → `IngestError`; `max_rows` fixture with cap+1 rows →
`IngestError`; `../escape` query → `PathSecurityError`; missing `root` dir and
missing `query` file → `IngestError` (not bare `FileNotFoundError`); BOM fixture →
first header cell clean.
- Pattern: local `tmp_path` CSV-builder helper (conftest's LLM fixtures are irrelevant
to ingest — no conftest changes)
- **Verify:** `uv run pytest tests/test_ingest_materialize.py -q` → expected: all pass
- **On failure:** revert — `git checkout -- src/portfolio_optimiser/ingest.py && rm -f tests/test_ingest_materialize.py`
(`ingest.py` is tracked after Step 1's checkpoint; the test file is new in this step)
- **Checkpoint:** `git commit -m "feat(ingest): CSV connector with boundary check, row cap and escaped table body (I2)"`
- **Manifest:**
```yaml
manifest:
expected_paths:
- src/portfolio_optimiser/ingest.py
- tests/test_ingest_materialize.py
min_file_count: 2
commit_message_pattern: "^feat\\(ingest\\): CSV connector with boundary check, row cap and escaped table body \\(I2\\)$"
bash_syntax_check: []
forbidden_paths:
- src/portfolio_optimiser/okf.py
- tests/conftest.py
must_contain:
- path: src/portfolio_optimiser/ingest.py
pattern: "safe_resolve"
- path: src/portfolio_optimiser/ingest.py
pattern: "utf-8-sig"
```
### Step 3: Materialization — provenance frontmatter, LF byte-writer, in-memory staging
- **Files:** `src/portfolio_optimiser/ingest.py`, `tests/test_ingest_materialize.py`
- **Changes:** Add the per-extraction concept-file builder and the public entry point
`materialize(manifest_path: str | Path, bundle_dir: str | Path, *, ingested_at: str) ->
list[Path]` (explicit required `ingested_at` keyword, NO wall-clock default — mirrors
`promote_verdict`; validate against the regex `^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z$`
NOT `datetime.fromisoformat`, which rejects the `Z` suffix on Python 3.10, the repo's
version floor — then stamp the string verbatim). This programmatic function IS the I2
invocation surface (recorded decision, resolving the brief's open question: a `python -m`
CLI is not demanded by the I2 verification set and is deferred). `materialize` performs
the §8 source-call log per extraction on logger `portfolio_optimiser.ingest`: source id,
the `ingested_at` argument (deterministic "when" — never wall-clock), row count; never
log cell contents or (in later source types) resolved secrets. It creates the bundle
directory first: `Path(bundle_dir).mkdir(parents=True, exist_ok=True)``safe_resolve`
never creates directories, and unlike `okf.write_concept_file` the byte-writer has no
implicit mkdir. Frontmatter: ordered dict with EXACTLY the §5 keys in §5 order — `type`
(= `okf_type`), `title`, `source_system` (= `source.id`), `source_query` (the query,
whitespace-collapsed), `ingested_at`, `ingest_manifest` (the Step-1 stamp), `generated`
(`true`) — rendered via `okf.render_frontmatter` (insertion-ordered, single-lining).
Filenames: `ingest-{extraction.id}.md` (§5; grammar keeps the namespace disjoint from
`index.md`/`promoted-verdict-*` by construction). File content assembled as ONE string
`---\n{fm}\n---\n\n{body}` (body already ends in exactly one `\n`) and written as bytes
(`content.encode("utf-8")`) to a `safe_resolve`d path — LF-only and exactly one trailing
newline guaranteed on every platform (§5). **All extractions execute and render in
memory BEFORE the first disk mutation** (crash-window mitigation for §5's non-atomic
replace sequence; recovery = idempotent re-run, §10).
- **Reuses:** `okf.render_frontmatter` (`okf.py:146`), `retrieval.safe_resolve`;
`promote_verdict`'s explicit-timestamp shape (`verdicts.py:465-472`).
- **Test first:**
- File: `tests/test_ingest_materialize.py` (extend)
- Verifies: generated file's RAW TEXT has the 7 frontmatter keys in exact §5 order (read
the file text, not just the parsed dict); `okf.parse_frontmatter` round-trips the
values (`generated == "true"` as string; `ingest_manifest == {stem}@{hash16}` computed
independently in the test from the manifest's raw bytes; `ingested_at` verbatim;
`source_query` whitespace-collapsed); file bytes are LF-only with exactly one trailing
`\n` (`read_bytes()` assertions: no `\r`, endswith exactly one `b"\n"`);
missing/invalid `ingested_at` (no argument has no default — non-`Z`/non-ISO string
raises `ValueError`; the golden timestamp `2026-07-03T12:00:00Z` passes the regex);
materializing into a NON-EXISTENT nested `bundle_dir` succeeds (mkdir path); §8 log
record (via `caplog`) carries source id, the explicit `ingested_at` string, and row
count; two `materialize` runs with identical inputs produce byte-identical files
(§10 idempotence at file level).
- Pattern: `tests/test_okf.py` frontmatter round-trip assertions
- **Verify:** `uv run pytest tests/test_ingest_materialize.py -q` → expected: all pass
- **On failure:** revert — `git checkout -- src/portfolio_optimiser/ingest.py tests/test_ingest_materialize.py`
- **Checkpoint:** `git commit -m "feat(ingest): deterministic materialization with §5/§7 provenance stamp (I2)"`
- **Manifest:**
```yaml
manifest:
expected_paths:
- src/portfolio_optimiser/ingest.py
- tests/test_ingest_materialize.py
min_file_count: 2
commit_message_pattern: "^feat\\(ingest\\): deterministic materialization with .5/.7 provenance stamp \\(I2\\)$"
bash_syntax_check: []
forbidden_paths:
- src/portfolio_optimiser/okf.py
must_contain:
- path: src/portfolio_optimiser/ingest.py
pattern: "ingested_at"
- path: src/portfolio_optimiser/ingest.py
pattern: "ingest_manifest"
```
### Step 4: Index generation + stamped-replacement semantics
- **Files:** `src/portfolio_optimiser/ingest.py`, `tests/test_ingest_materialize.py`
- **Changes:** Complete `materialize` with §3/§5/§6 bundle semantics. (a) **Ownership
scan:** classify a bundle file as ingest-owned iff `okf.parse_frontmatter` yields
`generated == "true"` AND an `ingest_manifest` key (string comparisons; sorted directory
scan for determinism). (b) **Collision gate:** if a target filename `ingest-{id}.md`
exists WITHOUT the stamp → `IngestError`, nothing written (§3: never overwrite curated
content). (c) **Replace:** delete every stamped file, then write the new staged set.
(d) **Index:** if `index.md` missing → create it with `bundle_summary` as the body (no
frontmatter — spec-minimal, pinned decision; content = `{bundle_summary}\n` written as
bytes). If present → preserve every unmanaged line byte-for-byte; remove ONLY
whole lines matching the managed-link form `- [<label>](<target>)` whose `<target>` is a
previously-stamped file deleted in this run and NOT re-created (exact full-line
target match, never bare-substring — a promoted verdict's or curated link must be
unreachable by this filter). (e) **Link:** `okf.link_in_index(bundle_dir,
"ingest-{id}.md", title)` per extraction in manifest order (idempotent by target: links
for re-created targets keep their existing position — byte-stability across re-ingest).
(f) **Label refresh:** for a re-created target whose existing managed line's label no
longer equals the (normalized) extraction `title`, rewrite THAT line in place —
`- [{new title}](ingest-{id}.md)` — preserving its position; §6 defines the label AS
the title, so a title change must never leave a stale label behind. The refresh uses
the SAME anchoring as the removal filter: only an exact full-line managed form
`- [<label>](ingest-{id}.md)` targeting THIS run's ingest files is rewritten;
non-conforming lines (curated prose mentioning the target inline, links to non-ingest
files) are untouched. Index update is the LAST disk mutation.
- **Reuses:** `okf.link_in_index` (`okf.py:168` — the exact primitive spec §6 cites),
`okf.parse_frontmatter` (`okf.py:31`).
- **Test first:**
- File: `tests/test_ingest_materialize.py` (extend)
- Verifies: fresh dir → `index.md` created with `bundle_summary` body + one link per
extraction in manifest order; existing `index.md` with a curated line → line preserved
byte-for-byte after materialization; re-materialization with an extraction REMOVED
from the manifest → its file gone AND its index link gone, other links intact;
re-materialization with a CHANGED extraction `title` (same id) → the managed link's
label updated in place, position preserved (no stale label); unstamped
`ingest-foo.md` present → `IngestError` and bundle unmodified (assert directory bytes
unchanged); re-materialization twice with identical inputs → byte-identical bundle
including `index.md` (§10).
- Pattern: `tests/test_okf.py::test_link_in_index_makes_concept_file_navigable`
- **Verify:** `uv run pytest tests/test_ingest_materialize.py -q` → expected: all pass
- **On failure:** revert — `git checkout -- src/portfolio_optimiser/ingest.py tests/test_ingest_materialize.py`
- **Checkpoint:** `git commit -m "feat(ingest): index generation and stamped-replacement semantics (I2)"`
- **Manifest:**
```yaml
manifest:
expected_paths:
- src/portfolio_optimiser/ingest.py
- tests/test_ingest_materialize.py
min_file_count: 2
commit_message_pattern: "^feat\\(ingest\\): index generation and stamped-replacement semantics \\(I2\\)$"
bash_syntax_check: []
forbidden_paths:
- src/portfolio_optimiser/okf.py
- src/portfolio_optimiser/verdicts.py
must_contain:
- path: src/portfolio_optimiser/ingest.py
pattern: "link_in_index"
- path: src/portfolio_optimiser/ingest.py
pattern: "bundle_summary"
```
### Step 5: Load-bearing seam quartet + MAF-free guard (detach-RED proofs)
- **Files:** `tests/test_ingest_loadbearing.py` (new)
- **Changes:** The durable seam tests per spec §11's table, in the repo's load-bearing
idiom (module docstring naming each seam + its detach-RED condition; scenarios built so
the seam is the ONLY thing preventing the observable artifact). (1) **Verdict
reservation:** an otherwise fully-valid manifest with `okf_type: "Verdict"`
`ValidationError` at load, zero files/links written (RED if the case-insensitive
reservation validator is removed). Plus the defensive namespace invariant: the id
grammar + `ingest-` prefix make `promoted-verdict-*`/`index.md` collisions structurally
unreachable — asserted as invariant, with the honest note that its load-bearing bite is
the `okf_type` check. (2) **Provenance stamping:** materialize, then assert the §7 layer
on every generated file via UNCHANGED `okf.parse_frontmatter` (RED if the stamp layer is
dropped). (3) **Navigability via UNCHANGED okf.py** (the durable version of the planning
session's ad-hoc proof): `okf.navigate_bundle` reaches every `ingest-{id}.md` through
index cross-links; each is classified by its `okf_type`; provenance fields ride through
`frontmatter` (unknown-field preservation); `okf.bundle_context` renders the generated
content (RED if index linking detaches — file unreachable). (4) **Re-ingest layer
safety — with the removal filter ACTIVE:** on a `shutil.copytree` copy of
`shared/examples/bygg-energi-mikro/`, run `materialize` (manifest with ≥2 extractions),
then the REAL `verdicts.promote_verdict` (approved verdict, explicit timestamp), then
`materialize` AGAIN with a REDUCED manifest (one extraction dropped) — the §6
managed-line removal filter must actually FIRE while the promoted link and the bundle's
curated managed-form links are present (an identical-manifest second run never removes
anything, so it cannot catch a loose filter — the green-but-dead trap): the dropped
extraction's file AND index link are gone; the promoted verdict file and its index link
survive verbatim; every curated link survives verbatim; only stamped files were
replaced (RED if the filter over-matches — `promote_verdict` writes exactly the
managed-line shape a bare-substring filter would wrongly delete — or if replacement
stops honouring the stamp ownership rule). (5) **MAF-free guard:** AST-parse
`src/portfolio_optimiser/ingest.py` (pattern: `tests/test_okf.py::test_okf_is_maf_free`)
asserting no import roots in `{agent_framework, mcp}` AND no `portfolio_optimiser.*`
imports outside `{okf, retrieval}` (transitively stdlib-pure — `contracts`/`backends`
would smuggle MAF in). **Execution rule:** each of (1)(4) is proven RED by temporarily
detaching its seam (comment out the validator / stamp keys / `link_in_index` call / the
stamp filter), observing the failure, reattaching; the detach-proof log (seam → detached
how → observed failure) goes into the session verification log for STATE/commit message.
- **Reuses:** `_copy_bundle` copytree idiom (`tests/test_step8_promotion_loadbearing.py:51`),
AST guard (`tests/test_okf.py:165-181`), real `promote_verdict` (`verdicts.py:465`).
- **Test first:**
- File: `tests/test_ingest_loadbearing.py` (new — this whole step IS tests; the seams it
guards were built in Steps 14, so these must pass immediately and the detach-proofs
demonstrate they can fail)
- Verifies: the four §11 seams + module purity, as above
- Pattern: `tests/test_step8_promotion_loadbearing.py` (docstring naming detach-RED per test)
- **Verify:** `uv run pytest tests/test_ingest_loadbearing.py -q` → expected: all pass;
then 4 documented detach→RED→reattach cycles each showing ≥1 failure
- **On failure:** escalate — a quartet test that cannot go RED at detach means the seam is
not actually load-bearing; stop and redesign the test before proceeding (do not weaken
the assertion). Cleanup on abandon: `rm -f tests/test_ingest_loadbearing.py` (new file —
untracked until this step's checkpoint)
- **Checkpoint:** `git commit -m "test(ingest): load-bearing seam quartet + MAF-free guard, detach-proven (I2)"`
- **Manifest:**
```yaml
manifest:
expected_paths:
- tests/test_ingest_loadbearing.py
min_file_count: 1
commit_message_pattern: "^test\\(ingest\\): load-bearing seam quartet \\+ MAF-free guard, detach-proven \\(I2\\)$"
bash_syntax_check: []
forbidden_paths:
- src/portfolio_optimiser/okf.py
- shared/examples/bygg-energi-mikro/index.md
must_contain:
- path: tests/test_ingest_loadbearing.py
pattern: "promote_verdict"
- path: tests/test_ingest_loadbearing.py
pattern: "navigate_bundle"
```
### Step 6: Spec §12 field cross-check (discharge the I1-recorded obligation)
- **Files:** `tests/test_method_spec_loadbearing.py`
- **Changes:** Add `test_ingest_spec_documents_every_contract_field`, mirroring the
existing `test_spec_documents_every_contract_field` (line 106): iterate the REAL pydantic
models' `model_fields` (ManifestV1, recursively into the three source variants and
Extraction) plus the seven §5 frontmatter keys plus the four golden-case entries
(`manifest.json`, `fixture/`, `ingested-at.txt`, `expected-bundle/`), asserting each
token appears in `shared/ingest-spec.md` (the §12 cross-check table's completeness,
test-enforced at field level). This discharges the deferred note recorded at
`tests/test_method_spec_loadbearing.py:156-162` ("arrives with the I2 contracts").
RED when contract code and spec drift apart. Surgical: only ADD the test — the I1
structure/neutrality guards in the file are untouched.
- **Reuses:** the file's own test-3 pattern (`model_fields`-driven spec assertion).
- **Test first:**
- File: `tests/test_method_spec_loadbearing.py` (existing)
- Verifies: every machine-readable contract field is documented in the frozen spec; a
hypothetical new model field without a spec entry turns it RED (detach-proof: add a
dummy field to a throwaway copy check — verified by temporarily adding a fake field
name to the iteration list, observing RED, removing)
- Pattern: `tests/test_method_spec_loadbearing.py::test_spec_documents_every_contract_field`
- **Verify:** `uv run pytest tests/test_method_spec_loadbearing.py -q` → expected: all pass
- **On failure:** escalate — a genuine field↔spec mismatch means either the contract
deviates from the frozen spec (fix the contract) or the spec is defective (STOP: spec
changes are a separate GATED commons round, never folded into I2)
- **Checkpoint:** `git commit -m "test(spec-guard): ingest contract field cross-check vs spec §12 (I2)"`
- **Manifest:**
```yaml
manifest:
expected_paths:
- tests/test_method_spec_loadbearing.py
min_file_count: 1
commit_message_pattern: "^test\\(spec-guard\\): ingest contract field cross-check vs spec .12 \\(I2\\)$"
bash_syntax_check: []
forbidden_paths:
- shared/ingest-spec.md
- shared/method-spec.md
must_contain:
- path: tests/test_method_spec_loadbearing.py
pattern: "test_ingest_spec_documents_every_contract_field"
```
### Step 7: Golden extraction case + byte-for-byte regression test
- **Files:** `examples/ingest-golden-file/` (new directory), `tests/test_ingest_golden.py` (new)
- **Changes:** Author the golden case per spec §11 layout, LOCALLY in this repo (brief
Non-Goal: commons sharing is a later gated round; `shared/` is untouchable). New
top-level `examples/` directory (does not exist yet; outside ruff's `src` config —
fixture data, not lint targets): `manifest.json` (file source, `root: "fixture"`
resolved against the manifest's directory, 2 extractions: a plain cost table AND an
edge-case table whose quoted CSV cells exercise `\`, `|`, an embedded newline, a cell
with both `\` and `|`, AND number-shaped cells that DISCRIMINATE the text-verbatim
reading — `007` and `1.50`, strings whose numeric normalization differs byte-wise
(`7`, `1.5`); Assumption 1 is only pinned if a number-normalizing implementation CANNOT
reproduce the golden — a cell like `30000.0` discriminates nothing since
`repr(30000.0)` round-trips identically), `fixture/*.csv`, `ingested-at.txt`
(`2026-07-03T12:00:00Z`, one line). `expected-bundle/` is generated ONCE by the (now fully behaviour-tested)
implementation, its content REVIEWED against the spec §5/§6/§7 rules by hand, then
frozen in git (brief Assumption 4: golden = regression pin; correctness is carried by
Steps 16's tests; the freeze happens LAST). Golden test: read `ingested-at.txt`, run
`materialize` into `tmp_path`, then assert (a) the produced file SET equals the
`expected-bundle/` file set (catches extra AND missing files) and (b) every file's
`read_bytes()` matches exactly. A second `materialize` run over the same output must
leave every byte unchanged (golden-level idempotence, §10).
- **Reuses:** `BUNDLE_DIR`-style module-level fixture-path idiom (`tests/test_okf.py`,
`tests/test_step8_promotion_loadbearing.py`).
- **Test first:**
- File: `tests/test_ingest_golden.py` (new — written BEFORE `expected-bundle/` exists;
observed RED against the empty expectation, then the reviewed freeze turns it green)
- Verifies: byte-for-byte golden regression + file-set equality + repeat-run idempotence
- Pattern: `tests/test_okf.py` fixture-path idiom
- **Verify:** `uv run pytest tests/test_ingest_golden.py -q` → expected: all pass; then the
full gate battery: `uv run pytest` (pre-existing suite unchanged-green vs the Step-1
baseline + all new tests), `uv run ruff check .` → 0 issues,
`uv run ruff format --check .` → clean, `uv run mypy src` → 0 errors; honesty grep
(brief SC8 — scoped to PROSE per the brief: docstrings/comments of the new module and
new test files, plus any touched docs):
`grep -rinE "sql|http|live|commons" src/portfolio_optimiser/ingest.py tests/test_ingest_*.py`
every PROSE hit is a defer-mention (I4/I6/extension point/gated commons round) or a
spec citation; hits that are polymorphic-contract SCHEMA IDENTIFIERS (`SqlSource`,
`HttpSource`, `Literal["sql"]`, `Literal["http"]`, `connection_ref`, `base_url`,
`credential_ref`) are expected and allowed — they validate variants per §4 and claim
no delivered connector support
- **On failure:** escalate — a golden mismatch after Steps 16 are green means a
determinism defect (platform newline, ordering, escaping); diagnose root cause before
re-freezing — NEVER re-freeze to make a mismatch disappear. Cleanup on abandon:
`rm -rf examples/ tests/test_ingest_golden.py` (new, untracked until checkpoint)
- **Checkpoint:** `git commit -m "test(ingest): golden extraction case examples/ingest-golden-file, byte-frozen (I2)"`
- **Manifest:**
```yaml
manifest:
expected_paths:
- examples/ingest-golden-file/manifest.json
- examples/ingest-golden-file/ingested-at.txt
- examples/ingest-golden-file/fixture/costs.csv
- examples/ingest-golden-file/expected-bundle/index.md
- tests/test_ingest_golden.py
min_file_count: 5
commit_message_pattern: "^test\\(ingest\\): golden extraction case examples/ingest-golden-file, byte-frozen \\(I2\\)$"
bash_syntax_check: []
forbidden_paths:
- src/portfolio_optimiser/okf.py
- shared/ingest-spec.md
must_contain:
- path: tests/test_ingest_golden.py
pattern: "read_bytes"
- path: examples/ingest-golden-file/ingested-at.txt
pattern: "2026-07-03T12:00:00Z"
```
## Execution Strategy
Seven steps, strictly sequential — every step builds on the previous step's code and the
session's operating model is single-session TDD (one session = one closed delivery). No
parallel waves.
### Session 1: I2 complete (this session)
- **Steps:** 1, 2, 3, 4, 5, 6, 7
- **Wave:** 1
- **Depends on:** none
- **Scope fence:**
- Touch: `src/portfolio_optimiser/ingest.py`, `tests/test_ingest_*.py`,
`tests/test_ingest_golden.py`, `tests/test_method_spec_loadbearing.py` (additive only),
`examples/ingest-golden-file/**`
- Never touch: `src/portfolio_optimiser/okf.py`, `src/portfolio_optimiser/run.py`,
`src/portfolio_optimiser/verdicts.py`, `src/portfolio_optimiser/contracts.py`,
`shared/**`, `tests/conftest.py`, `pyproject.toml` (no new deps needed)
### Execution Order
- **Wave 1:** Session 1 (sequential steps 1→7)
### Grouping rules applied
- All steps share `src/portfolio_optimiser/ingest.py` → one session by necessity.
- TDD dependency chain (contract → connector → materializer → semantics → seams → guard →
golden freeze) forbids reordering.
## Alternatives Considered
| Approach | Pros | Cons | Why rejected |
|----------|------|------|--------------|
| Add a link-removal primitive to `okf.py` | Symmetric API next to `link_in_index` | Violates the brief's hard constraint (navigability criterion is "consumable via UNCHANGED okf.py"); widens D7-shared surface mid-program | Brief Constraints; removal lives in `ingest.py` |
| Define manifest models in `contracts.py` | One contracts home | `contracts.py:25` imports `backends``agent_framework`; ingest module would fail its MAF-free guard; also violates surgical-scope | Brief Constraints (MAF-free ingest seam) |
| Reuse `okf.write_concept_file` for file writes | Maximum reuse, path-safety built in | `Path.write_text` translates newlines per platform and doesn't guarantee exactly one trailing `\n` — §5 LF-only byte-determinism at risk | Risk assessment (High); ingest writes bytes itself, still via `safe_resolve` + `render_frontmatter` |
| Validate only the `file` source variant in I2 | Smaller Step 1 | Spec §4's schema is polymorphic; I4 would retrofit the union; brief assumption already commits to schema breadth with connector-narrowness | Brief Assumption 1 (schema polymorphic, connector `file`-only) |
| Hand-author `expected-bundle/` before implementing | "Purer" golden TDD | Byte-level hand-typing of hash stamps and escaping is error-prone; correctness is already carried by Steps 16 behaviour tests; golden is a regression PIN (spec §11) | Brief Assumption 4 (generate once, review, freeze LAST) |
| Remove + re-append index links for re-created targets | Simpler §6 filter | Moves ingest links to index end whenever anything sits between; unnecessary byte-churn breaks repeat-run stability | Idempotence (§10): keep links in place for re-created targets — WITH the Step-4(f) in-place label refresh, so a changed title never leaves a stale label (plan-critic major finding) |
## Test Strategy
- **Framework:** pytest ≥8, plain sync tests (ingest is deterministic — no asyncio), no new
markers, no conftest changes, no mocking library (repo convention: hand-built doubles;
ingest needs only `tmp_path` + small CSV builders).
- **Existing patterns:** fail-fast contract tests (`tests/test_contracts.py`), detach-RED
load-bearing modules (`tests/test_step8_promotion_loadbearing.py`), navigability
round-trips (`tests/test_okf.py`), spec cross-check (`tests/test_method_spec_loadbearing.py`).
- **New tests in this plan:** ~30 tests across 4 new files + 1 extended file.
### Tests to write
| Type | File | Verifies | Model test |
|------|------|----------|------------|
| Unit | `tests/test_ingest_manifest.py` | §4 schema fail-fast, verdict reservation, grammar, fail-before-source-access | `tests/test_contracts.py` |
| Unit | `tests/test_ingest_materialize.py` | connector boundary/cap/escaping, §5 frontmatter order, LF bytes, §6 index semantics, collision gate, idempotence | `tests/test_okf.py` |
| Load-bearing | `tests/test_ingest_loadbearing.py` | the four §11 seams + MAF-free module guard, each detach-RED-proven | `tests/test_step8_promotion_loadbearing.py` |
| Spec guard | `tests/test_method_spec_loadbearing.py` | §12 field-level contract↔spec cross-check | its own test 3 |
| Golden | `tests/test_ingest_golden.py` | byte-for-byte regression + file-set equality + repeat-run idempotence | `tests/test_okf.py` fixture idiom |
## Risks and Mitigations
| Priority | Risk | Location | Impact | Mitigation |
|----------|------|----------|--------|------------|
| Critical | §6 link removal implemented as substring match deletes a promoted verdict's or curated link | new removal filter in `ingest.py`; `okf.py:168-181` is append-only | Corrupted `index.md`; §11 re-ingest seam broken | Exact whole-line managed-link match, only for targets deleted this run and not re-created (Step 4); re-ingest safety test uses the REAL `promote_verdict` (Step 5) |
| Critical | Crash mid-replace leaves half-materialized bundle (§5 sequence is non-atomic) | `materialize` orchestration | Loop navigates a broken bundle until re-run | Full in-memory staging before first disk mutation; index update last; recovery = idempotent re-run (§10) — documented, not hidden |
| High | Reformatting numeric-looking CSV text (e.g. `"30000.0"`, `"007"`) breaks golden bytes | renderer, spec §5 reading | Golden divergence; sibling (I3) mismatch | Pinned decision: CSV cells text-verbatim; §5 number rules are typed-`sql` territory (I4). Recorded as Assumption 1; golden fixture pins it |
| High | Escape-order bug (`|` before `\`) double-escapes | renderer | Silent cell corruption | Fixed order `\``\\` then `|``\|`; both-chars cell in unit test AND golden fixture |
| High | Platform newline translation / trailing-newline drift breaks byte-determinism | file writes | Golden mismatch cross-platform | Bytes writer with explicit `\n` for concept files (Step 3); `read_bytes()` assertions; `okf.write_concept_file` not used for writes. Index appends stay on the frozen `link_in_index` → LF-ness there is platform-scoped (Assumption 6), nil on macOS/Linux |
| Medium | BOM/CRLF/embedded-newline CSV mishandling | connector | First header cell polluted; golden flaky | `utf-8-sig` + `newline=""` + `csv.reader` (pinned); BOM + CRLF + embedded-newline unit fixtures |
| Medium | Stamp misclassification (string `"true"` vs bool) deletes curated file or strands stale ingest files | ownership scan; `okf.py:46` returns strings | §3 violation | Classify on `generated == "true"` AND `ingest_manifest` present; collision gate test (unstamped `ingest-foo.md` → error, bundle unmodified) |
| Medium | `link_in_index` raises when `index.md` missing | `okf.py:176` | Mid-run crash on fresh bundle | Create `index.md` (bundle_summary body) before any linking (Step 4 order) |
| Medium | Title containing `](x.md)` injects a parseable index link | index label = raw title; `okf.py:28` | Spurious navigation targets (skipped if missing — fail-closed) | Spec-exact validation only (single-line); limitation documented in module docstring — spec change would be a gated commons round |
| Low | Golden re-frozen to hide a real determinism defect | Step 7 | Regression pin becomes a lie | Step 7 On-failure rule: diagnose root cause; NEVER re-freeze to green a mismatch |
## Assumptions
| # | Assumption | Why unverifiable | Impact if wrong |
|---|-----------|-----------------|-----------------|
| 1 | §5's integer/float/NULL rendering rules apply to typed (`sql`) values; `file`/CSV cells are text-verbatim (escape-only) | Spec prose lists both under one bullet; the frozen spec cannot be amended inside I2 | Golden bytes + sibling (I3) diverge; if I3 reads it differently, a GATED commons clarification round is triggered — the golden fixture makes the chosen reading explicit and shareable |
| 2 | Fresh `index.md` is created WITHOUT frontmatter (body = `bundle_summary` only) — spec §6 is silent on index frontmatter | Spec silence; curated example (`bygg-energi-mikro/index.md`) has `type: index` but is hand-authored, not generated | Cosmetic divergence pinned by the golden; `navigate_bundle` handles both (frontmatter-less parse returns `{}`) — verified `okf.py:38-39` |
| 3 | Relative `root` resolves against the manifest file's directory | Spec §4 does not name the resolution base | Golden becomes cwd-dependent if wrong; pinned decision documented in docstring + spec cross-check exercise for I3 |
| 4 | `utf-8-sig` reading (BOM tolerated and stripped) is the correct connector-side choice | Spec silent on encoding | A BOM-carrying golden fixture in the sibling could differ; our golden contains no BOM in `fixture/`, so the shared surface is unaffected |
| 5 | Empty CSV (no header), ragged rows, and missing `root`/`query` paths are `IngestError` fail-fast (never silent coercion/leaked `FileNotFoundError`) | Spec §5's MUST-fail clause addresses typed values; these input-shape failures are unaddressed spec silence | I3 sibling could choose differently; the decisions are documented in the module docstring and exercised by unit tests so the I3 cross-check surfaces any divergence — if the sibling disagrees, a GATED commons clarification round resolves it |
| 6 | LF-only bytes are platform-scoped for `index.md`: concept files go through the ingest byte-writer (LF everywhere), but index appends REUSE the frozen `okf.link_in_index`, whose `Path.write_text` translates newlines per platform (and `read_text` universal-newline-decodes a CRLF curated index on round-trip) | The brief mandates reusing `link_in_index` and forbids changing `okf.py` — the platform behaviour is frozen with it | On the LF-only development/target platform (macOS/Linux) impact is nil and the golden holds; a Windows deployment would need its own golden verification — documented limitation, not a universal cross-platform claim |
*6 items — above the template's 3-item investigation threshold, accepted deliberately:
all are spec-SILENCE or platform-scope points (not unknowns about this codebase) pinned
by the golden fixture and/or documented decisions — exactly the mechanism the program
uses to surface such choices to I3; none blocks execution.*
## Verification
*Per-step manifests verify automatically during execution. These are the end-to-end
checks crossing step boundaries — they restate the brief's Success Criteria (SC1SC8).*
- [ ] SC1: `uv run pytest tests/test_ingest_golden.py -q` → all pass (byte-for-byte +
repeat-run idempotence)
- [ ] SC2SC5: `uv run pytest tests/test_ingest_loadbearing.py -q` → all pass, AND the
session log records four detach→RED→reattach proofs (provenance, navigability,
verdict reservation, re-ingest safety)
- [ ] SC3 (unchanged okf.py): `git diff main@{session-start} -- src/portfolio_optimiser/okf.py`
→ empty output
- [ ] SC6: `uv run pytest tests/test_ingest_manifest.py -q` → all pass with no network
available (key assumption 2 closed)
- [ ] SC7: `uv run pytest` → exit 0, pre-existing suite unchanged-green vs the baseline
recorded in Step 1 (pre-change `uv run pytest -q` summary in the session log) + new
tests; `uv run ruff check .` → exit 0; `uv run ruff format --check .` → clean;
`uv run mypy src` → exit 0
- [ ] SC8: `grep -rinE "sql|http|live|commons" src/portfolio_optimiser/ingest.py tests/test_ingest*.py`
→ every PROSE hit (docstrings/comments — the brief's scope) is a defer-mention
(I4/I6/extension point/gated) or spec citation; polymorphic-contract schema
identifiers (`SqlSource`/`HttpSource`/`Literal["sql"|"http"]`/`connection_ref`/
`base_url`/`credential_ref`) are allowed hits claiming no delivered support
- [ ] Layer hygiene: `uv run pytest tests/test_okf.py tests/test_method_spec_loadbearing.py -q`
→ all pass (MAF-free + spec guards intact)
## Estimated Scope
- **Files to modify:** 1 (`tests/test_method_spec_loadbearing.py`, additive)
- **Files to create:** 5 code/test files (`src/portfolio_optimiser/ingest.py`,
`tests/test_ingest_manifest.py`, `tests/test_ingest_materialize.py`,
`tests/test_ingest_loadbearing.py`, `tests/test_ingest_golden.py`) + the
`examples/ingest-golden-file/` fixture tree (~6 small files)
- **Complexity:** medium
## Plan Quality Score
| Dimension | Weight | Score | Notes |
|-----------|--------|-------|-------|
| Structural integrity | 0.15 | 90 | strict TDD dependency chain; Step-2/3 logging ownership clarified post-review |
| Step quality | 0.20 | 88 | 1 module + 1 test file per step; all 3 critic majors fixed in revision |
| Coverage completeness | 0.20 | 88 | all 8 SCs + §11 seams mapped; title-refresh, fresh-dir, missing-source cases added post-review |
| Specification quality | 0.15 | 88 | 5 spec-silence points pinned as explicit assumptions |
| Risk & pre-mortem | 0.15 | 88 | risk table incl. 2 critical collisions; review added revert/mkdir/staleness fixes |
| Headless readiness | 0.10 | 85 | On-failure clauses corrected for untracked files; baseline capture now explicit in Step 1 |
| Manifest quality | 0.05 | 88 | Step-7 manifest now pins the byte-frozen golden artifacts |
| **Weighted total** | **1.00** | **88** | **Grade: B+** |
**Adversarial review:**
- **Plan critic:** APPROVE_WITH_NOTES (81/100, grade B) — 0 blockers, 3 major, 10 minor;
all 3 majors and 9 of 10 minors resolved in the revision below (the remaining minor —
SC4's filename-namespace clause being structurally covered — is accepted as honest
by-construction coverage, matching scope-guardian's reading).
- **Scope guardian:** ALIGNED — 0 creep, 0 dependency issues, 3 gaps (1 major: SC8 grep
scope; 2 minor: baseline step, SC4 structural note); the major and the baseline gap are
resolved below.
## Revisions
*Added by adversarial review (plan-critic + scope-guardian, deduped 16 findings).*
| # | Finding | Severity | Resolution |
|---|---------|----------|------------|
| 1 | Step 1 On-failure `git checkout --` cannot remove untracked new files | major | Step 1 revert → `rm -f`; Step 2 revert split (tracked checkout + `rm` new test file); Steps 5/7 got explicit cleanup-on-abandon commands |
| 2 | Changed extraction `title` on re-ingest left a stale index label (kept-in-place links) | major | Step 4 gained (f) in-place label refresh preserving line position + a changed-title test; Alternatives row updated |
| 3 | `materialize` wrote bytes without creating `bundle_dir` (byte-writer has no mkdir) | major | Step 3 now mkdirs `bundle_dir` first + fresh-nested-dir test added |
| 4 | SC8 whole-file grep unachievable — schema identifiers (`SqlSource` etc.) would false-positive | major | SC8 (Step 7 + Verification) rescoped to PROSE hits per the brief, with schema identifiers as an explicitly allowed category |
| 5 | `ingested_at` validation via `fromisoformat` breaks on py3.10 (`Z` unsupported) | minor | Pinned to an explicit regex; py3.10 trap documented in Step 3 |
| 6 | Missing `root`/`query` leaked bare `FileNotFoundError` | minor | Wrapped in `IngestError` + tests added (Step 2) |
| 7 | `render_frontmatter` collapses title whitespace → frontmatter/label divergence | minor | `title` whitespace-normalized at validation so both representations agree (Step 1) |
| 8 | CRLF could become two spaces | minor | CRLF replaced as one unit (`\r\n` first) + explicit CRLF test (Step 2) |
| 9 | Step 2 logged `ingested_at` it does not own | minor | Connector is pure; §8 logging moved to `materialize` (Step 3) with the caplog test |
| 10 | `HttpSource` credential validator under-defined | minor | Defined as `urlsplit` userinfo check; query-param heuristics explicitly out of scope (Step 1) |
| 11 | Ragged/empty-file decisions unpinned vs I3 sibling | minor | Added as Assumption 5 (documented + unit-tested divergence surface) |
| 12 | No baseline-capture step for SC7 | minor | Step 1 now opens with a pre-change `uv run pytest -q` baseline recording; SC7 references it |
| 13 | Step 7 manifest omitted the byte-frozen golden artifacts | minor | `expected_paths` now includes `fixture/costs.csv` + `expected-bundle/index.md`, `min_file_count: 5` |
| 14 | Invocation surface (API vs CLI) unstated | minor | Recorded in Step 3: programmatic `materialize` IS the I2 surface; CLI deferred |
| 15 | SC4 filename-namespace clause covered by construction, not detach-testable | minor | Accepted as-is: honestly documented invariant (both reviewers concur); load-bearing bite is the `okf_type` check |
## Adversarial Pass 2 (independent second pass, v5.1.1 high-effort)
*Mandated by the brief's `effort: high` plan signal. The prescribed `gemini-bridge` pass
was SUBSTITUTED with an independent fresh-context contrarian reviewer: Gemini deep-research
result retrieval is verified broken as of 2026-07-03 (research completes, result
unretrievable — operator memory), so a bridge run would burn time with no retrievable
output. The substitute preserves the intent — an independent perspective that does not
re-tread Pass 1's dedup space (it was given Pass 1's findings as excluded ground).*
Verdict: **PASS2_FINDINGS(5)** — 0 blocker, 3 major, 2 minor. All five incorporated:
| # | Finding | Severity | Resolution |
|---|---------|----------|------------|
| P2-1 | Re-ingest safety test ran `materialize` twice with an IDENTICAL manifest — the §6 removal filter never fired, so the plan's own Critical risk (loose filter deleting a promoted/curated link) was untested (green-but-dead) | major | Step 5(4) rewritten: second run uses a REDUCED manifest so removal fires with the promoted verdict + curated managed-form links present; over-match → RED |
| P2-2 | Golden fixture had no cell discriminating text-verbatim vs number-normalization — Assumption 1 was not actually pinned (`30000.0` round-trips identically) | major | Step 7 fixture now mandates `007` and `1.50` cells whose normalized forms differ byte-wise |
| P2-3 | Index writes route through the frozen `link_in_index` (`Path.write_text`) — the "LF-only on every platform" claim contradicted the plan's own reason for avoiding `write_concept_file` | major | Determinism claim rescoped: concept files LF-guaranteed via byte-writer; `index.md` LF-ness platform-scoped (nil on macOS/Linux) — Assumption 6 + risk-row update |
| P2-4 | Header-cell escaping unspecified (spec §5 separates "column names" from "cell values") — a `|`/newline header would break the table | minor | Step 2: headers escaped identically to data cells (pinned) + header edge-case tests |
| P2-5 | Step 4(f) label refresh did not restate the removal filter's full-line/ingest-target anchoring | minor | Step 4(f) now requires the same exact managed-line anchoring; non-conforming lines untouched |
Probed and clean (per the reviewer): §12 cross-check test CAN go red; file-deletion
ownership seam holds against real `promote_verdict`; no invariant violations (okf.py
unchanged, `shared/` untouched, MAF-free, no new deps); escape order correct; golden
`tmp_path` second run does exercise re-ingest-over-existing.