Add ask_expert and eval capture (A14/A16) — step 7c
CI / checks (push) Skipped
CI / checks (pull_request) Successful in 1m7s
CI / checks (push) Skipped
CI / checks (pull_request) Successful in 1m7s
Completes step 7. - expert.py: one stateless call for a question needing no repo access. Structured output is contracted at the prompt level, since vendors differ on JSON enforcement, so the reply is parsed and validated locally with one retry — tolerating a fenced code block, because models wrap JSON habitually, but not missing fields - The per-task budget is counted from the canonical log rather than memory, so it survives a restart and cannot hand a circling executor a fresh allowance. Exhaustion raises rather than answering: a caller out of consults should get on with the work or escalate - Consults are recorded as delegation_started/completed with role consultant. A16 asks for a "consult/delegation eval set", and reusing the composite events gets consult evaluation for free rather than inventing a parallel record - evals.py pairs starts with completions into eval records, with verdicts kept outside the canonical log: the log records what happened, a verdict is a later judgement about it, and judgements get revised. Schema v3 adds the outcomes table, so a v1 store now traverses two migrations — a better exercise of the hook than one - ask_expert is offered to the executor as a tool only when a desk is configured, in addition to the observable escalation triggers and never instead of them (A12). Consulting is not mutating, so a completion claim backed only by consults is still challenged Found live: passing an empty message list and putting the question in the system prompt fails outright — the Messages API requires at least one non-system message. The question now travels as a synthesised user message that is never appended to the log, which keeps the consult stateless while remaining a valid request. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
dd22eda528
commit
2b7651fb37
9 files changed
+846
-8
No files matched your search
@@ -0,0 +1,88 @@
|
||||
"""Delegation and consult evaluation capture (spec A16).
|
||||
|
||||
Each ``delegation_started`` / ``delegation_completed`` pair is already a clean
|
||||
input-output record; this pairs them up and attaches a verdict, so "best model
|
||||
this week" can be decided by re-running a captured set rather than by benchmark
|
||||
chatter. Without it, swapping the frontier tier becomes something to avoid,
|
||||
because the effect is unmeasurable.
|
||||
|
||||
Verdicts live outside the canonical log: the log records what happened, a
|
||||
verdict is a later judgement about it, and judgements get revised.
|
||||
"""
|
||||
|
||||
import json
|
||||
from dataclasses import asdict, dataclass
|
||||
from typing import Any
|
||||
|
||||
from nisse_dev.canonical import DelegationCompleted, DelegationStarted
|
||||
from nisse_dev.state import StateStore
|
||||
|
||||
UNMARKED = "unmarked"
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class EvalRecord:
|
||||
event_id: int # of the completion, which is what a verdict attaches to
|
||||
role: str
|
||||
model: str
|
||||
brief: str
|
||||
caps: dict[str, Any]
|
||||
handback: str
|
||||
artifacts: list[str]
|
||||
tokens: int
|
||||
duration_s: float
|
||||
exit_status: str
|
||||
verdict: str = UNMARKED
|
||||
note: str = ""
|
||||
|
||||
|
||||
def collect(store: StateStore, after: int = 0) -> list[EvalRecord]:
|
||||
"""Pair each start with the completion that follows it.
|
||||
|
||||
A start with no completion is skipped rather than reported half-formed:
|
||||
it means the process died mid-delegation, which A3 handles, and an
|
||||
unfinished record would pollute the eval set.
|
||||
"""
|
||||
verdicts = store.outcomes()
|
||||
records: list[EvalRecord] = []
|
||||
pending: tuple[int, DelegationStarted] | None = None
|
||||
for stored in store.read_events(after=after):
|
||||
if isinstance(stored.event, DelegationStarted):
|
||||
pending = (stored.id, stored.event)
|
||||
elif isinstance(stored.event, DelegationCompleted) and pending is not None:
|
||||
_, started = pending
|
||||
verdict, note = verdicts.get(stored.id, (UNMARKED, ""))
|
||||
records.append(
|
||||
EvalRecord(
|
||||
event_id=stored.id,
|
||||
role=started.role,
|
||||
model=started.model,
|
||||
brief=started.brief,
|
||||
caps=started.caps,
|
||||
handback=stored.event.handback,
|
||||
artifacts=stored.event.artifacts,
|
||||
tokens=stored.event.tokens,
|
||||
duration_s=stored.event.duration_s,
|
||||
exit_status=stored.event.exit_status,
|
||||
verdict=verdict,
|
||||
note=note,
|
||||
)
|
||||
)
|
||||
pending = None
|
||||
return records
|
||||
|
||||
|
||||
def to_jsonl(records: list[EvalRecord]) -> str:
|
||||
"""JSON lines, so a captured set can be diffed and re-run per record."""
|
||||
return "\n".join(json.dumps(asdict(r)) for r in records)
|
||||
|
||||
|
||||
def summarise(records: list[EvalRecord]) -> dict[str, Any]:
|
||||
by_model: dict[str, dict[str, int]] = {}
|
||||
for record in records:
|
||||
bucket = by_model.setdefault(
|
||||
record.model, {"total": 0, "good": 0, "bad": 0, UNMARKED: 0}
|
||||
)
|
||||
bucket["total"] += 1
|
||||
bucket[record.verdict if record.verdict in ("good", "bad") else UNMARKED] += 1
|
||||
return by_model
|
||||
@@ -11,12 +11,13 @@ the human with the failures attached rather than engaging a frontier model.
|
||||
"""
|
||||
|
||||
from dataclasses import dataclass
|
||||
from typing import Literal
|
||||
from typing import Any, Literal
|
||||
|
||||
from nisse_dev.canonical import ToolCall, ToolResult
|
||||
from nisse_dev.expert import ASK_EXPERT_TOOL, BudgetExhausted, ExpertDesk
|
||||
from nisse_dev.llm import LLMClient, LLMError, RoleMap
|
||||
from nisse_dev.state import StateStore
|
||||
from nisse_dev.tools import EXECUTOR_TOOLS, MUTATING_TOOLS, Workspace
|
||||
from nisse_dev.tools import EXECUTOR_TOOLS, MUTATING_TOOLS, ToolOutcome, Workspace
|
||||
|
||||
EXECUTOR_SYSTEM = """You are nisse-dev's executor, doing mechanical work.
|
||||
|
||||
@@ -56,6 +57,7 @@ class LLMExecutor:
|
||||
*,
|
||||
max_turns: int = DEFAULT_MAX_TURNS,
|
||||
failure_limit: int = DEFAULT_FAILURE_LIMIT,
|
||||
expert: ExpertDesk | None = None,
|
||||
) -> None:
|
||||
self._client = client
|
||||
self._store = store
|
||||
@@ -63,6 +65,28 @@ class LLMExecutor:
|
||||
self._workspace = workspace
|
||||
self._max_turns = max_turns
|
||||
self._failure_limit = failure_limit
|
||||
self._expert = expert
|
||||
|
||||
def _tools(self) -> list[dict[str, Any]]:
|
||||
# A12: ask_expert exists in addition to the observable escalation
|
||||
# triggers, never as a replacement for them.
|
||||
if self._expert is None:
|
||||
return EXECUTOR_TOOLS
|
||||
return [*EXECUTOR_TOOLS, ASK_EXPERT_TOOL]
|
||||
|
||||
def _consult(self, arguments: dict[str, Any]) -> ToolOutcome:
|
||||
assert self._expert is not None
|
||||
question = str(arguments.get("question", "")).strip()
|
||||
if not question:
|
||||
return ToolOutcome("ask_expert needs a question", success=False)
|
||||
try:
|
||||
return ToolOutcome(self._expert.ask(question).render(), success=True)
|
||||
except BudgetExhausted as exc:
|
||||
# Not an error the model should route around: it is the signal
|
||||
# that it is circling and should get on with the work or fail.
|
||||
return ToolOutcome(str(exc), success=False)
|
||||
except LLMError as exc:
|
||||
return ToolOutcome(f"the consultant did not answer: {exc}", success=False)
|
||||
|
||||
def run(self, milestone: int) -> ExecutorOutcome:
|
||||
model = self._roles.resolve("executor")
|
||||
@@ -78,7 +102,7 @@ class LLMExecutor:
|
||||
try:
|
||||
produced = self._client.complete(
|
||||
self._store.read_events(),
|
||||
tools=EXECUTOR_TOOLS,
|
||||
tools=self._tools(),
|
||||
model=model,
|
||||
system=system,
|
||||
)
|
||||
@@ -110,7 +134,10 @@ class LLMExecutor:
|
||||
for call in calls:
|
||||
if call.tool in MUTATING_TOOLS:
|
||||
did_work = True
|
||||
outcome = self._workspace.dispatch(call.tool, call.arguments)
|
||||
if call.tool == "ask_expert":
|
||||
outcome = self._consult(call.arguments)
|
||||
else:
|
||||
outcome = self._workspace.dispatch(call.tool, call.arguments)
|
||||
self._store.append_event(
|
||||
ToolResult(
|
||||
call_id=call.call_id,
|
||||
|
||||
@@ -0,0 +1,214 @@
|
||||
"""`ask_expert` (spec A14).
|
||||
|
||||
The one case delegation does not cover: a question needing no repo access —
|
||||
"which of these two approaches is better", "is this API deprecated". A single
|
||||
stateless call, a text answer, no session state, no diff application.
|
||||
|
||||
Two constraints from A14 shape the implementation:
|
||||
|
||||
**Structured output is contracted at the prompt level, not the API level.**
|
||||
Vendors differ on JSON mode and schema enforcement, so the answer is parsed and
|
||||
validated against a local schema, with one retry. This runs over LiteLLM's
|
||||
*normalized* route, which A13 reserves for calls where fidelity is cheap.
|
||||
|
||||
**A per-task budget**, so a confused executor cannot spend twenty consults
|
||||
circling. The count comes from the canonical log rather than a counter in
|
||||
memory: it survives a restart, and it is observable from outside the model,
|
||||
which is the same reasoning A12 applies to escalation triggers.
|
||||
|
||||
Consults are recorded as ``delegation_started`` / ``delegation_completed`` with
|
||||
role ``consultant``. That is deliberate — A16 wants a "consult/delegation eval
|
||||
set", and reusing the composite events gets consult evaluation for free rather
|
||||
than inventing a parallel record.
|
||||
"""
|
||||
|
||||
import json
|
||||
import re
|
||||
from dataclasses import dataclass
|
||||
|
||||
from nisse_dev.canonical import (
|
||||
DelegationCompleted,
|
||||
DelegationStarted,
|
||||
StoredEvent,
|
||||
UserMessage,
|
||||
)
|
||||
from nisse_dev.llm import LLMClient, LLMError, RoleMap
|
||||
from nisse_dev.state import StateStore
|
||||
|
||||
CONSULTANT_ROLE = "consultant"
|
||||
DEFAULT_BUDGET = 3
|
||||
|
||||
SYSTEM_PROMPT = """You are a consultant answering one self-contained question.
|
||||
|
||||
You have no access to the caller's repository, so answer from the question
|
||||
alone. If the question cannot be answered without seeing the code, say so
|
||||
plainly in `answer` rather than guessing.
|
||||
|
||||
Reply with a single JSON object and nothing else:
|
||||
|
||||
{"answer": "...", "confidence": "high" | "medium" | "low", "caveats": "..."}
|
||||
|
||||
Keep `answer` direct and specific. Put what would change your mind, or what you
|
||||
are assuming, in `caveats`."""
|
||||
|
||||
_RETRY_NUDGE = (
|
||||
"\n\nYour previous reply was not a single valid JSON object with the keys "
|
||||
"answer, confidence and caveats. Reply with only that object."
|
||||
)
|
||||
_CONFIDENCES = {"high", "medium", "low"}
|
||||
|
||||
|
||||
ASK_EXPERT_TOOL: dict = {
|
||||
"type": "function",
|
||||
"function": {
|
||||
"name": "ask_expert",
|
||||
"description": (
|
||||
"Ask a stronger model one self-contained question that needs no "
|
||||
"access to this repository — a design trade-off, or how a library "
|
||||
"behaves. It cannot see your files. Use sparingly; the budget is "
|
||||
"small."
|
||||
),
|
||||
"parameters": {
|
||||
"type": "object",
|
||||
"properties": {"question": {"type": "string"}},
|
||||
"required": ["question"],
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
|
||||
class BudgetExhausted(RuntimeError):
|
||||
"""Raised rather than silently answering: a caller that has burned its
|
||||
consult budget is circling, and should escalate or ask the operator."""
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class ExpertAnswer:
|
||||
answer: str
|
||||
confidence: str
|
||||
caveats: str
|
||||
|
||||
def render(self) -> str:
|
||||
body = f"{self.answer}\n\n(confidence: {self.confidence})"
|
||||
return f"{body}\n\nCaveats: {self.caveats}" if self.caveats else body
|
||||
|
||||
|
||||
class ExpertDesk:
|
||||
def __init__(
|
||||
self,
|
||||
client: LLMClient,
|
||||
store: StateStore,
|
||||
roles: RoleMap,
|
||||
*,
|
||||
budget: int = DEFAULT_BUDGET,
|
||||
) -> None:
|
||||
self._client = client
|
||||
self._store = store
|
||||
self._roles = roles
|
||||
self._budget = budget
|
||||
|
||||
def consults_used(self, since_event_id: int = 0) -> int:
|
||||
return sum(
|
||||
1
|
||||
for e in self._store.read_events(after=since_event_id)
|
||||
if isinstance(e.event, DelegationStarted)
|
||||
and e.event.role == CONSULTANT_ROLE
|
||||
)
|
||||
|
||||
def ask(self, question: str, *, since_event_id: int = 0) -> ExpertAnswer:
|
||||
used = self.consults_used(since_event_id)
|
||||
if used >= self._budget:
|
||||
raise BudgetExhausted(
|
||||
f"consult budget of {self._budget} is spent for this task "
|
||||
f"({used} used); escalate or ask the operator instead"
|
||||
)
|
||||
model = self._roles.resolve(CONSULTANT_ROLE)
|
||||
self._store.append_event(
|
||||
DelegationStarted(
|
||||
brief=question,
|
||||
role=CONSULTANT_ROLE,
|
||||
model=model,
|
||||
caps={"consults": self._budget, "used": used},
|
||||
)
|
||||
)
|
||||
answer, note = self._answer(question, model)
|
||||
self._store.append_event(
|
||||
DelegationCompleted(
|
||||
handback=answer.render() if answer else note,
|
||||
artifacts=[],
|
||||
tokens=0,
|
||||
duration_s=0.0,
|
||||
exit_status="answered" if answer else "failed",
|
||||
)
|
||||
)
|
||||
if answer is None:
|
||||
raise LLMError(f"ask_expert produced no usable answer: {note}")
|
||||
return answer
|
||||
|
||||
def _answer(self, question: str, model: str) -> tuple[ExpertAnswer | None, str]:
|
||||
system = SYSTEM_PROMPT
|
||||
last = "no reply"
|
||||
for attempt in (1, 2):
|
||||
try:
|
||||
# Stateless by design (A14): the question travels as a
|
||||
# synthesised message that is never appended to the log, so a
|
||||
# consult neither depends on nor leaks repo context. It cannot
|
||||
# ride in the system prompt alone — the Messages API requires
|
||||
# at least one non-system message.
|
||||
produced = self._client.complete(
|
||||
[
|
||||
StoredEvent(
|
||||
id=0,
|
||||
ts="",
|
||||
event=UserMessage(text=question, source="ask_expert"),
|
||||
)
|
||||
],
|
||||
tools=None,
|
||||
model=model,
|
||||
system=system,
|
||||
)
|
||||
except LLMError as exc:
|
||||
return None, str(exc)
|
||||
text = "\n".join(
|
||||
getattr(e, "text", "") for e in produced if hasattr(e, "text")
|
||||
).strip()
|
||||
parsed = parse_answer(text)
|
||||
if parsed is not None:
|
||||
return parsed, "ok"
|
||||
last = text[:300] or "empty reply"
|
||||
if attempt == 1:
|
||||
system = SYSTEM_PROMPT + _RETRY_NUDGE
|
||||
return None, f"unparseable reply: {last}"
|
||||
|
||||
|
||||
def parse_answer(text: str) -> ExpertAnswer | None:
|
||||
"""Validated locally because the prompt-level contract is all we have.
|
||||
|
||||
Tolerates a fenced code block, since models wrap JSON in one habitually,
|
||||
but not missing or malformed fields — those are a failed turn.
|
||||
"""
|
||||
candidate = text.strip()
|
||||
fenced = re.search(r"```(?:json)?\s*(.*?)```", candidate, re.DOTALL)
|
||||
if fenced:
|
||||
candidate = fenced.group(1).strip()
|
||||
start, end = candidate.find("{"), candidate.rfind("}")
|
||||
if start == -1 or end <= start:
|
||||
return None
|
||||
try:
|
||||
data = json.loads(candidate[start : end + 1])
|
||||
except json.JSONDecodeError:
|
||||
return None
|
||||
if not isinstance(data, dict):
|
||||
return None
|
||||
answer = data.get("answer")
|
||||
confidence = data.get("confidence")
|
||||
if not isinstance(answer, str) or not answer.strip():
|
||||
return None
|
||||
if not isinstance(confidence, str) or confidence.lower() not in _CONFIDENCES:
|
||||
return None
|
||||
caveats = data.get("caveats")
|
||||
return ExpertAnswer(
|
||||
answer=answer.strip(),
|
||||
confidence=confidence.lower(),
|
||||
caveats=caveats.strip() if isinstance(caveats, str) else "",
|
||||
)
|
||||
@@ -27,7 +27,7 @@ from nisse_dev.canonical import (
|
||||
)
|
||||
from nisse_dev.oplog import OpLog
|
||||
|
||||
CURRENT_SCHEMA_VERSION = 2
|
||||
CURRENT_SCHEMA_VERSION = 3
|
||||
|
||||
Migration = Callable[[sqlite3.Connection], None]
|
||||
|
||||
@@ -41,12 +41,29 @@ CREATE TABLE canonical_events (
|
||||
"""
|
||||
|
||||
|
||||
_EVAL_OUTCOMES_DDL = """
|
||||
CREATE TABLE eval_outcomes (
|
||||
event_id INTEGER PRIMARY KEY,
|
||||
verdict TEXT NOT NULL,
|
||||
note TEXT NOT NULL DEFAULT '',
|
||||
marked_at TEXT NOT NULL
|
||||
)
|
||||
"""
|
||||
|
||||
|
||||
def _migrate_v1_add_canonical_events(conn: sqlite3.Connection) -> None:
|
||||
conn.execute(_CANONICAL_EVENTS_DDL)
|
||||
|
||||
|
||||
def _migrate_v2_add_eval_outcomes(conn: sqlite3.Connection) -> None:
|
||||
conn.execute(_EVAL_OUTCOMES_DDL)
|
||||
|
||||
|
||||
# Maps version N to the migration that takes the schema from N to N+1.
|
||||
MIGRATIONS: Mapping[int, Migration] = {1: _migrate_v1_add_canonical_events}
|
||||
MIGRATIONS: Mapping[int, Migration] = {
|
||||
1: _migrate_v1_add_canonical_events,
|
||||
2: _migrate_v2_add_eval_outcomes,
|
||||
}
|
||||
|
||||
|
||||
class StateSchemaError(RuntimeError):
|
||||
@@ -93,6 +110,7 @@ _SCHEMA = [
|
||||
)
|
||||
""",
|
||||
_CANONICAL_EVENTS_DDL,
|
||||
_EVAL_OUTCOMES_DDL,
|
||||
]
|
||||
|
||||
|
||||
@@ -294,6 +312,27 @@ class StateStore:
|
||||
for row in rows
|
||||
]
|
||||
|
||||
def mark_outcome(self, event_id: int, verdict: str, note: str = "") -> None:
|
||||
"""Record whether a delegation or consult was good (A16).
|
||||
|
||||
Kept out of the canonical log on purpose: the log is an append-only
|
||||
record of what happened, and a verdict is a later judgement about it
|
||||
that may be revised.
|
||||
"""
|
||||
self._conn.execute(
|
||||
"INSERT INTO eval_outcomes (event_id, verdict, note, marked_at) "
|
||||
"VALUES (?, ?, ?, ?) ON CONFLICT(event_id) DO UPDATE SET "
|
||||
"verdict = excluded.verdict, note = excluded.note, "
|
||||
"marked_at = excluded.marked_at",
|
||||
(event_id, verdict, note, _now()),
|
||||
)
|
||||
|
||||
def outcomes(self) -> dict[int, tuple[str, str]]:
|
||||
rows = self._conn.execute(
|
||||
"SELECT event_id, verdict, note FROM eval_outcomes"
|
||||
).fetchall()
|
||||
return {r["event_id"]: (r["verdict"], r["note"]) for r in rows}
|
||||
|
||||
def close(self) -> None:
|
||||
self._conn.close()
|
||||
|
||||
|
||||
Reference in new issue
Block a user