Skip to content

Commit

Permalink
fix: Get by index from response wrapper (#290)
Browse files Browse the repository at this point in the history
  • Loading branch information
ZhongpinWang authored Nov 14, 2024
1 parent 3c6cb4f commit 91df549
Show file tree
Hide file tree
Showing 7 changed files with 19 additions and 63 deletions.
6 changes: 6 additions & 0 deletions .changeset/stale-pans-approve.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@sap-ai-sdk/foundation-models': patch
'@sap-ai-sdk/orchestration': patch
---

[Fixed Issue] Get choice via index by comparing the `index` property instead of using array index.
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import { createLogger } from '@sap-cloud-sdk/util';
import { jest } from '@jest/globals';
import { parseMockResponse } from '../../../../test-util/mock-http.js';
import { AzureOpenAiChatCompletionResponse } from './azure-openai-chat-completion-response.js';
import type { HttpResponse } from '@sap-cloud-sdk/http-client';
Expand Down Expand Up @@ -51,14 +49,7 @@ describe('OpenAI chat completion response', () => {
});

it('should return undefined when convenience function is called with incorrect index', () => {
const logger = createLogger({
package: 'foundation-models',
messageContext: 'azure-openai-chat-completion-response'
});
const errorSpy = jest.spyOn(logger, 'error');
expect(azureOpenAiChatResponse.getFinishReason(1)).toBeUndefined();
expect(errorSpy).toHaveBeenCalledWith('Choice index 1 is out of bounds.');
expect(azureOpenAiChatResponse.getContent(1)).toBeUndefined();
expect(errorSpy).toHaveBeenCalledTimes(2);
});
});
Original file line number Diff line number Diff line change
@@ -1,12 +1,6 @@
import { createLogger } from '@sap-cloud-sdk/util';
import type { HttpResponse } from '@sap-cloud-sdk/http-client';
import type { AzureOpenAiCreateChatCompletionResponse } from './client/inference/schema/index.js';

const logger = createLogger({
package: 'foundation-models',
messageContext: 'azure-openai-chat-completion-response'
});

/**
* Azure OpenAI chat completion response.
*/
Expand All @@ -32,11 +26,8 @@ export class AzureOpenAiChatCompletionResponse {
* @param choiceIndex - The index of the choice to parse.
* @returns The finish reason.
*/
getFinishReason(
choiceIndex = 0
): this['data']['choices'][0]['finish_reason'] {
this.logInvalidChoiceIndex(choiceIndex);
return this.data.choices[choiceIndex]?.finish_reason;
getFinishReason(choiceIndex = 0): string | undefined {
return this.data.choices.find(c => c.index === choiceIndex)?.finish_reason;
}

/**
Expand All @@ -45,13 +36,7 @@ export class AzureOpenAiChatCompletionResponse {
* @returns The message content.
*/
getContent(choiceIndex = 0): string | undefined | null {
this.logInvalidChoiceIndex(choiceIndex);
return this.data.choices[choiceIndex]?.message?.content;
}

private logInvalidChoiceIndex(choiceIndex: number): void {
if (choiceIndex < 0 || choiceIndex >= this.data.choices.length) {
logger.error(`Choice index ${choiceIndex} is out of bounds.`);
}
return this.data.choices.find(c => c.index === choiceIndex)?.message
?.content;
}
}
3 changes: 1 addition & 2 deletions packages/orchestration/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
"dependencies": {
"@sap-ai-sdk/core": "workspace:^",
"@sap-ai-sdk/ai-api": "workspace:^",
"@sap-cloud-sdk/http-client": "^3.22.2",
"@sap-cloud-sdk/util": "^3.22.2"
"@sap-cloud-sdk/http-client": "^3.22.2"
}
}
9 changes: 0 additions & 9 deletions packages/orchestration/src/orchestration-response.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import { createLogger } from '@sap-cloud-sdk/util';
import { jest } from '@jest/globals';
import { parseMockResponse } from '../../../test-util/mock-http.js';
import { OrchestrationResponse } from './orchestration-response.js';
import type { HttpResponse } from '@sap-cloud-sdk/http-client';
Expand Down Expand Up @@ -48,15 +46,8 @@ describe('OrchestrationResponse', () => {
});

it('should return undefined when convenience function is called with incorrect index', () => {
const logger = createLogger({
package: 'orchestration',
messageContext: 'orchestration-response'
});
const errorSpy = jest.spyOn(logger, 'error');
expect(orchestrationResponse.getFinishReason(1)).toBeUndefined();
expect(errorSpy).toHaveBeenCalledWith('Choice index 1 is out of bounds.');
expect(orchestrationResponse.getContent(1)).toBeUndefined();
expect(errorSpy).toHaveBeenCalledTimes(2);
});

it('should throw if content that was filtered is accessed', () => {
Expand Down
23 changes: 5 additions & 18 deletions packages/orchestration/src/orchestration-response.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,9 @@
import { createLogger } from '@sap-cloud-sdk/util';
import type { HttpResponse } from '@sap-cloud-sdk/http-client';
import type {
CompletionPostResponse,
TokenUsage
} from './client/api/schema/index.js';

const logger = createLogger({
package: 'orchestration',
messageContext: 'orchestration-response'
});

/**
* Representation of an orchestration response.
*/
Expand All @@ -35,8 +29,7 @@ export class OrchestrationResponse {
* @returns The finish reason.
*/
getFinishReason(choiceIndex = 0): string | undefined {
this.logInvalidChoiceIndex(choiceIndex);
return this.getChoices()[choiceIndex]?.finish_reason;
return this.getChoices().find(c => c.index === choiceIndex)?.finish_reason;
}

/**
Expand All @@ -46,25 +39,19 @@ export class OrchestrationResponse {
* @returns The message content.
*/
getContent(choiceIndex = 0): string | undefined {
this.logInvalidChoiceIndex(choiceIndex);
const choice = this.getChoices().find(c => c.index === choiceIndex);
if (
this.getChoices()[choiceIndex]?.message?.content === '' &&
this.getChoices()[choiceIndex]?.finish_reason === 'content_filter'
choice?.message?.content === '' &&
choice?.finish_reason === 'content_filter'
) {
throw new Error(
'Content generated by the LLM was filtered by the output filter. Please try again with a different prompt or filter configuration.'
);
}
return this.getChoices()[choiceIndex]?.message?.content;
return choice?.message?.content;
}

private getChoices() {
return this.data.orchestration_result.choices;
}

private logInvalidChoiceIndex(choiceIndex: number): void {
if (choiceIndex < 0 || choiceIndex >= this.getChoices().length) {
logger.error(`Choice index ${choiceIndex} is out of bounds.`);
}
}
}
9 changes: 3 additions & 6 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 91df549

Please sign in to comment.