fix(dashboard): keep steers pending on an empty consumed echo (#8481)

[bolichen97]

Problem / Motivation

A mid-turn steer (the user's correction typed while a turn is running) could be silently lost. When the ACP backend sends an EVENT_STEER_CONSUMED echo with empty text — a legitimate case per session_handle.py's emitter and steer_settle's own docstring (older backend, dropped echo) — the main chat's _settle_consumed_steers called settle_consumed_steers(..., settle_all_on_empty=True), which cleared all pending steers with no evidence. That suppressed _requeue_unconsumed_steers, so the correction ran nowhere while its transcript row claimed delivery.

Why it matters

The user's correction is exactly the message they care most about mid-turn. Losing it silently — with the UI asserting it was delivered — is the worst failure shape: unrecoverable and invisible. Every sibling call site (the _refusal_notices settle in the same event branch, and the /side sidecar) already settles nothing on an empty echo for this precise reason; the main chat was the sole holdout.

What changed (motivation → approach → change)

• Symptom: an empty consumed echo makes a mid-turn steer vanish while its row reads as delivered.
• Root cause: settle_all_on_empty=True at the main chat's _settle_consumed_steers call treats an evidence-free frame as proof of consumption. The stated reason for deferring the flip ("the requeue mechanism does not exercise this path") is stale: _requeue_unconsumed_steers is wired into _run_chat's outer finally and runs on every turn-exit path (normal return, exception, cancellation).
• Change: delete the settle_all_on_empty opt-in outright — after the flip it had zero production selectors of True (both review lanes converged on this), so the empty-echo rule is now unconditional in settle_consumed_steers: an empty echo settles nothing, entries stay pending, and the turn-end requeue degrades them to queue cards. The accepted cost, named at the call site: on a backend that injected the steer but echoed no text, the steer runs again — usually immediately via the turn-exit drain, but visibly as its own turn (and holds as a cancellable card when the drain is withheld, e.g. sign-in required), unlike a silent loss.
• Comments/docstrings in chat_runner.py, steer_settle.py, state.py, and chat_delivery.py that described the sweep as live behaviour were restated as present-tense invariants.

Tests

• test_empty_snapshot_settles_nothing (rewritten from test_empty_snapshot_falls_back_to_settling_all, which pinned the bug): an empty echo leaves _pending_steers untouched.
• test_an_empty_echo_never_claims_consumption (rewritten as the regression test): empty echo → entry stays pending → no row promoted → _requeue_unconsumed_steers emits a requeued card carrying the steer.
• test_an_empty_echo_during_the_rpc_persists_as_written: updated — the entry now survives the empty echo, exercising the still-registered → written path.
• test_settle_delegates_to_the_shared_rules: the delegation assert flipped to settle_all_on_empty is False (load-bearing).
• Mutation-verified: flipping the empty-echo branch back to settle-everything fails 4 distinct tests.

Manual verification

N/A — unit coverage sufficient: the settle and requeue paths are exercised end-to-end through steer_into_running_turn + _settle_consumed_steers + _requeue_unconsumed_steers in the tests above, and the full backend suite's failure set is byte-identical to pristine main on this host (environmental only).

Related Issues

Closes #8481. Context: #7997 (merged prerequisite; this flip was deliberately held until it landed), #7998 (distinct no-echo-arrives case, already requeues correctly — not changed here).

Pattern harvest

Rule candidate: review-prompt
Pattern: a boolean escape-hatch parameter whose only selector is "long-standing behaviour" — the deferral rationale goes stale when the blocking mechanism lands; audit such flags when their named prerequisite merges.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable)
• No secrets, credentials, or internal references in the diff

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of 201cab26f910fe58ccad9b90c440972b6f75efc1 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

Root-cause fix that converges the last holdout call site onto the fail-closed rule its siblings already use, deleting the selector-less escape hatch entirely.

The one cost — a visible duplicate run on a backend that injected the steer but echoed empty — is named at the call site, bounded (the requeued card runs as a plain turn, so no loop), and correctly traded against silent loss. The requeue dependency the old deferral rationale doubted is verified wired into _run_chat's outer finally, and the spec (side.md) already documents the two-argument signature, so removing the parameter closes doc drift rather than opening it.

[DESIGN-REVIEWED] 201cab2

[github-actions]

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of 201cab26f910fe58ccad9b90c440972b6f75efc1 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All claims verified. The parameter settle_all_on_empty has zero remaining references in the repo (grepped: 0 hits), all three settle_consumed_steers call sites (main chat, refusal notices, /side sidecar) now settle nothing on an empty echo, and _requeue_unconsumed_steers is confirmed wired into _run_chat's teardown at chat_runner.py:11801. The change deletes a concept rather than adding one, and sits at cause level. Two textual stragglers reference the removed sweep: the test name test_an_unusable_echo_settles_everything (whose body asserts the opposite) and side.md:396's "settle-all on an unusable echo" phrase — both pre-existing, noted as a subtraction.

First-Principles-Verdict: PASS

A reported silent-loss defect (#8481) fixed by deleting the evidence-free sweep concept outright — every item subtracts surface or pins the new invariant.

What this change ships

Intent: stop a mid-turn correction from being silently dropped when the backend echoes an empty consumed frame — a FIX.

1. A correction hit by an empty echo now stays pending and reappears as a visible, cancellable queue card — justified (reported defect Empty steering_consumed echo silently loses a mid-turn steer instead of requeueing it #8481).
2. On a backend that consumed but echoed nothing, the correction runs twice, visibly — justified, declared trade against silent loss.
3. settle_all_on_empty parameter deleted from settle_consumed_steers — justified subtraction; grepped settle_all_on_empty: 0 remaining references.
4. Empty-echo rule now unconditional and identical at all 3 call sites (main chat, refusal notices, /side) — justified; removes the sole divergent holdout.
5. Comments in 4 modules restated as present-tense invariants — declared; mandated by the code-style comment rule.
6. Tests rewritten to pin settle-nothing plus the requeue card — declared.

This is the shape this lane exists to endorse: the fix removes the decision that caused the defect (an opt-in that treated no evidence as proof), deletes the opt-in with zero surviving consumers, and leaves no unfixed code siblings — all 3 call sites of settle_consumed_steers now share one rule.

Subtractions

• Delete the last two references to the removed sweep: the name test_an_unusable_echo_settles_everything (test/test_steer_settle.py:74) asserts the opposite of its own body, and docs/system-specs/modules/side.md:396's "settle-all on an unusable echo" names behavior that no longer exists. Count: 2, grepped settle_all|settles_everything.

[FIRST-PRINCIPLES-REVIEWED] 201cab2

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 201cab26f910fe58ccad9b90c440972b6f75efc1 — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 201cab2

Verdict parsed from the review's SHA-scoped output markers for commit 201cab26f910fe58ccad9b90c440972b6f75efc1.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 201cab26f910fe58ccad9b90c440972b6f75efc1: <one-sentence reason>

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 201cab26f910fe58ccad9b90c440972b6f75efc1 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 201cab2

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 201cab26f910fe58ccad9b90c440972b6f75efc1: <one-sentence reason>

[bolichen97]

[SUPERSEDED — historical record; disposition marker removed] Earlier accept-and-defer response to the First Principles CONCERNS verdict on head 242d0a6. Superseded by the implementation on head 201cab2: the settle_all_on_empty subtraction was folded into this PR after the lane escalated to BLOCK; follow-up issue #8831 is closed as implemented-here. See the current disposition comment for the live record.

[bolichen97]

[SUPERSEDED — historical record; disposition marker removed] Implementation notice for the First Principles BLOCK, whose marker carried a mistyped head SHA and never matched. Superseded by the corrected disposition comment on head 201cab2.

[bolichen97]

• The fix orphans settle_all_on_empty, and the diff preserves the corpse instead of deleting it — IMPLEMENTED on head 201cab2. The parameter, its return [] arm, the explicit kwarg at the chat_runner.py call site, test_the_settle_all_on_empty_opt_in_settles_everything, and the delegation kwarg assert are all deleted; the empty-echo rule is unconditional in settle_consumed_steers.

Verified at commit level: git grep settle_all_on_empty 201cab26f returns zero hits. Targeted suites (382 tests) and all local gates re-green. Mutation check (empty branch flipped to return []) fails 4 distinct tests. Follow-up issue #8831, filed under the earlier deferred disposition, is closed as implemented-here. The lane re-reviewed this head and posted PASS.