fix(embodied): emit canonical spatial error_types + fix ok=false mitigation path - #19
Open
Pablomonte wants to merge 1 commit into
Conversation
…on ok=false mitigation path
Two related bugs were keeping the Tier 2a auto-recovery path dead in
practice:
1. **Lost specificity in `foldBotResponse`** (`lib/dispatcher.js`).
The bot throws rich error strings such as `"target space is occupied
by stone..."` or `"no solid adjacent block to place against..."`, but
`foldBotResponse` collapsed every non-200 response to the generic
`error_type: "bot_action_failed"` and every soft-failure to
`bot_soft_failure`. The runner's Tier 2a check
(`local_agent/embodied.py:SPATIAL_ERRORS = {"target_occupied",
"bot_in_target", "no_solid_neighbor"}`) and the captain tool's
equivalent retry block (`hermes-agent/tools/embodied_plan_tool.py`)
are both keyed on those canonical names, so they never fired and
recovery fell entirely to the SOUL pattern-matching catalog one turn
later.
Fix: introduce a conservative `classifyBotError(details)` that
promotes the bot's free-text diagnostic into one of those three
canonical error_types when a known pattern matches. Anything else
falls through to the generic types — no behavioral change for soft
failures we don't classify (partial mine, can't see, etc.) or for
unrelated hard failures (path timeouts, etc.).
2. **`ok=false` mitigation path missing `error` key** (`index.js`).
When Ollama returned an empty raw response the service synthesized a
`raise_guardian_event` mitigation and returned `ok: false` with a
populated `plan` + `execution_results` + `mitigations` block but
*without* a top-level `error` object. Downstream consumers
(`embodied_plan_tool.py`) read `result["error"]` first and fell
through to literal `"unknown error"` defaults — visible in real
captain sessions as `error_type: "other", details: "unknown error"`,
which blinds the SOUL recovery catalog.
Fix: populate `error: {error_type: "empty_model_response", details:
"..."}` alongside the existing observability blocks. No shape change
for callers that already used `execution_results`; new callers (and
legacy ones reading `error`) now see a structured signal.
Tests added to `test/dispatcher.test.js`:
- `classifyBotError`: 3 positive (target_occupied / bot_in_target /
no_solid_neighbor) + 2 negatives (unrelated strings, empty/invalid
input) — 5 cases.
- `foldBotResponse with spatial classifier`: end-to-end through
`foldBotResponse` for the same 3 canonical promotions + 1
fall-through regression check.
Existing suite was 59 tests; now 68. All pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pablomonte
added a commit
to Pablomonte/DaemonCraft
that referenced
this pull request
May 17, 2026
… player intent The previous §6 promised three canonical error_types to the captain LLM: "target_occupied", "no_solid_neighbor", "bot_in_target". These are referenced in agents/local_agent/embodied.py (SPATIAL_ERRORS set) and used to gate Tier 2a auto-retry. But the embodied service dispatcher (foldBotResponse in lib/dispatcher.js) collapsed ALL bot errors to a generic `error_type: "bot_action_failed"`. The canonical labels were never emitted, so the captain's recovery path keyed on them was dead. The bug is preexisting upstream — neither the dispatcher nor the bot re-classified placement failure strings into canonical error_types, so the recovery contract the SOUL described did not match runtime reality. This commit closes the gap at the SOUL layer (lowest blast radius — zero code change, deploys via profile sync). Two things change: 1. **Pattern-match `details` rather than `error_type`.** The captain is taught to read the bot's free-text diagnostic string against a 9-row catalog of real bot phrases. Each pattern was verified against the actual error throw site in agents/bot/server.js. 2. **Spatial recovery: think before relocating.** The first iteration of this SOUL listed "place at adjacent cell" as the default response to `target_occupied`. Field test exposed the flaw: the original coordinate often carried player intent (sealing a hole, hitting a named cell, completing a structure). Silently placing one cell over is a worse failure than refusing — it leaves the player with an incorrect world AND the false belief the task succeeded. New shape: §6 starts with a "pause and decide" checklist asking whether the cell was load-bearing, approximate, or just blocked by the body. The table lists OPTIONS, not prescriptions. Default tie-breaker is "ask the player" via a short chat message. Pattern table (each row anchored in a real bot string): | pattern in details | source | |-----------------------------------|--------------------------------------| | target space is occupied | server.js:2363 (place) | | inside my own body / footprint | server.js:2354 (place — PR nicoechaniz#17) | | no solid adjacent block / against | server.js:2407 + 2358 (place) | | did not materialize | server.js:2343 (place) | | No {item} in inventory | server.js:2227 + 2317 (equip/place) | | crafting_table nearby | server.js:2163, 2169 (craft_item) | | Mined K/N | dispatcher.js:detectSoftFailure | | Can't ... / Failed to ... | bot soft-failure prefix | | timeout | generic | §1 worked example also rewritten: instead of silently relocating to an adjacent cell, the captain now thinks aloud about whether relocation serves the player's intent — and asks if it doesn't. Companion PR with the dispatcher-side fix that ALSO emits canonical error_types (so SOUL pattern matching + canonical error_types both work, defense-in-depth): nicoechaniz#19 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two related bugs were silently breaking the Tier 2a auto-recovery path in the canonical-loop runner and the captain tool.
Bug 1 —
foldBotResponselost specificityThe bot throws rich error strings like
target space is occupied by stone...,no solid adjacent block to place against..., orthat cell is inside my own body....lib/dispatcher.js:foldBotResponsecollapsed all of them to the genericerror_type: \"bot_action_failed\"(orbot_soft_failure). The runner's Tier 2a check is keyed on the canonical names:agents/local_agent/embodied.py:82—SPATIAL_ERRORS = {\"target_occupied\", \"bot_in_target\", \"no_solid_neighbor\"}hermes-agent/tools/embodied_plan_tool.pyretry block — same setNeither ever fired. Recovery fell to the SOUL pattern-matching catalog one full turn later, costing tokens and latency.
Fix: added a conservative
classifyBotError(details)that promotes the bot's free-text to one of those three canonical types when a known regex matches. Everything else falls through unchanged — no behavior shift for partial mines, can't-see warnings, refusal messages, or unrelated hard failures.Bug 2 —
ok=falsemitigation path missingerrorkeyWhen Ollama returned an empty raw response,
index.jssynthesized araise_guardian_eventmitigation and returnedok: falsewithplan+execution_results+mitigationspopulated but without a top-levelerrorobject. The captain's tool (embodied_plan_tool.py) readsresult[\"error\"]first; when missing, it fell through to literal\"unknown error\"defaults — visible in real captain sessions aserror_type: \"other\", details: \"unknown error\", which blinds the SOUL recovery catalog and forces blind retries.Fix: populate
error: {error_type: \"empty_model_response\", details: \"...\"}alongside the existing observability blocks. Additive — no shape change for callers that already usedexecution_results.Companion PRs
execution_resultswhenerroris missing as a defense-in-depth): https://github.com/nicoechaniz/hermes-agent/pull/These three together close the loop: dispatcher emits canonical types → runner Tier 2a fires automatically → captain SOUL still has the pattern catalog as graceful fallback for everything else.
Test plan
node --test agents/embodied-service/test/— was 59 tests, now 68 (added 9 forclassifyBotError+ foldBotResponse classifier integration). All pass.error_type: \"target_occupied\"(notbot_action_failed), and the runner emits a Tier 2a auto-retry log line BEFORE the captain's next turn.error_type: \"empty_model_response\"in the captain's view (not the legacy\"other\" / \"unknown error\").🤖 Generated with Claude Code