fix(run): --embedder-config is refused, not silently dropped
Walked from a fresh clone: `--embedder-config` was accepted in every mode without `--semantic-retrieval` and then had no effect whatsoever. MEASURED, not inferred — an injected embedder is consulted ZERO times with the flag off and once with it on, because the only consumer is the HybridRanker that flag builds; the default StructuralRetriever takes no embedder at all. That is the silent-ignore this CLI's flag contract exists to prevent, and the same ground on which `--semantic-retrieval` itself is already refused when it cannot take effect. REFUSED, not wired — the opposite call from `--scripted-replies` in portfolio mode, and for a stated reason: there the seam already existed, so refusing would have left a whole mode without an offline door. Here there is nothing to wire to. Mode-independent (both modes gate the embedder on the same flag) and placed ABOVE the scripted door, mirroring the required-args hoist: a refused run must not first print a banner claiming a scripted loop closed. Five mutations against the WHOLE suite, all red, each isolating one seam: detach the refusal (3 red) · scope it to single-project mode (portfolio arm red) · move it below the banner (banner arm red, rc intact) · build the ranker unconditionally (the zero-consultation measurement red) · ignore the injected embedder (its control red). 663 -> 668 tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GyAbxJoyypnLLUDcMvnKh8
This commit is contained in:
parent
f87d555840
commit
a109a703e2
2 changed files with 169 additions and 0 deletions
|
|
@ -1273,6 +1273,33 @@ def main(argv: list[str] | None = None) -> int:
|
||||||
)
|
)
|
||||||
return 1
|
return 1
|
||||||
|
|
||||||
|
# --embedder-config selects the embedder for the HybridRanker, and that ranker is built ONLY
|
||||||
|
# when semantic_retrieval is on; the default StructuralRetriever takes no embedder at all. So
|
||||||
|
# without --semantic-retrieval the config is loaded fail-fast and then dropped on the floor —
|
||||||
|
# MEASURED, not inferred: an injected embedder is consulted ZERO times with the flag off and
|
||||||
|
# once with it on (tests/test_run_cli.py::test_injected_embedder_is_never_consulted_with_the
|
||||||
|
# _flag_off + its control). That is the silent-ignore this CLI's flag contract exists to
|
||||||
|
# prevent, and the same ground on which --semantic-retrieval itself is refused above when it
|
||||||
|
# cannot take effect.
|
||||||
|
#
|
||||||
|
# REFUSED, not wired — the opposite call from --scripted-replies in portfolio mode, and for a
|
||||||
|
# stated reason: there the seam already existed (run_portfolio takes the same client_factory),
|
||||||
|
# so refusing would have left a whole mode without an offline door. Here there is nothing to
|
||||||
|
# wire to; an embedder has no job outside the hybrid ranker.
|
||||||
|
#
|
||||||
|
# MODE-INDEPENDENT (hence above the portfolio dispatch, not inside either branch): both modes
|
||||||
|
# gate the embedder on the same flag, since run_portfolio forwards it to run_project unchanged.
|
||||||
|
# Placed ABOVE the scripted door for the reason the required-args guard was hoisted there — a
|
||||||
|
# refused run must not first print a banner claiming a scripted loop closed.
|
||||||
|
if args.embedder_config is not None and not args.semantic_retrieval:
|
||||||
|
print(
|
||||||
|
"run refused: --embedder-config requires --semantic-retrieval (the embedder is only "
|
||||||
|
"consulted by the hybrid ranker that flag builds — without it the config would be "
|
||||||
|
"loaded and then ignored)",
|
||||||
|
file=sys.stderr,
|
||||||
|
)
|
||||||
|
return 1
|
||||||
|
|
||||||
# The scripted door (offline WHOLE-loop run over the caller's own data). Resolved BEFORE the
|
# The scripted door (offline WHOLE-loop run over the caller's own data). Resolved BEFORE the
|
||||||
# dry-run branch so the two offline modes cannot both be honoured — and BEFORE the portfolio
|
# dry-run branch so the two offline modes cannot both be honoured — and BEFORE the portfolio
|
||||||
# dispatch, because the door serves BOTH modes. It originally sat below that dispatch, which
|
# dispatch, because the door serves BOTH modes. It originally sat below that dispatch, which
|
||||||
|
|
|
||||||
|
|
@ -679,6 +679,78 @@ def test_embedder_config_unknown_type_refuses(tmp_path, capsys) -> None:
|
||||||
assert "my_pkg" not in sys.modules
|
assert "my_pkg" not in sys.modules
|
||||||
|
|
||||||
|
|
||||||
|
def test_embedder_config_without_semantic_retrieval_is_refused(tmp_path, capsys) -> None:
|
||||||
|
"""The embedder is consulted ONLY by the hybrid ranker ``--semantic-retrieval`` builds (proved
|
||||||
|
by ``test_injected_embedder_is_never_consulted_with_the_flag_off`` below), so without that flag
|
||||||
|
the config is loaded and then dropped. That is the silent-ignore this CLI's flag contract
|
||||||
|
refuses — the same ground on which ``--semantic-retrieval`` itself is refused when it cannot
|
||||||
|
take effect."""
|
||||||
|
cfg = tmp_path / "embedder.json"
|
||||||
|
cfg.write_text('{"type": "fake"}', encoding="utf-8")
|
||||||
|
rc = run.main(
|
||||||
|
[
|
||||||
|
_PID,
|
||||||
|
"--docs-dir",
|
||||||
|
str(BUNDLE_DIR),
|
||||||
|
"--bundle-dir",
|
||||||
|
str(BUNDLE_DIR),
|
||||||
|
"--verdict-dir",
|
||||||
|
str(tmp_path / "inbox"),
|
||||||
|
"--embedder-config",
|
||||||
|
str(cfg),
|
||||||
|
"--live-dry-run",
|
||||||
|
]
|
||||||
|
)
|
||||||
|
err = capsys.readouterr().err
|
||||||
|
assert rc == 1
|
||||||
|
assert "--embedder-config" in err
|
||||||
|
assert "--semantic-retrieval" in err
|
||||||
|
|
||||||
|
|
||||||
|
def test_embedder_config_without_semantic_retrieval_is_refused_in_portfolio_mode(
|
||||||
|
tmp_path, capsys
|
||||||
|
) -> None:
|
||||||
|
"""Mode-independent: BOTH run modes gate the embedder on the same flag (``run_portfolio``
|
||||||
|
forwards it to ``run_project``, which builds the ranker or nothing), so a refusal scoped to
|
||||||
|
single-project mode would leave portfolio mode with the silent drop."""
|
||||||
|
cfg = tmp_path / "embedder.json"
|
||||||
|
cfg.write_text('{"type": "fake"}', encoding="utf-8")
|
||||||
|
rc = run.main(["--portfolio", "--embedder-config", str(cfg)])
|
||||||
|
err = capsys.readouterr().err
|
||||||
|
assert rc == 1
|
||||||
|
assert "--embedder-config" in err
|
||||||
|
assert "--semantic-retrieval" in err
|
||||||
|
|
||||||
|
|
||||||
|
def test_embedder_config_is_refused_before_the_scripted_banner(tmp_path, capsys) -> None:
|
||||||
|
"""The refusal sits ABOVE the scripted door, mirroring the required-args hoist: a refused run
|
||||||
|
must never print the honesty banner, which claims a scripted loop actually closed."""
|
||||||
|
cfg = tmp_path / "embedder.json"
|
||||||
|
cfg.write_text('{"type": "fake"}', encoding="utf-8")
|
||||||
|
replies = tmp_path / "replies.json"
|
||||||
|
replies.write_text(
|
||||||
|
json.dumps({"proposer": _ENERGY_REPLY, "checker": "VERDICT: APPROVE"}), encoding="utf-8"
|
||||||
|
)
|
||||||
|
rc = run.main(
|
||||||
|
[
|
||||||
|
_PID,
|
||||||
|
"--docs-dir",
|
||||||
|
str(BUNDLE_DIR),
|
||||||
|
"--bundle-dir",
|
||||||
|
str(BUNDLE_DIR),
|
||||||
|
"--verdict-dir",
|
||||||
|
str(tmp_path / "inbox"),
|
||||||
|
"--embedder-config",
|
||||||
|
str(cfg),
|
||||||
|
"--scripted-replies",
|
||||||
|
str(replies),
|
||||||
|
]
|
||||||
|
)
|
||||||
|
out = capsys.readouterr()
|
||||||
|
assert rc == 1
|
||||||
|
assert "SCRIPTED OFFLINE RUN" not in out.out
|
||||||
|
|
||||||
|
|
||||||
def test_report_with_embedder_config_is_refused(tmp_path, capsys) -> None:
|
def test_report_with_embedder_config_is_refused(tmp_path, capsys) -> None:
|
||||||
"""--report stays an ALLOWLIST: a new config flag must be refused there like every other one,
|
"""--report stays an ALLOWLIST: a new config flag must be refused there like every other one,
|
||||||
else it would be silently dropped."""
|
else it would be silently dropped."""
|
||||||
|
|
@ -878,3 +950,73 @@ async def test_run_portfolio_without_the_flag_keeps_the_structural_pick(
|
||||||
assert all(_TIE_MARKER not in p for p in recorded), (
|
assert all(_TIE_MARKER not in p for p in recorded), (
|
||||||
"the marker reached a prompt without the flag — the portfolio default is not structural"
|
"the marker reached a prompt without the flag — the portfolio default is not structural"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _counting_embedder(calls: list) -> object:
|
||||||
|
"""A real embedder (the shipped ``FakeEmbedder``) that records every consultation, so the
|
||||||
|
flag-ON arm completes normally and the difference between the arms is the CALL COUNT alone."""
|
||||||
|
from portfolio_optimiser.semretrieval import FakeEmbedder
|
||||||
|
|
||||||
|
fake = FakeEmbedder()
|
||||||
|
|
||||||
|
def embed(features):
|
||||||
|
calls.append(features)
|
||||||
|
return fake(features)
|
||||||
|
|
||||||
|
return embed
|
||||||
|
|
||||||
|
|
||||||
|
async def test_injected_embedder_is_never_consulted_with_the_flag_off(
|
||||||
|
make_recording_client_factory,
|
||||||
|
) -> None:
|
||||||
|
"""THE MEASUREMENT BEHIND THE REFUSAL — an injected embedder is consulted ZERO times when
|
||||||
|
``semantic_retrieval`` is off, because the ``HybridRanker`` that holds it is only built when
|
||||||
|
the flag is on; the structural retriever takes no embedder at all.
|
||||||
|
|
||||||
|
This is why ``--embedder-config`` without ``--semantic-retrieval`` is REFUSED rather than
|
||||||
|
WIRED (contrast ``--scripted-replies`` in portfolio mode, where a seam to wire existed). If
|
||||||
|
someone later gives the embedder a job on the default path, this test goes RED and the CLI
|
||||||
|
refusal above becomes wrong — which is exactly the signal wanted."""
|
||||||
|
calls: list = []
|
||||||
|
factory, _ = make_recording_client_factory(_ENERGY_REPLY)
|
||||||
|
|
||||||
|
await run_project(
|
||||||
|
_PID,
|
||||||
|
"local",
|
||||||
|
docs_dir=str(BUNDLE_DIR),
|
||||||
|
bundle_dir=str(BUNDLE_DIR),
|
||||||
|
verdict_input=_VERDICT_INPUT,
|
||||||
|
store=_tied_pair_store(),
|
||||||
|
client_factory=factory,
|
||||||
|
top_k=1,
|
||||||
|
embedder=_counting_embedder(calls),
|
||||||
|
)
|
||||||
|
|
||||||
|
assert calls == [], (
|
||||||
|
"the injected embedder was consulted with semantic_retrieval OFF — the CLI refusal of "
|
||||||
|
"--embedder-config without --semantic-retrieval is no longer justified"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
async def test_injected_embedder_is_consulted_with_the_flag_on(
|
||||||
|
make_recording_client_factory,
|
||||||
|
) -> None:
|
||||||
|
"""CAUSALITY CONTROL — the identical run with the flag ON consults the same embedder. Without
|
||||||
|
this the zero above could equally mean the embedder was never reachable at all."""
|
||||||
|
calls: list = []
|
||||||
|
factory, _ = make_recording_client_factory(_ENERGY_REPLY)
|
||||||
|
|
||||||
|
await run_project(
|
||||||
|
_PID,
|
||||||
|
"local",
|
||||||
|
docs_dir=str(BUNDLE_DIR),
|
||||||
|
bundle_dir=str(BUNDLE_DIR),
|
||||||
|
verdict_input=_VERDICT_INPUT,
|
||||||
|
store=_tied_pair_store(),
|
||||||
|
client_factory=factory,
|
||||||
|
top_k=1,
|
||||||
|
embedder=_counting_embedder(calls),
|
||||||
|
semantic_retrieval=True,
|
||||||
|
)
|
||||||
|
|
||||||
|
assert calls, "the injected embedder was never consulted with the flag ON"
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue