fix(memory): address participant context review feedback

Author: vsumner

src/agent/channel.rs

@@ -86,6 +86,25 @@ fn should_flush_coalesce_buffer_for_event(event: &ProcessEvent) -> bool {
     )
 }
 
+fn sentence_contains_decision_marker(sentence: &str, explicit_markers: &[&str]) -> bool {
+    let sentence_lower = sentence.to_ascii_lowercase();
+    explicit_markers
+        .iter()
+        .any(|marker| sentence_lower.contains(marker))
+        || (sentence_lower.contains(" instead of ")
+            && [
+                "use ",
+                "switch",
+                "adopt ",
+                "choose ",
+                "pick ",
+                "go with ",
+                "proceed with ",
+            ]
+            .iter()
+            .any(|marker| sentence_lower.contains(marker)))
+}
+
 fn extract_decision_summary_from_reply(reply_text: &str) -> Option<String> {
     let normalized = reply_text.split_whitespace().collect::<Vec<_>>().join(" ");
     let trimmed = normalized.trim();
@@ -136,9 +155,17 @@ fn extract_decision_summary_from_reply(reply_text: &str) -> Option<String> {
         return None;
     }
 
-    let mut summary = trimmed
+    let sentences: Vec<&str> = trimmed
         .split_terminator(['.', '!', '?', '\n'])
-        .find(|sentence| !sentence.trim().is_empty())
+        .map(str::trim)
+        .filter(|sentence| !sentence.is_empty())
+        .collect();
+
+    let mut summary = sentences
+        .iter()
+        .copied()
+        .find(|sentence| sentence_contains_decision_marker(sentence, &explicit_markers))
+        .or_else(|| sentences.first().copied())
         .unwrap_or(trimmed)
         .trim()
         .to_string();
@@ -152,6 +179,23 @@ fn extract_decision_summary_from_reply(reply_text: &str) -> Option<String> {
     Some(summary)
 }
 
+fn decision_user_id(
+    humans: &[crate::config::HumanDef],
+    message: &InboundMessage,
+    is_retrigger: bool,
+) -> Option<String> {
+    if is_retrigger || message.source == "system" {
+        return None;
+    }
+
+    let source = message.source.trim();
+    if source.is_empty() || message.sender_id.is_empty() {
+        return None;
+    }
+
+    Some(participant_memory_key(humans, source, &message.sender_id))
+}
+
 /// Shared state that channel tools need to act on the channel.
 ///
 /// Wrapped in Arc and passed to tools (branch, spawn_worker, route, cancel)
@@ -556,6 +600,27 @@ impl Drop for MessageDurationGuard {
 }
 
 impl Channel {
+    fn record_decision_event(&self, reply_text: Option<&str>, user_id: Option<String>) {
+        let Some(decision_summary) = reply_text.and_then(extract_decision_summary_from_reply)
+        else {
+            return;
+        };
+
+        let mut event = self
+            .deps
+            .working_memory
+            .emit(
+                crate::memory::WorkingMemoryEventType::Decision,
+                decision_summary,
+            )
+            .channel(self.id.as_ref())
+            .importance(0.8);
+        if let Some(user_id) = user_id {
+            event = event.user(user_id);
+        }
+        event.record();
+    }
+
     /// Create a new channel.
     ///
     /// All tunable config (prompts, routing, thresholds, browser, skills) is read
@@ -1587,7 +1652,7 @@ impl Channel {
         }
 
         // Run agent turn with any image/audio attachments preserved
-        let (result, skip_flag, replied_flag, _, _) = self
+        let (result, skip_flag, replied_flag, _, reply_text) = self
             .run_agent_turn(
                 &combined_text,
                 &system_prompt,
@@ -1600,6 +1665,9 @@ impl Channel {
 
         self.handle_agent_result(result, &skip_flag, &replied_flag, false)
             .await;
+        if replied_flag.load(std::sync::atomic::Ordering::Relaxed) {
+            self.record_decision_event(reply_text.as_deref(), None);
+        }
         // Check compaction
         if let Err(error) = self.compactor.check_and_compact().await {
             tracing::warn!(channel_id = %self.id, %error, "compaction check failed");
@@ -1954,34 +2022,10 @@ impl Channel {
         self.handle_agent_result(result, &skip_flag, &replied_flag, is_retrigger)
             .await;
 
-        if replied_flag.load(std::sync::atomic::Ordering::Relaxed)
-            && let Some(decision_summary) = reply_text
-                .as_deref()
-                .and_then(extract_decision_summary_from_reply)
-        {
-            let user_id = if message.source.trim().is_empty() || message.sender_id.is_empty() {
-                None
-            } else {
-                let humans = self.deps.humans.load();
-                Some(participant_memory_key(
-                    humans.as_ref(),
-                    message.source.trim(),
-                    &message.sender_id,
-                ))
-            };
-            let mut event = self
-                .deps
-                .working_memory
-                .emit(
-                    crate::memory::WorkingMemoryEventType::Decision,
-                    decision_summary,
-                )
-                .channel(self.id.as_ref())
-                .importance(0.8);
-            if let Some(user_id) = user_id {
-                event = event.user(user_id);
-            }
-            event.record();
+        if replied_flag.load(std::sync::atomic::Ordering::Relaxed) {
+            let humans = self.deps.humans.load();
+            let user_id = decision_user_id(humans.as_ref(), &message, is_retrigger);
+            self.record_decision_event(reply_text.as_deref(), user_id);
         }
 
         // Safety-net: in quiet mode, explicit mention/reply should never be dropped silently.
@@ -3739,7 +3783,7 @@ fn is_dm_conversation_id(conv_id: &str) -> bool {
 #[cfg(test)]
 mod tests {
     use super::{
-        QuietModeFallbackState, compute_listen_mode_invocation,
+        QuietModeFallbackState, compute_listen_mode_invocation, decision_user_id,
         extract_decision_summary_from_reply, is_dm_conversation_id, recv_channel_event,
         should_process_event_for_channel, should_send_discord_quiet_mode_ping_ack,
         should_send_quiet_mode_fallback,
@@ -3851,6 +3895,40 @@ mod tests {
             )
             .is_none()
         );
+        assert_eq!(
+            extract_decision_summary_from_reply("Got it. We'll switch to the new routing config.")
+                .as_deref(),
+            Some("We'll switch to the new routing config")
+        );
+    }
+
+    #[test]
+    fn decision_user_id_skips_retrigger_messages() {
+        let humans = vec![crate::config::HumanDef {
+            id: "victor".to_string(),
+            display_name: Some("Victor".to_string()),
+            role: None,
+            bio: None,
+            description: None,
+            discord_id: Some("12345".to_string()),
+            telegram_id: None,
+            slack_id: None,
+            email: None,
+        }];
+        let message = InboundMessage {
+            id: "message-1".to_string(),
+            source: "system".to_string(),
+            adapter: None,
+            conversation_id: "discord:chan-1".to_string(),
+            sender_id: "12345".to_string(),
+            agent_id: None,
+            content: crate::MessageContent::Text("retrigger".to_string()),
+            timestamp: chrono::Utc::now(),
+            metadata: HashMap::new(),
+            formatted_author: None,
+        };
+
+        assert!(decision_user_id(&humans, &message, true).is_none());
     }
 
     #[test]

src/api/channels.rs

@@ -612,7 +612,14 @@ pub(super) async fn inspect_prompt(
         &participant_config,
     )
     .await
-    .unwrap_or_default();
+    .unwrap_or_else(|error| {
+        tracing::warn!(
+            %error,
+            channel_id = %query.channel_id,
+            "failed to render participant context for prompt inspection"
+        );
+        String::new()
+    });
 
     // ── Available channels ──
     let available_channels = {

src/conversation/participants.rs

@@ -5,6 +5,8 @@ use chrono::{DateTime, Utc};
 
 use std::collections::HashMap;
 
+const MAX_DISPLAY_NAME_CHARS: usize = 80;
+
 /// Active participant state tracked for a live channel session.
 ///
 /// This is the bridge between today's lightweight config-backed participant
@@ -59,9 +61,7 @@ pub fn track_active_participant(
     let human = find_human_for_sender(humans, platform, &message.sender_id);
     let display_name = human
         .and_then(|entry| entry.display_name.as_deref())
-        .map(str::trim)
-        .filter(|name| !name.is_empty())
-        .map(ToOwned::to_owned)
+        .and_then(sanitize_display_name)
         .unwrap_or_else(|| participant_display_name(message));
     let participant_key = participant_memory_key(humans, platform, &message.sender_id);
     let fallback_key = format!("{platform}:{}", message.sender_id);
@@ -112,23 +112,45 @@ pub fn renderable_participants(
 /// Best-effort display name for an inbound message sender.
 pub fn participant_display_name(message: &InboundMessage) -> String {
     message
-        .metadata
-        .get("sender_display_name")
-        .and_then(|value| value.as_str())
-        .map(str::trim)
-        .filter(|name| !name.is_empty())
-        .map(ToOwned::to_owned)
+        .formatted_author
+        .as_deref()
+        .and_then(sanitize_display_name)
         .or_else(|| {
             message
-                .formatted_author
-                .as_deref()
-                .map(str::trim)
-                .filter(|name| !name.is_empty())
-                .map(ToOwned::to_owned)
+                .metadata
+                .get("sender_display_name")
+                .and_then(|value| value.as_str())
+                .and_then(sanitize_display_name)
         })
+        .or_else(|| sanitize_display_name(&message.sender_id))
         .unwrap_or_else(|| message.sender_id.clone())
 }
 
+fn sanitize_display_name(value: &str) -> Option<String> {
+    let collapsed = value
+        .chars()
+        .map(|character| {
+            if character.is_control() {
+                ' '
+            } else {
+                character
+            }
+        })
+        .collect::<String>();
+    let mut sanitized = collapsed.split_whitespace().collect::<Vec<_>>().join(" ");
+    if sanitized.is_empty() {
+        return None;
+    }
+
+    if sanitized.len() > MAX_DISPLAY_NAME_CHARS {
+        let boundary = sanitized.floor_char_boundary(MAX_DISPLAY_NAME_CHARS);
+        sanitized.truncate(boundary);
+        sanitized.push_str("...");
+    }
+
+    Some(sanitized)
+}
+
 fn find_human_for_sender<'a>(
     humans: &'a [HumanDef],
     platform: &str,
@@ -259,6 +281,27 @@ mod tests {
         );
     }
 
+    #[test]
+    fn participant_display_name_prefers_formatted_author() {
+        let mut message = test_message();
+        message.formatted_author = Some("Victor Summit".to_string());
+
+        assert_eq!(participant_display_name(&message), "Victor Summit");
+    }
+
+    #[test]
+    fn participant_display_name_sanitizes_user_input() {
+        let mut message = test_message();
+        message.formatted_author = Some("  Victor\n\t<System>\u{0007} ".repeat(10));
+
+        let display_name = participant_display_name(&message);
+        assert!(display_name.starts_with("Victor <System>"));
+        assert!(display_name.ends_with("..."));
+        assert!(display_name.len() <= MAX_DISPLAY_NAME_CHARS + 3);
+        assert!(!display_name.contains('\n'));
+        assert!(!display_name.contains('\u{0007}'));
+    }
+
     #[test]
     fn upgrades_fallback_participant_entry_without_duplicates() {
         let mut participants = HashMap::new();