Skip to content

Commit

Permalink
Make fallback user mentions support also match against the bot's room…
Browse files Browse the repository at this point in the history
…-specific username

It seems like Element iOS benefits from this.
  • Loading branch information
spantaleev committed Oct 3, 2024
1 parent 393be9b commit d9a045a
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 26 deletions.
18 changes: 18 additions & 0 deletions src/bot/implementation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,24 @@ impl Bot {
self.matrix_link().user_id()
}

pub(crate) async fn user_display_name_in_room(&self, room: &Room) -> Option<String> {
let bot_display_name = self
.room_display_name_fetcher()
.own_display_name_in_room(room)
.await;

match bot_display_name {
Ok(value) => value,
Err(err) => {
tracing::warn!(
?err,
"Failed to fetch bot display name. Proceeding without it"
);
None
}
}
}

pub(crate) fn reacting(&self) -> super::reacting::Reacting {
super::reacting::Reacting::new(self.clone())
}
Expand Down
6 changes: 5 additions & 1 deletion src/bot/messaging.rs
Original file line number Diff line number Diff line change
Expand Up @@ -239,8 +239,11 @@ impl Messaging {
}
};

let bot_display_name = self.bot.user_display_name_in_room(&room).await;

let interaction_context = determine_interaction_context_for_room_event(
self.bot.user_id(),
&bot_display_name,
&room,
&event,
&payload,
Expand Down Expand Up @@ -279,7 +282,8 @@ impl Messaging {
self.bot.admin_pattern_regexes().clone(),
trigger_event_info,
interaction_context.thread_info.clone(),
);
)
.with_bot_display_name(bot_display_name);

let controller_type = crate::controller::determine_controller(
self.bot.command_prefix(),
Expand Down
22 changes: 4 additions & 18 deletions src/controller/chat_completion/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -383,24 +383,10 @@ async fn handle_stage_text_generation(
_ => vec![],
};

let bot_display_name = bot
.room_display_name_fetcher()
.own_display_name_in_room(message_context.room())
.await;

let bot_display_name = match bot_display_name {
Ok(value) => value,
Err(err) => {
tracing::warn!(
?err,
"Failed to fetch bot display name. Proceeding without it"
);
None
}
};

let bot_user_prefixes_to_strip =
create_list_of_bot_user_prefixes_to_strip(bot.user_id(), &bot_display_name);
let bot_user_prefixes_to_strip = create_list_of_bot_user_prefixes_to_strip(
bot.user_id(),
message_context.bot_display_name(),
);

let allowed_users = match controller_type {
// Regular chat completion only operates on messages from allowed users.
Expand Down
28 changes: 21 additions & 7 deletions src/conversation/matrix/utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -277,13 +277,14 @@ pub fn convert_matrix_native_event_to_matrix_message(
#[tracing::instrument(name = "determine_interaction_context_for_room_event", skip_all, fields(room_id = room.room_id().as_str(), event_id = current_event.event_id.as_str()))]
pub async fn determine_interaction_context_for_room_event(
bot_user_id: &OwnedUserId,
bot_display_name: &Option<String>,
room: &Room,
current_event: &OriginalSyncRoomMessageEvent,
current_event_payload: &MessagePayload,
event_fetcher: &Arc<RoomEventFetcher>,
) -> anyhow::Result<Option<InteractionContext>> {
let current_event_is_mentioning_bot =
is_event_mentioning_bot(&current_event.content, bot_user_id);
is_event_mentioning_bot(&current_event.content, bot_user_id, bot_display_name);

let Some(relation) = &current_event.content.relates_to else {
// This is a top-level message. We consider it the start of the thread.
Expand All @@ -305,6 +306,7 @@ pub async fn determine_interaction_context_for_room_event(
Relation::Thread(thread) => {
determine_interaction_context_for_room_event_related_to_thread(
bot_user_id,
bot_display_name,
room,
current_event,
event_fetcher,
Expand All @@ -329,6 +331,7 @@ pub async fn determine_interaction_context_for_room_event(

async fn determine_interaction_context_for_room_event_related_to_thread(
bot_user_id: &OwnedUserId,
bot_display_name: &Option<String>,
room: &Room,
current_event: &OriginalSyncRoomMessageEvent,
event_fetcher: &Arc<RoomEventFetcher>,
Expand Down Expand Up @@ -389,6 +392,7 @@ async fn determine_interaction_context_for_room_event_related_to_thread(
thread_start_timeline_event,
thread_info.clone(),
bot_user_id,
bot_display_name,
)?;

let Some(detailed_message_payload) = thread_start_detailed_message_payload else {
Expand Down Expand Up @@ -431,6 +435,7 @@ async fn determine_interaction_context_for_room_event_related_to_reply(
fn is_event_mentioning_bot(
event_content: &RoomMessageEventContent,
bot_user_id: &OwnedUserId,
bot_display_name: &Option<String>,
) -> bool {
if let Some(mentions) = &event_content.mentions {
mentions
Expand All @@ -445,12 +450,17 @@ fn is_event_mentioning_bot(
// As of 2024-10-03, at least Element iOS does not support the new Mentions specification
// and is still quite widespread.
//
// It may be even better to match not only against the MXID, but also against the bot's
// room-specific display name.
//
// We may consider dropping this string-matching behavior altogether in the future,
// so improving this compatibility block is not a high priority.
event_content.body().contains(bot_user_id.as_str())
if event_content.body().contains(bot_user_id.as_str()) {
return true;
}

if let Some(bot_display_name) = bot_display_name {
return event_content.body().contains(bot_display_name);
}

false
}
}

Expand All @@ -459,6 +469,7 @@ fn timeline_event_to_detailed_message_payload(
timeline_event: TimelineEvent,
thread_info: ThreadInfo,
bot_user_id: &OwnedUserId,
bot_display_name: &Option<String>,
) -> anyhow::Result<Option<DetailedMessagePayload>> {
let timeline_event_deserialized = match timeline_event.event.deserialize() {
Ok(value) => value,
Expand Down Expand Up @@ -510,8 +521,11 @@ fn timeline_event_to_detailed_message_payload(
return Ok(None);
};

let is_mentioning_bot =
is_event_mentioning_bot(&room_message_original.content, bot_user_id);
let is_mentioning_bot = is_event_mentioning_bot(
&room_message_original.content,
bot_user_id,
bot_display_name,
);

(is_mentioning_bot, room_message_payload)
} else {
Expand Down
13 changes: 13 additions & 0 deletions src/entity/message_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ pub struct MessageContext {
admin_whitelist_regexes: Vec<regex::Regex>,
trigger_event_info: TriggerEventInfo,
thread_info: ThreadInfo,

bot_display_name: Option<String>,
}

impl MessageContext {
Expand All @@ -31,9 +33,20 @@ impl MessageContext {
admin_whitelist_regexes,
trigger_event_info,
thread_info,

bot_display_name: None,
}
}

pub fn with_bot_display_name(mut self, value: Option<String>) -> Self {
self.bot_display_name = value;
self
}

pub fn bot_display_name(&self) -> &Option<String> {
&self.bot_display_name
}

pub fn room(&self) -> &Room {
&self.room
}
Expand Down

0 comments on commit d9a045a

Please sign in to comment.