Skip to content

Commit 6aa4848

Browse files
refactor: tidy conversation tool after simplify review
Post-implementation simplify pass (reuse + efficiency + comment cleanup), no behavior change: - Reuse the existing buildTurnUserContent helper for both lapUserMessage constructions (sync + async) instead of re-inlining the identical context-or-prompt branch. - Compute the lap transcript once per path and reuse it for the assistant message, persisted state, and response content (was building the same string twice in each of the sync and async paths). - Collapse `x ? x : y` framing ternaries to `x || y`. - Drop spec-narration/WHAT comments; keep the genuine constraint comments (SDK last-user-message, numeric async progress, abort rethrow, and the formatStatus result.content requirement). 25/25 conversation unit tests, eslint, and typecheck pass.
1 parent 0b99879 commit 6aa4848

1 file changed

Lines changed: 17 additions & 22 deletions

File tree

src/tools/conversation.js

Lines changed: 17 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -110,12 +110,8 @@ function buildFramingText({ i, models, turn_prompt }) {
110110
const nextModel = i < total - 1 ? models[i + 1] : null;
111111

112112
const order = models.join(', ');
113-
const prevText = prevModel
114-
? prevModel
115-
: 'no one (you open the round)';
116-
const nextText = nextModel
117-
? nextModel
118-
: 'no one (you close this round)';
113+
const prevText = prevModel || 'no one (you open the round)';
114+
const nextText = nextModel || 'no one (you close this round)';
119115
const handoffText = nextModel
120116
? `Your response will be passed to the next participant (${nextModel}).`
121117
: 'Your response will be returned to the user, as you are the last participant this round.';
@@ -564,7 +560,6 @@ export async function conversationTool(args, dependencies) {
564560
}
565561
}
566562

567-
// Process context (files and images) attached to the lap user message
568563
const contextMessage = await buildContextMessage(
569564
files,
570565
images,
@@ -661,14 +656,16 @@ export async function conversationTool(args, dependencies) {
661656
// Build the lap user message (lap prompt, with context if present)
662657
const lapUserMessage = {
663658
role: 'user',
664-
content: contextMessage?.content
665-
? [...contextMessage.content, { type: 'text', text: prompt }]
666-
: prompt,
659+
content: buildTurnUserContent(prompt, contextMessage),
667660
};
668661

662+
// Labeled lap transcript (### <model> (turn <n>):) — computed once and reused
663+
// for the assistant message, the persisted state, and the response content.
664+
const transcript = formatLapTranscript(lapTurns);
665+
669666
const assistantMessage = {
670667
role: 'assistant',
671-
content: formatLapTranscript(lapTurns),
668+
content: transcript,
672669
};
673670

674671
// Save conversation state (skip on abort to avoid persisting incomplete history)
@@ -721,10 +718,6 @@ export async function conversationTool(args, dependencies) {
721718

722719
const continuationIdLine = `continuation_id: ${continuationId}\n\n`;
723720

724-
// Rendered lap transcript with labeled turns (### <model> (turn <n>):).
725-
// Per spec requirement 11, the response content must include this transcript.
726-
const transcript = formatLapTranscript(lapTurns);
727-
728721
const result = {
729722
status: 'conversation_complete',
730723
content: transcript,
@@ -1046,14 +1039,16 @@ async function executeConversationWithStreaming(args, dependencies, context) {
10461039

10471040
const lapUserMessage = {
10481041
role: 'user',
1049-
content: contextMessage?.content
1050-
? [...contextMessage.content, { type: 'text', text: prompt }]
1051-
: prompt,
1042+
content: buildTurnUserContent(prompt, contextMessage),
10521043
};
10531044

1045+
// Final lap transcript — computed once and reused for the assistant message,
1046+
// the persisted state, and the returned top-level content.
1047+
const transcript = formatLapTranscript(lapTurns);
1048+
10541049
const assistantMessage = {
10551050
role: 'assistant',
1056-
content: formatLapTranscript(lapTurns),
1051+
content: transcript,
10571052
};
10581053

10591054
// Save conversation state
@@ -1114,11 +1109,11 @@ async function executeConversationWithStreaming(args, dependencies, context) {
11141109

11151110
const messageCount = (conversationState?.messages || []).length;
11161111

1117-
// CRITICAL: top-level `content` is required so check_status renders the
1118-
// transcript on completion (formatStatus.js only shows result.content).
1112+
// Top-level `content` is required: formatStatus only renders result.content
1113+
// when displaying a completed async job.
11191114
return {
11201115
status: 'conversation_complete',
1121-
content: formatLapTranscript(lapTurns),
1116+
content: transcript,
11221117
models_consulted: models.length,
11231118
successful_turns: turnsSuccessful,
11241119
failed_turns: turnsFailed,

0 commit comments

Comments
 (0)