fix: classifier robustness — schema in prompt, retries, kickoff fallback
The kickoff parse-and-confirm route was 500-ing intermittently because
Hermes-3 + Featherless's response_format={"type":"json_object"} only
guarantees JSON output, NOT a particular schema. The model was inventing
its own field names (sceneTime, entities, settingDetails) instead of
the KickoffParse fields, causing Pydantic validation to fail on both
classify() retries.
Three changes:
1. Include the Pydantic JSON schema in the system prompt so the model
knows exactly which keys to produce. Affects every classify() call
(kickoff parse, turn parse, scene-close detect, significance,
state-update, scene summarize). Strip ```json fences if the model
wraps its output. Bump retries 2 → 3 (model is stochastic; one extra
attempt closes most of the remaining gap).
2. parse_kickoff() now passes a default empty KickoffParse so the
route degrades to a fillable form instead of 500 when the classifier
ultimately fails. The confirm form is the human-in-the-loop; an
empty form is strictly better UX than a stack trace.
3. Tests updated: bumped canned-failure arrays from 2 → 3 entries to
match the new attempt count; renamed kickoff test from
"raises_when_classifier_fails_twice" to
"falls_back_to_empty_when_classifier_fails" reflecting the new
degraded-but-usable behavior.
Verified live with all 3 sample bots (maya/eli/sam) — kickoff route
returns 200 across multiple attempts. Full suite: 168 passed.
This commit is contained in:
+26
-5
@@ -10,6 +10,18 @@ T = TypeVar("T", bound=BaseModel)
|
|||||||
REFUSAL_PATTERNS = ("i can't", "i cannot", "i'm sorry, but", "as an ai")
|
REFUSAL_PATTERNS = ("i can't", "i cannot", "i'm sorry, but", "as an ai")
|
||||||
|
|
||||||
|
|
||||||
|
def _strip_json_fences(text: str) -> str:
|
||||||
|
"""Strip ```json ... ``` markdown fences if the model wraps its JSON output."""
|
||||||
|
s = text.strip()
|
||||||
|
if s.startswith("```"):
|
||||||
|
# Drop the first fence line (which may be ``` or ```json)
|
||||||
|
s = s.split("\n", 1)[1] if "\n" in s else s[3:]
|
||||||
|
# Drop the trailing fence
|
||||||
|
if s.rstrip().endswith("```"):
|
||||||
|
s = s.rstrip()[:-3]
|
||||||
|
return s.strip()
|
||||||
|
|
||||||
|
|
||||||
async def classify(
|
async def classify(
|
||||||
client: LLMClient,
|
client: LLMClient,
|
||||||
*,
|
*,
|
||||||
@@ -20,21 +32,30 @@ async def classify(
|
|||||||
default: T | None = None,
|
default: T | None = None,
|
||||||
timeout_s: float = 10.0,
|
timeout_s: float = 10.0,
|
||||||
) -> T:
|
) -> T:
|
||||||
|
schema_json = json.dumps(schema.model_json_schema(), indent=2)
|
||||||
|
schema_block = (
|
||||||
|
f"\n\nRespond with a single JSON object matching this exact schema. "
|
||||||
|
f"Use these field names exactly; do not invent your own keys:\n```json\n{schema_json}\n```"
|
||||||
|
)
|
||||||
msgs = [
|
msgs = [
|
||||||
Message(role="system", content=system + "\n\nRespond with JSON only matching the schema."),
|
Message(role="system", content=system + schema_block),
|
||||||
Message(role="user", content=user),
|
Message(role="user", content=user),
|
||||||
]
|
]
|
||||||
for attempt in range(2):
|
for attempt in range(3):
|
||||||
try:
|
try:
|
||||||
text = await asyncio.wait_for(
|
text = await asyncio.wait_for(
|
||||||
client.generate(msgs, model=model, response_format={"type": "json_object"}),
|
client.generate(msgs, model=model, response_format={"type": "json_object"}),
|
||||||
timeout=timeout_s,
|
timeout=timeout_s,
|
||||||
)
|
)
|
||||||
if any(p in text.lower()[:80] for p in REFUSAL_PATTERNS) and not text.strip().startswith("{"):
|
cleaned = _strip_json_fences(text)
|
||||||
|
if any(p in cleaned.lower()[:80] for p in REFUSAL_PATTERNS) and not cleaned.lstrip().startswith("{"):
|
||||||
raise ValueError("refusal-shaped response")
|
raise ValueError("refusal-shaped response")
|
||||||
return schema.model_validate_json(text)
|
return schema.model_validate_json(cleaned)
|
||||||
except (ValidationError, ValueError, json.JSONDecodeError, asyncio.TimeoutError):
|
except (ValidationError, ValueError, json.JSONDecodeError, asyncio.TimeoutError):
|
||||||
msgs[0] = Message(role="system", content=system + "\n\nRespond with valid JSON ONLY. No prose.")
|
msgs[0] = Message(
|
||||||
|
role="system",
|
||||||
|
content=system + schema_block + "\n\nRespond with valid JSON ONLY. No prose, no markdown fences.",
|
||||||
|
)
|
||||||
continue
|
continue
|
||||||
if default is None:
|
if default is None:
|
||||||
raise RuntimeError(f"classify failed for schema {schema.__name__} with no default")
|
raise RuntimeError(f"classify failed for schema {schema.__name__} with no default")
|
||||||
|
|||||||
@@ -85,6 +85,36 @@ def _build_user_prompt(
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _empty_activity() -> ActivityShape:
|
||||||
|
return ActivityShape(
|
||||||
|
posture="",
|
||||||
|
action_verb="",
|
||||||
|
action_interruptible=True,
|
||||||
|
action_required_attention="low",
|
||||||
|
action_expected_duration="brief",
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _empty_kickoff_parse() -> KickoffParse:
|
||||||
|
"""Default returned when the classifier can't produce a valid parse.
|
||||||
|
|
||||||
|
The user gets a mostly-empty confirm form they can fill in by hand
|
||||||
|
instead of a 500. ``initial_time_iso`` is left as the current UTC.
|
||||||
|
"""
|
||||||
|
from datetime import datetime, timezone
|
||||||
|
|
||||||
|
return KickoffParse(
|
||||||
|
container_name="",
|
||||||
|
container_type="",
|
||||||
|
container_properties={},
|
||||||
|
you_activity=_empty_activity(),
|
||||||
|
bot_activity=_empty_activity(),
|
||||||
|
initial_time_iso=datetime.now(timezone.utc).isoformat(timespec="seconds"),
|
||||||
|
edge_seed_summary="",
|
||||||
|
edge_seed_knowledge_facts=[],
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
async def parse_kickoff(
|
async def parse_kickoff(
|
||||||
client: LLMClient,
|
client: LLMClient,
|
||||||
*,
|
*,
|
||||||
@@ -98,11 +128,9 @@ async def parse_kickoff(
|
|||||||
) -> KickoffParse:
|
) -> KickoffParse:
|
||||||
"""Parse authored kickoff prose into a structured ``KickoffParse``.
|
"""Parse authored kickoff prose into a structured ``KickoffParse``.
|
||||||
|
|
||||||
Internally calls :func:`chat.llm.classify.classify` with a labeled
|
Falls back to a mostly-empty default if the classifier fails — the
|
||||||
user prompt. Raises ``RuntimeError`` if the classifier fails twice in
|
confirm-and-edit form is the human-in-the-loop, so a degraded form
|
||||||
a row — no default is supplied at this layer, since the caller (T13's
|
that the user can fill in is preferable to a 500.
|
||||||
confirm form) is responsible for showing an error and letting the
|
|
||||||
user edit.
|
|
||||||
"""
|
"""
|
||||||
user_prompt = _build_user_prompt(
|
user_prompt = _build_user_prompt(
|
||||||
bot_name=bot_name,
|
bot_name=bot_name,
|
||||||
@@ -117,5 +145,6 @@ async def parse_kickoff(
|
|||||||
system=_SYSTEM_PROMPT,
|
system=_SYSTEM_PROMPT,
|
||||||
user=user_prompt,
|
user=user_prompt,
|
||||||
schema=KickoffParse,
|
schema=KickoffParse,
|
||||||
|
default=_empty_kickoff_parse(),
|
||||||
timeout_s=timeout_s,
|
timeout_s=timeout_s,
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -18,7 +18,7 @@ async def test_classify_parses_valid_json():
|
|||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_classify_falls_back_on_unparseable_after_retry():
|
async def test_classify_falls_back_on_unparseable_after_retry():
|
||||||
mock = MockLLMClient(canned=["nope", "still nope"])
|
mock = MockLLMClient(canned=["nope", "still nope", "nope3"])
|
||||||
default = Verdict(score=1, reason="fallback")
|
default = Verdict(score=1, reason="fallback")
|
||||||
result = await classify(mock, model="m", system="x", user="y", schema=Verdict, default=default)
|
result = await classify(mock, model="m", system="x", user="y", schema=Verdict, default=default)
|
||||||
assert result.reason == "fallback"
|
assert result.reason == "fallback"
|
||||||
|
|||||||
+23
-12
@@ -117,15 +117,26 @@ async def test_parse_kickoff_applies_activity_defaults_for_missing_fields():
|
|||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_parse_kickoff_raises_when_classifier_fails_twice():
|
async def test_parse_kickoff_falls_back_to_empty_when_classifier_fails():
|
||||||
mock = MockLLMClient(canned=["nope", "still nope"])
|
"""When the classifier fails three times, return an empty KickoffParse
|
||||||
with pytest.raises(RuntimeError):
|
instead of raising — the confirm form lets the user fill in by hand.
|
||||||
await parse_kickoff(
|
"""
|
||||||
mock,
|
mock = MockLLMClient(canned=["nope", "still nope", "still bad"])
|
||||||
model="m",
|
result = await parse_kickoff(
|
||||||
bot_name="BotA",
|
mock,
|
||||||
bot_persona="x",
|
model="m",
|
||||||
initial_relationship_to_you="y",
|
bot_name="BotA",
|
||||||
kickoff_prose="z",
|
bot_persona="x",
|
||||||
you_name="You",
|
initial_relationship_to_you="y",
|
||||||
)
|
kickoff_prose="z",
|
||||||
|
you_name="You",
|
||||||
|
)
|
||||||
|
assert isinstance(result, KickoffParse)
|
||||||
|
assert result.container_name == ""
|
||||||
|
assert result.container_type == ""
|
||||||
|
assert result.edge_seed_summary == ""
|
||||||
|
assert result.edge_seed_knowledge_facts == []
|
||||||
|
# Activity defaults sane (action_interruptible defaults to True so the
|
||||||
|
# confirm form's checkbox is in a reasonable initial state).
|
||||||
|
assert result.you_activity.action_interruptible is True
|
||||||
|
assert result.bot_activity.action_interruptible is True
|
||||||
|
|||||||
@@ -84,7 +84,7 @@ async def test_summarize_scene_default_on_failure():
|
|||||||
"""Two consecutive non-JSON returns trip the classifier's retry-then-default
|
"""Two consecutive non-JSON returns trip the classifier's retry-then-default
|
||||||
path; we should get the empty fallback rather than crashing the close
|
path; we should get the empty fallback rather than crashing the close
|
||||||
flow."""
|
flow."""
|
||||||
mock = MockLLMClient(canned=["bad", "still bad"])
|
mock = MockLLMClient(canned=["bad", "still bad", "bad3"])
|
||||||
result = await summarize_scene(
|
result = await summarize_scene(
|
||||||
mock,
|
mock,
|
||||||
model="x",
|
model="x",
|
||||||
|
|||||||
@@ -58,7 +58,7 @@ async def test_detect_scene_close_default_on_failure():
|
|||||||
"""Two consecutive non-JSON returns trip the classifier's retry-then-default
|
"""Two consecutive non-JSON returns trip the classifier's retry-then-default
|
||||||
path; we should get the safe ``should_close=False`` fallback rather than
|
path; we should get the safe ``should_close=False`` fallback rather than
|
||||||
crashing the turn flow."""
|
crashing the turn flow."""
|
||||||
mock = MockLLMClient(canned=["nope", "still nope"])
|
mock = MockLLMClient(canned=["nope", "still nope", "nope3"])
|
||||||
decision = await detect_scene_close(
|
decision = await detect_scene_close(
|
||||||
mock,
|
mock,
|
||||||
model="x",
|
model="x",
|
||||||
|
|||||||
@@ -45,7 +45,7 @@ async def test_compute_significance_parses_score():
|
|||||||
async def test_compute_significance_default_on_failure():
|
async def test_compute_significance_default_on_failure():
|
||||||
# Both attempts return non-JSON text; the classify wrapper falls back
|
# Both attempts return non-JSON text; the classify wrapper falls back
|
||||||
# to the SignificanceVerdict default (score=1, "fallback").
|
# to the SignificanceVerdict default (score=1, "fallback").
|
||||||
mock = MockLLMClient(canned=["nope", "still nope"])
|
mock = MockLLMClient(canned=["nope", "still nope", "nope3"])
|
||||||
score = await compute_significance(
|
score = await compute_significance(
|
||||||
mock,
|
mock,
|
||||||
model="x",
|
model="x",
|
||||||
|
|||||||
@@ -62,7 +62,7 @@ async def test_compute_state_update_parses_classifier_output():
|
|||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_compute_state_update_returns_default_on_failure():
|
async def test_compute_state_update_returns_default_on_failure():
|
||||||
"""Two malformed classifier responses -> default StateUpdate (zeros)."""
|
"""Two malformed classifier responses -> default StateUpdate (zeros)."""
|
||||||
mock = MockLLMClient(canned=["nope", "still nope"])
|
mock = MockLLMClient(canned=["nope", "still nope", "nope3"])
|
||||||
result = await compute_state_update(
|
result = await compute_state_update(
|
||||||
mock,
|
mock,
|
||||||
model="x",
|
model="x",
|
||||||
|
|||||||
@@ -74,7 +74,7 @@ async def test_parse_turn_empty_prose_short_circuits_without_classifier_call():
|
|||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_parse_turn_raises_when_classifier_fails_twice():
|
async def test_parse_turn_raises_when_classifier_fails_twice():
|
||||||
mock = MockLLMClient(canned=["nope", "still nope"])
|
mock = MockLLMClient(canned=["nope", "still nope", "nope3"])
|
||||||
with pytest.raises(RuntimeError):
|
with pytest.raises(RuntimeError):
|
||||||
await parse_turn(
|
await parse_turn(
|
||||||
mock,
|
mock,
|
||||||
|
|||||||
Reference in New Issue
Block a user