fix(s31): close 1 review MAJOR — VECLIB_MAXIMUM_THREADS + honest determinism claim + the missing pin guard

src/portfolio_optimiser/semretrieval.py

@@ -17,9 +17,20 @@ Registered in ``tests/test_okf.py``'s ``_MAF_FREE_MODULES`` (direct-import AST g
 catches a lazy runtime import an import-time guard would miss — by
 ``tests/test_semretrieval_loadbearing.py``.
 
-Determinism: BLAS reduction order depends on the thread count, so the thread pins below are set
+Determinism, stated precisely because the two halves rest on different things:
-before numpy is imported (this module is the only numpy importer in the package). Vectors are
+
-float64 and C-contiguous; ranking uses the total-order key ``(-round(score, 9), id)``.
+* **Ranking order** is guaranteed by the total-order key ``(-round(score, 9), id)`` — rounding
+  absorbs last-bit float noise and ``id`` settles exact ties. This is the guarantee that actually
+  holds, and it does NOT depend on the thread pins. It has to do real work: ``np.dot`` over two
+  separately allocated but bit-identical vectors can differ by one ulp, because the reduction path
+  varies with buffer alignment. Rounding is what makes that unobservable in the ranking.
+* **Byte-identical vector artifacts** (the ``.npy``/``.jsonl`` store, across machines) are what the
+  thread pins below defend: BLAS reduction order depends on the thread count, and ``FakeEmbedder``
+  calls ``np.linalg.norm``, which can dispatch to BLAS. The pins are defence-in-depth for that
+  narrower claim — never the basis of the ranking guarantee.
+
+Vectors are float64 and C-contiguous. This module is the only numpy importer in the package, which
+is what makes an import-time pin viable at all.
 """
 
 from __future__ import annotations
@@ -31,10 +42,21 @@ from collections.abc import Callable, Sequence
 from pathlib import Path
 from typing import TYPE_CHECKING, Protocol, runtime_checkable
 
-# Pin BLAS threads BEFORE numpy is imported — OpenBLAS reads these at import time, and a
+# Pin BLAS threads BEFORE numpy is imported — every one of these is latched by the time the numpy
-# thread-count-dependent reduction order is the one thing that makes float64 dot products drift
+# import completes (measured: setting them afterwards has no effect, even before the first BLAS
-# between environments. Pinned to 1 => bit-exact run-to-run. ``setdefault`` so an operator can
+# call), so placement above the import is load-bearing, not stylistic. ``setdefault`` so an
-# still override deliberately.
+# operator can still override deliberately.
+#
+# One variable per backend, because which one BITES depends on what numpy links:
+#   VECLIB_MAXIMUM_THREADS  -> Accelerate (macOS; what numpy >= 2.0 links here by default)
+#   OPENBLAS_NUM_THREADS    -> OpenBLAS   (numpy's default wheel on Linux/Windows, and numpy 1.x)
+#   MKL_NUM_THREADS         -> Intel MKL  (conda-forge / Intel distributions)
+#   OMP_NUM_THREADS         -> the OpenMP runtime underneath several of the above
+# On THIS build the effective one is VECLIB_MAXIMUM_THREADS; the other three are measured no-ops
+# against Accelerate. They stay because they are the correct knobs for an OpenBLAS/MKL/OpenMP
+# deployment — that is a PORTABILITY argument, not a determinism one, and the distinction matters:
+# before VECLIB was added, this block pinned nothing at all on the machine it ran on.
+os.environ.setdefault("VECLIB_MAXIMUM_THREADS", "1")
 os.environ.setdefault("OPENBLAS_NUM_THREADS", "1")
 os.environ.setdefault("OMP_NUM_THREADS", "1")
 os.environ.setdefault("MKL_NUM_THREADS", "1")
@@ -48,8 +70,11 @@ if TYPE_CHECKING:  # verdicts imports agent_framework — keep it out of the run
     # The structural score, passed in rather than imported — see module docstring.
     SimilarityFn = Callable[[ProposalFeatures, ProposalFeatures], float]
 
-# Dimension of the fake embedding. Small enough that a 500+ verdict base is a trivial matmul,
+# Dimension of the fake embedding. Large enough that distinct features do not collide, small
-# large enough that distinct features do not collide.
+# enough that a 500+ verdict base stays cheap. Note there is no matmul on the ranking path:
+# ``HybridRanker.rank`` calls ``cosine()`` once per candidate from inside the sort key — a Python
+# loop over per-pair ``np.dot`` on 64-vectors — and re-invokes the embedder for every candidate on
+# every call, caching nothing. Fine at this scale; the thing to change first if it ever is not.
 EMBED_DIM = 64
 
 # Blend weight for HybridRanker: score = weight * cosine + (1 - weight) * structural.

tests/test_semretrieval_loadbearing.py

@@ -26,6 +26,7 @@ system's own echo of the query can no longer outrank genuine expert prose.
 from __future__ import annotations
 
 import ast
+import os
 import subprocess
 import sys
 from pathlib import Path
@@ -106,6 +107,54 @@ def test_semretrieval_import_and_rank_pull_in_no_maf_and_no_verdicts() -> None:
     assert result.returncode == 0, result.stderr
 
 
+def test_blas_thread_pins_are_set_before_numpy_is_imported() -> None:
+    """SC3 GUARD — the determinism claim's one untested half, until now.
+
+    Two properties, both checked in a FRESH subprocess that execs the module standalone:
+    1. all four thread variables are set to ``"1"`` after the module body runs;
+    2. ``numpy`` is absent from ``sys.modules`` BEFORE the module's env writes execute — the
+       ordering the whole mechanism depends on, since every backend latches its variable by the
+       time the numpy import completes.
+
+    ``VECLIB_MAXIMUM_THREADS`` is the one that BITES on this build (numpy links Accelerate); the
+    other three cover OpenBLAS/MKL/OpenMP deployments. Before it was added this block pinned
+    nothing at all here, and no test could tell — ``grep -rn 'OPENBLAS\\|OMP_NUM\\|MKL_NUM' tests/``
+    returned zero hits.
+
+    Detach point: move any ``os.environ.setdefault`` below the ``import numpy`` line → RED."""
+    check = (
+        "import importlib.util, sys\n"
+        "assert 'numpy' not in sys.modules, 'numpy was imported before the probe started'\n"
+        f"spec = importlib.util.spec_from_file_location('semretrieval_pins', {str(_SEMRETRIEVAL)!r})\n"
+        "mod = importlib.util.module_from_spec(spec)\n"
+        "\n"
+        "# Trip-wire: record whether numpy was already imported at the moment the env writes ran.\n"
+        "import os\n"
+        "seen = {}\n"
+        "real_setdefault = os.environ.setdefault\n"
+        "def spy(key, value):\n"
+        "    seen.setdefault(key, 'numpy' in sys.modules)\n"
+        "    return real_setdefault(key, value)\n"
+        "os.environ.setdefault = spy\n"
+        "try:\n"
+        "    spec.loader.exec_module(mod)\n"
+        "finally:\n"
+        "    os.environ.setdefault = real_setdefault\n"
+        "\n"
+        "for var in ('VECLIB_MAXIMUM_THREADS', 'OPENBLAS_NUM_THREADS', 'OMP_NUM_THREADS',\n"
+        "            'MKL_NUM_THREADS'):\n"
+        "    assert os.environ.get(var) == '1', var + ' is ' + repr(os.environ.get(var))\n"
+        "    assert var in seen, var + ' was never pinned by the module body'\n"
+        "    assert seen[var] is False, var + ' was pinned AFTER numpy was imported'\n"
+        "assert 'numpy' in sys.modules, 'the module never imported numpy — probe is not exercising it'\n"
+    )
+    env = {
+        k: v for k, v in os.environ.items() if not k.endswith(("_NUM_THREADS", "_MAXIMUM_THREADS"))
+    }
+    result = subprocess.run([sys.executable, "-c", check], capture_output=True, text=True, env=env)
+    assert result.returncode == 0, result.stderr
+
+
 def test_semretrieval_imports_no_network_modules() -> None:
     """SC8 — the offline default must not be able to reach the network. Detach point: add
     ``import urllib.request`` (or any ``_NETWORK_ROOTS`` member) to semretrieval.py → RED."""