feat(s41): PORTFOLIO_MODEL_MAP override + placeholder fail-fast in resolve_model
This commit is contained in:
parent
0670b2a6d8
commit
eb552f854e
3 changed files with 97 additions and 8 deletions
|
|
@ -22,17 +22,40 @@ import json
|
||||||
import os
|
import os
|
||||||
from enum import Enum
|
from enum import Enum
|
||||||
from importlib.resources import files
|
from importlib.resources import files
|
||||||
from typing import Protocol, runtime_checkable
|
from pathlib import Path
|
||||||
|
from typing import Any, Protocol, runtime_checkable
|
||||||
|
|
||||||
from agent_framework import BaseChatClient
|
from agent_framework import BaseChatClient
|
||||||
from agent_framework_foundry import FoundryChatClient
|
from agent_framework_foundry import FoundryChatClient
|
||||||
from agent_framework_openai import OpenAIChatCompletionClient
|
from agent_framework_openai import OpenAIChatCompletionClient
|
||||||
|
|
||||||
_MODEL_MAP_RESOURCE = "data/model_map.json"
|
_MODEL_MAP_RESOURCE = "data/model_map.json"
|
||||||
|
# S4.1 — out-of-tree model-map override so tenant-specific deployment names are never committed.
|
||||||
|
_MODEL_MAP_ENV = "PORTFOLIO_MODEL_MAP"
|
||||||
|
# S4.1 — placeholder sentinel (mirrors costsim.PLACEHOLDER_PREFIX); an azure deployment left as
|
||||||
|
# ``REPLACE-WITH-*`` must never reach a client build.
|
||||||
|
_PLACEHOLDER_PREFIX = "REPLACE-WITH-"
|
||||||
# Loopback only — never a remote host (D6 / research 03 no-egress). Override via env.
|
# Loopback only — never a remote host (D6 / research 03 no-egress). Override via env.
|
||||||
_DEFAULT_LOCAL_BASE_URL = "http://127.0.0.1:11434/v1"
|
_DEFAULT_LOCAL_BASE_URL = "http://127.0.0.1:11434/v1"
|
||||||
|
|
||||||
|
|
||||||
|
def _load_effective_map() -> dict[str, Any]:
|
||||||
|
"""Load the role->model map (B12). ``PORTFOLIO_MODEL_MAP`` (an out-of-tree path) wins so
|
||||||
|
tenant-specific deployment names are never committed; otherwise the bundled resource. This is
|
||||||
|
the SINGLE source of truth shared by ``resolve_model`` and the S4.1 preflight — so the checker
|
||||||
|
and the run path can never validate different maps. Fail-fast (``FileNotFoundError``) when the
|
||||||
|
override path does not exist (mirror ``contracts.load_goal_config``)."""
|
||||||
|
override = os.environ.get(_MODEL_MAP_ENV)
|
||||||
|
if override:
|
||||||
|
path = Path(override)
|
||||||
|
if not path.is_file():
|
||||||
|
raise FileNotFoundError(f"model map not found: {override!r}")
|
||||||
|
return json.loads(path.read_text(encoding="utf-8"))
|
||||||
|
return json.loads(
|
||||||
|
files("portfolio_optimiser").joinpath(_MODEL_MAP_RESOURCE).read_text(encoding="utf-8")
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
class Profile(str, Enum):
|
class Profile(str, Enum):
|
||||||
"""Model-serving backend profile (D2)."""
|
"""Model-serving backend profile (D2)."""
|
||||||
|
|
||||||
|
|
@ -52,16 +75,22 @@ class ChatBackend(Protocol):
|
||||||
|
|
||||||
|
|
||||||
def resolve_model(profile: Profile | str, role: str) -> str:
|
def resolve_model(profile: Profile | str, role: str) -> str:
|
||||||
"""Resolve a role -> model/deployment id from ``data/model_map.json`` (B12). Falls back
|
"""Resolve a role -> model/deployment id from the effective model map (B12), honoring
|
||||||
to the profile's ``default``; fail-fast (``ValueError``) when nothing maps."""
|
``PORTFOLIO_MODEL_MAP``. Falls back to the profile's ``default``; fail-fast (``ValueError``)
|
||||||
|
when nothing maps, OR when the resolved id is still an unreplaced ``REPLACE-WITH-*`` placeholder
|
||||||
|
(S4.1 — never build a client from a placeholder; the guard reaches the run path at
|
||||||
|
``run.py`` too, not just the preflight)."""
|
||||||
prof = Profile(profile)
|
prof = Profile(profile)
|
||||||
table = json.loads(
|
table = _load_effective_map()
|
||||||
files("portfolio_optimiser").joinpath(_MODEL_MAP_RESOURCE).read_text(encoding="utf-8")
|
|
||||||
)
|
|
||||||
profile_map = table.get(prof.value, {})
|
profile_map = table.get(prof.value, {})
|
||||||
model = profile_map.get(role) or profile_map.get("default")
|
model = profile_map.get(role) or profile_map.get("default")
|
||||||
if not model:
|
if not model:
|
||||||
raise ValueError(f"no model mapped for profile={prof.value} role={role!r}")
|
raise ValueError(f"no model mapped for profile={prof.value} role={role!r}")
|
||||||
|
if model.startswith(_PLACEHOLDER_PREFIX):
|
||||||
|
raise ValueError(
|
||||||
|
f"model map has unresolved placeholder for profile={prof.value} role={role!r}: "
|
||||||
|
f"{model!r} — replace the placeholder or set PORTFOLIO_MODEL_MAP"
|
||||||
|
)
|
||||||
return model
|
return model
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -1,5 +1,8 @@
|
||||||
"""Tests for the backend profiles (D2) — now wired (Fase 2), no longer skeletons."""
|
"""Tests for the backend profiles (D2) — now wired (Fase 2), no longer skeletons."""
|
||||||
|
|
||||||
|
import json
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
from agent_framework import BaseChatClient
|
from agent_framework import BaseChatClient
|
||||||
|
|
||||||
|
|
@ -12,6 +15,19 @@ from portfolio_optimiser.backends import (
|
||||||
resolve_model,
|
resolve_model,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# A complete, non-placeholder map for the PORTFOLIO_MODEL_MAP override (S4.1). Both blocks are
|
||||||
|
# required — resolve_model does no structural validation, so an azure-only map would break local.
|
||||||
|
_VALID_MAP = {
|
||||||
|
"local": {"default": "qwen3:4b", "proposer": "qwen3:4b", "checker": "qwen3:4b"},
|
||||||
|
"azure": {"default": "gpt-4o-mini", "proposer": "gpt-4o-mini", "checker": "gpt-4o-mini"},
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def _write_map(tmp_path: Path, data: dict) -> Path:
|
||||||
|
p = tmp_path / "model_map.json"
|
||||||
|
p.write_text(json.dumps(data), encoding="utf-8")
|
||||||
|
return p
|
||||||
|
|
||||||
|
|
||||||
def test_get_backend_by_string() -> None:
|
def test_get_backend_by_string() -> None:
|
||||||
assert isinstance(get_backend("azure"), AzureFoundryBackend)
|
assert isinstance(get_backend("azure"), AzureFoundryBackend)
|
||||||
|
|
@ -48,7 +64,34 @@ def test_azure_backend_fails_fast_without_endpoint(monkeypatch: pytest.MonkeyPat
|
||||||
get_backend("azure").create_chat_client(model="dummy-deployment")
|
get_backend("azure").create_chat_client(model="dummy-deployment")
|
||||||
|
|
||||||
|
|
||||||
def test_model_map_resolves_role_to_model() -> None:
|
def test_model_map_resolves_role_to_model(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||||
|
monkeypatch.delenv("PORTFOLIO_MODEL_MAP", raising=False)
|
||||||
assert resolve_model("local", "proposer") == "qwen3:4b"
|
assert resolve_model("local", "proposer") == "qwen3:4b"
|
||||||
# Unknown role falls back to the profile default (still a non-empty id).
|
# Unknown role falls back to the profile default (still a non-empty id).
|
||||||
assert resolve_model(Profile.LOCAL, "no-such-role")
|
assert resolve_model(Profile.LOCAL, "no-such-role")
|
||||||
|
|
||||||
|
|
||||||
|
def test_resolve_model_azure_placeholder_fails_fast(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||||
|
# SC4: the bundled azure block ships REPLACE-WITH-* placeholders → never build a client from
|
||||||
|
# one. Detach point: remove the _PLACEHOLDER_PREFIX guard in resolve_model → this goes GREEN
|
||||||
|
# (returns the placeholder) → RED here.
|
||||||
|
monkeypatch.delenv("PORTFOLIO_MODEL_MAP", raising=False)
|
||||||
|
with pytest.raises(ValueError, match="placeholder"):
|
||||||
|
resolve_model("azure", "proposer")
|
||||||
|
|
||||||
|
|
||||||
|
def test_resolve_model_override_redirects_azure(
|
||||||
|
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||||
|
) -> None:
|
||||||
|
# SC5: PORTFOLIO_MODEL_MAP points at an out-of-tree valid map → resolves the real id (control:
|
||||||
|
# the raise fires only on the placeholder, not always). The committed model_map.json is never
|
||||||
|
# touched.
|
||||||
|
monkeypatch.setenv("PORTFOLIO_MODEL_MAP", str(_write_map(tmp_path, _VALID_MAP)))
|
||||||
|
assert resolve_model("azure", "proposer") == "gpt-4o-mini"
|
||||||
|
assert resolve_model("local", "proposer") == "qwen3:4b"
|
||||||
|
|
||||||
|
|
||||||
|
def test_resolve_model_override_missing_file_fails_fast(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||||
|
monkeypatch.setenv("PORTFOLIO_MODEL_MAP", "/nonexistent/model_map.json")
|
||||||
|
with pytest.raises(FileNotFoundError):
|
||||||
|
resolve_model("azure", "proposer")
|
||||||
|
|
|
||||||
|
|
@ -10,11 +10,22 @@ synthetic client (conftest ``make_portfolio_client_factory``) selects each proje
|
||||||
scanning the prompt for its id, so one production-shaped factory serves the whole portfolio.
|
scanning the prompt for its id, so one production-shaped factory serves the whole portfolio.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
|
import json
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
|
|
||||||
from portfolio_optimiser.run import PortfolioResult, RunResult, run_portfolio, run_project
|
from portfolio_optimiser.run import PortfolioResult, RunResult, run_portfolio, run_project
|
||||||
from portfolio_optimiser.validator import Rejection
|
from portfolio_optimiser.validator import Rejection
|
||||||
|
|
||||||
|
# S4.1: the azure block in the bundled model_map now ships REPLACE-WITH-* placeholders that
|
||||||
|
# resolve_model refuses. This test resolves azure directly (line ~224), so it points
|
||||||
|
# PORTFOLIO_MODEL_MAP at a complete, non-placeholder map (BOTH blocks — resolve_model does no
|
||||||
|
# structural validation) for BOTH parametrizations.
|
||||||
|
_OFFLINE_MODEL_MAP = {
|
||||||
|
"local": {"default": "qwen3:4b", "proposer": "qwen3:4b", "checker": "qwen3:4b"},
|
||||||
|
"azure": {"default": "gpt-4o-mini", "proposer": "gpt-4o-mini", "checker": "gpt-4o-mini"},
|
||||||
|
}
|
||||||
|
|
||||||
# The synthetic reply IS the proposal: generate._parse_ir builds affected_items (each with its
|
# The synthetic reply IS the proposal: generate._parse_ir builds affected_items (each with its
|
||||||
# own quantity/unit_cost) straight from this JSON, and the validator's P90 = 0.30 x Σ(qty·unit_cost)
|
# own quantity/unit_cost) straight from this JSON, and the validator's P90 = 0.30 x Σ(qty·unit_cost)
|
||||||
# ONLY when ``assumptions`` is empty (degenerate Monte Carlo, validator.py:108-113). All three
|
# ONLY when ``assumptions`` is empty (degenerate Monte Carlo, validator.py:108-113). All three
|
||||||
|
|
@ -198,7 +209,7 @@ async def test_c_execution_state_isolation_is_load_bearing(
|
||||||
|
|
||||||
@pytest.mark.parametrize("profile", ["local", "azure"])
|
@pytest.mark.parametrize("profile", ["local", "azure"])
|
||||||
async def test_d_both_profiles_run_offline(
|
async def test_d_both_profiles_run_offline(
|
||||||
make_portfolio_client_factory, fresh_store, profile
|
make_portfolio_client_factory, fresh_store, profile, tmp_path, monkeypatch
|
||||||
) -> None:
|
) -> None:
|
||||||
"""SC7: both profiles drive the portfolio contract path OFFLINE under the synthetic client
|
"""SC7: both profiles drive the portfolio contract path OFFLINE under the synthetic client
|
||||||
(the path is actually executed, not just the backend instantiated). Teeth: ``resolve_model``
|
(the path is actually executed, not just the backend instantiated). Teeth: ``resolve_model``
|
||||||
|
|
@ -210,6 +221,12 @@ async def test_d_both_profiles_run_offline(
|
||||||
``resolve_model`` is the only profile-dependent seam provable offline."""
|
``resolve_model`` is the only profile-dependent seam provable offline."""
|
||||||
from portfolio_optimiser.backends import resolve_model
|
from portfolio_optimiser.backends import resolve_model
|
||||||
|
|
||||||
|
# Unconditionally (both params): the azure branch of line ~224 resolves azure even when
|
||||||
|
# profile=="local", so the placeholder-free override must be active in both runs.
|
||||||
|
map_path = tmp_path / "model_map.json"
|
||||||
|
map_path.write_text(json.dumps(_OFFLINE_MODEL_MAP), encoding="utf-8")
|
||||||
|
monkeypatch.setenv("PORTFOLIO_MODEL_MAP", str(map_path))
|
||||||
|
|
||||||
result = await run_portfolio(
|
result = await run_portfolio(
|
||||||
_PORTFOLIO_IDS,
|
_PORTFOLIO_IDS,
|
||||||
profile,
|
profile,
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue