Skip to content

fix(anthropic): add non-whitespace trailing dummy #1891

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.

Sign up for GitHub

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

Open
rosetta-livekit-bot wants to merge 1 commit into
base: fix/anthropic-retry-fresh-stream
Choose a base branch
from
Open

fix(anthropic): add non-whitespace trailing dummy #1891

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 7 additions & 0 deletions plugins/anthropic/src/llm.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@ export class LLM extends llm.LLM {
* strict alternating-turn requirement.
* - A dummy `(empty)` user message is injected if the conversation doesn't
* start with a user turn.
* - A dummy user message is appended if the conversation ends with an
* assistant turn, since Claude 4.6+ does not support prefilling.
*/
protected _buildAnthropicContext(chatCtx: llm.ChatContext): {
system: Anthropic.TextBlockParam[];
Expand Down Expand Up @@ -172,6 +174,11 @@ export class LLM extends llm.LLM {
messages.unshift({ role: 'user', content: '(empty)' });
}

// Claude 4.6+ does not support prefilling (trailing assistant messages).
if (messages.length > 0 && messages[messages.length - 1]!.role === 'assistant') {
messages.push({ role: 'user', content: [{ type: 'text', text: '.' }] });

@devin-ai-integration devin-ai-integration Bot Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🚩 Dummy message content '.' differs from existing placeholder style and may influence model output

The new trailing dummy message uses { type: 'text', text: '.' } at plugins/anthropic/src/llm.ts:179, while the existing leading dummy message uses the string '(empty)' at plugins/anthropic/src/llm.ts:174. Beyond the stylistic inconsistency, a single period could be interpreted by Claude as actual user input (e.g., prompting a "How can I help you?" response), whereas '(empty)' more clearly signals a placeholder. This is a design choice rather than a correctness bug, but it may affect response quality in conversations where the assistant's last message needs a follow-up.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

}
Comment on lines +177 to +180

@devin-ai-integration devin-ai-integration Bot Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 New trailing-message fix breaks existing unit test for message merging

A dummy user message is unconditionally appended when the last message is from the assistant (messages.push(...) at plugins/anthropic/src/llm.ts:179), but the existing test that ends with an assistant reply was not updated, so it will fail its length assertion.

Impact: The test suite will fail, blocking CI and preventing the PR from merging.

Test failure trace for "merges consecutive same-role messages"

The test at plugins/anthropic/src/llm.test.ts:83-100 builds a chat context with two user messages and one assistant reply (line 89). After merging consecutive user messages, _buildAnthropicContext produces a 2-element array. However, the new code at plugins/anthropic/src/llm.ts:178-180 detects that the last message is from the assistant and appends a dummy { role: 'user', content: [{ type: 'text', text: '.' }] } entry, growing the array to length 3. The assertion at plugins/anthropic/src/llm.test.ts:95 — expect(messages).toHaveLength(2) — therefore fails.

The test file was not modified by this PR, so this is an oversight. The test needs updating to expect the new trailing user message.

Prompt for agents 
The new code at plugins/anthropic/src/llm.ts:177-180 appends a dummy user message whenever the conversation ends with an assistant turn. This is the intended behavior for Claude 4.6+ compatibility, but the existing test 'merges consecutive same-role messages' at plugins/anthropic/src/llm.test.ts:83-100 was not updated. That test builds a context ending with an assistant message (line 89) and asserts messages.toHaveLength(2) at line 95, which will now be 3.

The fix is to update the test at plugins/anthropic/src/llm.test.ts:
1. Change the length assertion at line 95 from toHaveLength(2) to toHaveLength(3)
2. Add an assertion for the new trailing dummy user message (e.g. expect(messages[2].role).toBe('user'))
3. Consider also adding a dedicated test case for the new trailing-message behavior, similar to the existing test at line 102 for the leading-message injection.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.


return { system, messages };
}

Expand Down