fix(reflect): let a fresh mental model short-circuit forced retrieval (no extra LLM call)

[nicoloboschi]

Summary

Fixes #1971.

Reflect forced the full hierarchical retrieval path on the first iterations:

search_mental_models -> search_observations -> recall

The root cause is one mechanism: those iterations set tool_choice to a named function, and a named tool_choice forbids the model from emitting done. So even when search_mental_models returned a fresh, directly-relevant mental model, the agent could not answer — it was obligated to call search_observations and then recall first, paying for the lower layers (and often re-reading the very facts the mental model already summarized).

The fix

After the forced search_mental_models result, decide deterministically — with no extra LLM call — whether to keep forcing:

• If the call is low/mid budget and every retrieved mental model is explicitly fresh (is_stale is False) with non-empty content → stop forcing from the next iteration onward.
• That next iteration happens regardless; it now runs under auto tool choice, so the agent either answers directly, or — having just read the mental model — issues its own targeted search_observations/recall.
• Stale, empty, or missing mental models keep the full forced path.
• High budget always keeps the full verification path.

The key insight: a reflect iteration always makes exactly one call_with_tools; tool_choice only changes what the model may emit, not whether a call happens. So the sufficiency decision can be folded into the next agentic step that already occurs, rather than added as a separate classifier call.

Why not a sufficiency-classifier LLM call

An alternative (see #2005) inserts a dedicated structured "is this mental model sufficient?" LLM call after the mental-model layer. That re-introduces a cost on the very path the issue is about:

Scenario	Before	Classifier approach	This PR
mental model sufficient	3 forced rounds	mm + classifier + done → net −1	mm + done → net −2
mental model not sufficient	3 forced rounds	mm + classifier + obs + recall → net +1	mm + obs/recall under auto → net 0

This PR never adds an LLM round-trip in any scenario, and the targeted follow-up query the issue asks for comes for free: under auto the model composes its own observations/recall query having just read the mental model, so it naturally targets the gap/conclusion.

The deterministic freshness/usability guard (is_stale is False + non-empty content; missing flag treated as unsafe) is what makes "release to auto" safe — stale/empty results never short-circuit. Reliability-over-latency callers use high budget, which preserves the full forced path.

Behavior

• No mental models found → continue to the enabled lower layers (unchanged).
• Any retrieved mental model stale or empty → continue to the lower layers.
• high budget → full forced verification path (unchanged).
• Fresh + usable mental models on low/mid budget → forced lower-level retrieval stops; the next iteration is auto.

Tests

hindsight-api-slim/tests/test_reflect_agent.py:

• TestMentalModelFreshnessHelper — the deterministic guard (fresh, stale, missing-flag, blank-content, empty-list).
• test_fresh_mental_model_releases_forced_retrieval — fresh mm → second iteration is auto, obs/recall never called, and no extra llm.call.
• test_short_circuited_agent_may_still_retrieve_under_auto — after release the agent can still recall, by its own choice, with its own query.
• test_stale_mental_model_keeps_forced_retrieval / test_no_mental_models_keeps_forced_retrieval — full forced path preserved.
• test_high_budget_keeps_forced_path_for_fresh_mental_model — high budget unaffected.

This is a deterministic control-flow change (no prompt or model-interpretation change), so fast MockLLM unit tests are the appropriate coverage.

Commands run:

• ./scripts/hooks/lint.sh
• uv run ty check hindsight_api/engine/reflect/agent.py
• uv run pytest tests/test_reflect_agent.py (50 passed)