Skip to content

fix(agent): cap consecutive turns that call no tool and give no answer - #76

Open
xepozz wants to merge 2 commits into
wippyai:masterfrom
xepozz:fix/empty-step-cap-and-child-output-test
Open

fix(agent): cap consecutive turns that call no tool and give no answer#76
xepozz wants to merge 2 commits into
wippyai:masterfrom
xepozz:fix/empty-step-cap-and-child-output-test

Conversation

@xepozz

@xepozz xepozz commented Sep 6, 2026

Copy link
Copy Markdown

What was changed

  • The agent loop counts consecutive turns that call no tool and give no answer, and fails the node at DEFAULTS.MAX_UNPRODUCTIVE_STEPS (3) with its own lifecycle reason. Any productive turn resets the streak.
  • check_completion reports that condition as a flag of its own, separate from "I wrote a feedback observation".
  • child_output_test.lua pins which of a child's pinned data ids outputs_from_yield_results returns. No production change there — see below.

Why?

The cap. check_completion answers an unproductive turn with feedback and lets the loop ask again. Nothing counts those turns, so a model that keeps returning nothing is asked until max_iterations, one full model round trip each time. Measured on a deployment running 0.7.13: three empty turns inside one five-minute run. 0.7.16 tells EMPTY_RESULT apart from NO_TOOLS_CALLED, which made the turns legible, but did not bound them.

check_completion already computed whether it wrote feedback; finalize_iteration dropped that flag on the floor. Reusing it alone is not enough: tool_calling = "none" writes no observation at all, so a run whose every turn is unusable produces nothing to count and escapes the cap entirely. Hence two flags — feedback_recorded still drives the durable yield, unproductive drives the cap.

This narrows what a run may do: a model that ignores the nudge three times in a row and recovers on the fourth now fails instead. It does not change the outcome of a run that never recovers — that run already failed at max_iterations, because completion needs either a usable result or a tool call. It fails sooner and says why.

The streak lives in the loop, not in node metadata, so a recovered node restarts it. max_iterations still bounds the total; recovery only loosens the cap.

The child_output test, and the change that is not here. An earlier version of this branch removed the node.output filter from outputs_from_yield_results, reasoning that output_ids come out of the child's own result envelope and are therefore already pinned exactly. Measured against the suite, that is wrong twice over.

complete() pins every routed data_id, not only the terminal one, and a yield reports one result envelope per run node — so a mid-graph child pins the node.input row it fed to a sibling. Removing the filter folds those rows into the parent's observation. On sqlite it turned a green suite (993/993 at the parent commit) into 989/993: func child output collection, func control-command chaining, cycle template chains and a signal burst all broke.

The delegation case the removal was meant to fix is not reachable through that call site. delegation_handler's child is collected by collect_delegation_result, which reads the row by data_id with no type filter, and the flow compiler always assigns node.output to a nested child's terminal target (session_parent_id is set for both agent and func hosts). Adding agent.delegation to the filter would mean importing agent-node constants into dataflow core for a case no caller produces.

So the production change is dropped and the invariant is pinned by tests instead.

Checklist

  • Closes #
  • Tested
    • Tested manually
    • Unit tests added

Full wippy test --profile sqlite: 1002/1002.

  • check_completion_test — five cases pin the new flag across none / auto / any, a tool-calling turn, and a warm-up turn below min_iterations. All five fail without the change (expected true, got nil).
  • agent_empty_result_testempty_until_limit with max_iterations = 12 records 3 empty-result observations, not 12, and the node result carries result.error. Without the cap the same test reads expected 3, got 12. The existing limit case (max_iterations = 3) is unchanged and still green.
  • child_output_test — pinned output rows are returned, a pinned node.input row the child routed to a sibling is not, and an envelope that pins nothing falls back to a node scan. The middle case fails without the filter (expected 1, got 2).

make lint: 172 entries, 0 errors, 2 pre-existing warnings.

One flake seen and not caused here: Signal Stress Tests > 10 wrong signals then 1 correct failed on one run out of five and passed on the immediate re-run. It drives signal nodes only and touches nothing in this change.

…tor returns

The previous version of this branch removed the node.output filter from
outputs_from_yield_results, on the reasoning that output_ids come out of the
child's own result envelope and are already pinned exactly. Measured against
the suite, that is wrong twice over.

complete() pins every routed data_id, not only the terminal one, and a yield
reports one result envelope per run node -- so a mid-graph child pins the
node.input row it fed to a sibling. Removing the filter folds those rows into
the parent's observation. On sqlite the removal turned a green suite (993/993
at the parent commit) into 989/993: func child output collection, func control
command chaining, cycle template chains and a signal burst all broke.

The delegation case the removal was meant to fix is not reachable through this
call site: delegation_handler's child is collected by collect_delegation_result,
which reads the row by data_id with no type filter, and the flow compiler always
assigns node.output to a nested child's terminal target. Adding agent.delegation
to the filter would mean importing agent-node constants into dataflow core for a
case no caller produces.

So the production change is dropped and the invariant is pinned by tests
instead: pinned output rows are returned, a pinned sibling input is not, and an
envelope that pins nothing falls back to a node scan. The middle case fails
without the filter. 996/996 on sqlite.

The stub no longer leaks the data type between queries -- the earlier one did,
so its own assertion read the result envelope back instead of the answer row
and the test could not pass.

Follow-up, not in this branch: check_completion has no cap on consecutive empty
steps. 0.7.16 tells EMPTY_RESULT apart from NO_TOOLS_CALLED, but a model that
returns nothing is still asked again until max_iterations -- three empty turns
in one five-minute run measured on 0.7.13. That changes loop-termination
semantics and belongs in its own change.
check_completion answers an unproductive turn with feedback and lets the loop
ask again. Nothing counts those turns, so a model that keeps returning nothing
is asked until max_iterations, one full round trip each time. Measured on a
deployment running 0.7.13: three empty turns inside one five-minute run.

The loop now carries the streak of consecutive unproductive turns and fails the
node at DEFAULTS.MAX_UNPRODUCTIVE_STEPS (3) with its own lifecycle reason. Any
productive turn resets the streak.

check_completion already computed whether it wrote feedback; finalize_iteration
was dropping that flag. It now reports a second, separate flag: tool_calling=none
writes no observation at all, so a run whose every turn is unusable produced no
feedback to count and would have escaped the cap. feedback_recorded still drives
the durable yield; unproductive drives the cap.

This narrows what a run may do: a model that ignores the nudge three times in a
row and recovers on the fourth now fails instead. It does not change the outcome
of a run that never recovers -- that run already failed at max_iterations, since
completion needs either a usable result or a tool call. It fails sooner and says
why.

The streak lives in the loop, not in node metadata, so a recovered node restarts
it. max_iterations still bounds the total; recovery only loosens the cap.

Tests: five check_completion cases pin the flag across none/auto/any, a tool
turn and a warm-up turn below min_iterations. At loop level, empty_until_limit
with max_iterations = 12 now records 3 empty-result observations, not 12, and
the node result carries result.error. Every one of the six fails without this
change. 1002/1002 on sqlite.
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