portfolio-optimiser/docs/extending.md
Kjell Tore Guttormsen c02c1addba fix(semretrieval): refuse a non-finite embedding instead of scoring it (kø-(l)/S3.1 MINOR)
`cosine`'s docstring claimed its guard was load-bearing because "a NaN reaching the
ranking sort key would corrupt ordering silently rather than failing loudly" — but the
guard tested `norm == 0.0` only, which a NaN or inf norm passes straight through. The
claim was prose, not behaviour.

Measured, not assumed: `cosine(unit, nan_vector)` AND `cosine(unit, inf_vector)` both
returned `nan`, and a NaN sort key made ranking INPUT-ORDER-DEPENDENT — six permutations
of the same three candidates produced four distinct orderings. That defeats the total
order `HybridRanker` documents ("`id` makes the result independent of input order").

Refuse rather than coerce, and deliberately NOT symmetric with the zero-norm branch: a
zero vector is a legitimate handled state (`FakeEmbedder` returns `np.zeros` by design),
whereas a non-finite component only ever means the INJECTED embedder is broken. Scoring
it `0.0` would launder that into "no semantic similarity" while ranking proceeded on a
forged signal — validation, never repair, mirroring `read_spend`.

Reachable via the documented `Embedder` extension point, not the shipped fake; scoped to
the norms (90% principle — a finite-normed dot-product overflow is not chased).

Also corrects `docs/extending.md`, which stated `SEMANTIC_WEIGHT_DEFAULT = 0.5` while the
code has said `0.25` since the weight was lowered.

625 -> 630 tests. Load-bearing MEASURED against the WHOLE suite, five mutations all red:
detach the guard entirely · coerce to 0.0 instead of raising · check only the first norm ·
drop "non-finite" from the message · (control) detach the zero-norm branch, which fails
ONLY the zero-norm test — the new guard does not mask the existing one.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018V9vNBmxAmgJ2JMoHByiHS
2026-08-03 21:48:50 +02:00

275 lines
19 KiB
Markdown

# Extending the framework (extension points)
portfolio-optimiser is a generic core with explicit **config seams** (D4/D5, 90 %-prinsippet):
you onboard a new project, a new data source, or a new model-map **without editing the core
`src/portfolio_optimiser/*.py`**. The three guides below name the exact seam for each.
> **Honesty note (rent teknisk rammeverk).** The bundled reference domain
> (`data/reference_projects.json` + `data/docs/<id>/`) is a set of **SYNTHETIC, AI-authored
> fixtures** — fictional construction-cost projects, dummy estimates, and placeholder
> `verdict_input` decisions. They are flagged in each file's `_note`. A production deployer
> **replaces the data source** with their own and supplies **Layer-2 verdicts via real HITL**
> (fageksperter), not static config. The static `verdict_input` field is a test-fixture
> convenience that stands in for the durable HITL verdict in the offline synthetic framework.
## Legg til eget prosjekt
A new project is **config + docs only** — no code change (this is exercised by the SC1 test
`test_e_new_project_flows_through_via_config_only` + the `test_f_no_hardcoded_project_ids_in_src`
guard, which fails if any project id leaks into `src/`).
1. Append an entry to `src/portfolio_optimiser/data/reference_projects.json` with the full key
set: `id`, `name`, `description`, `currency`, `cost_items`, **`docs_dir`**, and
**`verdict_input`** (`{"decision", "rationale"}`).
- `docs_dir` is a path **relative to the package `data/` root** (e.g. `"docs/MY-PROJECT"`);
the loader (`reference_domain.load_reference_projects`) resolves it to an absolute path.
- `verdict_input` carries the (synthetic) Layer-2 decision/rationale; flag it in the file's
`_note` as synthetic if it is not a real expert verdict.
2. Create the bundled docs folder `src/portfolio_optimiser/data/docs/<id>/` with at least one
text file whose content names the cost-saving measure/terms (so `retrieve_chunks` returns at
least one citable chunk).
3. Run the project: `run_portfolio(["MY-PROJECT"], "local", client_factory=...)` (or include it
in the default fan-out by passing no `project_ids`).
## Legg til egen datakilde
The retriever (`retrieval.py` / `datasource.py`) reads a **local docs folder** per project,
selected by the project's `docs_dir` in `reference_projects.json`. To point a project at your own
data, change its `docs_dir` to your folder and drop your cost documentation there — the citation
seam (`{file, locator, snippet, score}`) is identical on the in-process tool and MCP paths. The
folder is boundary-checked (fail-closed) against path traversal, so keep documents inside the
configured `docs_dir`. A real deployer swaps the bundled synthetic docs for their own source.
## Legg til egen modell-map
Model choice is **config, not code** (B12): `src/portfolio_optimiser/data/model_map.json` maps
`profile -> role -> model/deployment` (`resolve_model(profile, role)`). To use your own models:
- Edit the `local` block to your local model ids (Ollama/LM Studio), and/or
- Edit the `azure` block to your Foundry deployment names (the placeholders
`REPLACE-WITH-FOUNDRY-DEPLOYMENT` are tenant-specific — replace them or supply via env).
The role keys (`proposer`, `checker`, `default`) let you assign a distinct model per debate role;
`default` is the fallback when a role is unmapped.
## Legg til en ingest-kilde (http-familien som worked example)
The **ingest layer** (`ingest.py`, spec `shared/ingest-spec.md`) is a second, distinct source
seam from the retriever above: one JSON **manifest** per source coupling declares a `source.type`
and a list of extractions; `materialize(...)` runs the connector for that type and writes an OKF
bundle.
> **Where to change it (2026-07-20):** `ingest.py` is a thin adapter — Door A is implemented by
> the shared [`llm-ingestion-okf`](https://git.fromaitochitta.com/open/llm-ingestion-okf) library
> (git-pinned to `v0.3.1`), so connectors, materialization and index generation improve in ONE
> place across every consumer. `shared/ingest-spec.md` remains the normative spec — the library
> implements it, it does not replace it, and spec changes go via commons. The API below is
> unchanged; only the implementation moved. Note that Door A is **ungated**: it calls no
> security guard before writing to disk, so gating untrusted content is the caller's
> responsibility (see `docs/plan/2026-07-16-llm-ingestion-guard-inclusion.md`). The spec ships three source families — `file`/CSV (I2), `sql` (I4), and **`http`** (I6) —
and `http` is the framework's **worked extension-point example**: it shows exactly how a third,
network-transport family plugs into the same connector / materialization / gate contracts.
The `http` manifest contract (spec §4):
- `source.base_url` — the endpoint root; it **must not embed credentials** (userinfo is rejected
at schema validation).
- `source.credential_ref` — the **name of an environment variable** holding the secret, resolved
at run time (sent as `Authorization: Bearer <secret>`); the secret is never read from the
manifest, never logged, never stamped in the generated frontmatter. `null` → no auth.
- each extraction's `query` is joined onto `base_url` and its response body is rendered
**verbatim inside a fenced code block** (not a markdown table — a raw `|` or `\` survives
un-escaped); `max_rows` caps the response line count, fail-fast (never silent truncation).
**Network is a per-run grant, never a manifest field (spec §8).** `materialize` refuses an `http`
source unless the caller passes `allow_network=True`:
```python
# refused fail-fast — the manifest cannot grant itself network access:
materialize(manifest, bundle_dir, ingested_at="2026-07-04T12:00:00Z")
# opted in explicitly by the operator for this run:
materialize(manifest, bundle_dir, ingested_at="2026-07-04T12:00:00Z", allow_network=True)
```
This is the **local-only default, no silent egress** principle made mechanical: configuration is
data that cannot escalate its own authority; only the runtime `allow_network` grant can. The
transport itself is an **injectable seam**`materialize(..., http_get=<callable>)` swaps the GET
implementation (the default `_urllib_get` is the only socket path). The golden case
(`examples/ingest-golden-http/`) and every test inject a canned `get` over committed fixture
payloads, so the suite runs offline against a **local mock** — no live source, no credentials.
**The default transport is time-bounded (S2.4).** The library's `_urllib_get` calls the stdlib
opener without a timeout, which falls back to the process-wide default socket timeout — `None`
out of the box — so a source that accepts a connection and then never answers would hang a run,
contradicting the invariant that nothing runs unbounded. `materialize` therefore hands the library
that same socket path wrapped in `ingest.timeout_get`, which scopes `socket.setdefaulttimeout`
(`HTTP_TIMEOUT_SECONDS`, 30s) around the delegate call: the bound applies **without** a second
socket path and **without** duplicating the credential header, so the pinned library stays
untouched. An explicitly injected `http_get` is passed through **unwrapped** — a caller-supplied
transport owns its own timeout policy.
**Honest limit:** the default socket timeout is *process-global*. Under `concurrency=k` the runner
is asyncio on a single thread, so the scoping holds. Driving `read_http` from a thread-pool
executor would make it unsafe, and the bound would have to move to a per-call timeout argument —
i.e. to owning a socket path locally.
### D7 sibling hook — MCP as an extension of this family
The spec (§4) documents an **MCP-based connector as an extension of the `http` family**, not a new
client wired into the optimiser run path — that stays a **Non-Goal** here: the in-process
`FunctionTool` seam is the default in the run path, and MCP is demonstrated (via `build_mcp_server`
in `datasource.py`), not wired in.
**The connector (S2.2, 2026-08-02).** `ingest_mcp.py` implements it. There is **no fourth source
family and no schema change**: an MCP source is an ordinary `type: "http"` source whose transport is
declared by the `base_url` scheme, and whose two parts fall straight out of the join the library
already performs (`base_url` + `/` + `query`):
```json
{ "type": "http", "id": "docs", "base_url": "mcp+stdio://PORTFOLIO_DOCS_MCP" }
```
```python
from portfolio_optimiser.ingest import materialize
from portfolio_optimiser.ingest_mcp import mcp_get, stdio_call_tool
materialize(manifest, bundle_dir, ingested_at="2026-08-02T00:00:00Z",
allow_network=True, http_get=mcp_get(stdio_call_tool()))
```
`server_ref` is the **name of an environment variable** holding the server command — mirroring the
`sql` family's `connection_ref`, so the manifest carries a reference and never an executable path —
and the extraction's `query` is the tool name. Parsing is string-based, not `urlsplit`-based:
`urlsplit().hostname` lowercases the host, which would silently break a case-sensitive env lookup.
Two properties are worth stating because they are **inherited, not written**: the §8 network grant
covers MCP for free (an MCP source *is* `http`, so it is refused before any tool call unless
`allow_network=True`), as do the `max_rows` cap, verbatim fenced rendering, and the §7 provenance
stamp. Had MCP become a fourth family, each of those would have been ours to write — and ours to
forget. The transport discriminator **gates rather than labels**: `mcp_get` refuses a URL it does
not own, so an MCP transport can never quietly serve a plain `https://` manifest and leave the
bundle's provenance claiming a transport that was never used.
**Verified against a real MCP server subprocess (2026-08-03).** `stdio_call_tool` was previously
written but never executed end to end; `examples/ingest-golden-mcp/` + `tests/test_ingest_golden_mcp.py`
now run it against a live server process — byte-identical golden extraction, plus the tool-error and
missing-`server_ref` branches. Five detach mutations measured RED (unwrap, `initialize()`, error-code
identity, the `isError` branch, one body byte). A local subprocess costs no model tokens, so the cost
discipline is untouched; the contract tests still inject a canned tool and spawn nothing.
**What running it actually found — the error contract was broken.** `stdio_client` and
`ClientSession` are each an anyio task group, and anyio re-packages anything leaving one in a
`BaseExceptionGroup`. Every error raised inside the session (`mcp_tool_error`,
`mcp_non_text_content`) therefore reached callers as an exception group, never as the `IngestError`
the whole Door A path catches and switches on by `code`. Fixed by unwrapping the group and
re-raising the owned error; anything unowned is re-raised untouched. **No canned-tool test could
have caught this** — they never enter a task group. This is the case for running what you ship.
**A server on this path must expose a null-argument tool.** The URL carries both coordinates and the
tool is called with an empty argument dict, so `datasource.build_mcp_server` **cannot** serve ingest:
its `retrieve_cost_docs(query)` has a required parameter (verified — it returns an error result).
The two are separate seams by design: `build_mcp_server` serves the agents' retrieval path.
**Still true:** MCP remains **unwired in the optimiser run path** — the in-process `FunctionTool`
seam stays the default there. The timeout path (`asyncio.wait_for`) is not covered by a test.
**Where the D7 sibling stands (målbilde §11 boundary).** The Claude Agent SDK sibling built the
**file/CSV and SQL** connectors — mirroring I3/I5 — with bit-identical golden extractions. **HTTP
and MCP are implemented on the MAF side only**; the sibling ships no network connector and no
live-source integration. `ingest_mcp.py` is deliberately MAF-free (it imports the open `mcp`
protocol client, never `agent_framework`), so the seam is portable to D7 unchanged. On D7 the
in-process server hook is `create_sdk_mcp_server(name, version="1.0.0", tools=...) ->
McpSdkServerConfig` (package `claude-agent-sdk`) — verified 2026-07-04 against the official Claude
Agent SDK Python docs — but that is a **documented hook a deployer would reach for, not a shipped D7
connector**; no D7 MCP session is planned. Nothing here contacts a live endpoint.
## Bytt ut henteren (Embedder / Retriever, S3.1)
How prior verdicts are ranked is a seam, not a hard-coded sort. `semretrieval.py` declares two
protocols and the store delegates to them:
- **`Retriever`** — `rank(query, candidates, k) -> list[Verdict]`. `VerdictStore.retriever`
defaults to `None`, which means `StructuralRetriever`: the same weighted structural score and
`(-similarity, id)` ordering the store used before the seam existed. Assign your own object
with a `rank` method to replace ranking wholesale.
- **`Embedder`** — `__call__(features) -> np.ndarray`. `HybridRanker(embedder, similarity, weight)`
blends `weight * cosine + (1 - weight) * structural`; both terms live in `[0, 1]`, so `weight`
means what it reads as (`SEMANTIC_WEIGHT_DEFAULT = 0.25`).
**Your vectors must be finite.** `cosine` raises `ValueError` on a NaN or infinite norm rather
than scoring it — a non-finite score compares `False` against everything, which leaves the
ranking in whatever order the input happened to arrive in and defeats the total order
`HybridRanker` otherwise guarantees. A zero vector is fine and scores `0.0`; the asymmetry is
deliberate, because zero is a state the shipped `FakeEmbedder` produces on purpose whereas
non-finite only ever means the embedder is broken. Coercing it to `0.0` would hide that as
"no semantic similarity" and let ranking proceed on a forged signal.
```python
store.retriever = HybridRanker(MyEmbedder(), similarity, weight=0.3)
```
Note that `similarity` is **injected**, not imported by `semretrieval`. That is deliberate:
`verdicts.py` imports `agent_framework`, and injecting the score keeps the retrieval layer free
of it (guarded by `tests/test_semretrieval_loadbearing.py`, which ranks in a subprocess and then
asserts `verdicts` never entered `sys.modules`). Keep that property if you extend the module.
**A real embeddings client is a code-level extension point — it is not built here.** Selection goes
through a CLOSED registry: `--embedder-config` names a type, `build_embedder` dispatches on it, and
an unknown type is refused rather than resolved. A dotted `module:Class` import path is
deliberately *not* supported — that would be arbitrary code execution at config-load time and would
hand a config file the ability to import something that opens a socket, routing around the
no-network guard (which is scoped to `semretrieval.py` and blind to a third module by
construction). Adding a real embedder therefore means adding a registry branch in code, plus any
capability opt-in as a factory kwarg — the same discipline `NotifierConfig` applies to egress. The
shipped
`FakeEmbedder` is a deterministic sha256 projection with no semantics; it exists so the seam is
exercisable offline at zero cost, and the load-bearing proof is that removing the cosine term
flips the ranking, not that the projection is meaningful. A deployer supplying a real client owns
its network egress, cost, and the fact that it embeds proposal **text** — which is why the hybrid
is opt-in (`--semantic-retrieval`) rather than the default. If you wire one in, note that
`semretrieval` currently imports no network module at all, and a guard test asserts exactly that;
a real client belongs behind the `Embedder` protocol in *your* module, not inside this one.
**Optional persistence — an unwired authoring primitive.** `save_vector_store(dir, verdicts,
embedder)` / `load_vector_store(dir)` write and read a byte-deterministic `vectors.npy` +
`vectors.jsonl` pair (sorted by verdict id, atomic replace). A missing store loads as `None`; a
row/line mismatch raises rather than silently mis-ranking. `*.npy` is gitignored.
They have **no caller in `src/`**, and ranking never reads a persisted matrix: `HybridRanker.rank`
re-invokes the embedder for every candidate on every call, so a persisted store would be bypassed
even if one existed. They are offered to extenders, on the same footing as `write_verdict`,
`promote_verdict` and `build_mcp_server` — public primitives the core deliberately does not wire
into `run_project`. Wiring the cache in only becomes worth anything once the ranker is changed to
consult it, which is a redesign rather than a hookup.
*Known limitation:* the empty-store branch hardcodes `EMBED_DIM`, so a third-party `Embedder` of a
different dimension writes a shape-inconsistent empty store. Stated, not fixed.
## Bevisst ikke bygget (90 %-kuttlista)
Per the design philosophy (a ~90 % generic core with clear extension points — we do not chase
the last 10 %), the following are **deliberate cuts**, not roadmap debt. Each is extension
territory for a deployer, with the seam named:
- **B10 — full verdict-conflict taxonomy.** Chosen minimal semantics (documented in
`verdicts.py` + README): the in-memory store is first-write-wins per verdict id; the disk
layers (`write_verdict`, `promote_verdict`) are last-write-wins per file. The full taxonomy
(rejection categories + a rule for conflicting expert verdicts) is deferred until real experts
produce conflicting verdicts.
- **B11 — expert notification.** `run_project(notify=...)` remains a plain-callable seam
(`run.py` auto-wires no default notifier), but the core now ships the declared `Notifier`
contract (`notify.py`, exported from the package top) with three implementations:
`ConsoleNotifier`, `FileNotifier` (byte-deterministic JSONL), and `WebhookNotifier` — plus a
fail-fast `build_notifier(config, *, allow_egress=...)` factory. The webhook is the ONLY egress
point and is fail-closed behind an explicit per-run `allow_egress=True` opt-in (a code kwarg,
never a config field — mirroring the ingest layer's `allow_network`). SSRF guards, HMAC
signing, and auth headers remain deployer-owned extension points on the injectable
`WebhookPost` transport seam. Webhook URLs must start with `http://` or `https://` — a
scheme-less URL is rejected fail-fast at config construction.
- **U12 — checkpointing / crash-survival of a run.** A run either completes or is re-run; the
async verdict inbox (step 7) is the resumable boundary, not intra-run state.
- **U14 — OpenTelemetry / observability.** Provenance stamping is the audit trail the core
ships; OTEL wiring is a deployer concern.
- **Concurrent fan-out.** `run_portfolio` iterates projects sequentially by design (fresh
per-project execution state; one threaded `VerdictStore`); parallel orchestration is left to
the deployer and would need budget-cap coordination.