From f5dc2ceeecc86086584b0b93dd1951b2c764b7df Mon Sep 17 00:00:00 2001
From: m-szymanska <m.szymanska@libraxis.ai>
Date: Sun, 5 Jul 2026 21:24:00 -0700
Subject: [PATCH] fix(threads): do not persist legacy fallback thread when
 formatter failed
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 app/controller/helpers.rs | 66 ++++++++++++++++++++++++++++++++++++---
 1 file changed, 61 insertions(+), 5 deletions(-)

diff --git a/app/controller/helpers.rs b/app/controller/helpers.rs
index 388f6b11..391b0a36 100644
--- a/app/controller/helpers.rs
+++ b/app/controller/helpers.rs
@@ -832,6 +832,28 @@ async fn run_agent_send_path(
     }
 }
 
+/// Map a legacy formatter result to the assistant text that should be
+/// persisted, if any.
+///
+/// A `Failed` status carries no real assistant content (previously it was
+/// surfaced only as the "AI Failed" sentinel). A failed formatting attempt is
+/// NOT a conversation and must not be persisted (operator decision
+/// 2026-07-06): a dead API key would otherwise land a junk "AI Failed" thread
+/// on disk for every retry, producing 3-4 duplicate garbage threads per
+/// utterance. `Skipped` likewise has nothing to persist. Only genuine output
+/// (`Applied` / `AiNoop`, i.e. partial or full success) is persisted, exactly
+/// as before.
+fn legacy_fallback_assistant_text(
+    status: crate::ai_formatting::AiFormatStatus,
+    text: String,
+) -> Option<String> {
+    use crate::ai_formatting::AiFormatStatus;
+    match status {
+        AiFormatStatus::Applied | AiFormatStatus::AiNoop => Some(text),
+        AiFormatStatus::Failed | AiFormatStatus::Skipped => None,
+    }
+}
+
 async fn run_legacy_send_path(
     text: &str,
     whisper_language: crate::config::Language,
@@ -845,12 +867,14 @@ async fn run_legacy_send_path(
     )
     .await;
 
-    match result.status {
-        crate::ai_formatting::AiFormatStatus::Applied
-        | crate::ai_formatting::AiFormatStatus::AiNoop => Some(result.text),
-        crate::ai_formatting::AiFormatStatus::Failed => Some("AI Failed".to_string()),
-        crate::ai_formatting::AiFormatStatus::Skipped => None,
+    let status = result.status;
+    let assistant_text = legacy_fallback_assistant_text(status, result.text);
+    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)"
+        );
     }
+    assistant_text
 }
 
 async fn run_agent_send_with_fallback(
@@ -1159,6 +1183,38 @@ mod tests {
         assert_eq!(messages[1].content[0]["text"], "assistant reply");
     }
 
+    #[test]
+    fn legacy_fallback_skips_persist_on_failed_status() {
+        use crate::ai_formatting::AiFormatStatus;
+
+        // Failed: the formatter produced no real assistant content (dead API
+        // key -> "AI Failed"). Nothing to persist, so no thread is written and
+        // no messages are built. This is the regression guard for the ~12 junk
+        // "AI Failed" threads (incl. 3-4 duplicates per utterance) the operator
+        // saw after a dead key drove every retry through the legacy fallback.
+        assert_eq!(
+            legacy_fallback_assistant_text(AiFormatStatus::Failed, "AI Failed".to_string()),
+            None
+        );
+
+        // Skipped: also nothing to persist.
+        assert_eq!(
+            legacy_fallback_assistant_text(AiFormatStatus::Skipped, String::new()),
+            None
+        );
+
+        // Applied / AiNoop: genuine output (partial or full success) is still
+        // persisted exactly as before.
+        assert_eq!(
+            legacy_fallback_assistant_text(AiFormatStatus::Applied, "formatted reply".to_string()),
+            Some("formatted reply".to_string())
+        );
+        assert_eq!(
+            legacy_fallback_assistant_text(AiFormatStatus::AiNoop, "verbatim reply".to_string()),
+            Some("verbatim reply".to_string())
+        );
+    }
+
     struct NoopTestProvider;
 
     #[async_trait]