feat(persona): build the shared expert-reviewer persona as a framework-neutral Agent Skill
The expert reviewer was only a hardcoded verdict_input dict inside the offline simulation. Build it as the real, shared artifact target picture §8 calls for: shared/skills/expert-reviewer/ — a SKILL.md persona prompt (energy-advisor / M&V role + the realization-gap methodology the validator cannot compute) plus a canonical references/example-verdict.json. shared/ stays pure data; the MAF side reads it via portfolio_optimiser.persona.load_persona_example (call-time, fail-fast) and the Claude-SDK sibling reads the same JSON with its own loader. This de-stubs the simulation: its persona judgement (decision + rationale + traced marker) is now sourced from the artifact at call time, not an inline literal — so the shared persona is genuinely consumed and cannot rot silently. decision is binary (approved/rejected, the FeedbackContract the run path accepts); approved_with_adjustment is rejected there and lives only in the bundle seed frontmatter + the promotion gate, so the realization correction is carried in the rationale prose. Load-bearing trio (tests/test_persona_skill_loadbearing.py), each proven RED on its own detach: structure + framework-neutrality, the example is valid pipeline input (incl. FeedbackContract, on a throwaway copy), and the simulation's marker follows the artifact file. Suite 149->152. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MHR8iKxJRxDiDfNw8HZmWE
This commit is contained in:
parent
a9144cb9bb
commit
6f861a0078
8 changed files with 281 additions and 13 deletions
101
tests/test_persona_skill_loadbearing.py
Normal file
101
tests/test_persona_skill_loadbearing.py
Normal file
|
|
@ -0,0 +1,101 @@
|
|||
"""Load-bearing tests for the shared expert-reviewer persona skill (målbilde §8).
|
||||
|
||||
The persona is a framework-neutral Agent Skill in ``shared/skills/`` encoding the expert reviewer's
|
||||
judgement — the realization gap the deterministic validator cannot compute. These tests make the
|
||||
artifact load-bearing: the REAL pipeline consumes the persona's canonical example verdict, and the
|
||||
offline simulation derives its persona judgement from that artifact, not from a hardcoded literal.
|
||||
Each test goes RED when its seam is detached.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import re
|
||||
import shutil
|
||||
from pathlib import Path
|
||||
|
||||
from portfolio_optimiser import okf, persona, verdicts
|
||||
from portfolio_optimiser.contracts import FeedbackContract
|
||||
from portfolio_optimiser.simulation import simulate_learning_loop
|
||||
from portfolio_optimiser.verdicts import bundle_candidate_features, capture_verdict, promote_verdict
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parents[1]
|
||||
SKILL_DIR = REPO_ROOT / "shared" / "skills" / "expert-reviewer"
|
||||
BUNDLE_DIR = REPO_ROOT / "shared" / "examples" / "bygg-energi-mikro"
|
||||
|
||||
# Import-shaped framework references. Prose mentioning a framework is fine; an import/dependency is
|
||||
# not — this mirrors test_okf_is_maf_free's intent for the shared/skills/ data tree (which has no
|
||||
# AST to parse, so the guard is constrained to import-shaped patterns, not bare substrings).
|
||||
_FORBIDDEN = re.compile(r"\bagent_framework\b|\b(?:import|from)\s+mcp\b")
|
||||
|
||||
|
||||
def test_persona_skill_structure_and_framework_neutral() -> None:
|
||||
"""Test 1: the skill exists with valid non-empty frontmatter, ships its example, and carries no
|
||||
framework import (mirrors test_okf_is_maf_free for shared/skills/). RED before the artifact
|
||||
exists."""
|
||||
skill_md = SKILL_DIR / "SKILL.md"
|
||||
assert skill_md.is_file(), "shared/skills/expert-reviewer/SKILL.md missing"
|
||||
assert (SKILL_DIR / "references" / "example-verdict.json").is_file()
|
||||
|
||||
fm = okf.parse_frontmatter(skill_md)
|
||||
assert fm.get("name") == "expert-reviewer"
|
||||
assert fm.get("description", "").strip(), "SKILL.md description must be non-empty"
|
||||
|
||||
for f in SKILL_DIR.rglob("*"):
|
||||
if f.is_file():
|
||||
assert not _FORBIDDEN.search(f.read_text(encoding="utf-8")), (
|
||||
f"framework import-shaped reference in shared/ persona artifact: {f}"
|
||||
)
|
||||
|
||||
|
||||
def test_persona_example_is_valid_pipeline_input(tmp_path: Path) -> None:
|
||||
"""Test 2: the persona's canonical example verdict is a VALID input to the REAL pipeline —
|
||||
capture_verdict -> promote_verdict accepts it and yields a navigable concept file. Operates on a
|
||||
throwaway COPY (promote_verdict mutates index.md — never touch the git-tracked fixture). RED if
|
||||
the example drifts from the schema the pipeline consumes. Also asserts the marker-in-rationale
|
||||
invariant the simulation requires (simulation.py raises otherwise)."""
|
||||
ex = persona.load_persona_example()
|
||||
assert ex.marker in ex.rationale, "marker must be a substring of rationale (sim invariant)"
|
||||
# The run path feeds verdict_input through FeedbackContract (a BINARY decision); the promotion
|
||||
# gate accepts the same value. The realization correction lives in the rationale, not a third
|
||||
# enum value the run-path contract would reject (contracts.py: Literal["approved", "rejected"]).
|
||||
assert ex.decision == "approved"
|
||||
assert ex.decision in verdicts._APPROVED_DECISIONS
|
||||
FeedbackContract(
|
||||
decision=ex.decision, rationale=ex.rationale
|
||||
) # run-path contract: must not raise
|
||||
|
||||
copy = tmp_path / "bundle"
|
||||
shutil.copytree(BUNDLE_DIR, copy)
|
||||
features = bundle_candidate_features(str(copy))
|
||||
verdict = capture_verdict(features, ex.decision, ex.rationale)
|
||||
promoted = promote_verdict(
|
||||
str(copy),
|
||||
verdict,
|
||||
approver="test",
|
||||
experiment="persona-loadbearing",
|
||||
timestamp="2026-06-30",
|
||||
)
|
||||
assert promoted.exists() and promoted.name.startswith("promoted-verdict-")
|
||||
|
||||
|
||||
async def test_simulation_persona_is_derived_from_artifact(tmp_path: Path, monkeypatch) -> None:
|
||||
"""Test 3 (de-stub causal seam): point the loader at a temp artifact carrying a DIFFERENT marker
|
||||
and confirm the simulation's traced marker follows the FILE — proving the persona judgement is
|
||||
sourced from the artifact, not inlined. RED the moment simulation re-hardcodes the persona
|
||||
(result.marker would stay fixed regardless of the file)."""
|
||||
swapped_marker = "realiseringsgrad=0.41"
|
||||
fake = {
|
||||
"decision": "approved",
|
||||
"marker": swapped_marker,
|
||||
"rationale": f"Godkjent med justering. I drift realiseres erfaringsvis ~41% ({swapped_marker}).",
|
||||
}
|
||||
fake_path = tmp_path / "example-verdict.json"
|
||||
fake_path.write_text(json.dumps(fake), encoding="utf-8")
|
||||
monkeypatch.setattr(persona, "_EXAMPLE_PATH", fake_path)
|
||||
|
||||
result = await simulate_learning_loop(str(BUNDLE_DIR), str(tmp_path / "w"))
|
||||
|
||||
assert result.marker == swapped_marker, "simulation did not source the marker from the artifact"
|
||||
assert not result.marker_in_run_a_prompt, "swapped marker leaked into Run A (causality broken)"
|
||||
assert result.marker_in_run_b_prompt, "the artifact's marker did not cross into Run B's prompt"
|
||||
Loading…
Add table
Add a link
Reference in a new issue