feat(explore): U4 kallsted 2 - hostet explore_prompt, og whitelisten blir en TREDELING (ORDRE 20260823T204216Z) [skip-docs]
Kallsted (2) av oerkt 57s fire. explore_prompt + explore_contract whitelistes paa den hostede flaten. De er ikke run_project-parametre - utforskningen kjoerer FOERST og gir run_project et Mandate - saa de KONSUMERES i stedet for aa videresendes, og whitelisten er dermed en TREDELING (_REQUIRED / _OPTIONAL / _CONSUMED). Ett sted (_run_kwargs) avgjoer hvilke felt som naar signaturen: et konsumert felt som blir liggende i kwargs er et argument run_project ikke har, som containeren svarer 500 paa for det som egentlig er en wiring-feil. FASE 4e-REGELEN UTVIDET MED EN NEGATIV HALVDEL, og det var paakrevd: 4e-testen sender HVERT whitelistet felt gjennom den EKTE run_project og asserterer dekning mot _ALLOWED_FIELDS - en assert et konsumert felt ALDRI kan oppfylle. Den positive halvdelen dekker naa _REQUIRED|_OPTIONAL (hvert felt maalt mot inspect.signature(run_project)), og den negative at _CONSUMED er DISJUNKT fra samme signatur. Uten den ville et felt som glir fra konsumert til videresendt vaere nettopp driften 4e finnes for. Fire nekter, alle ved navn og alle paa KALLERENS kanal (400): explore_contract uten explore_prompt - explore_prompt uten explore_contract - explore_prompt uten bundle_dir - enable_plan_review=true. Den siste er nektet HER og ikke i explore(), som ogsaa nekter den: ExplorationError er en RuntimeError, saa aa overlate den til sloeyfa ville svart en kallers konfigurasjonsfeil paa KRASJ-kanalen (500) - samme sammenblanding BudgetExceeded fikk sin egen 429 for aa avslutte. U13-doera er dessuten SYNKRON: den blokkerer sloeyfa paa et menneske, og en HTTP-forespoersel har ingen - invocationen ville hengt i stedet for aa svare. TracingConfigError er derimot en ValueError, saa 400- armen dekket den alt (verifisert, ikke antatt). Load-bearing MAALT (tests/test_explore_callsites_loadbearing.py, 6 nye tester), fire mutasjoner alle roede mot HELE suiten + groenn kontroll 996/5: videresend de konsumerte feltene igjen (2 roede) - detach mandate=-wiringen (1 roed) - detach enable_plan_review-nekten (1) - detach bundle_dir-kravet (1). Golden-transkriptet byte-uendret (ea8c534773acdbe41ae68f2c55724d69aaf8be4f). mypy + ruff rene. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YRZhBJcxqTcqWyMW6hBttx
This commit is contained in:
parent
8ba824c96f
commit
234c8d138a
3 changed files with 242 additions and 14 deletions
|
|
@ -66,9 +66,11 @@ import json
|
|||
import os
|
||||
import signal
|
||||
import sys
|
||||
from collections.abc import Mapping
|
||||
from typing import Any
|
||||
|
||||
from portfolio_optimiser.budget import BudgetExceeded
|
||||
from portfolio_optimiser.explore import ExplorationContract, explore
|
||||
from portfolio_optimiser.outbox import outcome_payload
|
||||
from portfolio_optimiser.run import RunResult, run_project
|
||||
from portfolio_optimiser.tracing import configure_tracing, tracing_notice
|
||||
|
|
@ -77,7 +79,13 @@ DEFAULT_PORT = 8088
|
|||
_HOSTED_DEFAULT_PROFILE = "azure"
|
||||
_REQUIRED_FIELDS = ("project_id", "docs_dir", "verdict_input")
|
||||
_OPTIONAL_FIELDS = ("bundle_dir", "profile", "max_rounds", "max_tokens", "top_k")
|
||||
_ALLOWED_FIELDS = frozenset(_REQUIRED_FIELDS + _OPTIONAL_FIELDS)
|
||||
#: Fields this surface CONSUMES rather than forwards (U4). They are not ``run_project``
|
||||
#: parameters — the exploration runs first and hands ``run_project`` a ``Mandate`` — so passing one
|
||||
#: through would be a ``TypeError`` answered as a 500. The whitelist is therefore a THREE-way
|
||||
#: partition, and the Fase 4e proof gained a negative half to match: every forwarded field must be
|
||||
#: a real ``run_project`` parameter, and every consumed field must not be.
|
||||
_CONSUMED_FIELDS = ("explore_prompt", "explore_contract")
|
||||
_ALLOWED_FIELDS = frozenset(_REQUIRED_FIELDS + _OPTIONAL_FIELDS + _CONSUMED_FIELDS)
|
||||
_REASONS = {
|
||||
200: "OK",
|
||||
400: "Bad Request",
|
||||
|
|
@ -99,10 +107,15 @@ def resolve_port() -> int:
|
|||
return int(os.environ.get("PORT") or DEFAULT_PORT)
|
||||
|
||||
|
||||
def _run_kwargs(payload: Any) -> tuple[str, dict[str, Any]]:
|
||||
def _run_kwargs(payload: Any) -> tuple[str, dict[str, Any], dict[str, Any]]:
|
||||
"""Whitelist the JSON payload onto ``run_project``'s signature. Everything not named in
|
||||
the whitelist — including server-side seams like ``outbox_dir``, ``client_factory`` or
|
||||
``verdict_dir`` — is refused by name, never silently dropped."""
|
||||
``verdict_dir`` — is refused by name, never silently dropped.
|
||||
|
||||
Returns ``(project_id, forwarded_kwargs, consumed)``. The consumed half is split out HERE
|
||||
rather than filtered at the call site so there is one place that decides which fields reach
|
||||
``run_project``: a consumed field left in ``kwargs`` is an argument the signature does not have,
|
||||
which the container answers as a 500 for what is really a wiring mistake."""
|
||||
if not isinstance(payload, dict):
|
||||
raise InvocationRefused("body must be a JSON object")
|
||||
unknown = sorted(set(payload) - _ALLOWED_FIELDS)
|
||||
|
|
@ -111,9 +124,58 @@ def _run_kwargs(payload: Any) -> tuple[str, dict[str, Any]]:
|
|||
missing = [field for field in _REQUIRED_FIELDS if field not in payload]
|
||||
if missing:
|
||||
raise InvocationRefused(f"missing required field(s): {', '.join(missing)}")
|
||||
kwargs: dict[str, Any] = {k: payload[k] for k in payload if k != "project_id"}
|
||||
consumed = {k: payload[k] for k in _CONSUMED_FIELDS if k in payload}
|
||||
kwargs: dict[str, Any] = {
|
||||
k: payload[k] for k in payload if k != "project_id" and k not in _CONSUMED_FIELDS
|
||||
}
|
||||
kwargs.setdefault("profile", _HOSTED_DEFAULT_PROFILE)
|
||||
return payload["project_id"], kwargs
|
||||
return payload["project_id"], kwargs, consumed
|
||||
|
||||
|
||||
async def _shaped_mandate(consumed: Mapping[str, Any], kwargs: Mapping[str, Any]) -> Any:
|
||||
"""Run the U4 exploration this invocation asked for and return the mandate it shaped.
|
||||
|
||||
Every refusal here is the CALLER's error and therefore a ``ValueError`` (the 400 arm), by name.
|
||||
That placement is deliberate rather than incidental: ``explore()`` refuses two of these itself,
|
||||
but ``ExplorationError`` is a ``RuntimeError``, so leaving them to the loop would answer a
|
||||
caller's configuration mistake on the crash channel — the same conflation ``BudgetExceeded``
|
||||
was given its own 429 to end.
|
||||
|
||||
``enable_plan_review`` is refused outright. The U13 door is SYNCHRONOUS: it blocks the loop on
|
||||
a human or persona, and an HTTP request has neither — the invocation would hang rather than
|
||||
answer. The library API is where that door opens."""
|
||||
prompt = consumed.get("explore_prompt")
|
||||
raw_contract = consumed.get("explore_contract")
|
||||
if prompt is None:
|
||||
raise InvocationRefused(
|
||||
"explore_contract without explore_prompt: the bounds describe an exploration that "
|
||||
"would never run"
|
||||
)
|
||||
if raw_contract is None:
|
||||
raise InvocationRefused(
|
||||
"explore_prompt without explore_contract: an exploration's bounds are never defaulted "
|
||||
"(an omitted cap falls back to an unbounded loop)"
|
||||
)
|
||||
if not kwargs.get("bundle_dir"):
|
||||
raise InvocationRefused(
|
||||
"explore_prompt without bundle_dir: the exploration navigates knowledge bases, and "
|
||||
"with none configured it would spend its budget reading nothing"
|
||||
)
|
||||
if not isinstance(raw_contract, dict):
|
||||
raise InvocationRefused("explore_contract must be a JSON object")
|
||||
contract = ExplorationContract(**raw_contract) # ValidationError subclasses ValueError -> 400
|
||||
if contract.enable_plan_review:
|
||||
raise InvocationRefused(
|
||||
"explore_contract sets enable_plan_review, but this surface has no reviewer to answer "
|
||||
"it: the synchronous plan review would block the request on nobody"
|
||||
)
|
||||
result = await explore(
|
||||
str(prompt),
|
||||
contract=contract,
|
||||
bundle_dirs=(kwargs["bundle_dir"],),
|
||||
profile=kwargs["profile"],
|
||||
)
|
||||
return result.mandate
|
||||
|
||||
|
||||
def _response_payload(result: RunResult) -> dict[str, Any]:
|
||||
|
|
@ -132,8 +194,14 @@ def _response_payload(result: RunResult) -> dict[str, Any]:
|
|||
async def invoke(payload: Any) -> dict[str, Any]:
|
||||
"""One invocation: validate → ``run_project`` → outbox-shaped response payload.
|
||||
``run_project`` is resolved through this module's namespace at call time (the test
|
||||
seam). ``live_dry_run`` is not on the whitelist, so the union narrows to RunResult."""
|
||||
project_id, kwargs = _run_kwargs(payload)
|
||||
seam). ``live_dry_run`` is not on the whitelist, so the union narrows to RunResult.
|
||||
|
||||
With ``explore_prompt`` the exploration runs FIRST and its mandate is what the pipeline then
|
||||
evaluates — level 2 and 3 of the guarantee table are unchanged, and the exploration itself
|
||||
still writes nothing."""
|
||||
project_id, kwargs, consumed = _run_kwargs(payload)
|
||||
if consumed:
|
||||
kwargs["mandate"] = await _shaped_mandate(consumed, kwargs)
|
||||
result = await run_project(project_id, **kwargs)
|
||||
assert isinstance(result, RunResult)
|
||||
return _response_payload(result)
|
||||
|
|
|
|||
|
|
@ -37,7 +37,7 @@ import pytest
|
|||
from agent_framework import BaseChatClient
|
||||
|
||||
import portfolio_optimiser
|
||||
from portfolio_optimiser import explore, okf, run
|
||||
from portfolio_optimiser import explore, hosting, okf, run
|
||||
from portfolio_optimiser.budget import BudgetExceeded
|
||||
from portfolio_optimiser.explore import ExplorationContract, ExplorationTrace
|
||||
from portfolio_optimiser.mandate import Approach, Mandate
|
||||
|
|
@ -115,7 +115,11 @@ def _hypothesis_line(label: str, rationale: str) -> str:
|
|||
|
||||
|
||||
def _factory(
|
||||
*, ledgers: list[str], hypothesiser: list[str], fallback: str = "ok"
|
||||
*,
|
||||
ledgers: list[str],
|
||||
hypothesiser: list[str],
|
||||
fallback: str = "ok",
|
||||
sink: list[str] | None = None,
|
||||
) -> Callable[[str], BaseChatClient]:
|
||||
"""One fresh ``ScriptedChatClient`` per role — the exploration's three plus everyone else.
|
||||
|
||||
|
|
@ -136,7 +140,7 @@ def _factory(
|
|||
return ScriptedChatClient(reply_selector=_hyp, role=role)
|
||||
if role == explore.NAVIGATOR_ROLE:
|
||||
return ScriptedChatClient("NAVIGATOR: index read.", role=role)
|
||||
return ScriptedChatClient(fallback, role=role)
|
||||
return ScriptedChatClient(fallback, sink, role=role)
|
||||
|
||||
return factory
|
||||
|
||||
|
|
@ -536,3 +540,148 @@ def test_a_seeded_mandate_still_leads_the_shaped_one() -> None:
|
|||
minted = explore._mint_approaches((seed,), [(_LABEL, "shaped in the loop")])
|
||||
assert [a.id for a in minted] == ["expert-1", "hypothesis-1"]
|
||||
assert isinstance(Mandate(objective="o", approaches=minted, allow_own_proposals=True), Mandate)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------------------------
|
||||
# 4. The hosted surface — a THREE-way whitelist, and the Fase 4e rule extended to cover it
|
||||
# ---------------------------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _hosted_payload(**extra: Any) -> dict[str, Any]:
|
||||
return {
|
||||
"project_id": _PID,
|
||||
"docs_dir": str(_BUNDLE_DIR),
|
||||
"verdict_input": {"decision": "approved", "rationale": "expert reviewed (explore)"},
|
||||
# LOCAL, never the hosted AZURE default: the AZURE arm resolves a Foundry deployment name
|
||||
# from the model map before any client is built, so it cannot complete offline.
|
||||
"profile": "local",
|
||||
**extra,
|
||||
}
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
def _hosted_backend(monkeypatch: pytest.MonkeyPatch) -> list[str]:
|
||||
"""The scripted backend behind a hosted invocation, plus the prompt sink that proves it ran.
|
||||
|
||||
``client_factory`` is refused by the invocations whitelist on purpose — the caller of a hosted
|
||||
agent never chooses the server's model client — so ``run._default_factory`` is the only
|
||||
injection point the surface leaves, and ``explore()`` resolves through the same one.
|
||||
"""
|
||||
sink: list[str] = []
|
||||
factory = _factory(
|
||||
ledgers=[_ledger_json(satisfied=False), _ledger_json(satisfied=True)],
|
||||
hypothesiser=[_hypothesis_line(_LABEL, "the index says the fittings are old")],
|
||||
fallback=_ENERGY_REPLY,
|
||||
sink=sink,
|
||||
)
|
||||
monkeypatch.setattr("portfolio_optimiser.run._default_factory", lambda profile: factory)
|
||||
return sink
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_hosted_exploration_shapes_the_mandate_the_run_evaluates(_hosted_backend) -> None:
|
||||
"""H1: ``explore_prompt`` over the hosted surface reaches the pipeline as a mandate.
|
||||
|
||||
Driven through ``hosting.invoke`` and the REAL ``run_project`` (Fase 4e): every other
|
||||
invocations test hands ``invoke`` a recorder that swallows ``**kwargs`` and therefore cannot
|
||||
see whether a new field composes with the signature at all.
|
||||
|
||||
The proof is the PROMPT, not the status code: an ``Approach``'s description reaches the
|
||||
proposer VERBATIM, and this label exists nowhere in the bundle or the reference projects — so
|
||||
finding it in a generation prompt means it travelled prompt → ``explore()`` → ``Mandate`` →
|
||||
``run_project(mandate=…)``.
|
||||
|
||||
Detach point: stop passing the shaped mandate into ``run_project`` → RED.
|
||||
"""
|
||||
body = await hosting.invoke(
|
||||
_hosted_payload(
|
||||
bundle_dir=str(_BUNDLE_DIR),
|
||||
explore_prompt="Find the cheapest saving.",
|
||||
explore_contract=dict(_CONTRACT_JSON),
|
||||
)
|
||||
)
|
||||
|
||||
assert body["outcome_type"] in {"validated", "rejected"}
|
||||
assert any(_LABEL in prompt for prompt in _hosted_backend), (
|
||||
"the shaped approach never reached a prompt — the hosted door does not wire the mandate"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_consumed_field_is_never_forwarded_to_run_project() -> None:
|
||||
"""H2: the whitelist is a THREE-way partition, and the consumed half is proved NEGATIVELY.
|
||||
|
||||
``explore_prompt``/``explore_contract`` are accepted by the surface and consumed BY it — they
|
||||
are not ``run_project`` parameters, and forwarding one would be a ``TypeError`` answered as a
|
||||
500. The positive half of Fase 4e (every forwarded field reaches the real signature) cannot
|
||||
see that; without this arm a field sliding from consumed to forwarded is exactly the drift 4e
|
||||
exists to catch.
|
||||
|
||||
Detach point: build ``kwargs`` from the whole payload again → RED.
|
||||
"""
|
||||
import inspect
|
||||
|
||||
_, kwargs, consumed = hosting._run_kwargs(
|
||||
_hosted_payload(
|
||||
bundle_dir=str(_BUNDLE_DIR),
|
||||
explore_prompt="p",
|
||||
explore_contract=dict(_CONTRACT_JSON),
|
||||
)
|
||||
)
|
||||
assert set(hosting._CONSUMED_FIELDS).isdisjoint(kwargs), (
|
||||
"a consumed field was forwarded to run_project, which does not take it"
|
||||
)
|
||||
assert set(consumed) == set(hosting._CONSUMED_FIELDS)
|
||||
|
||||
parameters = inspect.signature(run.run_project).parameters
|
||||
assert set(hosting._CONSUMED_FIELDS).isdisjoint(parameters), (
|
||||
"a CONSUMED field is a run_project parameter — it belongs in the forwarded half"
|
||||
)
|
||||
for name in (*hosting._REQUIRED_FIELDS, *hosting._OPTIONAL_FIELDS):
|
||||
# project_id is positional; every other forwarded field must be a real keyword.
|
||||
assert name in parameters, f"whitelisted field {name!r} is not a run_project parameter"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
("payload", "expected"),
|
||||
[
|
||||
pytest.param(
|
||||
{"bundle_dir": str(_BUNDLE_DIR), "explore_contract": dict(_CONTRACT_JSON)},
|
||||
"explore_prompt",
|
||||
id="bounds-without-an-exploration",
|
||||
),
|
||||
pytest.param(
|
||||
{"bundle_dir": str(_BUNDLE_DIR), "explore_prompt": "p"},
|
||||
"explore_contract",
|
||||
id="exploration-without-bounds",
|
||||
),
|
||||
pytest.param(
|
||||
{"explore_prompt": "p", "explore_contract": dict(_CONTRACT_JSON)},
|
||||
"bundle_dir",
|
||||
id="exploration-without-a-knowledge-base",
|
||||
),
|
||||
pytest.param(
|
||||
{
|
||||
"bundle_dir": str(_BUNDLE_DIR),
|
||||
"explore_prompt": "p",
|
||||
"explore_contract": {**_CONTRACT_JSON, "enable_plan_review": True},
|
||||
},
|
||||
"enable_plan_review",
|
||||
id="a-review-nobody-can-answer",
|
||||
),
|
||||
],
|
||||
)
|
||||
async def test_the_hosted_door_refuses_by_name_on_the_callers_channel(payload, expected) -> None:
|
||||
"""H3: each hosted refusal names the field, and each is a ``ValueError`` — the 400 arm.
|
||||
|
||||
``enable_plan_review`` is the one that had to be refused HERE rather than in ``explore()``:
|
||||
``ExplorationError`` is a ``RuntimeError``, so leaving it to the loop would answer a caller's
|
||||
configuration mistake on the crash channel (500), which is where a fallen-over endpoint lives.
|
||||
A synchronous plan review would also block the HTTP request on a reviewer that does not exist.
|
||||
|
||||
Detach point: drop any one of the four guards → RED.
|
||||
"""
|
||||
with pytest.raises(ValueError) as excinfo:
|
||||
await hosting.invoke(_hosted_payload(**payload))
|
||||
assert expected in str(excinfo.value)
|
||||
|
|
|
|||
|
|
@ -507,8 +507,15 @@ async def test_invocations_answers_through_the_real_run_project(
|
|||
to break that are invisible to a recorder and fatal in a container: naming a field
|
||||
``run_project`` does not take, and passing it an argument twice. Both surface here as a 500.
|
||||
|
||||
The payload names EVERY whitelisted field, and the coverage assertion below is what keeps that
|
||||
true: a field added to the whitelist later cannot slip past this test unexercised."""
|
||||
The payload names every FORWARDED field, and the coverage assertion below is what keeps that
|
||||
true: a field added to the whitelist later cannot slip past this test unexercised.
|
||||
|
||||
The whitelist became a THREE-way partition when U4's ``explore_prompt`` arrived: those fields
|
||||
are accepted by the surface and CONSUMED by it (the exploration runs first and hands
|
||||
``run_project`` a mandate), so they can never satisfy a "reaches ``run_project``" assertion.
|
||||
The consumed half has its own, negative proof in
|
||||
``tests/test_explore_callsites_loadbearing.py`` — without it a field sliding from consumed to
|
||||
forwarded is exactly the drift 4e exists to catch."""
|
||||
payload = {
|
||||
"project_id": _BUNDLE_PID,
|
||||
"docs_dir": str(_BUNDLE_DIR),
|
||||
|
|
@ -522,10 +529,14 @@ async def test_invocations_answers_through_the_real_run_project(
|
|||
"max_tokens": 100_000,
|
||||
"top_k": 3,
|
||||
}
|
||||
assert set(payload) == hosting._ALLOWED_FIELDS, (
|
||||
"the payload must exercise every whitelisted field — a field the whitelist accepts but "
|
||||
assert set(payload) == set(hosting._REQUIRED_FIELDS) | set(hosting._OPTIONAL_FIELDS), (
|
||||
"the payload must exercise every FORWARDED field — a field the whitelist forwards but "
|
||||
"this test never sends is a field no test proves ``run_project`` accepts"
|
||||
)
|
||||
assert hosting._ALLOWED_FIELDS == set(payload) | set(hosting._CONSUMED_FIELDS), (
|
||||
"the whitelist is a three-way partition and this arm covers the forwarded half; a field "
|
||||
"in neither half would be accepted by the surface with nothing proving what it does"
|
||||
)
|
||||
|
||||
status, body = await _post(served, "/invocations", payload)
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue