fix(threads): stop persisting legacy fallback threads when AI failed#51
Conversation
…iled A dead API key drove every assistive send through the legacy formatter fallback, and each attempt persisted an "AI Failed" thread — the operator saw ~12 junk conversations (with 3-4 duplicates per utterance) after a key outage. A failed formatting attempt is not a conversation, so it must not land on disk. Extract the status->assistant-text mapping into a pure `legacy_fallback_assistant_text` helper: Failed and Skipped now yield None (no persist), while Applied and AiNoop still return the text and persist as before. run_legacy_send_path warns on Failed so the skipped attempt is still visible in logs. Since the wild duplicates were all failed attempts (both send callsites invoke the runtime exactly once per stop/hold — there is no per-attempt retry loop in code), suppressing Failed persistence removes the duplicate garbage at the source; no dedup loop change is needed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request introduces a helper function legacy_fallback_assistant_text to prevent persisting failed or skipped legacy formatter attempts as threads, addressing an issue where failed attempts resulted in duplicate garbage threads. It also adds corresponding unit tests. The review feedback suggests simplifying a redundant condition in run_legacy_send_path where assistant_text.is_none() is checked alongside the Failed status.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if assistant_text.is_none() && status == crate::ai_formatting::AiFormatStatus::Failed { | ||
| warn!( | ||
| "Legacy formatter failed; skipping thread persist for this attempt (a failed attempt is not a conversation)" | ||
| ); | ||
| } |
There was a problem hiding this comment.
The check assistant_text.is_none() is redundant. Since legacy_fallback_assistant_text is guaranteed to return None when the status is AiFormatStatus::Failed, checking status == crate::ai_formatting::AiFormatStatus::Failed is sufficient. Simplifying this condition improves code readability.
| if assistant_text.is_none() && status == crate::ai_formatting::AiFormatStatus::Failed { | |
| warn!( | |
| "Legacy formatter failed; skipping thread persist for this attempt (a failed attempt is not a conversation)" | |
| ); | |
| } | |
| if status == crate::ai_formatting::AiFormatStatus::Failed { | |
| warn!( | |
| "Legacy formatter failed; skipping thread persist for this attempt (a failed attempt is not a conversation)" | |
| ); | |
| } |
Problem
When the AI runtime is unavailable (dead/missing API key), the legacy assistive fallback persisted every failed attempt as a conversation thread with an "AI Failed" assistant message. After the thread-heal fix (#49) these junk threads became visible, cluttering the history rail — including repeated recordings of the same utterance producing 3-4 identical junk threads.
Fix
legacy_fallback_assistant_text(status, text) -> Option<String>:Failed/Skipped→None(no thread persisted,warn!logged),Applied/AiNoop→Some(text)(persisted as before — partial success unaffected).Tests
legacy_fallback_skips_persist_on_failed_statuscovering all four status arms.cargo fmt --check,cargo clippy --all-targets -- -D warnings,cargo test -p codescribe(239 passed),cargo test -p codescribe-core(645 passed) — all green.🤖 Generated with Claude Code