From a3ee2814d65cd5f8f1e1fa53b474a0acf5d9317c Mon Sep 17 00:00:00 2001 From: Blomios Date: Thu, 13 Aug 2026 12:45:32 +0200 Subject: [PATCH] =?UTF-8?q?fix(cli):=20progress=20agent=5Fmessage=20=C3=A9?= =?UTF-8?q?mis=20en=20live=20+=20anti-doublon=20running/done=20par=20progr?= =?UTF-8?q?essId=20=E2=80=94=20#187=20(QA=20verte)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Codex: les items agent_message sont désormais tapés en live comme progress (Message/Delta) dès item.completed, au lieu d'être bufferisés dans pending_final jusqu'à l'événement tool/MCP suivant. - Corrélation stable progressId (codex:item:) ajoutée sur ReplyProgress, propagée jusqu'au DTO (ReplyProgressDto.progressId) et au frontend ; le ProgressSettler et la vue procèdent par upsert running -> done au lieu d'empiler des lignes dupliquées. - Tests: chat_bridge (settler par progressId), dto_chat, session codex, CustomAgentChatView (upsert par progressId). --- crates/app-tauri/src/chat.rs | 8 ++ crates/app-tauri/tests/chat_bridge.rs | 48 +++++++++++ crates/app-tauri/tests/dto_chat.rs | 3 + crates/backend/src/dto.rs | 4 + crates/domain/src/ports.rs | 15 ++++ crates/infrastructure/src/session/codex.rs | 68 +++++++++++++-- crates/infrastructure/src/session/mod.rs | 27 ++++-- frontend/src/domain/index.ts | 1 + .../agents/CustomAgentChatView.test.tsx | 86 +++++++++++++++++++ .../features/agents/CustomAgentChatView.tsx | 5 +- 10 files changed, 250 insertions(+), 15 deletions(-) diff --git a/crates/app-tauri/src/chat.rs b/crates/app-tauri/src/chat.rs index 05dd8808..db903f06 100644 --- a/crates/app-tauri/src/chat.rs +++ b/crates/app-tauri/src/chat.rs @@ -237,6 +237,14 @@ fn progress_entity_key(progress: &ReplyProgressDto) -> Option { if !is_settleable_progress(progress) { return None; } + if let Some(progress_id) = progress + .progress_id + .as_deref() + .map(str::trim) + .filter(|id| !id.is_empty()) + { + return Some(progress_id.to_owned()); + } let name = progress .tool_name .as_ref() diff --git a/crates/app-tauri/tests/chat_bridge.rs b/crates/app-tauri/tests/chat_bridge.rs index 1be22e08..9af76705 100644 --- a/crates/app-tauri/tests/chat_bridge.rs +++ b/crates/app-tauri/tests/chat_bridge.rs @@ -70,6 +70,28 @@ fn progress_chunk( ) -> ReplyChunk { ReplyChunk::Progress { progress: ReplyProgressDto { + progress_id: None, + source: ReplyProgressSourceDto::ProviderNative, + kind, + stage, + label: label.to_owned(), + text: None, + provider: None, + native_event: None, + tool_name: Some(label.to_owned()), + }, + } +} + +fn progress_chunk_with_id( + kind: ReplyProgressKindDto, + stage: ReplyProgressStageDto, + label: &str, + progress_id: &str, +) -> ReplyChunk { + ReplyChunk::Progress { + progress: ReplyProgressDto { + progress_id: Some(progress_id.to_owned()), source: ReplyProgressSourceDto::ProviderNative, kind, stage, @@ -824,6 +846,32 @@ fn progress_settler_finish_completes_active_progress_at_stream_end() { assert!(settler.finish().is_none()); } +#[test] +fn progress_settler_matches_running_and_done_by_progress_id_before_label() { + let bridge = ChatBridge::new(); + let session = sid(); + let sink = Arc::new(Mutex::new(Vec::new())); + bridge.register(session, capturing_channel(Arc::clone(&sink))); + let mut settler = ProgressSettler::default(); + let started = progress_chunk_with_id( + ReplyProgressKindDto::Message, + ReplyProgressStageDto::Delta, + "message intermédiaire", + "codex:item:msg-1", + ); + let completed = progress_chunk_with_id( + ReplyProgressKindDto::Message, + ReplyProgressStageDto::Completed, + "message terminé", + "codex:item:msg-1", + ); + + assert!(bridge.send_output_settled(&session, &mut settler, started.clone())); + assert!(bridge.send_output_settled(&session, &mut settler, completed.clone())); + assert!(settler.finish().is_none()); + assert_eq!(sink.lock().unwrap().clone(), vec![started, completed]); +} + // --------------------------------------------------------------------------- // Registry basics, mirrored from pty_bridge.rs for parity // --------------------------------------------------------------------------- diff --git a/crates/app-tauri/tests/dto_chat.rs b/crates/app-tauri/tests/dto_chat.rs index d8e0b051..8c32eb21 100644 --- a/crates/app-tauri/tests/dto_chat.rs +++ b/crates/app-tauri/tests/dto_chat.rs @@ -72,6 +72,7 @@ fn reply_chunk_tool_activity_serialises_exact_camel_case() { fn reply_chunk_progress_serialises_exact_camel_case() { let v = serde_json::to_value(ReplyChunk::Progress { progress: ReplyProgressDto { + progress_id: Some("codex:item:tool0".into()), source: ReplyProgressSourceDto::IdeaLocal, kind: ReplyProgressKindDto::Mcp, stage: ReplyProgressStageDto::Started, @@ -90,6 +91,7 @@ fn reply_chunk_progress_serialises_exact_camel_case() { "progress": { "source": "ideaLocal", "kind": "mcp", + "progressId": "codex:item:tool0", "stage": "started", "label": "idea_ask_agent", "text": "vers QA", @@ -133,6 +135,7 @@ fn reply_chunk_round_trips_through_json_for_every_variant() { }, ReplyChunk::Progress { progress: ReplyProgressDto { + progress_id: None, source: ReplyProgressSourceDto::ProviderNative, kind: ReplyProgressKindDto::Turn, stage: ReplyProgressStageDto::Started, diff --git a/crates/backend/src/dto.rs b/crates/backend/src/dto.rs index 0dedeff1..f1114b86 100644 --- a/crates/backend/src/dto.rs +++ b/crates/backend/src/dto.rs @@ -3297,6 +3297,9 @@ impl From for ReplyProgressStageDto { #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct ReplyProgressDto { + /// Optional stable progress entity id for frontend upsert. + #[serde(skip_serializing_if = "Option::is_none")] + pub progress_id: Option, /// Event source. pub source: ReplyProgressSourceDto, /// Canonical event family. @@ -3322,6 +3325,7 @@ pub struct ReplyProgressDto { impl From for ReplyProgressDto { fn from(value: ReplyProgress) -> Self { Self { + progress_id: value.progress_id, source: value.source.into(), kind: value.kind.into(), stage: value.stage.into(), diff --git a/crates/domain/src/ports.rs b/crates/domain/src/ports.rs index e0535c41..7ed96a57 100644 --- a/crates/domain/src/ports.rs +++ b/crates/domain/src/ports.rs @@ -648,6 +648,10 @@ pub enum ReplyProgressStage { /// un contrat métier à parser côté application. #[derive(Debug, Clone, PartialEq, Eq)] pub struct ReplyProgress { + /// Identifiant stable optionnel de l'entité de progress, quand le provider ou + /// IdeA expose une corrélation native. Sert à faire évoluer running -> done + /// sans dépendre d'un libellé affichable. + pub progress_id: Option, /// Origine de l'événement (provider natif ou observabilité locale IdeA). pub source: ReplyProgressSource, /// Famille canonique. @@ -676,6 +680,7 @@ impl ReplyProgress { label: impl Into, ) -> Self { Self { + progress_id: None, source, kind, stage, @@ -694,6 +699,16 @@ impl ReplyProgress { self } + /// Ajoute l'identité stable de l'entité de progress. + #[must_use] + pub fn with_progress_id(mut self, progress_id: impl Into) -> Self { + let progress_id = progress_id.into(); + if !progress_id.trim().is_empty() { + self.progress_id = Some(progress_id); + } + self + } + /// Ajoute les métadonnées provider natives. #[must_use] pub fn with_provider_event( diff --git a/crates/infrastructure/src/session/codex.rs b/crates/infrastructure/src/session/codex.rs index 85cdbe3a..39447e8a 100644 --- a/crates/infrastructure/src/session/codex.rs +++ b/crates/infrastructure/src/session/codex.rs @@ -196,6 +196,7 @@ fn item_progress( stage: ReplyProgressStage, native_event: impl Into, ) -> ReplyProgress { + let progress_id = item_progress_id(item); let progress = if label.starts_with("idea_") { ReplyProgress::new( ReplyProgressSource::IdeaLocal, @@ -214,6 +215,11 @@ fn item_progress( ) .with_tool_name(label.to_owned()) }; + let progress = if let Some(progress_id) = progress_id { + progress.with_progress_id(progress_id) + } else { + progress + }; if let Some(preview) = item_preview(item) { progress.with_text(preview) } else { @@ -221,6 +227,29 @@ fn item_progress( } } +fn agent_message_progress(item: &Value, native_event: &str) -> Option { + let content = item_text(item)?; + let mut progress = ReplyProgress::new( + ReplyProgressSource::ProviderNative, + ReplyProgressKind::Message, + ReplyProgressStage::Delta, + "message intermédiaire", + ) + .with_provider_event("codex", native_event) + .with_text(content); + if let Some(progress_id) = item_progress_id(item) { + progress = progress.with_progress_id(progress_id); + } + Some(ReplyEvent::Progress { progress }) +} + +fn item_progress_id(item: &Value) -> Option { + non_blank_string(item.get("id")) + .or_else(|| non_blank_string(item.get("item_id"))) + .or_else(|| non_blank_string(item.get("call_id"))) + .map(|id| format!("codex:item:{id}")) +} + fn append_item_progress( events: &mut Vec, item: &Value, @@ -580,20 +609,18 @@ impl CodexExecSession { let parser = std::thread::spawn(move || { let mut pending_final: Option = None; for line in line_rx { + if let Ok(Some(progress)) = agent_message_progress_from_line(&line) { + let _ = event_tap.send(progress); + } if let Ok(parsed) = parse_event(&line) { for event in parsed.events { match event { ReplyEvent::Final { content } => { - if let Some(text) = pending_final.replace(content) { - let _ = event_tap.send(ReplyEvent::Announcement { text }); - } + let _ = pending_final.replace(content); } other => { if event_supersedes_pending_final(&other) { - if let Some(text) = pending_final.take() { - let _ = - event_tap.send(ReplyEvent::Announcement { text }); - } + let _ = pending_final.take(); } let _ = event_tap.send(other); } @@ -684,3 +711,30 @@ fn event_supersedes_pending_final(event: &ReplyEvent) -> bool { } ) } + +fn agent_message_progress_from_line(line: &str) -> Result, AgentSessionError> { + let trimmed = line.trim(); + if trimmed.is_empty() { + return Ok(None); + } + let value: Value = serde_json::from_str(trimmed) + .map_err(|e| AgentSessionError::Decode(format!("ligne JSON illisible: {e}")))?; + let Some("item.completed" | "conversation.item.done" | "response.output_item.done") = + value.get("type").and_then(Value::as_str) + else { + return Ok(None); + }; + let Some(item) = value.get("item") else { + return Ok(None); + }; + if item_type(item).as_deref() != Some("agent_message") { + return Ok(None); + } + Ok(agent_message_progress( + item, + value + .get("type") + .and_then(Value::as_str) + .unwrap_or("item.completed"), + )) +} diff --git a/crates/infrastructure/src/session/mod.rs b/crates/infrastructure/src/session/mod.rs index 045b36ee..4c915812 100644 --- a/crates/infrastructure/src/session/mod.rs +++ b/crates/infrastructure/src/session/mod.rs @@ -1016,6 +1016,7 @@ mod tests { && progress.stage == ReplyProgressStage::Completed && progress.label == "reasoning" && progress.tool_name.as_deref() == Some("reasoning") + && progress.progress_id.as_deref() == Some("codex:item:i0") ), "reasoning item must be represented by one canonical progress event: {:?}", r.events @@ -1032,6 +1033,7 @@ mod tests { && progress.stage == ReplyProgressStage::Completed && progress.label == "command" && progress.tool_name.as_deref() == Some("command") + && progress.progress_id.as_deref() == Some("codex:item:i1") ), "command item must be represented by one canonical progress event: {:?}", c.events @@ -1055,6 +1057,7 @@ mod tests { && progress.label == "read_file" && progress.tool_name.as_deref() == Some("read_file") && progress.native_event.as_deref() == Some("conversation.item.added") + && progress.progress_id.as_deref() == Some("codex:item:call_1") ), "Codex item.added must surface tool start immediately: {:?}", r.events @@ -1083,6 +1086,7 @@ mod tests { && progress.label == "idea_ask_agent" && progress.tool_name.as_deref() == Some("idea_ask_agent") && progress.native_event.as_deref() == Some("conversation.item.added") + && progress.progress_id.as_deref() == Some("codex:item:call_1") ), "Codex MCP tool start must be tagged as IdeaLocal/Mcp: {:?}", r.events @@ -1106,6 +1110,7 @@ mod tests { && progress.label == "idea_ask_agent" && progress.tool_name.as_deref() == Some("idea_ask_agent") && progress.native_event.as_deref() == Some("conversation.item.done") + && progress.progress_id.as_deref() == Some("codex:item:call_1") ), "Codex MCP tool completion must keep the real tool name: {:?}", r.events @@ -1436,12 +1441,20 @@ mod tests { matches!(live.first(), Some(ReplyEvent::Progress { progress }) if progress.native_event.as_deref() == Some("turn.started")), "le démarrage du tour provider doit être tapé live en premier" ); - let announcement_idx = live + let message_idx = live .iter() - .position( - |event| matches!(event, ReplyEvent::Announcement { text } if text == "je délègue"), - ) - .expect("l'annonce d'intention doit être tapée live"); + .position(|event| { + matches!( + event, + ReplyEvent::Progress { progress } + if progress.source == ReplyProgressSource::ProviderNative + && progress.kind == ReplyProgressKind::Message + && progress.stage == ReplyProgressStage::Delta + && progress.text.as_deref() == Some("je délègue") + && progress.progress_id.as_deref() == Some("codex:item:i0") + ) + }) + .expect("le message agent doit être tapé live comme progress"); let mcp_idx = live .iter() .position(|event| { @@ -1456,8 +1469,8 @@ mod tests { }) .expect("le début MCP doit être tapé live"); assert!( - announcement_idx < mcp_idx, - "l'annonce d'intention doit précéder le premier MCP dans le tap live: {:?}", + message_idx < mcp_idx, + "le message agent doit précéder le premier MCP dans le tap live: {:?}", live ); assert!( diff --git a/frontend/src/domain/index.ts b/frontend/src/domain/index.ts index 678128d2..db9f8fc6 100644 --- a/frontend/src/domain/index.ts +++ b/frontend/src/domain/index.ts @@ -1680,6 +1680,7 @@ export type ReplyProgressKind = "turn" | "message" | "tool" | "mcp" | "other"; export type ReplyProgressStage = "started" | "delta" | "completed" | "info"; export interface ReplyProgress { + progressId?: string; source: ReplyProgressSource; kind: ReplyProgressKind; stage: ReplyProgressStage; diff --git a/frontend/src/features/agents/CustomAgentChatView.test.tsx b/frontend/src/features/agents/CustomAgentChatView.test.tsx index 95f29984..37c50555 100644 --- a/frontend/src/features/agents/CustomAgentChatView.test.tsx +++ b/frontend/src/features/agents/CustomAgentChatView.test.tsx @@ -1259,6 +1259,92 @@ describe("CustomAgentChatView", () => { expect(screen.getByText("Progress").parentElement?.textContent).toContain("done"); }); + it("updates message progress by progressId instead of appending running and done rows", async () => { + let emitChunk: ((chunk: unknown) => void) | null = null; + const agent = { + launchAgentChat: vi.fn(), + reattachAgentChat: vi.fn(async (sessionId: string) => ({ + sessionId, + scrollback: [], + })), + sendAgentChat: vi.fn( + (_sessionId: string, _prompt: string, onChunk: (chunk: unknown) => void) => + new Promise((resolve) => { + emitChunk = (chunk: unknown) => { + onChunk(chunk); + if ((chunk as { kind?: string }).kind === "final") resolve(); + }; + }), + ), + cancelAgentChat: vi.fn(async () => {}), + closeAgentChat: vi.fn(async () => {}), + }; + + render( + null) }, + } as unknown as Gateways} + > + + , + ); + + await waitFor(() => + expect(agent.reattachAgentChat).toHaveBeenCalledWith( + "chat-session-1", + expect.any(Function), + ), + ); + + fireEvent.change(screen.getByLabelText(/message CLI custom/), { + target: { value: "ship progress ids" }, + }); + fireEvent.click(screen.getByRole("button", { name: "Envoyer" })); + await waitFor(() => expect(emitChunk).not.toBeNull()); + + const progress = (stage: "delta" | "completed", text: string) => ({ + kind: "progress" as const, + progress: { + progressId: "codex:message:item-1", + source: "providerNative" as const, + kind: "message" as const, + stage, + label: "Analyse", + text, + }, + }); + + act(() => { + emitChunk?.(progress("delta", "Lecture du contexte disponible.")); + }); + + expect(screen.getAllByTestId("chat-progress-status")).toHaveLength(1); + expect(screen.getByText("Lecture du contexte disponible.")).toBeTruthy(); + expect(screen.getByText("Progress").parentElement?.textContent).toContain("running"); + + act(() => { + emitChunk?.(progress("completed", "Contexte lu.")); + }); + + expect(screen.getAllByTestId("chat-progress-status")).toHaveLength(1); + expect(screen.queryByText("Lecture du contexte disponible.")).toBeNull(); + expect(screen.getByText("Contexte lu.")).toBeTruthy(); + expect(screen.getByText("Progress").parentElement?.textContent).toContain("done"); + }); + it("does not append late progress chunks after the final answer", async () => { let emitChunk: ((chunk: unknown) => void) | null = null; const agent = { diff --git a/frontend/src/features/agents/CustomAgentChatView.tsx b/frontend/src/features/agents/CustomAgentChatView.tsx index 7935043a..07c15f77 100644 --- a/frontend/src/features/agents/CustomAgentChatView.tsx +++ b/frontend/src/features/agents/CustomAgentChatView.tsx @@ -214,6 +214,7 @@ function normalizeProgress(raw: unknown): ReplyProgress | null { const label = optionalString(record.label); if (!label) return null; return { + progressId: optionalString(record.progressId), source: isReplyProgressSource(record.source) ? record.source : "providerNative", kind: isReplyProgressKind(record.kind) ? record.kind : "other", stage: isReplyProgressStage(record.stage) ? record.stage : "info", @@ -227,6 +228,7 @@ function normalizeProgress(raw: unknown): ReplyProgress | null { function progressKey(progress: ReplyProgress): string { return [ + progress.progressId ?? "", progress.source, progress.kind, progress.stage, @@ -248,6 +250,7 @@ function isDelegationProgress(progress: ReplyProgress): boolean { } function progressEntityKey(progress: ReplyProgress): string | null { + if (progress.progressId) return `id:${progress.progressId}`; const name = normalizedProgressToolName(progress); if (!name) return null; if (isDelegationProgress(progress)) return `delegation:${name}`; @@ -256,7 +259,7 @@ function progressEntityKey(progress: ReplyProgress): string | null { } function progressStageBucket(progress: ReplyProgress): string { - if (!isCenteredToolProgress(progress)) return progress.stage; + if (!progressEntityKey(progress)) return progress.stage; if (progress.stage === "started" || progress.stage === "delta") return "started"; if (progress.stage === "completed") return "completed"; return "info";