Skip to content

Close the action contract: one registry, no prose fallback (#241 follow-up) - #441

Merged
jeremymanning merged 1 commit into
mainfrom
fix/action-contract-closure
Aug 1, 2026
Merged

Close the action contract: one registry, no prose fallback (#241 follow-up)#441
jeremymanning merged 1 commit into
mainfrom
fix/action-contract-closure

Conversation

@jeremymanning

Copy link
Copy Markdown
Member

Narrowly bounded follow-up to #241, as requested, before the Result API.

What was measured first

A census of 130 pipeline documents, counting only steps without a tool: key — steps with one route by tool and never touch the action vocabulary:

steps
dispatched by action name 430
already a registered action 247 (57%)
relying on the prose → model fallback 167 (53 distinct)
<AUTO> instructions 14
declared prose families (echo …) 2

My first census was wrong and counted write/execute/read as prose — they are tool operations in the two-field form. The numbers above are from parsed steps, not raw action: lines.

Among the 167: generate-text, used six times, had never dispatched. A hyphen slip of generate_text — the same defect class as generate_structured in #437, which is exactly the drift this PR closes.

The registry

ActionSpec carries canonical name, aliases, handler, whether a model or tool is required, required parameters, result schema and v1 membership. Everything else is derived:

Consumer Derived as
Executor dispatch resolve_action(...).handler
Validator recognition is_known_action(...)
supported_actions SUPPORTED_ACTIONS
Alias normalisation canonical_action(...), applied by the compiler
Documentation docs/actions.md, generated + drift-tested

The ten advertised-but-unimplemented names are gone. Anything advertised is now executable, with a test asserting exactly that.

Unknown actions are refused twice

At compile time, and again at dispatch — a caller can build a Task and reach execution without going through YAML, so compile-time validation is not the only boundary. Both are tested.

<AUTO>...</AUTO> stays supported and is now a declared ActionFamily rather than an accident of the fallback: an author explicitly asking the model to interpret an instruction is not the same as a typo falling through. echo … and the prose filesystem forms are declared the same way; their regexes moved into the registry and the two now-unused predicate methods are deleted.

Three real bugs fell out of writing the tests

  1. Registered actions handled one level down (generate_structured lives on ModelBasedControlSystem) hit the new raise instead of delegating.
  2. The filesystem handler keys off a hardcoded list containing "file" but not "filesystem". Normalising the alias sent canonical steps past that branch into a default that tried to write to the read-only root.
  3. Every failed step recorded the error handler's recovery decision{"action": "retry", "delay": 5.0, ...} — as its error. The reason a step failed was being discarded system-wide, and Task.error was not even an exception. Now the original error is recorded with the recovery decision beside it, and a compile failure leads with the actual messages instead of Issues: 1 errors.

Number 3 is the one worth noting for the Result API: structured errors were not merely missing, they were actively overwritten.

Also in here

  • A pre-existing SyntaxError in scripts/utilities/organization_maintenance.pyextend([ closed with ]. That file could not be imported at all. Found by extending the correctness lint to scripts/.
  • The cleanliness guard now catches untracked files, per your point 3: git status --porcelain --untracked-files=all, since git diff cannot see newly created ones. legacy-results.txt is excluded as the single expected artifact.
  • The exit-code boundary is documented, per your point 4: compile-time failures exit 2; runtime template and execution failures exit 1, with the reasoning about multi-pass resolution written down.
  • tests/test_builtin_actions.py folded into tests/test_action_contract.py (53 tests); its assertions were subsumed once the registry became authoritative.

Numbers

before after
Blocking suite 456 passed 476 passed, 13 skipped, 0 failed, 0 xfailed
Collection 3251 3271
Examples validating 3 / 111 3 / 111 — tightening cost no valid pipeline
ruff correctness (src + scripts) 1 SyntaxError clean
uv lock --check, git diff --check, ci.yml parse clean

Deliberately not here

Parameter schemas, only required-parameter lists. ActionSpec has the field, and the validator checks required names, but full JSON Schema per action means deciding the parameter vocabulary for every action — that belongs with the canonical YAML schema work, not smuggled into a closure PR.

Result schemas are declared for generate and generate_structured only — the two whose envelope shape is actually fixed today. Asserting a schema for handlers that return ad-hoc dicts would mean inventing their contracts here rather than in the Result API, where the per-step envelope gets designed once.

…ow-up)

The vocabulary had three disagreeing definitions. #241 reconciled the
first pair -- the executor's dispatch chain and the validator -- but left:

  - ModelBasedControlSystem.supported_actions, advertising transform,
    search, extract, filter, synthesize, create, optimize, review, write
    and compile, none of which had a handler anywhere;
  - an implicit fallback turning ANY unrecognised action into a prompt,
    so `action: gernate` was not an error. It became a model call and
    reported success.

core/actions.py is now a registry of ActionSpec objects carrying
canonical name, aliases, handler, whether a model or tool is required,
required parameters, result schema and v1 membership. Dispatch,
validator recognition, supported_actions, alias normalisation and the
documentation are all *derived* from it. None is maintained by hand, so
none can drift.

Unknown actions are refused twice: at compile time, and again at
dispatch, because a caller can build a Task and reach execution without
going through YAML at all. <AUTO>...</AUTO> stays supported -- an author
explicitly asking the model to interpret an instruction is not the same
as a typo falling through -- and is now a declared ActionFamily rather
than an accident of the fallback. `echo ...` and the prose filesystem
forms are declared the same way; their regexes moved into the registry
and the two now-unused predicate methods are gone.

Aliases are normalised by the compiler with a DeprecationWarning, so one
spelling reaches the task graph and the trace. This exposed
`generate-text`, a hyphen slip of `generate_text` used six times in the
examples that had never dispatched -- the same defect class as
`generate_structured` in #437.

Three real bugs fell out of writing the tests:

  - Registered actions whose handler lives one level down (i.e.
    generate_structured) hit the new raise instead of delegating.
  - The filesystem handler keys off a hardcoded list containing "file"
    but not "filesystem", so normalising the alias sent canonical steps
    down a default path that tried to write to the read-only root.
  - Every failed step recorded the error handler's *recovery decision*
    -- {"action": "retry", "delay": 5.0, ...} -- as its error, so the
    reason a step failed was discarded system-wide and Task.error was
    not even an exception. The original error is now recorded, with the
    recovery decision kept beside it, and a compile failure leads with
    the actual messages rather than "Issues: 1 errors".

Also fixed: a pre-existing SyntaxError in
scripts/utilities/organization_maintenance.py -- `extend([` closed with
`]` -- found by extending the correctness lint to scripts/. That file
could not be imported at all.

The CI cleanliness guard now uses `git status --porcelain
--untracked-files=all`, since `git diff` cannot see newly created files;
legacy-results.txt is excluded as the one expected artifact.

docs/actions.md is generated by scripts/generate_action_docs.py and
drift-tested, and documents the exit-code boundary: compile-time
failures exit 2, runtime template and execution failures exit 1.

tests/test_builtin_actions.py is folded into tests/test_action_contract.py
(53 tests), whose assertions subsumed it once the registry became
authoritative.

Blocking suite 456 -> 476 passed, 13 skipped, 0 failed, 0 xfailed.
Collection 3251 -> 3271. Examples validating unchanged at 3 of 111, so
tightening the vocabulary cost no currently-valid pipeline.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jeremymanning

Copy link
Copy Markdown
Member Author

CI 9/9 green, and the legacy backlog is unchanged — exactly, not approximately.

main @ 18d18c0 this PR
failed 475 475
passed 1845 1845
skipped 200 200
errors 189 189
deselected 555 575

Every count identical. The only movement is deselected +20 — the new contract-marked tests, which the legacy selection excludes by construction.

Notably zero UnknownActionErrors appear anywhere in the legacy failure list, so removing the prose fallback did not break a single legacy test that was previously passing or failing for another reason.

How this number was obtained, because the last two attempts were wrong

First I inferred main's baseline (474/1846/189) by projecting #440's effect onto #439's numbers. That produced an apparent +1 failed / −1 passed regression, and I went looking for a test that had broken. It had not — the inference was simply wrong.

gh run view --log returned nothing for main's legacy job, so log scraping was not available either. The reliable source turned out to be the workflow annotation:

gh api repos/OWNER/REPO/check-runs/<job-id>/annotations --jq '.[] | .message'
475 failed, 1845 passed, 200 skipped, 555 deselected, 9 warnings, 189 errors in 176.50s

The job publishes its tally as an annotation precisely so the backlog stays visible past continue-on-error, and that annotation is queryable per job. That is the route to use for future comparisons — not inference, and not log scraping.

Unrelated, but visible in the same annotations

Node.js 20 is deprecated. The following actions target Node.js 20 but are being
forced to run on Node.js 24: actions/checkout@v4, actions/setup-python@v5,
actions/upload-artifact@v4, astral-sh/setup-uv@v5

Four actions need bumping. Not touched here — it belongs with the release-engineering item that also wants actions pinned by commit SHA.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant