fix(refresh): preserve mental model content when reflect returns no usable answer

[gvilleg]

Summary

Fixes #2959.

When the reflect agent finishes without a usable answer it substitutes a human-readable placeholder
(NO_ANSWER_TEXT, or the iteration-limit text). Both are non-empty, so refresh_mental_model's
"refuse to overwrite with an empty render" guard — which tests not final_content.strip() — does not
fire, and the placeholder is written over the existing document while the operation reports success.

This makes the refresh fail closed: when reflect produced nothing usable, the existing content stays,
the failure is auditable, and MentalModelRefreshError is raised.

Changes

A required failure reason at the source (engine/reflect/models.py, engine/reflect/agent.py)

ReflectAgentResult.answer_failure_reason: Literal["empty_answer", "iteration_limit"] | None — a
required field with no default, supplied at all seven terminal construction sites. Each site already
knows whether it produced an answer, so the signal is recorded where the fact is known rather than
inferred later from text. _process_done_tool computes it before substituting the placeholder.

No default is deliberate: a future terminal path that forgets the field raises ValidationError
rather than silently reporting success. Placeholder strings moved to reflect/models.py (with
agent.py re-exporting NO_ANSWER_TEXT for existing importers) so persistence code can import them
without pulling in the agent module.

Check it before delta (engine/memory_engine.py)

refresh_mental_model now inspects answer_failure_reason immediately after reflect and before any
delta work. Placement matters: delta applies operations to the stored document and re-renders it, so
a guard after that step would inspect ordinary-looking markdown and pass. On failure it persists the
reflect_response — including the full based_on evidence and accumulated delta provenance — with
refresh_skipped set to the specific reason, leaves content/structured_content untouched, and
raises.

One predicate for storing and reporting (engine/memory_engine.py)

New pure helper is_populated_content(content) rejects empty/whitespace, MENTAL_MODEL_PENDING_CONTENT,
NO_ANSWER_TEXT, and ITERATION_LIMIT_TEXT. The text-level guard now uses it instead of a bare
emptiness test, and _write_refresh_outcome_metadata uses the same helper, so what gets stored and
what gets reported cannot drift. (This also gives the outcome metadata iteration-limit coverage it
did not previously have.) The authoritative signal is answer_failure_reason; this is defense in
depth.

Failed refreshes no longer advance query tracking (engine/memory_engine.py)

Both failure paths previously passed last_refreshed_source_query=current_source_query while
deliberately preserving old content. Since use_delta requires the stored query to match the current
one, that combination corrupts the retry: after a source-query change, a failed full regeneration
would record the new query, and the retry would then select delta and amend the previous topic's
document from recall scoped to new memories only. Failure paths now persist only the audit payload.
The success path and the delta no_new_facts path still advance tracking, since in both the stored
content really is current for that query.

Behavior change

A refresh that produces no usable answer now raises MentalModelRefreshError instead of completing.
MentalModelRefreshError is already classified retryable and converts to RetryTaskAt, so async
refreshes retry as before and settle terminal only after exhaustion. Retry is safe precisely because
each attempt is now non-destructive — the risk was never the retry, it was the overwrite.

Tests

New: hindsight-api-slim/tests/test_refresh_preserves_on_failed_answer.py (8 cases). They patch
reflect_async rather than refresh_mental_model, so the real persistence path — delta, guards, the
write — actually executes; stubbing the refresh itself would assert nothing about storage.

• blank answer preserves content, raises, records refresh_skipped == "empty_answer"
• iteration-limit preserves content, raises, records refresh_skipped == "iteration_limit"
• a failed refresh preserves its based_on evidence
• delta mode cannot launder a failed answer: asserts delta was actually selected (created_after
  present) and that no delta operation was emitted
• a failed refresh does not advance source-query tracking, so its retry is still a full regeneration
• a successful refresh still writes content
• is_populated_content unit coverage for all three placeholders

Each was mutation-checked against the pre-fix behavior: reverting the guards fails 5 of 7; disabling
only the early check still fails 3 (the text guard catches the rest); restoring
last_refreshed_source_query on a failure write fails the tracking test; disabling the early check
fails the delta test with real laundering — delta runs, renders ordinary prose, and no error is
raised. A test that passes with and without the fix would prove nothing, so this was verified rather
than assumed.

tests/test_refresh_outcome_metadata.py passes unchanged: its no-answer case stubs
refresh_mental_model entirely, so it exercises only the metadata writer, where populated_content=False
remains correct.

tests/test_refresh_preserves_on_failed_answer.py  tests/test_mental_model_delta.py  tests/test_refresh_outcome_metadata.py
21 passed, 4 skipped

scripts/hooks/lint.sh reports no diagnostics in any changed file.

Notes for reviewers

• The trigger — why a large synthesis returns a blank done.answer in the first place — is
  environment-specific and not addressed here. This change is about what happens when it does:
  a failed render must not destroy a working document.
• Sequencing is the crux of the fix. populated_content on main already recognizes the
  placeholder, but it is computed after refresh_mental_model has persisted and returned, so it can
  only annotate the loss. The check has to happen before delta and before the write.