Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ai): history fix #423

Merged
merged 8 commits into from
Feb 23, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 12 additions & 13 deletions src/api/ai_help.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ use crate::{
db::{
self,
ai_help::{
add_help_history, add_help_history_message, create_or_increment_total,
delete_full_help_history, delete_help_history, get_count, help_history,
help_history_get_message, list_help_history, update_help_history_label, AI_HELP_LIMIT,
add_help_history_message, create_or_increment_total, delete_full_help_history,
delete_help_history, get_count, help_history, help_history_get_message,
list_help_history, update_help_history_label, AI_HELP_LIMIT,
},
model::{AIHelpHistoryMessage, AIHelpHistoryMessageInsert, Settings},
model::{AIHelpHistoryMessage, AIHelpHistoryMessageInsert, Settings, UserQuery},
settings::get_settings,
SupaPool,
},
Expand Down Expand Up @@ -195,7 +195,7 @@ fn record_question(
pool: &Data<Pool>,
message: &ChatCompletionRequestMessage,
history_enabled: bool,
user_id: i64,
user: &UserQuery,
help_ids: HelpIds,
) -> Result<Option<NaiveDateTime>, ApiError> {
if !history_enabled {
Expand All @@ -207,11 +207,9 @@ fn record_question(
message_id,
parent_id,
} = help_ids;
if let Err(err) = add_help_history(&mut conn, user_id, chat_id) {
error!("AI Help log: {err}");
}

let insert = AIHelpHistoryMessageInsert {
user_id,
user_id: user.id,
chat_id,
message_id,
parent_id,
Expand All @@ -233,7 +231,7 @@ fn record_sources(
pool: &Data<Pool>,
sources: &Vec<RefDoc>,
history_enabled: bool,
user_id: i64,
user: &UserQuery,
help_ids: HelpIds,
) -> Result<Option<NaiveDateTime>, ApiError> {
if !history_enabled {
Expand All @@ -245,8 +243,9 @@ fn record_sources(
message_id,
parent_id,
} = help_ids;

let insert = AIHelpHistoryMessageInsert {
user_id,
user_id: user.id,
chat_id,
message_id,
parent_id,
Expand Down Expand Up @@ -393,7 +392,7 @@ pub async fn ai_help(
&diesel_pool,
question,
history_enabled(&settings),
user.id,
&user,
help_ids,
)?;
}
Expand All @@ -405,7 +404,7 @@ pub async fn ai_help(
&diesel_pool,
&sources,
history_enabled(&settings),
user.id,
&user,
help_ids,
)? {
Some(x) => Utc.from_utc_datetime(&x),
Expand Down
63 changes: 49 additions & 14 deletions src/db/ai_help.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,31 +132,63 @@ pub fn add_help_history(
.do_update()
.set(ai_help_history::updated_at.eq(diesel::dsl::now))
.execute(conn)?;

Ok(())
}

pub fn add_help_history_message(
pub fn update_help_history(
conn: &mut PgConnection,
mut message: AIHelpHistoryMessageInsert,
) -> Result<NaiveDateTime, DbError> {
let updated_at = update(ai_help_history::table)
user_id: i64,
chat_id: Uuid,
) -> Result<(), DbError> {
update(ai_help_history::table)
.filter(
ai_help_history::user_id
.eq(message.user_id)
.and(ai_help_history::chat_id.eq(message.chat_id)),
.eq(user_id)
.and(ai_help_history::chat_id.eq(chat_id)),
)
.set(ai_help_history::updated_at.eq(diesel::dsl::now))
.returning(ai_help_history::updated_at)
.get_result::<NaiveDateTime>(conn)?;
message.created_at = Some(updated_at);
insert_into(ai_help_history_messages::table)
.execute(conn)?;
Ok(())
}

pub fn add_help_history_message(
conn: &mut PgConnection,
message: AIHelpHistoryMessageInsert,
) -> Result<NaiveDateTime, DbError> {
// If a `parent_id` is present, we execute an update on the help history
// record because one of these are true:
// * We created a history record at the beginning of the conversation.
// * History was switched off, we did not create a record and the update
// will simply not match/change any record.
//
// With no `parent_id`, we create a new record because at this point, history
// _is_ enabled and we are at the start of a new conversation.
let res = if message.parent_id.is_some() {
update_help_history(conn, message.user_id, message.chat_id)
} else {
add_help_history(conn, message.user_id, message.chat_id)
};
if let Err(err) = res {
error!("AI Help log: {err}");
}

let res = insert_into(ai_help_history_messages::table)
.values(&message)
.on_conflict(ai_help_history_messages::message_id)
.do_update()
.set(&message)
.execute(conn)?;
Ok(updated_at)
.returning(ai_help_history_messages::created_at)
.get_result::<NaiveDateTime>(conn);
match res {
Ok(created_at) => Ok(created_at),
// Ignore foreign key violations deliberately
// because of the edge cases described above
Err(diesel::result::Error::DatabaseError(
diesel::result::DatabaseErrorKind::ForeignKeyViolation,
_info,
)) => Ok(Utc::now().naive_utc()),
Err(e) => Err(e.into()),
}
}

pub fn help_history_get_message(
Expand Down Expand Up @@ -255,7 +287,10 @@ pub fn update_help_history_label(
.eq(user.id)
.and(ai_help_history::chat_id.eq(chat_id)),
)
.set(ai_help_history::label.eq(label))
.set((
ai_help_history::label.eq(label),
ai_help_history::updated_at.eq(diesel::dsl::now),
))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.set((
ai_help_history::label.eq(label),
ai_help_history::updated_at.eq(diesel::dsl::now),
))
.set(ai_help_history::label.eq(label))

I see that the naming is bad but updated_at refers to the messages and not the with that chat id and the label is not considered an update. Currently (if not mistaken) the front-end will trigger the label creation even for old chats (if the user closed the tab before the label was generated). And those would receive a "wrong"/ misleading updated_at.

.execute(conn)?;
Ok(())
}
Loading