portfolio-optimiser/src/portfolio_optimiser/ingest.py
Kjell Tore Guttormsen 8910a673ea feat(ingest): bound the default http transport in time, in front of the pinned library (S2.4)
The hole: `read_http`'s default transport is the library's `urllib_get`, which invokes the
stdlib opener with no `timeout=`. urllib's documented fallback is then the process-wide default
socket timeout — `None` out of the box — so an http source that accepts a connection and never
answers hangs a run indefinitely. That contradicts the invariant that nothing runs unbounded.

The spec text for S2.4 ("a timeout parameter on `_urllib_get`") could NOT be followed literally:
that function is UPSTREAM library code (`llm_ingestion_okf.connectors`, pinned v0.3.1, pull-only),
signature `(url, credential) -> str` — measured, not assumed. Same failure class as S2.2's
"implement it in `ingest.py`": spec text that says "change X" has to be checked against whether
X is ours at all.

So the fix goes in FRONT of the library: `timeout_get` scopes `socket.setdefaulttimeout` around
a delegate call to the library's own `urllib_get`, and `materialize` now hands the library that
wrapped transport instead of letting it resolve its own untimed default. This meets S2.4's own
verification criterion — a bound WITHOUT a second socket path — and avoids duplicating the
credential-header logic. An explicitly injected `http_get` is passed through UNWRAPPED: a
caller-owned transport (MCP fronts a subprocess with its own `timeout_seconds`) keeps its own
policy, and a process-global side effect is not ours to impose on it.

Honest limit, carried in the code comment, the test docstring and `docs/extending.md`, not just
in the commit: the default socket timeout is PROCESS-global. Under `concurrency=k` the runner is
asyncio on one thread, so the scoping holds; driving `read_http` from a thread-pool executor
would make it unsafe.

Half of S2.4's scope was already delivered upstream — transport failures are categorised as
`SourceError(code="http_transport")`. Coarser than the plan envisaged, but not ours to rewrite.

Two pre-existing guards went red on the first pass, both on PROSE only: `ingest.py` must not
contain "urlopen" (no forked connector) or "ingest_mcp" (AST-guarded mcp-free). No code violated
either — my docstrings merely named them. The guards were left exactly as strict as they were and
the prose was reworded; weakening a real guard to save a comment is the trade this repo refuses.

578 -> 583 tests. Five mutations MEASURED red (restored from scratchpad + `shasum -c` each time,
never `git checkout`):
  1. remove the timeout scoping entirely            -> RED
  2. apply the bound AFTER the delegate call        -> RED
  3. set the bound but never restore it (no finally)-> RED  (the unconditional control)
  4. hand the library a bare None again (pre-S2.4)  -> RED  (the wiring)
  5. make the wrapping unconditional                -> RED  (the conditional control)

Mutations 1 and 2 take ~10s to fail rather than failing instantly: that is the loopback test's
join deadline expiring. It is the measurement that the bound actually BITES — a black-hole
listener on 127.0.0.1 that completes the handshake and never answers, run on a daemon thread so
a detached seam fails an assertion instead of hanging the suite forever. Every other assertion
here only proves we set a global; that one proves the global does something.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TdLGwd33vqhkToh98Ym34P
2026-08-03 14:54:04 +02:00

202 lines
9.2 KiB
Python

"""Door A ingest — the consumer seam over the shared ``llm-ingestion-okf`` library.
An addition IN FRONT of the loop (``shared/ingest-spec.md`` §1): a deterministic step that
couples the framework to a real data source and materializes the extract as an OKF knowledge
bundle, which the existing 8-step loop then consumes UNCHANGED. Zero model calls; no network
for the ``file``/``sql`` source types.
Since the library adoption (2026-07-20) the implementation IS the shared
``llm-ingestion-okf`` library (git-pinned to ``v0.3.1``). The spec in ``shared/ingest-spec.md``
remains the normative source — the library implements it, it does not replace it — and the
repo-local goldens under ``examples/`` remain the fasit (verified byte-exact across all three
source types, before and after the swap).
This module is the ONE place the repo touches the library for Door A. It is deliberately thin:
it re-exports the library's typed surface and keeps the historical ``materialize`` signature
(keyword-only ``ingested_at``, ``list[Path]`` return) so callers and tests bind to a stable
repo-local name rather than to the library's evolving one.
**Gating is the CALL SITE's responsibility (library README, "What is gated today: nothing").**
Door A calls no guard function before writing to disk — ``materialize_bundle`` writes what it
is given. The repo's own local-only posture still holds at this seam via ``allow_network``
(default ``False``): an ``http`` source is refused fail-fast at the library's network gate
unless a run explicitly opts in — the manifest can never grant itself network (§8, no silent
egress). Untrusted-content scanning remains the separate, still-planned ``llm-ingestion-guard``
wiring (see ``docs/plan/2026-07-16-llm-ingestion-guard-inclusion.md``); adopting this library
does NOT provide it.
MAF-free (D7-portable), like the rest of the context seam: the library has zero runtime
dependencies and imports no ``agent_framework`` / ``mcp``. Guarded by
``tests/test_ingest_loadbearing.py::test_ingest_module_is_maf_free_and_context_layer_pure``.
"""
from __future__ import annotations
import hashlib
import socket
from pathlib import Path
from llm_ingestion_okf import (
Extraction,
FileSource,
HttpSource,
IngestError,
IngestResult,
Manifest,
ManifestError,
MaterializationError,
NetworkGateError,
RenderError,
SourceError,
SqlSource,
materialize_bundle,
)
from llm_ingestion_okf import (
load_manifest as _load_manifest,
)
from llm_ingestion_okf.connectors import (
HttpGet,
read_csv,
read_http,
read_sql,
)
from llm_ingestion_okf.manifest import generated_filename
from llm_ingestion_okf.render import render_fenced_block, render_table
# Two historical PRIVATE names the repo's existing tests bind to, re-exported so those bindings
# survive the adoption unchanged. `_urllib_get` backs an identity assertion that the default http
# transport is the real socket path rather than a stub (tests/test_ingest_http.py), and is the
# delegate `timeout_get` wraps below; `_sql_value_to_text` is exercised directly for the §5
# bool/BLOB refusals (tests/test_ingest_sql.py). Not in `__all__` — not part of this contract.
from llm_ingestion_okf.connectors import urllib_get as _urllib_get
from llm_ingestion_okf.render import sql_value_to_text as _sql_value_to_text # noqa: F401
#: The repo's historical name for the §4 top-level model. The library calls it ``Manifest`` and
#: carries the version in the ``manifest_version`` field (still ``1``); the alias keeps the
#: repo-local name stable for callers that bound to it before the library adoption.
ManifestV1 = Manifest
#: The wall-clock bound applied to the default http transport (S2.4). A source that accepts a
#: connection and then never answers must not be able to hang a run: the repo requires stop
#: criteria and budget caps at startup precisely so nothing runs unbounded, and an untimed socket
#: is the same failure wearing a different hat. Chosen to match the MCP transport's own
#: `timeout_seconds` default so both transports in the `http` family fail on the same clock.
HTTP_TIMEOUT_SECONDS = 30.0
__all__ = [
"HTTP_TIMEOUT_SECONDS",
"Extraction",
"FileSource",
"HttpGet",
"HttpSource",
"IngestError",
"IngestResult",
"Manifest",
"ManifestError",
"ManifestV1",
"MaterializationError",
"NetworkGateError",
"RenderError",
"SourceError",
"SqlSource",
"generated_filename",
"load_manifest",
"materialize",
"materialize_bundle",
"read_csv",
"read_http",
"read_sql",
"render_fenced_block",
"render_table",
"timeout_get",
]
def timeout_get(get: HttpGet, *, timeout: float = HTTP_TIMEOUT_SECONDS) -> HttpGet:
"""Wrap an http transport so its socket operations are time-bounded (S2.4).
The library's ``urllib_get`` is the only socket path, and it invokes the stdlib opener with no
``timeout=`` argument. urllib's documented fallback in that case is the process-wide default
socket timeout — ``None`` unless set — so an unanswered connection blocks forever. Scoping
``socket.setdefaulttimeout`` around the delegate call therefore bounds the real transport
WITHOUT opening a second socket path and WITHOUT re-implementing the credential header, which
is what S2.4 asks for: the fix belongs in front of the pinned library, not inside it.
The previous default is restored in a ``finally``, so a transport failure cannot leak the
bound onto unrelated sockets.
KNOWN CAVEAT — the default socket timeout is PROCESS-global, not per-call. Under
``concurrency=k`` the runner is asyncio on a single thread, so this scoping holds. If
``read_http`` is ever driven from a thread-pool executor, this is NOT thread-safe and the
bound has to move to a per-call timeout argument on the opener — i.e. to owning a socket
path in this module, which is exactly what this wrapper exists to avoid.
"""
def bounded(url: str, credential: str | None) -> str:
previous = socket.getdefaulttimeout()
socket.setdefaulttimeout(timeout)
try:
return get(url, credential)
finally:
socket.setdefaulttimeout(previous)
return bounded
def load_manifest(path: str | Path) -> tuple[Manifest, str]:
"""Validate a manifest fail-fast, BEFORE any source access (§4, §9).
Returns the validated manifest and the §5 provenance stamp ``{stem}@{hash16}`` — the first
16 hex chars of SHA-256 over the manifest file's RAW bytes, so every generated file points
at the exact manifest version that produced it. Raises ``ManifestError`` without touching
any source.
The two-value return is the repo-local shape: the library returns the manifest alone and
mints the stamp INSIDE ``materialize_bundle``, exposing no stamp helper (v0.3.1). The
formula is therefore restated here, which is the one place this adapter is not purely
delegating — so it is pinned by
``tests/test_ingest_library_seam.py::test_adapter_stamp_equals_library_stamp``, which
compares this value against the ``ingest_manifest`` the library actually writes. That test
goes RED if either side's formula drifts."""
manifest_path = Path(path)
manifest = _load_manifest(manifest_path)
stamp = f"{manifest_path.stem}@{hashlib.sha256(manifest_path.read_bytes()).hexdigest()[:16]}"
return manifest, stamp
def materialize(
manifest_path: str | Path,
bundle_dir: str | Path,
*,
ingested_at: str,
allow_network: bool = False,
http_get: HttpGet | None = None,
) -> list[Path]:
"""Materialize a manifest's extractions into an OKF bundle (§5), returning the generated
concept-file paths in extraction order.
The historical repo signature over the library's ``materialize_bundle``: ``ingested_at`` is
a REQUIRED keyword (no wall-clock default, stamped verbatim; mirrors the promotion gate's
timestamp rule), and the return is the ``written`` paths as a list rather than the
library's ``IngestResult``. ``allow_network`` (§8) is the per-run network opt-in — an
``http`` source is refused fail-fast unless it is set, so the manifest itself can never
grant network access. ``http_get`` optionally injects the transport seam; both are ignored
for ``file``/``sql`` sources. Call ``materialize_bundle`` directly when the stamp is
wanted alongside the paths.
The DEFAULT transport is time-bounded (S2.4): rather than letting the library resolve its own
untimed ``urllib_get``, this passes that same socket path wrapped in :func:`timeout_get`, so
an unanswered http source fails typed instead of hanging the run. An explicitly injected
``http_get`` is passed through UNWRAPPED — a caller-supplied transport (the MCP one fronts a
subprocess with its own ``timeout_seconds``; test stubs touch nothing) owns its own timeout
policy, and a process-global socket bound is not ours to impose on it."""
transport = http_get if http_get is not None else timeout_get(_urllib_get)
return list(
materialize_bundle(
Path(manifest_path),
Path(bundle_dir),
ingested_at,
allow_network=allow_network,
http_get=transport,
).written
)