docs(s31): close the review's honesty gap — narrow semantic claims to the shipped mechanism

CHANGELOG.md

@@ -35,19 +35,30 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   `costsim` seam note was reworded from the stale "fylles av S5.4 verdirapport" to a truthful
   forward reference so `costsim`'s own output no longer claims the wiring is done.
 - Semantic retrieval seam (S3.1): a new MAF-free `semretrieval.py` adds an `Embedder`/`Retriever`
-  pair and a `HybridRanker` blending brute-force numpy cosine over embedded proposal features with
+  pair and a `HybridRanker` blending a numpy cosine term over the embedded feature triple (sorted
-  the existing structural score, exposed as `--semantic-retrieval` (valid in both run modes).
+  cost codes, measure type, magnitude bucket) with the existing structural score, exposed as
-  **Off by default and additive**: with no retriever installed the store delegates to
+  `--semantic-retrieval`. **What ships is the seam, not better retrieval quality**: the bundled
+  `FakeEmbedder` is a deterministic sha256 projection with no semantics, so over a structural tie
+  the order is deterministic but arbitrary. A real embedder is selected from a CLOSED registry via
+  `--embedder-config` / `build_embedder` — deliberately never an import path, so a config file can
+  never name arbitrary code to load.
+  **Off by default and additive**: with no retriever passed the store delegates to
   `StructuralRetriever`, which reproduces the pre-seam ranking exactly, so the text-excluded
-  default and every existing test are unchanged. Turning it on is a deliberate, gated reversal of
+  default and every existing test are unchanged. The embedding excludes `description`, matching
-  that default, since an embedder does see proposal text. Determinism is pinned rather than hoped
+  `similarity` ("text is ignored by design") and the verdict-id hash — so a flag-on run reads no
-  for: BLAS threads are fixed before numpy is imported, vectors are C-contiguous float64, and
+  surface text either, and a genuine expert verdict can no longer be outranked by the framework's
-  ranking uses the total order `(-round(score, 9), id)`. Ships an optional, rebuildable
+  own echo of the query. The ranker is passed PER CALL, never assigned to the caller's store, so
-  `vectors.npy` + `vectors.jsonl` store (byte-identical regardless of insertion order; fail-fast
+  the opt-in cannot outlive the run that asked for it. Accepted in both run modes; in
-  on a row/line mismatch; missing loads as `None`). numpy is confined to `semretrieval.py` and
+  single-project mode it requires `--bundle-dir` and `--verdict-dir` and is refused — never
-  never enters `okf.py`, `retrieval.py` or `shared/`. The real embeddings client remains a
+  silently ignored — without them.
-  config-only extension point — the shipped `FakeEmbedder` is a deterministic hash projection with
+  Determinism, precisely: ranking order rests on the total order `(-round(score, 9), id)`; the
-  no semantics, so this buys a scaling *seam*, not better retrieval quality.
+  BLAS thread pins set before numpy is imported (`VECLIB_MAXIMUM_THREADS` for Accelerate,
+  `OPENBLAS`/`MKL`/`OMP` for other backends) defend the narrower claim that vector artifacts are
+  byte-identical across environments. Ships an optional, rebuildable `vectors.npy` +
+  `vectors.jsonl` store (byte-identical regardless of insertion order; fail-fast on a row/line
+  mismatch; missing loads as `None`) — an authoring primitive with no caller in `src/`, offered to
+  extenders like `write_verdict` and `promote_verdict`. numpy is confined to `semretrieval.py` and
+  never enters `okf.py`, `retrieval.py` or `shared/`.
 - Azure/Foundry offline preflight config gate (`preflight.py`, S4.1).
 - Offline live-dry-run drill (`--live-dry-run`, S4.2): walks the whole path up to the eager
   client build and stops before the first model call — zero chat calls.
@@ -58,8 +69,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   `allow_egress` opt-in. Not auto-wired into `run.py`.
 - `docs/knowledge-base-recipe.md` (S5.3, D-H item 1): the documented team process (technical +
   domain expert) for building a knowledge base, with the honest 1–2 week expectation.
-- Test suite: 488 passing tests (4 skips are live-provider-only), every wired seam covered by a
+- Test suite: 512 passing tests (4 skips are live-provider-only). Every wired seam is covered by a
-  load-bearing test that goes red when the seam is detached.
+  load-bearing test that goes red when the seam is detached — including, since the S3.1 review
+  remediation, the `--semantic-retrieval` wiring at `main()` level. That claim did not hold for
+  S3.1 as first shipped: the flag was covered only below the CLI, so hardcoding it off left the
+  suite green. Each remediated seam now carries a recorded detach point, verified by mutation.
 
 ### Notes
 - Licensed under the MIT License (see `LICENSE`).

README.md

@@ -122,14 +122,24 @@ when the seam is detached, so the loop cannot silently degrade into theater.
   uv run python -m portfolio_optimiser.run --report --ledger ledger.json
   ```
 
-  `--semantic-retrieval` (S3.1) is an **opt-in** ranking change, valid in both run modes and
+  `--semantic-retrieval` (S3.1) is an **opt-in** ranking change, **off by default**. Off, prior
-  **off by default**. Off, prior verdicts are ranked exactly as before: a structural score over
+  verdicts are ranked exactly as before: a structural score over the affected cost-code set,
-  the affected cost-code set, measure type and magnitude bucket, with surface text deliberately
+  measure type and magnitude bucket, with surface text deliberately excluded. On, that score is
-  excluded. On, that score is blended with brute-force cosine over embedded features, so a
+  blended with a cosine term over the *same* structural triple, which lets a prior verdict on a
-  semantically related prior verdict carrying a *different* code set can also reach the
+  *different* cost-code set outrank one that ties structurally.
-  hypothesis prompt. Since an embedder sees the proposal text, turning the flag on is a
+
-  deliberate, gated reversal of the text-excluded default — which is why it is a flag and not a
+  **What this ships is the seam, not better retrieval.** The bundled `FakeEmbedder` is a
-  new default. Nothing about a flag-off run changes, and no savings claim depends on it.
+  deterministic sha256 projection carrying no semantics, so over a structural tie the resulting
+  order is deterministic but arbitrary. Retrieval *quality* depends entirely on injecting a real
+  embedder — `--embedder-config` selects one from a closed registry (never an import path; a
+  config file can never name arbitrary code to load), and `docs/extending.md` documents the
+  `Embedder` protocol. The embedding excludes `description`, matching the structural score and the
+  verdict-id hash, so a flag-on run reads no surface text either.
+
+  The flag is accepted in both run modes, but in single-project mode it **requires** `--bundle-dir`
+  and `--verdict-dir`: without them it cannot take effect, and the run is refused rather than
+  silently ignoring the flag. Nothing about a flag-off run changes, and no savings claim depends
+  on it.
 
   The **prior-verdict fold — the learning step — happens only on the `--bundle-dir` path**; a
   plain `--docs-dir`-only run is single-shot (no fold). `--decision`/`--rationale` apply to the

docs/extending.md

@@ -137,7 +137,15 @@ Note that `similarity` is **injected**, not imported by `semretrieval`. That is
 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 config-only extension point — it is not built here.** The shipped
+**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

src/portfolio_optimiser/run.py

@@ -283,11 +283,14 @@ async def run_project(
     (S4.2, comparison protocol §4 pkt 2/3) is the offline drill: it walks the whole path up to the
     EAGER client build, writes the run-config artefact (when ``outbox_dir`` is set), and returns a
     ``DryRunReport`` BEFORE the first model call (``debate.run``) — zero chat calls.
-    ``semantic_retrieval`` (S3.1) is the opt-in scaling seam: when true, the store's ranker is
+    ``semantic_retrieval`` (S3.1) is the opt-in scaling SEAM — the deliverable is the extension
-    swapped for a ``HybridRanker`` that blends brute-force cosine over embedded features with the
+    point, not better retrieval. When true, a ``HybridRanker`` blends a cosine term over the
-    structural score, so a semantically related prior verdict carrying a DIFFERENT cost-code set
+    embedded feature triple (sorted cost codes, measure type, magnitude bucket) with the structural
-    can reach the Step-1 fold. Default false keeps the structural, text-excluded ranking exactly
+    score, which lets a prior verdict on a DIFFERENT cost-code set outrank one that ties
-    as before."""
+    structurally. The shipped ``FakeEmbedder`` is a deterministic sha256 projection carrying NO
+    semantics, so over a structural tie the resulting order is deterministic but arbitrary;
+    retrieval *quality* arrives only with an embedder injected via ``embedder=`` or
+    ``--embedder-config``. Default false keeps the structural ranking exactly as before."""
     # 0. Fail-fast: an outbox write is byte-deterministic and keyed on run_id — no wall-clock default.
     if outbox_dir is not None and run_id is None:
         raise ValueError(
@@ -390,8 +393,14 @@ async def run_project(
     # hypothesis context BEFORE generation, keyed on the OKF bundle's candidate features (available
     # pre-hypothesis). THIS is the one missing dataflow — previously ExpeL was computed
     # post-generation into a discarded SessionContext (step 7 below), so a prior verdict could not
-    # reach the next hypothesis. Bundle-driven path with a populated store only; the road path is
+    # reach the next hypothesis. Bundle-driven path with a populated store only.
-    # untouched (its post-hoc, proposal-keyed retrieval below is unchanged).
+    #
+    # Scope of the --semantic-retrieval opt-in, stated precisely (an earlier version of this
+    # comment claimed "the road path is untouched", which the flag made false): the ranker built
+    # below is passed to ALL THREE retrievals this run performs — this fold, and the post-hoc
+    # ExpeLContextProvider + store.retrieve in step 7 — so the flag reaches the road path's
+    # proposal-keyed retrieval too. What IS untouched on the road path is the fold itself: it stays
+    # bundle-gated, so a --docs-dir-only run remains single-shot either way.
     # S3.1 opt-in: build the hybrid ranker as a LOCAL, then pass it explicitly at each retrieval
     # this run performs. It is deliberately not assigned to ``store.retriever``: the store is
     # caller-owned (``run_portfolio`` threads one store across every project, and a library caller
@@ -724,10 +733,13 @@ def main(argv: list[str] | None = None) -> int:
     parser.add_argument(
         "--semantic-retrieval",
         action="store_true",
-        help="S3.1 opt-in scaling seam: rank prior verdicts with a hybrid of brute-force cosine "
+        help="S3.1 opt-in scaling SEAM: blend a cosine term over the embedded feature triple with "
-        "over embedded features and the structural score, so a semantically related verdict with a "
+        "the structural score, so a prior verdict on a DIFFERENT cost-code set can outrank one that "
-        "DIFFERENT cost-code set can reach the hypothesis prompt. Valid in both modes; OFF by "
+        "ties structurally. The shipped embedder is a semantics-free sha256 projection — this buys "
-        "default, and off means the structural, text-excluded ranking is unchanged",
+        "the extension point, not better retrieval; inject a real one with --embedder-config. "
+        "Accepted in both modes, but in single-project mode it REQUIRES --bundle-dir and "
+        "--verdict-dir (without them it cannot take effect, and is refused rather than ignored). "
+        "OFF by default, and off means the structural ranking is unchanged",
     )
     parser.add_argument("--decision", default="approved", choices=["approved", "rejected"])
     parser.add_argument("--rationale", default="reviewed by expert")

src/portfolio_optimiser/semretrieval.py

@@ -92,8 +92,9 @@ _MAGNITUDE_BUCKETS = [(0.0, 1e5), (1e5, 5e5), (5e5, 1e6), (1e6, float("inf"))]
 class Embedder(Protocol):
     """Maps structured proposal features to a fixed-length float64 vector.
 
-    The real implementation (an embeddings client) is a config-only extension point and is NOT
+    The real implementation (an embeddings client) is a CODE-LEVEL extension point and is NOT
-    built here — tests and the offline path use ``FakeEmbedder``."""
+    built here — tests and the offline path use ``FakeEmbedder``. Config selects from the closed
+    ``build_embedder`` registry; it can never name a module to import (see ``EmbedderConfig``)."""
 
     def __call__(self, features: ProposalFeatures) -> np.ndarray: ...