Skip to content

Commit 9740663

Browse files
authored
πŸ€– Skip [CONTINUE] sentinel when user message follows (#176)
## Problem Currently we always add the `[CONTINUE]` sentinel after partial messages: ``` [user1] β†’ [partial assistant] β†’ [CONTINUE] β†’ [user2] ``` But when a user message already follows the partial, the sentinel is **redundant** - the user message itself provides the continuation signal. ## Solution **Conditionally skip the sentinel when unnecessary:** ```typescript // Before: Always add sentinel [user] β†’ [partial] β†’ [CONTINUE] β†’ [user] β†’ ... // After: Only add when needed [user] β†’ [partial] β†’ [user] β†’ ... // Skip - user follows [user] β†’ [partial] β†’ [CONTINUE] // Keep - last message [user] β†’ [partial] β†’ [CONTINUE] β†’ [assistant] // Keep - assistant follows ``` **Implementation:** ```typescript if (msg.role === "assistant" && msg.metadata?.partial) { const nextMsg = messages[i + 1]; // Only add sentinel if NO user message follows if (!nextMsg || nextMsg.role !== "user") { result.push(createSentinel()); } } ``` ## Benefits - βœ… **Saves tokens** - One fewer message per resume-with-user-message (common case!) - βœ… **More natural** - Models handle "user interrupts then continues" well - βœ… **Cleaner history** - No redundant synthetic messages - βœ… **Stateless logic** - Just looks at message array structure ## When Sentinel Is Added - Partial is **last message** (empty resume) - Partial followed by **assistant message** (rare but possible) ## When Sentinel Is Skipped - Partial followed by **user message** (most common!) - User interrupts stream and adds follow-up - This is the typical resume pattern ## Testing **Updated tests:** - βœ… Multi-partial test now expects 5 messages (not 6) - βœ… New test specifically for skip-sentinel behavior - βœ… All 298 unit tests pass **Test coverage:** 1. Sentinel added when partial is last message βœ… 2. Sentinel skipped when user follows partial βœ… 3. No synthetic messages when sentinel skipped βœ… ## Risk Assessment **Very low risk:** - Only affects case where sentinel was already redundant - Models naturally understand "user continues conversation" - Existing behavior unchanged for empty resumes - Stateless transform, no history dependencies ## Related Builds on #170 which changed sentinel text from `[INTERRUPTED]` to `[CONTINUE]`. _Generated with `cmux`_
1 parent e864cb4 commit 9740663

File tree

2 files changed

+65
-18
lines changed

2 files changed

+65
-18
lines changed

β€Žsrc/utils/messages/modelMessageTransform.test.tsβ€Ž

Lines changed: 42 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -508,12 +508,48 @@ describe("modelMessageTransform", () => {
508508

509509
const result = addInterruptedSentinel(messages);
510510

511-
// Should have 6 messages (4 original + 2 sentinels)
512-
expect(result).toHaveLength(6);
513-
expect(result[2].id).toBe("interrupted-assistant-1");
514-
expect(result[2].role).toBe("user");
515-
expect(result[5].id).toBe("interrupted-assistant-2");
516-
expect(result[5].role).toBe("user");
511+
// Should have 5 messages:
512+
// - user-1, assistant-1 (partial), user-2 (NO SENTINEL - user follows), assistant-2 (partial), SENTINEL (last message)
513+
expect(result).toHaveLength(5);
514+
expect(result[0].id).toBe("user-1");
515+
expect(result[1].id).toBe("assistant-1");
516+
expect(result[2].id).toBe("user-2"); // No sentinel between assistant-1 and user-2
517+
expect(result[3].id).toBe("assistant-2");
518+
expect(result[4].id).toBe("interrupted-assistant-2"); // Sentinel after last partial
519+
expect(result[4].role).toBe("user");
520+
});
521+
522+
it("should skip sentinel when user message follows partial", () => {
523+
const messages: CmuxMessage[] = [
524+
{
525+
id: "user-1",
526+
role: "user",
527+
parts: [{ type: "text", text: "Question" }],
528+
metadata: { timestamp: 1000 },
529+
},
530+
{
531+
id: "assistant-1",
532+
role: "assistant",
533+
parts: [{ type: "text", text: "Starting response..." }],
534+
metadata: { timestamp: 2000, partial: true },
535+
},
536+
{
537+
id: "user-2",
538+
role: "user",
539+
parts: [{ type: "text", text: "Follow-up question" }],
540+
metadata: { timestamp: 3000 },
541+
},
542+
];
543+
544+
const result = addInterruptedSentinel(messages);
545+
546+
// Should have 3 messages (no sentinel added because user-2 follows partial)
547+
expect(result).toHaveLength(3);
548+
expect(result[0].id).toBe("user-1");
549+
expect(result[1].id).toBe("assistant-1");
550+
expect(result[2].id).toBe("user-2");
551+
// No synthetic sentinel should exist
552+
expect(result.every((msg) => !msg.metadata?.synthetic)).toBe(true);
517553
});
518554
});
519555

β€Žsrc/utils/messages/modelMessageTransform.tsβ€Ž

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,10 @@ export function stripReasoningForOpenAI(messages: CmuxMessage[]): CmuxMessage[]
6666
* This helps the model understand that a message was interrupted and to continue.
6767
* The sentinel is ONLY for model context, not shown in UI.
6868
*
69+
* OPTIMIZATION: If a user message already follows the partial assistant message,
70+
* we skip the sentinel - the user message itself provides the continuation signal.
71+
* This saves tokens and creates more natural conversation flow.
72+
*
6973
* We insert a separate user message instead of modifying the assistant message
7074
* because if the assistant message only has reasoning (no text), it will be
7175
* filtered out, and we'd lose the interruption context. A user message always
@@ -74,21 +78,28 @@ export function stripReasoningForOpenAI(messages: CmuxMessage[]): CmuxMessage[]
7478
export function addInterruptedSentinel(messages: CmuxMessage[]): CmuxMessage[] {
7579
const result: CmuxMessage[] = [];
7680

77-
for (const msg of messages) {
81+
for (let i = 0; i < messages.length; i++) {
82+
const msg = messages[i];
7883
result.push(msg);
7984

80-
// If this is a partial assistant message, insert [CONTINUE] user message after it
85+
// If this is a partial assistant message, conditionally insert [CONTINUE] sentinel
8186
if (msg.role === "assistant" && msg.metadata?.partial) {
82-
result.push({
83-
id: `interrupted-${msg.id}`,
84-
role: "user",
85-
parts: [{ type: "text", text: "[CONTINUE]" }],
86-
metadata: {
87-
timestamp: msg.metadata.timestamp,
88-
// Mark as synthetic so it can be identified if needed
89-
synthetic: true,
90-
},
91-
});
87+
const nextMsg = messages[i + 1];
88+
89+
// Only add sentinel if there's NO user message following
90+
// If user message follows, it provides the continuation context itself
91+
if (!nextMsg || nextMsg.role !== "user") {
92+
result.push({
93+
id: `interrupted-${msg.id}`,
94+
role: "user",
95+
parts: [{ type: "text", text: "[CONTINUE]" }],
96+
metadata: {
97+
timestamp: msg.metadata.timestamp,
98+
// Mark as synthetic so it can be identified if needed
99+
synthetic: true,
100+
},
101+
});
102+
}
92103
}
93104
}
94105

0 commit comments

Comments
Β (0)