fix: normalize trailing assistant messages - #222
Conversation
77efe5e to
7090b64
Compare
7090b64 to
53cc6d0
Compare
Greptile SummaryThis PR fixes two related issues in
Confidence Score: 4/5The changes are narrowly scoped to two paths in transformBody and are well-covered by regression tests; the risk of introducing new breakage is low. Both changes are correct for the described compaction scenarios. The one gap is that the trailing-assistant guard appends a plain-text user turn unconditionally, without first checking whether the last assistant message still contains tool_use blocks after repairToolPairs. In the rare case of a structurally unusual compaction output, that would produce a request the API rejects for a different reason than the one this PR targets. Files Needing Attention: The trailing-assistant block in src/transforms.ts (lines 267-278) is the one spot worth a second look.
|
| Filename | Overview |
|---|---|
| src/transforms.ts | Two targeted changes: tool_result-aware splice for system-text relocation, and trailing-assistant normalization appended after repairToolPairs. Logic is sound for all realistic compaction outputs. |
| src/transforms.test.ts | Four new tests cover the two changed paths. The model-agnostic trailing-assistant test verifies length and roles but omits a deepEqual on the appended content block. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[transformBody called] --> B{body is string?}
B -- No --> Z[return body as-is]
B -- Yes --> C[Parse JSON]
C --> D[Add billing header to system]
D --> E[Split identity prefix]
E --> F{Non-core system entries?}
F -- Yes --> G[Find first user message]
G --> H{First user content is array?}
H -- Yes --> I[findIndex: first non-tool_result block]
I --> J{insertAt == -1?}
J -- Yes --> K[Splice text AFTER last tool_result]
J -- No --> L[Splice text BEFORE first non-tool_result]
H -- No --> M[Prepend text to string content]
K --> N[Strip effort if disableEffort model]
L --> N
M --> N
F -- No --> N
N --> O[Prefix tool names]
O --> P[repairToolPairs]
P --> Q{Last message role == assistant?}
Q -- Yes --> R[Push synthetic user turn]
Q -- No --> S[JSON.stringify and return]
R --> S
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
src/transforms.ts:267-278
**Trailing-assistant guard does not check for active `tool_use` blocks**
After `repairToolPairs`, a last assistant message can still contain `tool_use` blocks if those blocks are considered non-orphaned (i.e. there happens to be a matching `tool_result` earlier in the (already broken) conversation). In that case the code appends a plain-text user turn, producing a conversation where the `tool_use` call is never answered by a `tool_result` — a structure the API rejects with a different error than the one this PR targets. Adding a check that the last assistant message contains no `tool_use` content before appending the synthetic turn would make the guard more defensive against unusual compaction outputs.
### Issue 2 of 2
src/transforms.test.ts:881-898
**Second trailing-assistant test omits content assertion**
The Opus 4.7 test (above) verifies the exact shape of the appended content block with `deepEqual`, but this model-agnostic variant only checks `messages.length` and `messages[2].role`. Adding a `deepEqual` on `messages[2].content` here would confirm the injected text is consistent across models and guard against a future regression where the content accidentally diverges per model.
Reviews (1): Last reviewed commit: "fix: normalize trailing assistant messag..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const lastMessage = parsed.messages.at(-1) | ||
| if (lastMessage?.role === "assistant") { | ||
| parsed.messages.push({ | ||
| role: "user", | ||
| content: [ | ||
| { | ||
| type: "text", | ||
| text: "Please pause and wait for further instructions.", | ||
| }, | ||
| ], | ||
| }) | ||
| } |
There was a problem hiding this comment.
tool_use blocks
After repairToolPairs, a last assistant message can still contain tool_use blocks if those blocks are considered non-orphaned (i.e. there happens to be a matching tool_result earlier in the (already broken) conversation). In that case the code appends a plain-text user turn, producing a conversation where the tool_use call is never answered by a tool_result — a structure the API rejects with a different error than the one this PR targets. Adding a check that the last assistant message contains no tool_use content before appending the synthetic turn would make the guard more defensive against unusual compaction outputs.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/transforms.ts
Line: 267-278
Comment:
**Trailing-assistant guard does not check for active `tool_use` blocks**
After `repairToolPairs`, a last assistant message can still contain `tool_use` blocks if those blocks are considered non-orphaned (i.e. there happens to be a matching `tool_result` earlier in the (already broken) conversation). In that case the code appends a plain-text user turn, producing a conversation where the `tool_use` call is never answered by a `tool_result` — a structure the API rejects with a different error than the one this PR targets. Adding a check that the last assistant message contains no `tool_use` content before appending the synthetic turn would make the guard more defensive against unusual compaction outputs.
How can I resolve this? If you propose a fix, please make it concise.
| it("transformBody appends a user turn for any model ending with assistant", () => { | ||
| const input = JSON.stringify({ | ||
| model: "claude-sonnet-4-5", | ||
| messages: [ | ||
| { role: "user", content: "summarize the session" }, | ||
| { role: "assistant", content: "Here is the summary." }, | ||
| ], | ||
| }) | ||
|
|
||
| const output = transformBody(input) | ||
| const parsed = JSON.parse(output as string) as { | ||
| messages: Array<{ role: string; content: unknown }> | ||
| } | ||
|
|
||
| assert.equal(parsed.messages.length, 3) | ||
| assert.equal(parsed.messages[1].role, "assistant") | ||
| assert.equal(parsed.messages[2].role, "user") | ||
| }) |
There was a problem hiding this comment.
The Opus 4.7 test (above) verifies the exact shape of the appended content block with deepEqual, but this model-agnostic variant only checks messages.length and messages[2].role. Adding a deepEqual on messages[2].content here would confirm the injected text is consistent across models and guard against a future regression where the content accidentally diverges per model.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/transforms.test.ts
Line: 881-898
Comment:
**Second trailing-assistant test omits content assertion**
The Opus 4.7 test (above) verifies the exact shape of the appended content block with `deepEqual`, but this model-agnostic variant only checks `messages.length` and `messages[2].role`. Adding a `deepEqual` on `messages[2].content` here would confirm the injected text is consistent across models and guard against a future regression where the content accidentally diverges per model.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
2 participants
Summary
assistantturn.assistant.tool_resultblocks when relocating system text into the first user message.Rationale
After OpenCode compaction, some sessions can produce a follow-up request whose final message is an
assistantmessage. Models such as Opus 4.7 reject that request as unsupported assistant prefill with:This model does not support assistant message prefill. The conversation must end with a user message.This keeps the fix model-agnostic so the plugin does not need per-model maintenance as new Anthropic models are released.
The inserted user turn asks the model to pause and wait for further instructions, which keeps manual compaction from immediately continuing work while still satisfying Anthropic's requirement that normal chat requests end with a user message.
The system-text relocation path now inserts after any leading
tool_resultblocks. This keeps compaction requests valid when the first user message is the required tool-result response to a preceding assistant tool-use.Closes #212.
Verification
pnpm testpnpm run build