Skip to content

fix: handle case when last message is not from user without throwing … - #33

Merged
mdjastrzebski merged 8 commits into
mainfrom
fix/last-message-from-users-fix
Dec 16, 2024
Merged

fix: handle case when last message is not from user without throwing …#33
mdjastrzebski merged 8 commits into
mainfrom
fix/last-message-from-users-fix

Conversation

@Q1w1N

@Q1w1N Q1w1N commented Dec 12, 2024

Copy link
Copy Markdown
Collaborator

Slack app error: UnknownError: Last message in the "messages" list should be a "UserMessage”

Summary

Added logic that removes last messages from AI

Test plan

Tested with websocket
Could add some units though

@Q1w1N
Q1w1N requested a review from mdjastrzebski December 12, 2024 13:41
Comment thread .changeset/gorgeous-jobs-protect.md Outdated
Comment thread packages/core/src/__tests__/application.test.ts Outdated
Comment thread packages/core/src/__tests__/application.test.ts Outdated
Comment thread packages/core/src/__tests__/application.test.ts Outdated

@mdjastrzebski mdjastrzebski left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

modify MockChatModel to capture arguments passed to processMessage. Do not mutate passed messages, make a copy

Comment thread packages/core/src/application.ts Outdated
Comment thread packages/core/src/application.ts Outdated
if (lastMessage?.role !== 'user') {
throw new Error('Last message in the "messages" list should be a "UserMessage"');
}
const cleanMessages = ignoreLastAssistantMessage(messages);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

edge-case: It is technically possible that after ignoreLastAssistantMessage there will be zero messages in the array

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Not really, as this would require AI to trigger itself with a call.

Comment thread packages/core/src/application.ts Outdated
Comment thread packages/core/src/application.ts Outdated
Comment thread packages/core/src/__tests__/application.test.ts Outdated
Comment thread packages/core/src/__tests__/application.test.ts Outdated
Comment thread packages/core/src/__tests__/application.test.ts Outdated
Comment thread packages/core/src/__tests__/application.test.ts Outdated
Comment thread packages/core/src/__tests__/application.test.ts
Comment thread packages/core/src/application.ts
@mdjastrzebski
mdjastrzebski merged commit 6152fb3 into main Dec 16, 2024
@mdjastrzebski
mdjastrzebski deleted the fix/last-message-from-users-fix branch December 16, 2024 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants