fix: prevent consecutive messages with same role

[louis-jan]

Describe Your Changes

This PR fixes an issue where two consecutive messages with the same role (user or assistant) could appear in the conversation history.

Changes

• Insert a fallback message (".") with the opposite role when two consecutive messages share the same role.
• Ensures the message sequence always alternates between user and assistant.

Fixes Issues

• Closes bug: API request failed with 500 due to wrongly formatted compeltions API #6254

Self Checklist

• Added relevant comments, esp in complex areas
• Updated docs (for bug fixes / features)
• Created issues for follow-up changes or refactoring needed

---

Important

Fixes consecutive message role issue by inserting fallback messages to ensure alternation between user and assistant roles in CompletionMessagesBuilder.

• Behavior:
  • In CompletionMessagesBuilder in messages.ts, insert a fallback message . with the opposite role when two consecutive messages share the same role.
  • Ensures message sequence alternates between user and assistant roles.
• Tests:
  • Update tests in messages.test.ts to verify insertion of fallback messages between consecutive messages of the same role.
  • Modify tests in useChat.test.ts and useChat.instructions.test.ts to align with new message alternation behavior.
  • Adjust tests in ChatInput.test.tsx and sheet.test.tsx to ensure consistent behavior with new message handling.

^{This description was created by for a749b05. You can customize this summary. It will automatically update as commits are pushed.}

[ellipsis-dev]

Caution

Changes requested ❌

Reviewed everything up to c07abc6 in 1 minute and 55 seconds. Click for details.

• Reviewed 229 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 2 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. web-app/src/lib/messages.ts:85

• Draft comment:
  In addUserMessage(), the logic pops a duplicate 'user' message instead of inserting a fallback. Verify that handling consecutive user messages differently from consecutive assistant messages is intentional.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

2. web-app/src/hooks/useChat.ts:87

• Draft comment:
  Directly accessing state via getState() (e.g. useAssistant.getState()) inside callbacks is acceptable with Zustand, but ensure this pattern is intentional to always get fresh state rather than relying on hook subscriptions.
• Reason this comment was not posted:
  Confidence changes required: 30% <= threshold 50% None

Workflow ID: wflow_MOkTinqRvrdLDi2j

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed c613752 in 1 minute and 52 seconds. Click for details.

• Reviewed 619 lines of code in 5 files
• Skipped 0 files when reviewing.
• Skipped posting 5 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. web-app/src/components/ui/__tests__/sheet.test.tsx:55

• Draft comment:
  Added in SheetContent tests to improve component coverage. Consider adding style assertions if needed.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is purely informative, mentioning the addition of in tests. It suggests considering style assertions, but this is not a specific request or suggestion. It doesn't ask for a specific test to be added or a specific change to be made, which would make it actionable.

2. web-app/src/containers/__tests__/ChatInput.test.tsx:191

• Draft comment:
  Wrapping render and event calls in async act() improves test reliability and handles asynchronous behavior correctly.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is purely informative, explaining the benefits of using async act() in tests. It doesn't provide a specific suggestion or ask for a change in the code.

3. web-app/src/hooks/__tests__/useChat.instructions.test.ts:173

• Draft comment:
  The try-catch wrapper around the async act call aids in logging errors during message processing. Consider asserting on errors if applicable for stricter test validation.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The test's primary purpose is to verify date replacement in instructions. The try-catch seems to be added for debugging purposes, not as a core part of the test assertions. Adding error assertions would shift focus from the main test purpose and could make the test more brittle. The current error logging is sufficient for debugging. The comment has a point that error handling could be more robust. Silently catching and logging errors might hide important test failures. However, this test is specifically about instruction rendering. Adding error assertions would be scope creep and could make the test less maintainable. The comment should be removed as it suggests adding complexity that's not aligned with the test's core purpose of verifying instruction rendering.

4. web-app/src/hooks/__tests__/useChat.test.ts:59

• Draft comment:
  Refactoring the useModelProvider mock with Object.assign to include getState ensures consistency across tests, which is a good improvement.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is purely informative and does not provide any actionable feedback or suggestions for improvement. It simply states that a change is a good improvement, which violates the rule against making purely informative comments.

5. web-app/src/lib/__tests__/messages.test.ts:64

• Draft comment:
  Updated CompletionMessagesBuilder tests now account for filler messages inserted between consecutive messages with the same role. The revised expected array lengths and role orders (with a filler message having role 'assistant' and content '.') reflect the intended alternating behavior. Verify consistency in filler message content for edge cases.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is purely informative, explaining what changes were made to the tests and what they now account for. It does not provide a specific suggestion or ask for a specific confirmation about the code. It also asks to verify consistency, which is against the rules.

Workflow ID: wflow_yOTuKxd7QRjKZfKn

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[github-actions]

Barecheck - Code coverage report

Total: 30.56%

Your code coverage diff: -0.19% ▾

Uncovered files and lines

File	Lines
web-app/src/hooks/useChat.ts	87-104, 110-119, 129-151, 154, 156, 158, 161, 164-169, 171-172, 177-179, 182-186, 189, 192-194, 196-206, 248-249, 253, 257-261, 267-268, 270-273, 275-288, 290-301, 303-310, 313-317, 319-322, 324-334, 336-340, 342-345, 347-374, 376-394, 396-398, 401-407, 409-410, 412-418, 420-425, 427-430, 432-440, 443, 445, 447-462, 464-469, 471-480, 482-492, 495-502, 504-516, 518, 521, 523-525, 531-532
web-app/src/lib/messages.ts	36-57, 92-97, 100-110, 112-115

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed 8af250a in 1 minute and 30 seconds. Click for details.

• Reviewed 50 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 2 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. web-app/src/lib/messages.ts:169

• Draft comment:
  The block handling the first message forces a fallback user message if the initial message isn’t 'user' or 'system'. Please confirm that inserting a fallback ('.') for non-user/system first messages (e.g. 'assistant') aligns with the desired conversation flow. Also, consider adding a comment to clarify this edge-case behavior.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

2. web-app/src/lib/messages.ts:194

• Draft comment:
  When consecutive messages share the same role, a fallback message with the opposite role and content '.' is inserted. Note that the logic uses a ternary operator (prevRole === 'assistant' ? 'user' : 'assistant'), which assumes only 'user' and 'assistant' roles. If additional roles (e.g. 'system') occur later, this may behave unexpectedly. Please consider documenting the fallback content and logic.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_z0Lfza3dZDzpN7NH

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed a749b05 in 1 minute and 58 seconds. Click for details.

• Reviewed 252 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 7 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. web-app/src/lib/__tests__/messages.test.ts:47

• Draft comment:
  The system instruction test now correctly expects a filler message (user role with '.') immediately after the system message, ensuring role alternation.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

2. web-app/src/lib/__tests__/messages.test.ts:68

• Draft comment:
  The 'should filter out messages with errors' test now expects three messages: the valid first user message, an inserted filler (assistant role), and the second user message, which properly prevents consecutive same-role messages.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

3. web-app/src/lib/__tests__/messages.test.ts:177

• Draft comment:
  The assistant message test correctly inserts a filler user message before adding the normalized assistant message, ensuring proper alternating roles.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

4. web-app/src/lib/__tests__/messages.test.ts:305

• Draft comment:
  The multiple tool messages test now expects four messages with appropriate filler messages inserted between consecutive tool messages, which ensures alternation (initial filler 'user' and filler 'assistant' between tool messages).
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

5. web-app/src/lib/__tests__/messages.test.ts:348

• Draft comment:
  Normalization tests (e.g. 'should remove thinking content from the beginning') have been adjusted to check index 1 for the assistant message, accounting for the inserted filler message at index 0.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

6. web-app/src/lib/__tests__/messages.test.ts:532

• Draft comment:
  The integration test for complex conversation flow now verifies a sequence of 7 messages with filler messages correctly inserted to alternate roles, reflecting complex interactions properly.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

7. web-app/src/lib/__tests__/messages.test.ts:549

• Draft comment:
  The test for empty thread messages with a system instruction now correctly expects a filler user message following the system message.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_ZYJg7XJC7F06UO8N

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[Minh141120]

LGTM!