From 2f7f11154511d837fa96bdfd66501c797307c104 Mon Sep 17 00:00:00 2001 From: Blomios Date: Wed, 15 Jul 2026 08:32:59 +0200 Subject: [PATCH] =?UTF-8?q?fix(ticket-assistant):=20rendre=20visible=20l'?= =?UTF-8?q?=C3=A9chec=20=C2=AB=20aucune=20r=C3=A9ponse=20=C2=BB=20de=20l'a?= =?UTF-8?q?ssistant=20de=20ticket=20(#60)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduit un variant terminal `ReplyEvent::Error { message }` (domain/ports) propagé jusqu'au chat : l'adapter OpenAI-compat récupère `reasoning_content` quand `content` est vide, et un `Final` vide est converti en `Error` visible plutôt qu'un tour silencieux qui pend. Le front (`ReplyChunk` + useTicketAssistant) rend ce message d'erreur dans la cellule. - domain: variant `ReplyEvent::Error`, bras non terminal (readiness, structured drain) - infra/session: parse `reasoning_content` (delta/completion), fallback visible - app-tauri: mapping ReplyEvent::Error -> ReplyChunk::Error, Final vide -> Error - frontend: ReplyChunk `error`, rendu dans useTicketAssistant (+ test .test.tsx) NB: commit sur la feature branch, PAS un merge. #60 reste inProgress. Les 10 tests `cargo test -p infrastructure --lib session` échouent SOUS SANDBOX uniquement (bind loopback 127.0.0.1:0 interdit -> Os PermissionDenied), pas une régression : revérification hors-sandbox requise avant tout merge vers develop. Co-Authored-By: Claude Opus 4.8 --- crates/app-tauri/src/chat.rs | 12 +- crates/app-tauri/src/commands.rs | 12 ++ crates/app-tauri/src/dto.rs | 15 +- crates/app-tauri/tests/chat_bridge.rs | 96 ++++++++++++ crates/app-tauri/tests/dto_chat.rs | 15 ++ crates/application/src/agent/structured.rs | 4 +- crates/domain/src/ports.rs | 15 +- crates/domain/src/readiness.rs | 2 + crates/infrastructure/src/session/codex.rs | 36 ++++- crates/infrastructure/src/session/mod.rs | 71 +++++++++ .../src/session/openai_compat.rs | 146 ++++++++++++++++-- frontend/src/domain/index.ts | 7 +- .../tickets/useTicketAssistant.test.tsx | 140 +++++++++++++++++ .../features/tickets/useTicketAssistant.ts | 25 ++- 14 files changed, 571 insertions(+), 25 deletions(-) create mode 100644 frontend/src/features/tickets/useTicketAssistant.test.tsx diff --git a/crates/app-tauri/src/chat.rs b/crates/app-tauri/src/chat.rs index 99dfdee..13d93ef 100644 --- a/crates/app-tauri/src/chat.rs +++ b/crates/app-tauri/src/chat.rs @@ -196,6 +196,7 @@ fn reply_chunk_bytes(chunk: &ReplyChunk) -> usize { ReplyChunk::TextDelta { text } => text.len(), ReplyChunk::ToolActivity { label } => label.len(), ReplyChunk::Final { content } => content.len(), + ReplyChunk::Error { message } => message.len(), } } @@ -214,7 +215,16 @@ pub fn chunk_from_event(event: ReplyEvent) -> Option { match event { ReplyEvent::TextDelta { text } => Some(ReplyChunk::TextDelta { text }), ReplyEvent::ToolActivity { label } => Some(ReplyChunk::ToolActivity { label }), - ReplyEvent::Final { content } => Some(ReplyChunk::Final { content }), + ReplyEvent::Error { message } => Some(ReplyChunk::Error { message }), + ReplyEvent::Final { content } => { + if content.trim().is_empty() { + Some(ReplyChunk::Error { + message: "Réponse vide du modèle.".to_owned(), + }) + } else { + Some(ReplyChunk::Final { content }) + } + } ReplyEvent::Announcement { .. } => None, ReplyEvent::Heartbeat => None, ReplyEvent::RateLimited { .. } => None, diff --git a/crates/app-tauri/src/commands.rs b/crates/app-tauri/src/commands.rs index 7944d75..fbfa602 100644 --- a/crates/app-tauri/src/commands.rs +++ b/crates/app-tauri/src/commands.rs @@ -1659,6 +1659,7 @@ pub async fn agent_send( let service = std::sync::Arc::clone(&state.session_limit_service); let meta = state.structured_sessions.meta_for_session(&sid); std::thread::spawn(move || { + let mut terminal_visible = false; for event in stream { // Non-terminal, sans contenu chat : un signal de limite alimente le service // (le badge UI vient du bus `AgentRateLimited`, pas du flux chat) puis on @@ -1682,12 +1683,23 @@ pub async fn agent_send( let Some(chunk) = crate::chat::chunk_from_event(event) else { continue; }; + if matches!(chunk, ReplyChunk::Final { .. } | ReplyChunk::Error { .. }) { + terminal_visible = true; + } // `send_output` always records into the conversation scrollback; the // boolean only reflects live delivery. If the view navigated away // (no channel at this generation) we keep draining so the turn still // completes and the scrollback stays whole for the next re-attach. let _ = bridge.send_output(&sid, chunk); } + if !terminal_visible { + let _ = bridge.send_output( + &sid, + ReplyChunk::Error { + message: "Le modèle n'a renvoyé aucune réponse.".to_owned(), + }, + ); + } bridge.detach_if(&sid, gen); }); diff --git a/crates/app-tauri/src/dto.rs b/crates/app-tauri/src/dto.rs index bf7e91a..50595c3 100644 --- a/crates/app-tauri/src/dto.rs +++ b/crates/app-tauri/src/dto.rs @@ -1745,10 +1745,11 @@ impl From for TerminalSessionDto { /// twin of a [`domain::ports::ReplyEvent`]: the `agent_send` pump maps each turn /// event to one of these and pushes it to the frontend `AgentChatView`. /// -/// Tagged on `kind` (camelCase: `"textDelta"` | `"toolActivity"` | `"final"`), so -/// the front branches without positional parsing. `Final` is the **deterministic -/// terminal** chunk of a turn — after it the turn is frozen and the stream ends. -/// `Deserialize` is derived too so tests (and a mock gateway) can round-trip it. +/// Tagged on `kind` (camelCase: `"textDelta"` | `"toolActivity"` | `"final"` | +/// `"error"`), so the front branches without positional parsing. `Final` is the +/// normal deterministic terminal chunk of a turn; `Error` is a visible terminal +/// fallback for an otherwise silent/empty turn. `Deserialize` is derived too so +/// tests (and a mock gateway) can round-trip it. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(tag = "kind", rename_all = "camelCase")] pub enum ReplyChunk { @@ -1770,6 +1771,12 @@ pub enum ReplyChunk { /// The aggregated final content of the turn. content: String, }, + /// A visible terminal fallback when the model stream produced no usable answer. + #[serde(rename_all = "camelCase")] + Error { + /// Human-readable explanation to render in the chat cell. + message: String, + }, } /// Response DTO for `reattach_agent_chat`: the retained **conversation diff --git a/crates/app-tauri/tests/chat_bridge.rs b/crates/app-tauri/tests/chat_bridge.rs index 3645145..fe59cce 100644 --- a/crates/app-tauri/tests/chat_bridge.rs +++ b/crates/app-tauri/tests/chat_bridge.rs @@ -51,11 +51,18 @@ fn final_chunk(s: &str) -> ReplyChunk { } } +fn error_chunk(s: &str) -> ReplyChunk { + ReplyChunk::Error { + message: s.to_owned(), + } +} + fn chunk_bytes(chunk: &ReplyChunk) -> usize { match chunk { ReplyChunk::TextDelta { text } => text.len(), ReplyChunk::ToolActivity { label } => label.len(), ReplyChunk::Final { content } => content.len(), + ReplyChunk::Error { message } => message.len(), } } @@ -83,6 +90,16 @@ fn chunk_from_event_maps_tool_activity() { ); } +#[test] +fn chunk_from_event_maps_error() { + assert_eq!( + chunk_from_event(ReplyEvent::Error { + message: "auth failed".into() + }), + Some(error_chunk("auth failed")) + ); +} + #[test] fn chunk_from_event_maps_final() { assert_eq!( @@ -95,6 +112,16 @@ fn chunk_from_event_maps_final() { ); } +#[test] +fn chunk_from_event_maps_blank_final_to_visible_error() { + assert_eq!( + chunk_from_event(ReplyEvent::Final { + content: " \n\t ".into() + }), + Some(error_chunk("Réponse vide du modèle.")) + ); +} + #[test] fn chunk_from_event_drops_heartbeat() { // A heartbeat is a non-terminal liveness proof with no chat content (lot 1) ⇒ no @@ -110,12 +137,22 @@ fn chunk_from_event_drops_heartbeat() { /// does (`chunk_from_event` + `send_output`), then `detach_if(gen)` at stream /// end. This is the body of the spawned pump thread, run inline. fn pump_turn(bridge: &ChatBridge, session: &SessionId, gen: u64, events: Vec) { + let mut terminal_visible = false; for event in events { // Mirror the real pump: heartbeats map to no chunk and are skipped. if let Some(chunk) = chunk_from_event(event) { + if matches!(chunk, ReplyChunk::Final { .. } | ReplyChunk::Error { .. }) { + terminal_visible = true; + } let _ = bridge.send_output(session, chunk); } } + if !terminal_visible { + let _ = bridge.send_output( + session, + error_chunk("Le modèle n'a renvoyé aucune réponse."), + ); + } bridge.detach_if(session, gen); } @@ -174,6 +211,65 @@ fn pump_with_only_a_final_delivers_just_the_final() { assert_eq!(sink.lock().unwrap().as_slice(), &[final_chunk("ok")]); } +#[test] +fn pump_with_heartbeat_then_eof_emits_visible_error() { + let bridge = ChatBridge::new(); + let session = sid(); + let sink = Arc::new(Mutex::new(Vec::new())); + let gen = bridge.register(session, capturing_channel(Arc::clone(&sink))); + + pump_turn(&bridge, &session, gen, vec![ReplyEvent::Heartbeat]); + + assert_eq!( + sink.lock().unwrap().as_slice(), + &[error_chunk("Le modèle n'a renvoyé aucune réponse.")] + ); +} + +#[test] +fn pump_with_blank_final_emits_visible_error_without_silent_detach() { + let bridge = ChatBridge::new(); + let session = sid(); + let sink = Arc::new(Mutex::new(Vec::new())); + let gen = bridge.register(session, capturing_channel(Arc::clone(&sink))); + + pump_turn( + &bridge, + &session, + gen, + vec![ReplyEvent::Final { + content: String::new(), + }], + ); + + assert_eq!( + sink.lock().unwrap().as_slice(), + &[error_chunk("Réponse vide du modèle.")] + ); +} + +#[test] +fn pump_with_existing_error_does_not_emit_generic_error() { + let bridge = ChatBridge::new(); + let session = sid(); + let sink = Arc::new(Mutex::new(Vec::new())); + let gen = bridge.register(session, capturing_channel(Arc::clone(&sink))); + + pump_turn( + &bridge, + &session, + gen, + vec![ReplyEvent::Error { + message: "auth failed".into(), + }], + ); + + assert_eq!( + sink.lock().unwrap().as_slice(), + &[error_chunk("auth failed")] + ); +} + // --------------------------------------------------------------------------- // Generation supersede — the critical invariant (zone 1) // --------------------------------------------------------------------------- diff --git a/crates/app-tauri/tests/dto_chat.rs b/crates/app-tauri/tests/dto_chat.rs index b9fb1e0..c802275 100644 --- a/crates/app-tauri/tests/dto_chat.rs +++ b/crates/app-tauri/tests/dto_chat.rs @@ -45,6 +45,18 @@ fn reply_chunk_final_serialises_exact_camel_case() { assert_eq!(v, json!({ "kind": "final", "content": "done" })); } +#[test] +fn reply_chunk_error_serialises_exact_camel_case() { + let v = serde_json::to_value(ReplyChunk::Error { + message: "Réponse vide du modèle.".into(), + }) + .unwrap(); + assert_eq!( + v, + json!({ "kind": "error", "message": "Réponse vide du modèle." }) + ); +} + #[test] fn reply_chunk_round_trips_through_json_for_every_variant() { for chunk in [ @@ -55,6 +67,9 @@ fn reply_chunk_round_trips_through_json_for_every_variant() { ReplyChunk::Final { content: "y".into(), }, + ReplyChunk::Error { + message: "empty".into(), + }, ] { let v = serde_json::to_value(&chunk).unwrap(); let back: ReplyChunk = serde_json::from_value(v).unwrap(); diff --git a/crates/application/src/agent/structured.rs b/crates/application/src/agent/structured.rs index 9f6b948..84acd95 100644 --- a/crates/application/src/agent/structured.rs +++ b/crates/application/src/agent/structured.rs @@ -383,9 +383,11 @@ fn drain_stream_to_final( match event { ReplyEvent::Final { content } => return Ok(TurnOutcome::Completed(content)), ReplyEvent::RateLimited { resets_at_ms } => last_rate_limit = Some(resets_at_ms), - // TextDelta / Announcement / ToolActivity / Heartbeat : non terminaux. + // TextDelta / Announcement / ToolActivity / Error / Heartbeat : non terminaux + // pour ce drain synchrone ; seul Final est une réponse réussie. ReplyEvent::TextDelta { .. } | ReplyEvent::Announcement { .. } + | ReplyEvent::Error { .. } | ReplyEvent::ToolActivity { .. } | ReplyEvent::Heartbeat => {} } diff --git a/crates/domain/src/ports.rs b/crates/domain/src/ports.rs index a24f754..938b1de 100644 --- a/crates/domain/src/ports.rs +++ b/crates/domain/src/ports.rs @@ -405,6 +405,13 @@ pub enum ReplyEvent { /// humain, §21.1 niveau 3). Jamais une `Instant` monotone (cf. §21.2-T1). resets_at_ms: Option, }, + /// Erreur terminale du moteur structurée et affichable côté chat. Porte un + /// message déjà nettoyé par l'adapter ; les détails propres à la CLI restent + /// confinés côté infrastructure. + Error { + /// Message humain-lisible à rendre dans la cellule chat. + message: String, + }, /// **Événement terminal déterministe** d'un tour : l'adapter l'émet quand il a /// lu le message `result` documenté de la CLI. Porte le contenu final agrégé. /// Après `Final`, le flux se termine (plus aucun événement). @@ -415,11 +422,13 @@ pub enum ReplyEvent { } /// Flux borné d'événements de réponse d'UN tour (ARCHITECTURE §17.1). Se termine -/// après le [`ReplyEvent::Final`] (ou sur erreur). Calqué sur [`OutputStream`], +/// après le [`ReplyEvent::Final`] / [`ReplyEvent::Error`] (ou sur erreur de transport). +/// Calqué sur [`OutputStream`], /// mais **typé** : deltas de texte → activités d'outil → battements de cœur* /// ([`ReplyEvent::Heartbeat`], non terminaux, en nombre quelconque et entrelacés) → -/// **un** `Final` terminal déterministe, plutôt que des octets bruts. Seul le `Final` -/// clôt le flux ; deltas, activités et heartbeats sont tous non terminaux. +/// **un** terminal déterministe, plutôt que des octets bruts. `Final` clôt un tour +/// réussi ; `Error` clôt un tour échoué avec un message visible. Deltas, activités +/// et heartbeats sont tous non terminaux. pub type ReplyStream = Box + Send>; /// Description pure d'un outil exposé à un modèle OpenAI-compatible. diff --git a/crates/domain/src/readiness.rs b/crates/domain/src/readiness.rs index 0a8078d..9749103 100644 --- a/crates/domain/src/readiness.rs +++ b/crates/domain/src/readiness.rs @@ -85,6 +85,7 @@ impl ReadinessPolicy { /// l'application (planifier la reprise à `resets_at_ms`). L'heure de reset est /// propagée telle quelle. /// - [`ReplyEvent::TextDelta`] / [`ReplyEvent::Announcement`] / + /// [`ReplyEvent::Error`] / /// [`ReplyEvent::ToolActivity`] / /// [`ReplyEvent::Heartbeat`] ⇒ `None` : tous **non terminaux** (le flux /// continue). Un heartbeat prouve la vivacité mais ne termine pas le tour. @@ -97,6 +98,7 @@ impl ReadinessPolicy { }), ReplyEvent::TextDelta { .. } | ReplyEvent::Announcement { .. } + | ReplyEvent::Error { .. } | ReplyEvent::ToolActivity { .. } | ReplyEvent::Heartbeat => None, } diff --git a/crates/infrastructure/src/session/codex.rs b/crates/infrastructure/src/session/codex.rs index 981fbc5..618f9bf 100644 --- a/crates/infrastructure/src/session/codex.rs +++ b/crates/infrastructure/src/session/codex.rs @@ -47,8 +47,10 @@ pub struct ParsedLine { /// - `{"type":"turn.started"}` ⇒ [`ReplyEvent::Heartbeat`] (vivacité non terminale). /// - `{"type":"item.completed","item":{"id":"item_0","type":"agent_message","text":"…"}}` /// ⇒ si `item.type=="agent_message"` ⇒ [`ReplyEvent::Final`] (`content` = `item.text`, -/// c'est la réponse) ; sinon (`reasoning`/`command`/autre) ⇒ -/// [`ReplyEvent::ToolActivity`] (`label` = `item.type`). +/// c'est la réponse) ; si `item.type=="error"` ⇒ [`ReplyEvent::Error`] avec message +/// extrait de `item.text`, `item.message`, `item.error.message`, `item.error`, puis +/// des champs top-level `stderr`/`message`/`error.message`/`error` ; sinon +/// (`reasoning`/`command`/autre) ⇒ [`ReplyEvent::ToolActivity`] (`label` = `item.type`). /// - `{"type":"turn.completed","usage":{…}}` ⇒ [`ReplyEvent::Heartbeat`] (le `Final` /// vient de l'`agent_message`, pas de `turn.completed`). /// @@ -91,6 +93,11 @@ pub fn parse_event(line: &str) -> Result { .to_owned(); events.push(ReplyEvent::Final { content }); } + Some("error") => { + events.push(ReplyEvent::Error { + message: codex_error_message(&value, item), + }); + } // reasoning / command / tout autre item ⇒ activité (label = type). Some(kind) => events.push(ReplyEvent::ToolActivity { label: kind.to_owned(), @@ -109,6 +116,31 @@ pub fn parse_event(line: &str) -> Result { }) } +const CODEX_ERROR_FALLBACK: &str = "Codex a renvoyé une erreur."; + +fn non_blank_string(value: Option<&Value>) -> Option { + value + .and_then(Value::as_str) + .map(str::trim) + .filter(|s| !s.is_empty()) + .map(str::to_owned) +} + +fn error_value_message(value: Option<&Value>) -> Option { + let value = value?; + non_blank_string(value.get("message")).or_else(|| non_blank_string(Some(value))) +} + +fn codex_error_message(root: &Value, item: &Value) -> String { + non_blank_string(item.get("text")) + .or_else(|| non_blank_string(item.get("message"))) + .or_else(|| error_value_message(item.get("error"))) + .or_else(|| non_blank_string(root.get("stderr"))) + .or_else(|| non_blank_string(root.get("message"))) + .or_else(|| error_value_message(root.get("error"))) + .unwrap_or_else(|| CODEX_ERROR_FALLBACK.to_owned()) +} + /// Adapter de session structurée Codex. /// /// Incarnation « un `codex exec` par tour » (§17.2 (b)) : chaque `send` relance diff --git a/crates/infrastructure/src/session/mod.rs b/crates/infrastructure/src/session/mod.rs index 28cfead..ecd6a05 100644 --- a/crates/infrastructure/src/session/mod.rs +++ b/crates/infrastructure/src/session/mod.rs @@ -887,6 +887,46 @@ mod tests { ); } + /// `item.type == error` ⇒ Error terminal visible, avec message réel. + #[test] + fn codex_error_item_is_error_event_with_real_message() { + let r = codex::parse_event( + r#"{"type":"item.completed","item":{"id":"i0","type":"error","text":"auth failed"}}"#, + ) + .unwrap(); + assert_eq!( + r.events, + vec![ReplyEvent::Error { + message: "auth failed".into() + }] + ); + + let nested = codex::parse_event( + r#"{"type":"item.completed","item":{"id":"i0","type":"error","error":{"message":"quota exceeded"}}}"#, + ) + .unwrap(); + assert_eq!( + nested.events, + vec![ReplyEvent::Error { + message: "quota exceeded".into() + }] + ); + } + + /// `item.type == error` sans champ exploitable ⇒ fallback stable. + #[test] + fn codex_error_item_without_message_uses_fallback() { + let r = + codex::parse_event(r#"{"type":"item.completed","item":{"id":"i0","type":"error"}}"#) + .unwrap(); + assert_eq!( + r.events, + vec![ReplyEvent::Error { + message: "Codex a renvoyé une erreur.".into() + }] + ); + } + /// `agent_message` ⇒ Final (porte le texte de réponse). #[test] fn codex_agent_message_is_final() { @@ -982,6 +1022,37 @@ mod tests { ); } + #[tokio::test] + async fn codex_error_item_surfaces_error_without_final() { + let fake = FakeCli::printing(&[ + r#"{"type":"thread.started","thread_id":"c"}"#, + r#"{"type":"turn.started"}"#, + r#"{"type":"item.completed","item":{"id":"i0","type":"error","text":"auth failed"}}"#, + r#"{"type":"turn.completed","usage":{}}"#, + ]); + let s = CodexExecSession::new( + SessionId::new_random(), + fake.command(), + "/", + None, + Vec::new(), + Vec::new(), + None, + None, + ); + let events: Vec<_> = s.send("x").await.expect("send").collect(); + assert!(events.iter().any(|event| matches!( + event, + ReplyEvent::Error { message } if message.contains("auth failed") + ))); + assert!( + events + .iter() + .all(|event| !matches!(event, ReplyEvent::Final { .. })), + "Codex error must not be fabricated as a successful Final" + ); + } + #[tokio::test] async fn codex_two_agent_messages_preserve_first_as_announcement_and_last_as_final() { let fake = FakeCli::printing(&[ diff --git a/crates/infrastructure/src/session/openai_compat.rs b/crates/infrastructure/src/session/openai_compat.rs index 326d222..81ab2cf 100644 --- a/crates/infrastructure/src/session/openai_compat.rs +++ b/crates/infrastructure/src/session/openai_compat.rs @@ -38,6 +38,8 @@ pub struct OpenAiToolCall { pub struct ParsedChatDelta { /// Fragment de texte assistant. pub text: String, + /// Fragment de raisonnement exposé par certains serveurs OpenAI-compatibles. + pub reasoning_content: String, /// Appels d'outils annoncés par ce chunk. pub tool_calls: Vec, } @@ -47,13 +49,26 @@ pub struct ParsedChatDelta { pub struct ParsedCompletion { /// Contenu assistant complet. pub content: String, + /// Contenu alternatif de raisonnement, quand le serveur laisse `content` vide. + pub reasoning_content: String, /// Appels d'outils demandés par le modèle. pub tool_calls: Vec, } +impl ParsedCompletion { + fn visible_content(&self) -> String { + if self.content.trim().is_empty() { + self.reasoning_content.clone() + } else { + self.content.clone() + } + } +} + #[derive(Debug, Default)] struct StreamState { content: String, + reasoning_content: String, tool_calls: Vec, } @@ -108,8 +123,18 @@ pub fn parse_chat_delta(line: &str) -> Result Result, content: String, + reasoning_content: String, tool_calls: Vec, } +impl TurnResult { + fn visible_content(&self) -> String { + if self.content.trim().is_empty() { + self.reasoning_content.clone() + } else { + self.content.clone() + } + } +} + fn handle_sse_line( line: &str, state: &mut StreamState, @@ -528,12 +576,23 @@ fn handle_sse_line( return Ok(()); } let parsed = parse_chat_delta(data)?; - if !parsed.text.is_empty() { + let has_text = !parsed.text.is_empty(); + if has_text { state.content.push_str(&parsed.text); let event = ReplyEvent::TextDelta { text: parsed.text }; send_tap(tap, &event); events.push(event); } + if !has_text && !parsed.reasoning_content.is_empty() { + state.reasoning_content.push_str(&parsed.reasoning_content); + let event = ReplyEvent::TextDelta { + text: parsed.reasoning_content, + }; + send_tap(tap, &event); + events.push(event); + } else if !parsed.reasoning_content.is_empty() { + state.reasoning_content.push_str(&parsed.reasoning_content); + } merge_tool_call_deltas(state, parsed.tool_calls); Ok(()) } @@ -802,15 +861,82 @@ mod tests { ) .expect("parse ok"); assert_eq!(parsed.text, "bonjour"); + assert!(parsed.reasoning_content.is_empty()); assert!(parsed.tool_calls.is_empty()); } + #[test] + fn parse_openai_sse_delta_reasoning_content() { + let parsed = parse_chat_delta( + r#"{"choices":[{"delta":{"reasoning_content":"je raisonne"},"finish_reason":null}]}"#, + ) + .expect("parse ok"); + assert!(parsed.text.is_empty()); + assert_eq!(parsed.reasoning_content, "je raisonne"); + } + #[test] fn parse_ollama_completion_message() { let parsed = parse_completion(r#"{"choices":[{"message":{"role":"assistant","content":"salut"}}]}"#) .expect("parse ok"); assert_eq!(parsed.content, "salut"); + assert!(parsed.reasoning_content.is_empty()); + } + + #[test] + fn parse_completion_reasoning_content() { + let parsed = parse_completion( + r#"{"choices":[{"message":{"role":"assistant","content":"","reasoning_content":"raisonnement visible"}}]}"#, + ) + .expect("parse ok"); + assert!(parsed.content.is_empty()); + assert_eq!(parsed.reasoning_content, "raisonnement visible"); + assert_eq!(parsed.visible_content(), "raisonnement visible"); + } + + #[test] + fn sse_done_without_content_leaves_empty_visible_content_for_chat_error_mapping() { + let mut state = StreamState::default(); + let mut events = Vec::new(); + handle_sse_line("data: [DONE]", &mut state, &mut events, &None).expect("handle done"); + let turn = TurnResult { + events, + content: state.content, + reasoning_content: state.reasoning_content, + tool_calls: Vec::new(), + }; + + assert!(turn.events.is_empty()); + assert!( + turn.visible_content().is_empty(), + "chat bridge maps this empty final to ReplyChunk::Error" + ); + } + + #[test] + fn sse_reasoning_content_is_surfaced_when_content_is_empty_without_http() { + let mut state = StreamState::default(); + let mut events = Vec::new(); + handle_sse_line( + r#"data: {"choices":[{"delta":{"reasoning_content":"raison"},"finish_reason":null}]}"#, + &mut state, + &mut events, + &None, + ) + .expect("handle reasoning"); + let turn = TurnResult { + events, + content: state.content, + reasoning_content: state.reasoning_content, + tool_calls: Vec::new(), + }; + + assert!(matches!( + turn.events.as_slice(), + [ReplyEvent::TextDelta { text }] if text == "raison" + )); + assert_eq!(turn.visible_content(), "raison"); } #[test] diff --git a/frontend/src/domain/index.ts b/frontend/src/domain/index.ts index 11cd490..1bcfd68 100644 --- a/frontend/src/domain/index.ts +++ b/frontend/src/domain/index.ts @@ -1221,9 +1221,12 @@ export interface TicketChat { * One streamed chunk of an assistant turn (mirror of the backend `ReplyChunk`, * tagged on `kind`). `textDelta` is an incremental text fragment; `toolActivity` * a best-effort activity badge; `final` the deterministic end-of-turn chunk - * carrying the aggregated content — after it the turn is frozen. + * carrying the aggregated content — after it the turn is frozen. `error` is a + * visible terminal fallback (ticket #60): the model produced no usable answer, + * so the turn ends with a human-readable explanation instead of a silent hang. */ export type ReplyChunk = | { kind: "textDelta"; text: string } | { kind: "toolActivity"; label: string } - | { kind: "final"; content: string }; + | { kind: "final"; content: string } + | { kind: "error"; message: string }; diff --git a/frontend/src/features/tickets/useTicketAssistant.test.tsx b/frontend/src/features/tickets/useTicketAssistant.test.tsx new file mode 100644 index 0000000..04ef025 --- /dev/null +++ b/frontend/src/features/tickets/useTicketAssistant.test.tsx @@ -0,0 +1,140 @@ +/** + * Ticket #60 — the ticket AI assistant must never hang silently. + * + * The backend now emits an explicit terminal `{ kind: "error", message }` chunk + * when a turn produces no usable answer. This hook must consume it: end the + * pending turn with a readable message and drop `busy`. It must also keep the + * input disabled until the REAL terminal (`final` or `error`) arrives — not the + * moment `agent_send` returns (the stream is still flowing then). + */ +import { describe, it, expect } from "vitest"; +import { renderHook, act, waitFor } from "@testing-library/react"; +import type { ReactNode } from "react"; + +import { DIProvider } from "@/app/di"; +import type { Gateways } from "@/ports"; +import type { ReplyChunk, TicketChat } from "@/domain"; +import { useTicketAssistant } from "./useTicketAssistant"; + +const P = "proj"; +const REF = "#1"; + +/** + * A controllable ticket gateway: `sendTicketChat` captures the chunk handler and + * stays pending until the test resolves it, so the terminal chunk timing is + * driven explicitly (no reliance on the transport promise settling). + */ +class FakeTicketGateway { + captured: ((chunk: ReplyChunk) => void) | null = null; + sendResolve: (() => void) | null = null; + sendCalls = 0; + + async openTicketChat(): Promise { + return { sessionId: "sess-1", requester: "a1", issueRef: REF } as TicketChat; + } + async closeTicketChat(): Promise {} + sendTicketChat( + _sessionId: string, + _message: string, + onChunk: (chunk: ReplyChunk) => void, + ): Promise { + this.sendCalls += 1; + this.captured = onChunk; + return new Promise((resolve) => { + this.sendResolve = resolve; + }); + } +} + +function wrapperFor(ticket: FakeTicketGateway) { + const gateways = { ticket } as unknown as Gateways; + return function Wrapper({ children }: { children: ReactNode }) { + return {children}; + }; +} + +/** Opens the session and fires a send that leaves the transport pending. */ +async function openAndSend(gw: FakeTicketGateway) { + const hook = renderHook(() => useTicketAssistant(P, REF), { + wrapper: wrapperFor(gw), + }); + await act(async () => { + await hook.result.current.open("prof-1"); + }); + expect(hook.result.current.sessionId).toBe("sess-1"); + act(() => { + void hook.result.current.send("édite le ticket"); + }); + await waitFor(() => expect(gw.captured).toBeTruthy()); + return hook; +} + +describe("useTicketAssistant — ticket #60 (no-reply terminal)", () => { + it("an error chunk ends the pending turn with a visible message and clears busy", async () => { + const gw = new FakeTicketGateway(); + const { result } = await openAndSend(gw); + + // Mid-turn: busy true, an empty pending assistant bubble is streaming. + expect(result.current.busy).toBe(true); + expect(result.current.turns.at(-1)).toMatchObject({ + role: "assistant", + pending: true, + }); + + // Backend terminal fallback: no usable answer. + act(() => { + gw.captured!({ + kind: "error", + message: "Le modèle n'a renvoyé aucune réponse.", + }); + }); + + await waitFor(() => expect(result.current.busy).toBe(false)); + const last = result.current.turns.at(-1)!; + expect(last.role).toBe("assistant"); + expect(last.pending).toBeFalsy(); + expect(last.text).toContain("Le modèle n'a renvoyé aucune réponse."); + }); + + it("a non-empty final renders the answer and clears busy (nominal, no regression)", async () => { + const gw = new FakeTicketGateway(); + const { result } = await openAndSend(gw); + + act(() => { + gw.captured!({ kind: "textDelta", text: "Titre " }); + }); + act(() => { + gw.captured!({ kind: "final", content: "Titre mis à jour." }); + }); + + await waitFor(() => expect(result.current.busy).toBe(false)); + const last = result.current.turns.at(-1)!; + expect(last.role).toBe("assistant"); + expect(last.text).toBe("Titre mis à jour."); + expect(last.pending).toBeFalsy(); + }); + + it("busy stays true after agent_send resolves, until a terminal chunk arrives", async () => { + const gw = new FakeTicketGateway(); + const { result } = await openAndSend(gw); + expect(result.current.busy).toBe(true); + + // `agent_send` returns (stream started) but NO terminal chunk yet: busy must + // NOT drop — the input stays disabled while the turn is still in flight. + await act(async () => { + gw.sendResolve!(); + }); + expect(result.current.busy).toBe(true); + expect(result.current.turns.at(-1)).toMatchObject({ pending: true }); + + // Only the real terminal (here `final`) re-enables the input. + act(() => { + gw.captured!({ kind: "final", content: "fini" }); + }); + await waitFor(() => expect(result.current.busy).toBe(false)); + expect(result.current.turns.at(-1)).toMatchObject({ + text: "fini", + pending: false, + }); + }); +}); diff --git a/frontend/src/features/tickets/useTicketAssistant.ts b/frontend/src/features/tickets/useTicketAssistant.ts index 7e342f8..440c119 100644 --- a/frontend/src/features/tickets/useTicketAssistant.ts +++ b/frontend/src/features/tickets/useTicketAssistant.ts @@ -135,19 +135,40 @@ export function useTicketAssistant( } else if (chunk.kind === "toolActivity") { // Best-effort inline activity badge. next[i] = { ...next[i], text: `${next[i].text}\n· ${chunk.label}` }; + } else if (chunk.kind === "error") { + // Terminal fallback (ticket #60): the model returned no usable answer. + // Freeze the turn with the readable explanation so the bubble is never + // left empty and pending — the assistant no longer hangs silently. + next[i] = { + role: "assistant", + text: `⚠️ ${chunk.message}`, + pending: false, + }; } else { // `final`: freeze the turn with the aggregated content. next[i] = { role: "assistant", text: chunk.content, pending: false }; } return next; }); - if (chunk.kind === "final") setBusy(false); + // `busy` tracks the REAL end-of-turn: a `final` or an `error` terminal — + // never merely `agent_send` returning (the stream is still flowing then, + // ticket #60). The input stays disabled until one of them arrives. + if (chunk.kind === "final" || chunk.kind === "error") setBusy(false); }; try { await ticket.sendTicketChat(sessionId, text, applyChunk); } catch (e) { + // Transport failure: no terminal chunk will arrive, so close the pending + // turn and drop `busy` here to guarantee the assistant never hangs. setError(describe(e)); - } finally { + setTurns((prev) => { + const next = [...prev]; + const i = next.length - 1; + if (i >= 0 && next[i].role === "assistant" && next[i].pending) { + next[i] = { ...next[i], pending: false }; + } + return next; + }); setBusy(false); } },