fix: report an unparseable action as FAIL, not as a completed task

[abrichr]

The defect

ActionType.DONE is a successful terminal outcome — a runner that sees it ends the episode as complete. Every parse failure and every conversion failure in this package produced exactly that value. So "the model said it finished the task" and "we could not read the model's output at all" were the same Action.

This is the canonical shape the sweep was hunting: a failure rendered as a successful result. ActionType.FAIL already existed in the enum for exactly this case and was never used.

1. openadapt_types/parsing.py — all 19 failure paths

The _done() helper backing every failure return logged a warning and then returned Action(type=ActionType.DONE). The module docstring documented this as intended behaviour, which made it worse: a logged failure returning a success-shaped value is the same bug with better documentation.

Now _failed() returns Action(type=ActionType.FAIL) carrying the reason in both reasoning and raw[PARSE_ERROR_KEY].

This also repairs a second-order bug in parse_action, which used result.type != ActionType.DONE as its own success test:

result = parse_action_json(text)
if result.type != ActionType.DONE:   # <- a legitimate {"type": "done"} fails this
    return result
dsl_result = parse_action_dsl(text)  # ...so a valid DONE was re-parsed as DSL

A model that genuinely emitted {"type": "done"} was treated as a parse failure and sent down the DSL path. With FAIL as the failure marker, the check is now correct and unambiguous.

2. openadapt_types/_compat.py — the converters were worse

action_type_str = act.get("type", "done")   # missing type -> "done"
try:
    action_type = ActionType(action_type_str)
except ValueError:
    action_type = ActionType.DONE            # unknown type -> DONE, no log at all

Present in from_benchmark_action and from_ml_action; from_omnimcp_action_decision did the same via type_map.get(action_type_str, ActionType.DONE). A recorded step with a missing or future action type became a successful task completion in converted training/eval data, silently. All three now return FAIL with raw[UNCONVERTIBLE_ACTION_KEY] and the source dict. A real omnimcp "finish" still converts to DONE.

3. ComputerState.get_children — [] meant two things

get_children("does_not_exist") returned [], identical to a node that genuinely has no children. A caller with a stale or mistyped node id observed "this element has no children" instead of "this element is not here". An unknown node now raises KeyError. get_node is unchanged — it documents None for not-found and its callers handle it.

4. __version__ was a stale literal

__version__ = "0.1.0" against a package on 0.6.3. Now read from installed distribution metadata; an unmeasurable version reports unknown rather than a confident wrong number.

Test changes

16 existing tests asserted the DONE-on-failure behaviour — they enshrined the defect and are updated to assert FAIL. Because they now expect FAIL, they fail against pre-fix code, which is itself the mutation proof. tests/test_no_false_success.py adds the direct regressions.

Mutation checks

Each production change was reverted individually and the matching test confirmed to fail:

Reverted	Failing tests	Failure
_failed() -> return Action(type=ActionType.DONE)	11 in TestParseFailureIsNotCompletion	assert <ActionType.DONE: 'done'> is <ActionType.FAIL: 'fail'>
_compat.py DONE fallbacks	6 in TestCompatConversionFailureIsNotCompletion	assert <ActionType.DONE: 'done'> is <ActionType.FAIL: 'fail'>
get_children -> return []	test_children_of_an_unknown_node_raises	Failed: DID NOT RAISE <class 'KeyError'>
__version__ = "0.1.0"	test_reported_version_matches_installed_distribution	AssertionError: assert '0.1.0' == '0.6.3'

One test in the new file, test_parse_action_does_not_retry_a_successful_done_as_dsl, passes against pre-fix code — the pre-fix retry produced the same value by a wasteful route. It is a contract test, not a regression proof, and is labelled as such here rather than being presented as evidence.

Verification

uv sync --locked --extra dev && uv run pytest -> 206 passed. uv.lock untouched.

Candidates examined and dismissed

• control_overlay_tracking.py tracking_for_observation / event_for_media_frame / tracking_for_media_frame returning None (5 sites) — deliberate and correct. These implement the AGENTS.md rule to omit a target rectangle rather than reconstruct one when the exact binding is absent, and they use compare_digest for the identity check. None here means "no verified binding", which is precisely the representable distinction this sweep asks for.
• HumanDecisionReceiptV1.verify_hmac returning False for an unsigned receipt — documented ("An unsigned receipt never verifies") and the safe direction; an unsigned receipt genuinely does not verify.
• _safe_float returning None — an internal helper whose every call site checks the result and routes to a failure return.
• ComputerState.get_node returning None — documented, and its callers all branch on it.
• scripts/verify_release_lock.py return 0 — reached only after verify_release_lock() has raised or passed; the failure path calls parser.exit(1, ...).
• _make_target returning None when every field is absent — a genuine "no target specified", not a failed computation.

Fake-gate inventory

.github/workflows/ contains no || true, continue-on-error, exit 0, set +e, or swallowed step. test.yml runs uv sync --locked && uv run pytest and fails properly; --locked is a real gate. One observation, not fixed here: there is no lint job in CI for this repo, only pytest.