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

19 KiB

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 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:

# 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 seammaterialize(..., 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):

{ "type": "http", "id": "docs", "base_url": "mcp+stdio://PORTFOLIO_DOCS_MCP" }
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:

  • Retrieverrank(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.

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.