Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 6 additions & 5 deletions packages/core/src/agent/agent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -774,15 +774,15 @@ export class Agent<
taskPrompt: string,
opt?: {
cacheable?: boolean;
thinkingLevel?: ThinkingLevel;
_thinkingLevel?: ThinkingLevel;
},
) {
const modelConfigForPlanning =
this.modelConfigManager.getModelConfig('planning');
const defaultIntentModelConfig =
this.modelConfigManager.getModelConfig('default');

let thinkingLevelToUse = opt?.thinkingLevel;
let thinkingLevelToUse = opt?._thinkingLevel;
if (!thinkingLevelToUse && this.opts.aiActionContext) {
Comment on lines +785 to 786

Choose a reason for hiding this comment

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

P1 Badge Restore thinkingLevel option handling

The aiAct options now only read opt._thinkingLevel, so any caller still passing the documented thinkingLevel flag has its request ignored and the method falls back to the medium/high defaults. For example, packages/web-integration/tests/ai/web/puppeteer/e2e.test.ts:42-44 still passes { thinkingLevel: 'high' }, which will now be treated as if no level was provided. This silently changes planning behaviour for consumers relying on that knob; please continue accepting the existing option or update the call sites.

Useful? React with 👍 / 👎.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

👎

thinkingLevelToUse = 'high';
} else if (!thinkingLevelToUse) {
Expand All @@ -791,10 +791,11 @@ export class Agent<

// should include bbox in planning if
// 1. the planning model is the same as the default intent model
// or 2. the thinking level is high
// and
// 2. the thinking level is not high
const includeBboxInPlanning =
modelConfigForPlanning.modelName === defaultIntentModelConfig.modelName ||
thinkingLevelToUse === 'high';
modelConfigForPlanning.modelName === defaultIntentModelConfig.modelName &&
thinkingLevelToUse !== 'high';
debug('setting includeBboxInPlanning to', includeBboxInPlanning);

const cacheable = opt?.cacheable;
Expand Down
49 changes: 2 additions & 47 deletions packages/core/src/ai-model/conversation-history.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,13 @@
import type { ChatCompletionMessageParam } from 'openai/resources/index';

export interface ConversationHistoryOptions {
maxUserImageMessages?: number;
initialMessages?: ChatCompletionMessageParam[];
}

const defaultMaxUserImagesCount = 6;

export class ConversationHistory {
private readonly maxUserImageMessages: number;
private readonly messages: ChatCompletionMessageParam[] = [];

constructor(options?: ConversationHistoryOptions) {
this.maxUserImageMessages =
options?.maxUserImageMessages ?? defaultMaxUserImagesCount;
if (options?.initialMessages?.length) {
this.seed(options.initialMessages);
}
Expand All @@ -34,47 +28,8 @@ export class ConversationHistory {
this.messages.length = 0;
}

snapshot(options?: {
maxImageMessages?: number;
}): ChatCompletionMessageParam[] {
const maxImageMessages =
options?.maxImageMessages ?? this.maxUserImageMessages;

// Count image_url messages from back to front
let imageCount = 0;
const processedMessages = [...this.messages]
.reverse()
.map((message): ChatCompletionMessageParam => {
if (
typeof message.content !== 'string' &&
Array.isArray(message.content)
) {
// Also process content items from back to front
const processedContent = [...message.content]
.reverse()
.map((item) => {
if (item.type === 'image_url') {
imageCount++;
if (imageCount > maxImageMessages) {
// Replace with text type
return {
type: 'text' as const,
text: '(omitted due to size limit)',
};
}
}
return item;
})
.reverse();
return {
...message,
content: processedContent,
} as ChatCompletionMessageParam;
}
return message;
});

return processedMessages.reverse();
snapshot(): ChatCompletionMessageParam[] {
return [...this.messages];
}

get length(): number {
Expand Down
19 changes: 10 additions & 9 deletions packages/core/src/ai-model/llm-planning.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ export async function plan(
}

const historyLog = opts.conversationHistory?.snapshot() || [];
// .filter((item) => item.role === 'assistant') || [];

const knowledgeContext: ChatCompletionMessageParam[] = opts.actionContext
? [
Expand Down Expand Up @@ -96,7 +95,7 @@ export async function plan(
content: [
{
type: 'text',
text: 'I have finished the action previously planned, and the last screenshot is as follows:',
text: 'I have finished the action previously planned, and the last screenshot is attached. Please going on according to the instruction.',
},
{
type: 'image_url',
Expand All @@ -117,13 +116,15 @@ export async function plan(
latestImageMessage,
];

const { content: planFromAI, usage } =
await callAIWithObjectResponse<RawResponsePlanningAIResponse>(
msgs,
AIActionType.PLAN,
modelConfig,
);
const rawResponse = JSON.stringify(planFromAI, undefined, 2);
const {
content: planFromAI,
contentString: rawResponse,
usage,
} = await callAIWithObjectResponse<RawResponsePlanningAIResponse>(
msgs,
AIActionType.PLAN,
modelConfig,
);

const actions = planFromAI.action ? [planFromAI.action] : [];
const returnValue: PlanningAIResponse = {
Expand Down
8 changes: 6 additions & 2 deletions packages/core/src/ai-model/service-caller/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -358,12 +358,16 @@ export async function callAIWithObjectResponse<T>(
messages: ChatCompletionMessageParam[],
AIActionTypeValue: AIActionType,
modelConfig: IModelConfig,
): Promise<{ content: T; usage?: AIUsageInfo }> {
): Promise<{ content: T; contentString: string; usage?: AIUsageInfo }> {
const response = await callAI(messages, AIActionTypeValue, modelConfig);
assert(response, 'empty response');
const vlMode = modelConfig.vlMode;
const jsonContent = safeParseJson(response.content, vlMode);
return { content: jsonContent, usage: response.usage };
return {
content: jsonContent,
contentString: response.content,
usage: response.usage,
};
}

export async function callAIWithStringResponse(
Expand Down
64 changes: 7 additions & 57 deletions packages/core/tests/unit-test/conversation-history.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,55 +45,8 @@ describe('ConversationHistory', () => {
expect(history.snapshot()).toEqual([assistantMessage('hello')]);
});

it('limits image messages in snapshot from back to front', () => {
const history = new ConversationHistory({ maxUserImageMessages: 2 });

history.append(userMessageWithImage('first', 'data:image1'));
history.append(assistantMessage('ack1'));
history.append(userMessageWithImage('second', 'data:image2'));
history.append(assistantMessage('ack2'));
history.append(userMessageWithImage('third', 'data:image3'));

const snapshot = history.snapshot();

// First image should be omitted (counting from back, it's the 3rd one)
expect(snapshot[0]).toEqual({
role: 'user',
content: [
{ type: 'text', text: 'first' },
{ type: 'text', text: '(omitted due to size limit)' },
],
});

// Second and third images should be preserved
expect(snapshot[2]).toEqual(userMessageWithImage('second', 'data:image2'));
expect(snapshot[4]).toEqual(userMessageWithImage('third', 'data:image3'));
});

it('respects maxImageMessages parameter in snapshot options', () => {
const history = new ConversationHistory({ maxUserImageMessages: 5 });

history.append(userMessageWithImage('first', 'data:image1'));
history.append(userMessageWithImage('second', 'data:image2'));
history.append(userMessageWithImage('third', 'data:image3'));

// Override with maxImageMessages: 1
const snapshot = history.snapshot({ maxImageMessages: 1 });

// Only the last image should be preserved
expect(snapshot[0].content).toEqual([
{ type: 'text', text: 'first' },
{ type: 'text', text: '(omitted due to size limit)' },
]);
expect(snapshot[1].content).toEqual([
{ type: 'text', text: 'second' },
{ type: 'text', text: '(omitted due to size limit)' },
]);
expect(snapshot[2]).toEqual(userMessageWithImage('third', 'data:image3'));
});

it('handles messages with multiple images in content', () => {
const history = new ConversationHistory({ maxUserImageMessages: 2 });
it('returns image messages without modification', () => {
const history = new ConversationHistory();

const messageWithTwoImages: ChatCompletionMessageParam = {
role: 'user',
Expand All @@ -104,17 +57,14 @@ describe('ConversationHistory', () => {
],
};

history.append(userMessageWithImage('first', 'data:image1'));
history.append(assistantMessage('ack1'));
history.append(messageWithTwoImages);
history.append(userMessageWithImage('another', 'data:image3'));

const snapshot = history.snapshot();

// From back to front: image3 (1st), image2 (2nd), image1 (3rd - should be omitted)
expect(snapshot[0].content).toEqual([
{ type: 'text', text: 'Look at these' },
{ type: 'text', text: '(omitted due to size limit)' },
{ type: 'image_url', image_url: { url: 'data:image2' } },
]);
expect(snapshot[1]).toEqual(userMessageWithImage('another', 'data:image3'));
expect(snapshot[0]).toEqual(userMessageWithImage('first', 'data:image1'));
expect(snapshot[1]).toEqual(assistantMessage('ack1'));
expect(snapshot[2]).toEqual(messageWithTwoImages);
});
});
2 changes: 1 addition & 1 deletion packages/shared/src/env/parse-model-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ export const modelFamilyToVLConfig = (

// Check if the modelFamily is valid
if (!MODEL_FAMILY_VALUES.includes(modelFamily as any)) {
throw new Error(`Invalid modelFamily: ${modelFamily}`);
throw new Error(`Invalid MIDSCENE_MODEL_FAMILY value: ${modelFamily}`);
Comment on lines 74 to +76

Choose a reason for hiding this comment

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

P1 Badge Align invalid modelFamily error with tests

The error message for invalid MIDSCENE_MODEL_FAMILY was changed to Invalid MIDSCENE_MODEL_FAMILY value: …, but packages/shared/tests/unit-test/env/parse.test.ts:52-54 still asserts the previous Invalid modelFamily: invalid. This change causes that unit test to fail and will break the test suite until the message or expectation is brought back into sync.

Useful? React with 👍 / 👎.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

👍

}

// For other model families, they directly map to vlMode
Expand Down
2 changes: 1 addition & 1 deletion packages/shared/tests/unit-test/env/parse.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ describe('modelFamilyToVLConfig', () => {

it('should throw on invalid value', () => {
expect(() => modelFamilyToVLConfig('invalid' as any)).toThrow(
'Invalid modelFamily: invalid',
'Invalid MIDSCENE_MODEL_FAMILY value: invalid',
);
});
});
Expand Down
Loading