Skip to content

Commit 24ddb5f

Browse files
authored
feat(core): Add BaseLlmClient.generateContent. (#13591)
1 parent 60ff661 commit 24ddb5f

File tree

2 files changed

+279
-75
lines changed

2 files changed

+279
-75
lines changed

packages/core/src/core/baseLlmClient.test.ts

Lines changed: 135 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
beforeEach,
1313
afterEach,
1414
type Mocked,
15+
type Mock,
1516
} from 'vitest';
1617

1718
import type { GenerateContentResponse } from '@google/genai';
@@ -299,21 +300,60 @@ describe('BaseLlmClient', () => {
299300
expect(result).toEqual({ color: 'orange' });
300301
expect(logMalformedJsonResponse).not.toHaveBeenCalled();
301302
});
303+
304+
it('should use the resolved model name when logging malformed JSON telemetry', async () => {
305+
const aliasModel = 'fast-alias';
306+
const resolvedModel = 'gemini-1.5-flash';
307+
308+
// Override the mock for this specific test to simulate resolution
309+
(
310+
mockConfig.modelConfigService.getResolvedConfig as unknown as Mock
311+
).mockReturnValue({
312+
model: resolvedModel,
313+
generateContentConfig: {
314+
temperature: 0,
315+
topP: 1,
316+
},
317+
});
318+
319+
const malformedResponse = '```json\n{"color": "red"}\n```';
320+
mockGenerateContent.mockResolvedValue(
321+
createMockResponse(malformedResponse),
322+
);
323+
324+
const options = {
325+
...defaultOptions,
326+
modelConfigKey: { model: aliasModel },
327+
};
328+
329+
const result = await client.generateJson(options);
330+
331+
expect(result).toEqual({ color: 'red' });
332+
333+
expect(logMalformedJsonResponse).toHaveBeenCalled();
334+
const calls = vi.mocked(logMalformedJsonResponse).mock.calls;
335+
const lastCall = calls[calls.length - 1];
336+
const event = lastCall[1] as MalformedJsonResponseEvent;
337+
338+
// This is the key assertion: it should be the resolved model, not the alias
339+
expect(event.model).toBe(resolvedModel);
340+
expect(event.model).not.toBe(aliasModel);
341+
});
302342
});
303343

304344
describe('generateJson - Error Handling', () => {
305345
it('should throw and report error for empty response after retry exhaustion', async () => {
306346
mockGenerateContent.mockResolvedValue(createMockResponse(''));
307347

308348
await expect(client.generateJson(defaultOptions)).rejects.toThrow(
309-
'Failed to generate JSON content: Retry attempts exhausted for invalid content',
349+
'Failed to generate content: Retry attempts exhausted for invalid content',
310350
);
311351

312352
// Verify error reporting details
313353
expect(reportError).toHaveBeenCalledTimes(1);
314354
expect(reportError).toHaveBeenCalledWith(
315355
expect.any(Error),
316-
'API returned invalid content (empty or unparsable JSON) after all retries.',
356+
'API returned invalid content after all retries.',
317357
defaultOptions.contents,
318358
'generateJson-invalid-content',
319359
);
@@ -324,13 +364,13 @@ describe('BaseLlmClient', () => {
324364
mockGenerateContent.mockResolvedValue(createMockResponse(invalidJson));
325365

326366
await expect(client.generateJson(defaultOptions)).rejects.toThrow(
327-
'Failed to generate JSON content: Retry attempts exhausted for invalid content',
367+
'Failed to generate content: Retry attempts exhausted for invalid content',
328368
);
329369

330370
expect(reportError).toHaveBeenCalledTimes(1);
331371
expect(reportError).toHaveBeenCalledWith(
332372
expect.any(Error),
333-
'API returned invalid content (empty or unparsable JSON) after all retries.',
373+
'API returned invalid content after all retries.',
334374
defaultOptions.contents,
335375
'generateJson-invalid-content',
336376
);
@@ -342,14 +382,14 @@ describe('BaseLlmClient', () => {
342382
mockGenerateContent.mockRejectedValue(apiError);
343383

344384
await expect(client.generateJson(defaultOptions)).rejects.toThrow(
345-
'Failed to generate JSON content: Service Unavailable (503)',
385+
'Failed to generate content: Service Unavailable (503)',
346386
);
347387

348388
// Verify generic error reporting
349389
expect(reportError).toHaveBeenCalledTimes(1);
350390
expect(reportError).toHaveBeenCalledWith(
351391
apiError,
352-
'Error generating JSON content via API.',
392+
'Error generating content via API.',
353393
defaultOptions.contents,
354394
'generateJson-api',
355395
);
@@ -464,4 +504,93 @@ describe('BaseLlmClient', () => {
464504
);
465505
});
466506
});
507+
508+
describe('generateContent', () => {
509+
it('should call generateContent with correct parameters and utilize retry mechanism', async () => {
510+
const mockResponse = createMockResponse('This is the content.');
511+
mockGenerateContent.mockResolvedValue(mockResponse);
512+
513+
const options = {
514+
modelConfigKey: { model: 'test-model' },
515+
contents: [{ role: 'user', parts: [{ text: 'Give me content.' }] }],
516+
abortSignal: abortController.signal,
517+
promptId: 'content-prompt-id',
518+
};
519+
520+
const result = await client.generateContent(options);
521+
522+
expect(result).toBe(mockResponse);
523+
524+
// Ensure the retry mechanism was engaged
525+
expect(retryWithBackoff).toHaveBeenCalledTimes(1);
526+
expect(retryWithBackoff).toHaveBeenCalledWith(
527+
expect.any(Function),
528+
expect.objectContaining({
529+
shouldRetryOnContent: expect.any(Function),
530+
}),
531+
);
532+
533+
// Validate the parameters passed to the underlying generator
534+
expect(mockGenerateContent).toHaveBeenCalledTimes(1);
535+
expect(mockGenerateContent).toHaveBeenCalledWith(
536+
{
537+
model: 'test-model',
538+
contents: options.contents,
539+
config: {
540+
abortSignal: options.abortSignal,
541+
temperature: 0,
542+
topP: 1,
543+
},
544+
},
545+
'content-prompt-id',
546+
);
547+
});
548+
549+
it('should validate content using shouldRetryOnContent function', async () => {
550+
const mockResponse = createMockResponse('Some valid content.');
551+
mockGenerateContent.mockResolvedValue(mockResponse);
552+
553+
const options = {
554+
modelConfigKey: { model: 'test-model' },
555+
contents: [{ role: 'user', parts: [{ text: 'Give me content.' }] }],
556+
abortSignal: abortController.signal,
557+
promptId: 'content-prompt-id',
558+
};
559+
560+
await client.generateContent(options);
561+
562+
const retryCall = vi.mocked(retryWithBackoff).mock.calls[0];
563+
const shouldRetryOnContent = retryCall[1]?.shouldRetryOnContent;
564+
565+
// Valid content should not trigger retry
566+
expect(shouldRetryOnContent!(mockResponse)).toBe(false);
567+
568+
// Empty response should trigger retry
569+
expect(shouldRetryOnContent!(createMockResponse(''))).toBe(true);
570+
expect(shouldRetryOnContent!(createMockResponse(' '))).toBe(true);
571+
});
572+
573+
it('should throw and report error for empty response after retry exhaustion', async () => {
574+
mockGenerateContent.mockResolvedValue(createMockResponse(''));
575+
const options = {
576+
modelConfigKey: { model: 'test-model' },
577+
contents: [{ role: 'user', parts: [{ text: 'Give me content.' }] }],
578+
abortSignal: abortController.signal,
579+
promptId: 'content-prompt-id',
580+
};
581+
582+
await expect(client.generateContent(options)).rejects.toThrow(
583+
'Failed to generate content: Retry attempts exhausted for invalid content',
584+
);
585+
586+
// Verify error reporting details
587+
expect(reportError).toHaveBeenCalledTimes(1);
588+
expect(reportError).toHaveBeenCalledWith(
589+
expect.any(Error),
590+
'API returned invalid content after all retries.',
591+
options.contents,
592+
'generateContent-invalid-content',
593+
);
594+
});
595+
});
467596
});

0 commit comments

Comments
 (0)