open/portfolio-optimiser
1
0
Fork 0
Code Issues Releases 1 Activity

fix(s31): close 2 review findings — drop description from the embedding + re-derive SC2 on the minted shape

Browse source
This commit is contained in:
Kjell Tore Guttormsen 2026-07-25 12:31:06 +02:00
commit 969d450b31
3 changed files with 172 additions and 43 deletions
Download patch file Download diff file Expand all files Collapse all files

21
tests/test_semretrieval.py
View file

@ -77,17 +77,36 @@ def test_fake_embedder_is_insensitive_to_code_set_ordering() -> None:
def test_distinct_features_give_distinct_vectors() -> None:
"""SC4, NARROWED: "distinct" means distinct in the EMBEDDED fields — the structural triple.
The description-only variant moved to the test below, which asserts the opposite."""
embedder = FakeEmbedder()
baseline = embedder(_features())
for variant in (
_features(codes=frozenset({"09.1"})),
_features(measure_type="rate_renegotiation"),
_features(saving=900_000.0), # different magnitude bucket
_features(description="totally different wording"),
):
assert not np.array_equal(baseline, embedder(variant))
def test_features_differing_only_in_description_embed_identically() -> None:
"""SC4's narrowing, asserted POSITIVELY rather than left implicit — this is the same
assertion the loop above used to carry, with the opposite sign.
``description`` is excluded from the embedding on purpose: ``verdicts.similarity`` already
ignores text by design and ``verdicts._mint_id`` already hashes only the structural triple, so
two proposals differing only in prose are ONE verdict as far as the rest of the system is
concerned. Leaving the embedding as the sole place surface text still counted made the
framework's own restatement of a query outrank real expert prose (defect P1).
Detach point: put ``features.description`` back into ``_canonical_feature_string`` → RED."""
embedder = FakeEmbedder()
assert np.array_equal(
embedder(_features()),
embedder(_features(description="totally different wording")),
)
def test_fake_embedder_components_are_non_negative() -> None:
"""Non-negative components keep ``cosine`` in [0, 1], matching the structural score's
range — no scale mismatch when the two are blended by HybridRanker."""

172
tests/test_semretrieval_loadbearing.py
View file

@ -12,9 +12,15 @@ the D7 fault line. Three guards hold that line, each with a named detach point:
3. **No network** — an AST sweep for network modules, since a real embeddings client is an
extension point and must never be smuggled into the offline default.
And one load-bearing behavioural seam (SC2): on a 500+ verdict base where two candidates are
structurally INDISTINGUISHABLE, the cosine term is the only thing that can separate them.
And one load-bearing behavioural seam (SC2): on a 500+ verdict base where two candidates TIE on
the structural score, the cosine term is the only thing that can separate them.
**Detach point: zero/remove the cosine term → CORRECT no longer top-1.**
The pair ties on equal Jaccard over DIFFERENT code sets rather than by being structurally
identical, so both candidates are constructible through ``capture_verdict`` — the path the
framework actually mints on. Plus one regression guard (defect P1): a framework-minted and an
externally-authored verdict that are structurally identical must embed identically, so the
system's own echo of the query can no longer outrank genuine expert prose.
"""
from __future__ import annotations
@ -24,13 +30,23 @@ import subprocess
import sys
from pathlib import Path
import numpy as np
from portfolio_optimiser.semretrieval import (
FakeEmbedder,
HybridRanker,
StructuralRetriever,
cosine,
)
from portfolio_optimiser.verdicts import ProposalFeatures, Verdict, VerdictStore, similarity
from portfolio_optimiser.verdicts import (
ProposalFeatures,
Verdict,
VerdictStore,
capture_verdict,
similarity,
verdict_from_dict,
verdict_to_dict,
)
_SEMRETRIEVAL = (
Path(__file__).resolve().parents[1] / "src" / "portfolio_optimiser" / "semretrieval.py"
@ -106,54 +122,69 @@ def test_semretrieval_imports_no_network_modules() -> None:
# --- SC2: the cosine term is load-bearing on a structurally tied 500+ base ---------------------
#
# The pair ties on the STRUCTURAL score while carrying DIFFERENT cost codes — equal Jaccard
# against the query (1/3 each), same measure, same magnitude bucket => similarity 0.6 == 0.6.
# That matters: the previous fixture tied by being structurally IDENTICAL and separated only by
# hand-written prose, a shape the framework cannot mint (``_mint_id`` ignores description, so
# both candidates would share one id and ``VerdictStore.add`` is first-write-wins). Differing in
# codes makes the pair genuinely constructible through ``capture_verdict`` — the real minting
# path — so the proof now runs on data the system can actually produce.
#
# Every feature set carries ``description == measure_type``, which is exactly what both live
# minting paths emit (``run._features_of`` sets ``description=proposal.measure``;
# ``verdicts._features_from_ir`` sets ``description=ir["measure"]``).
_TIED_CODES = frozenset({"05.2", "03.1"})
_MEASURE = "asfalt"
_QUERY_CODES = frozenset({"05.1", "05.2"})
_CORRECT_CODES = frozenset({"05.1", "07.4"})
_DISTRACTOR_CODES = frozenset({"05.2", "09.8"})
# CORRECT's id sorts AFTER the distractor's, so the structural ranking's `(-similarity, id)` key
# puts the DISTRACTOR first. Only the cosine term can overturn that. The id doubles as a unique
# grep marker: it appears nowhere else in the repo, so a stray match cannot fake a pass.
_CORRECT_ID = "zz-s31-cosine-tiebreak-4b7e"
_DISTRACTOR_ID = "aa-structural-tie-winner"
# Unique grep markers: these strings appear nowhere else in the repo, so a stray match cannot
# fake a pass. They ride in the RATIONALE (not the id) because ids are now minted content
# hashes — and because ``format_fewshot`` puts the rationale into the prompt, which is the
# channel the CLI-level proof downstream depends on.
_CORRECT_MARKER = "zz-s31-cosine-tiebreak-4b7e"
_DISTRACTOR_MARKER = "aa-structural-tie-winner"
_QUERY = ProposalFeatures(
affected_codes=_TIED_CODES,
measure_type="scope_reduction",
affected_codes=_QUERY_CODES,
measure_type=_MEASURE,
claimed_saving_nok=220_000.0,
description="reduce asphalt base course thickness on the school approach",
description=_MEASURE,
)
def _tied_verdict(verdict_id: str, description: str) -> Verdict:
"""Structurally IDENTICAL to the query — same codes, measure and magnitude bucket. Only the
(structurally ignored) description differs, which is exactly what makes these two candidates
inseparable without a semantic signal."""
return Verdict(
id=verdict_id,
proposal_features=ProposalFeatures(
affected_codes=_TIED_CODES,
measure_type="scope_reduction",
def _tied_verdict(codes: frozenset[str], decision: str, marker: str) -> Verdict:
"""Structurally TIED with the query — equal Jaccard, same measure, same magnitude bucket —
but on a different code set, so the canonical embedding string differs and cosine has
something to separate. Minted through ``capture_verdict``, never hand-assigned."""
return capture_verdict(
ProposalFeatures(
affected_codes=codes,
measure_type=_MEASURE,
claimed_saving_nok=200_000.0, # same bucket [100k, 500k) as the query
description=description,
description=_MEASURE,
),
decision="approved",
rationale=f"prior ruling reached via {verdict_id}",
decision,
f"prior ruling reached via {marker}",
)
# Minted ids, resolved once so the ordering property below is stated in terms of real values.
_CORRECT_ID = _tied_verdict(_CORRECT_CODES, "approved", _CORRECT_MARKER).id
_DISTRACTOR_ID = _tied_verdict(_DISTRACTOR_CODES, "rejected", _DISTRACTOR_MARKER).id
def _synthetic_base(n: int = 500) -> list[Verdict]:
"""Two structurally tied candidates plus ``n - 2`` strictly-lower-scoring fillers, so the
ranking has to hold up at a realistic base size rather than on a three-item toy store."""
correct = _tied_verdict(
_CORRECT_ID,
# Tuned, and admittedly so: a sha256 projection carries no inherent semantics, so this
# wording was selected because it lands nearer the query than the distractor does. The
# proof of the seam is the detach control below, not the plausibility of this string.
"shallower asphalt base layer along the school approach",
)
distractor = _tied_verdict(
_DISTRACTOR_ID,
"unrelated administrative rebate on office cleaning contract",
)
# Not tuned prose any more: the two candidates differ in their CODE SETS, and the cosine
# ordering over a sha256 projection is deterministic but semantically arbitrary. The proof of
# the seam is the detach control below — that removing the cosine term flips the ranking —
# never the plausibility of either candidate.
correct = _tied_verdict(_CORRECT_CODES, "approved", _CORRECT_MARKER)
distractor = _tied_verdict(_DISTRACTOR_CODES, "rejected", _DISTRACTOR_MARKER)
fillers = [
Verdict(
id=f"filler-{i:04d}",
@ -173,15 +204,32 @@ def _synthetic_base(n: int = 500) -> list[Verdict]:
def test_sc2_fixture_preconditions_hold() -> None:
"""The three properties the SC2 proof rests on, asserted rather than assumed: the pair is
structurally tied, CORRECT loses the id tie-break, and cosine favours CORRECT."""
"""Every property the SC2 proof rests on, asserted rather than assumed: the pair is
structurally tied on DIFFERENT code sets, both ids are minted by ``capture_verdict``, CORRECT
loses the id tie-break, and cosine favours CORRECT.
The code-set assertion is the one that keeps this fixture honest — a pair that tied by being
structurally identical could not be minted at all, since ``_mint_id`` would collapse them onto
one id and the store would keep only the first."""
base = _synthetic_base()
correct = next(v for v in base if v.id == _CORRECT_ID)
distractor = next(v for v in base if v.id == _DISTRACTOR_ID)
# Tied structurally, but NOT structurally identical.
assert similarity(_QUERY, correct.proposal_features) == similarity(
_QUERY, distractor.proposal_features
)
assert correct.proposal_features.affected_codes != distractor.proposal_features.affected_codes
# The framework-minted shape: description carries the measure, on every feature set.
for features in (_QUERY, correct.proposal_features, distractor.proposal_features):
assert features.description == features.measure_type == _MEASURE
# Ids are content hashes of the features, not hand-written — re-minting reproduces them.
assert correct.id != distractor.id
assert correct.id == capture_verdict(correct.proposal_features, "approved", "re-mint").id
assert distractor.id == capture_verdict(distractor.proposal_features, "rejected", "re-mint").id
assert _DISTRACTOR_ID < _CORRECT_ID # structural id tie-break favours the distractor
embedder = FakeEmbedder()
@ -192,6 +240,56 @@ def test_sc2_fixture_preconditions_hold() -> None:
assert len(base) >= 500
def test_minted_and_authored_verdicts_embed_identically_on_a_structural_tie() -> None:
"""REGRESSION (defect P1) — a framework-minted verdict and an externally-authored one that
are structurally identical must now embed IDENTICALLY.
Before ``description`` left the embedding, the framework minted every feature set with
``description == measure`` while a human/persona verdict arrived through the inbox carrying
real prose. On a mixed store the system's own echo of the query scored ~1.0 and genuine expert
prose ~0.72, so the hybrid pushed real rulings BELOW the framework's restatement of its own
question — the inverse of the feature's purpose, and the same self-contamination the Step-8
promotion gate exists to prevent.
Detach point: restore ``features.description`` to ``_canonical_feature_string`` → RED."""
features = ProposalFeatures(
affected_codes=_CORRECT_CODES,
measure_type=_MEASURE,
claimed_saving_nok=200_000.0,
description=_MEASURE, # the shape run._features_of / _features_from_ir emit
)
minted = capture_verdict(features, "approved", "framework-captured ruling")
# The same proposal as an expert would file it: identical structure, human prose.
authored = verdict_from_dict(
{
**verdict_to_dict(minted),
"rationale": "the base course reduction was accepted on the school approach",
"proposal_features": {
**verdict_to_dict(minted)["proposal_features"],
"description": "shallower asphalt base layer along the school approach",
},
}
)
embedder = FakeEmbedder()
query_vector = embedder(_QUERY)
# The load-bearing claim: the two embed to the SAME vector, bit for bit.
assert np.array_equal(
embedder(minted.proposal_features), embedder(authored.proposal_features)
), "prose still leaks into the embedding — the framework's own echo can outrank expert text"
# ...and therefore rank identically. Compared at the ranking key's own tolerance, not on raw
# equality: ``np.dot`` over two separately allocated but bit-identical vectors can differ by
# one ulp, because the reduction path varies with buffer alignment. That is precisely why
# ``HybridRanker`` orders on ``(-round(score, 9), id)`` rather than the raw score — asserting
# bit-equality here would pin a property the system deliberately does not rely on.
assert round(cosine(query_vector, embedder(minted.proposal_features)), 9) == round(
cosine(query_vector, embedder(authored.proposal_features)), 9
)
def test_sc2_control_structural_ranking_puts_the_distractor_first() -> None:
"""CONTROL — without a semantic term the CORRECT verdict is unreachable. If this ever goes
green with CORRECT on top, the positive below proves nothing."""