Ask the environment which outputs are reasoning steps

[12yuens2]

Closes #31.

The defect

_solver_stuck (mas.py:136) decides whether to summon the ground-truth agent. One of its three arms tested "think" in action — the embodied datasets' vocabulary, where a reasoning step is the command think:. FEVER and HotpotQA are ReAct and spell it Thought N:, and "think" in "Thought 1: ..." is False — "Thought" contains "hough". So on both Wikipedia datasets that arm never fired.

The other two arms didn't cover it: identical needs the same string three times, which three thoughts about three different hops are not; similar needs all three pairwise SequenceMatcher ratios above 0.8, and three plausible consecutive ReAct thoughts measure 0.35, 0.46, 0.51.

Why it matters here, having argued in #31 that it might not

I raised the possibility that the rule shouldn't fire on ReAct, since thought-then-action is the protocol rather than a stuck agent. On checking, that's wrong, and the reason is stop_strs=['\n'] — shared by all three workflows. The solver emits one line per turn, so a healthy ReAct rhythm is strictly thought, action, thought, action. Three thoughts running means no action reached the environment for three of the episode's trials. And a thought costs a trial exactly as an action does: the loop is for i in range(env.max_trials) and sets trials = i + 1 unconditionally.

So the arm should fire, and the failure was silent — the episode ran out of its 30 and was recorded unsolved, indistinguishable from a task the agent genuinely couldn't do.

There's a second path into it that makes this worse. A solver that puts both on one line —

Thought 1: I should search Milhouse. Action 1: Search[Milhouse]

— is classified a thought, so the Search never reaches the environment. Repeated, that burns every trial. Verified:

processed: 'Thought 1: I should search Milhouse. Action 1: Search[Milhouse]'
step     : ('OK.', 0, False)

Previously that ran silently to the trial budget. Now the ground-truth agent gets called on the third one.

The fix

The root cause isn't the spelling — it's that the workflow re-implemented a classification each environment already owned. There were four spellings of it in the tree, plus the workflow's own fifth:

	rule
ALFWorld	'think:' in action
ScienceWorld	'think' in action
PDDL	'think' in action
FEVER / HotpotQA	'thought' in action.lower()
_solver_stuck	"think" in action ← wrong for one family

So: is_thought on the Env protocol and @abstractmethod on BaseEnv, each environment keeping the rule it already applied, and each environment's step now reading its own classifier instead of an inline literal — one source per env, so the two can't drift again. _solver_stuck asks the env.

No behaviour change for ALFWorld, ScienceWorld or PDDL: same rules, same call sites.

Also renamed double_think → repeated_thought. It required three consecutive steps, not the two the name and docstring both claimed.

Tests

549 passing (was 540). Written before the fix and confirmed red; the fix turned them green.

Mutant	Caught by
workflow matches the substring itself again	test_detects_a_react_thought_loop
Wikipedia env stops recognising its own thoughts	test_every_env_knows_a_reasoning_step_from_an_action[fever, hotpotqa]
an environment omits the method entirely	same, [pddl]
step bypasses the classifier	test_a_reasoning_step_costs_a_trial_but_reaches_no_action

test_every_env_knows_a_reasoning_step_from_an_action is parametrized over ENVS, so a new dataset is covered by being registered — and it asserts against that dataset's own vocabulary, which is the thing a shared substring could never get right for everyone.

One test guards the behaviour that must not change: test_a_react_thought_between_actions_is_not_a_loop — alternating thought and action is the protocol working, and it passed before the fix and after.

One trap fixed on the way

make_env in test_autogen_validator.py returns a bare MagicMock, whose is_thought would answer truthily for every output and make any sufficiently long history look like a thought loop. It now classifies both spellings, as does FakeEnv. The existing episode tests only escaped this because their histories never reach three.

🤖 Generated with Claude Code

[12yuens2]

maybe add another test on the solver behaviour after the 3rd thought? how does the ground truth agent help?

[12yuens2]

Added test_ground_truth_acts_when_the_solver_only_thinks (fe1526d), and the question was worth asking — the two existing ground-truth tests both drive the identical arm, and neither asserts what reaches the environment. The stronger of them checks reasoning.call_count >= 7.

How it helps: for that one trial the ground truth agent is asked in the solver's place, and its output is what env.step receives. Its system prompt tells it not to continue the approach the solver has been repeating, so what comes back is an action rather than a fifth thought. That's the whole mechanism — it doesn't get extra tools or context, just a different instruction and the same prompt. The new test asserts both halves: who was asked, and that Lookup[named after] rather than a fourth thought reached the environment.

On timing, which the arm's name doesn't convey: the check reads the current output plus the last two, so the condition is three consecutive thoughts — but it's guarded by len(action_history) >= 3, which is stricter and delays it until a fourth is proposed. So three thoughts reach the environment before anything intervenes. The similar arm carries the same guard. The test pins this rather than papering over it.

I left the guard alone: it's pre-existing, identical on every dataset, and tightening it to match the check would change behaviour on all five. Happy to open an issue if you think three-vs-four is worth deciding.

Two things the test needed, both in test_autogen_validator.py helpers: the agents share one reasoning module, so a flat StubReasoning list can't say which agent produced an action — script_agents gives each named agent its own queue and returns the log of who was asked. And make_autogen grew a use_validator argument (defaulting to True, so existing callers are unchanged) so the stuck path can be exercised without validator calls interleaved into that log.

Confirmed red against the substring match this branch removes: ['solver', 'solver', 'solver', 'solver'] with no ground truth call, and the fourth thought reaching the environment.