Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add descriptive enum constant for Azure content filter configuration - Option 2 #476

Closed
wants to merge 7 commits into from
Closed

feat: Add descriptive enum constant for Azure content filter configuration - Option 2 #476

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c7e9120
chore: initial implementation
KavithaSiva Jan 21, 2025
5bc8251
fix: Changes from lint
Jan 21, 2025
9e44509
chore: add eslint disable
KavithaSiva Jan 21, 2025
32d48dd
chore: update tests
KavithaSiva Jan 21, 2025
3b15096
chore: align name with Java
KavithaSiva Jan 21, 2025
49b385d
chore: update documentation
KavithaSiva Jan 21, 2025
ccf71ab
Merge branch 'main' into feat/descriptive-enum-orchestration-filtering
KavithaSiva Jan 21, 2025
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
7 changes: 5 additions & 2 deletions packages/orchestration/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,10 @@ import {
buildAzureContentFilter
} from '@sap-ai-sdk/orchestration';

const filter = buildAzureContentFilter({ Hate: 2, Violence: 4 });
const filter = buildAzureContentFilter({
Hate: AzureFilterThreshold.ALLOW_SAFE_LOW,
Violence: AzureFilterThreshold.ALLOW_SAFE_LOW_MEDIUM
});
const orchestrationClient = new OrchestrationClient({
llm: {
model_name: 'gpt-4o',
Expand Down Expand Up @@ -291,7 +294,7 @@ Therefore, handle errors appropriately to ensure meaningful feedback for both ty

`buildAzureContentFilter()` is a convenience function that creates an Azure content filter configuration based on the provided inputs.
The Azure content filter supports four categories: `Hate`, `Violence`, `Sexual`, and `SelfHarm`.
Each category can be configured with severity levels of 0, 2, 4, or 6.
Each category can be configured with severity levels of `ALLOW_SAFE`, `ALLOW_SAFE_LOW`, `ALLOW_SAFE_LOW_MEDIUM` and `ALLOW_ALL` which correspond to Azure threshold values of 0, 2, 4, or 6 respectively.

### Data Masking

Expand Down
4 changes: 3 additions & 1 deletion packages/orchestration/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ export type {
StreamOptions,
DocumentGroundingServiceConfig,
DocumentGroundingServiceFilter,
LlmModelParams
LlmModelParams,
AzureContentSafety,
AzureFilterThreshold
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The idea is that AzureContentSafety defined in orchestration-types overrides the type generated from the specification.

Modifying the type doesn't seem to be a breaking change as in the end they would both resolve to be the same code in JS.

Copy link
Contributor

Choose a reason for hiding this comment

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

[req] Why are we not directly overriding AzureThreshold? Currently, the generated AzureThreshold can still be imported. If you are worrying about user hover on AzureContentSafety and got navigate to the generated AzureThreshold type, it is simply not avoidable. Even if we override AzureContentSafety, if user navigate from an outside type, they will still end up in the generated threshold. Unless we don't use wildcard..

} from './orchestration-types.js';

export { OrchestrationStreamResponse } from './orchestration-stream-response.js';
Expand Down
23 changes: 15 additions & 8 deletions packages/orchestration/src/orchestration-client.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
constructCompletionPostRequestFromJsonModuleConfig
} from './orchestration-utils.js';
import { OrchestrationResponse } from './orchestration-response.js';
import { AzureFilterThreshold } from './orchestration-types.js';
import type { CompletionPostResponse } from './client/api/schema/index.js';
import type {
OrchestrationModuleConfig,
Expand Down Expand Up @@ -162,8 +163,14 @@ describe('orchestration service client', () => {
]
},
filtering: {
input: buildAzureContentFilter({ Hate: 4, SelfHarm: 2 }),
output: buildAzureContentFilter({ Sexual: 0, Violence: 4 })
input: buildAzureContentFilter({
Hate: AzureFilterThreshold.ALLOW_SAFE_LOW_MEDIUM,
SelfHarm: AzureFilterThreshold.ALLOW_SAFE_LOW
}),
output: buildAzureContentFilter({
Sexual: AzureFilterThreshold.ALLOW_SAFE,
Violence: AzureFilterThreshold.ALLOW_SAFE_LOW_MEDIUM
})
Comment on lines +166 to +173
Copy link
Contributor

Choose a reason for hiding this comment

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

Also here, don't change the test. Add a new one (or type test).

}
};
const prompt = {
Expand Down Expand Up @@ -210,21 +217,21 @@ describe('orchestration service client', () => {
input: {
filters: [
{
type: 'azure_content_safety' as const,
type: 'azure_content_safety',
config: {
Hate: 4 as const,
SelfHarm: 2 as const
Hate: 4,
SelfHarm: 2
}
}
]
},
output: {
filters: [
{
type: 'azure_content_safety' as const,
type: 'azure_content_safety',
config: {
Sexual: 0 as const,
Violence: 4 as const
Sexual: 0,
Violence: 4
}
}
]
Expand Down
37 changes: 37 additions & 0 deletions packages/orchestration/src/orchestration-types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,3 +151,40 @@ export interface DocumentGroundingServiceConfig {
*/
output_param: string;
}

/**
* Filter configuration for Azure Content Safety.
*/
// eslint-disable-next-line @typescript-eslint/consistent-type-definitions
export type AzureContentSafety = {
/**
* The filter category for hate content.
*/
Hate?: AzureFilterThreshold;
/**
* The filter category for self-harm content.
*/
SelfHarm?: AzureFilterThreshold;
/**
* The filter category for sexual content.
*/
Sexual?: AzureFilterThreshold;
/**
* The filter category for violence content.
*/
Violence?: AzureFilterThreshold;
};

/**
* A descriptive constant for Azure content safety filter.
*/
export enum AzureFilterThreshold {
/** Only safe content is allowed. */
ALLOW_SAFE = 0,
/** Safe and low-risk content is allowed. */
ALLOW_SAFE_LOW = 2,
/** Safe, low-risk, and medium-risk content is allowed. */
ALLOW_SAFE_LOW_MEDIUM = 4,
/** All content is allowed. */
ALLOW_ALL = 6
}
44 changes: 30 additions & 14 deletions packages/orchestration/src/orchestration-utils.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,18 @@ import {
buildDocumentGroundingConfig,
constructCompletionPostRequest
} from './orchestration-utils.js';
import {
type OrchestrationModuleConfig,
type DocumentGroundingServiceConfig,
type StreamOptions,
AzureFilterThreshold
} from './orchestration-types.js';
import type {
CompletionPostRequest,
FilteringModuleConfig,
ModuleConfigs,
OrchestrationConfig
} from './client/api/schema/index.js';
import type {
OrchestrationModuleConfig,
DocumentGroundingServiceConfig,
StreamOptions
} from './orchestration-types.js';

describe('orchestration utils', () => {
describe('stream util tests', () => {
Expand Down Expand Up @@ -82,7 +83,10 @@ describe('orchestration utils', () => {
const config: OrchestrationModuleConfig = {
...defaultOrchestrationModuleConfig,
filtering: {
output: buildAzureContentFilter({ Hate: 4, SelfHarm: 0 })
output: buildAzureContentFilter({
Hate: AzureFilterThreshold.ALLOW_SAFE_LOW_MEDIUM,
SelfHarm: AzureFilterThreshold.ALLOW_SAFE
})
Comment on lines +86 to +89
Copy link
Contributor

Choose a reason for hiding this comment

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

[q] Can we avoid changing these tests and only add new tests (even just type tests) to avoid merge conflicts with #441? I think it is unnecessary to change those types.

}
};
const filteringConfig = addStreamOptionsToOutputFilteringConfig(
Expand All @@ -101,7 +105,10 @@ describe('orchestration utils', () => {
const config: ModuleConfigs = {
...defaultModuleConfigs,
filtering_module_config: {
output: buildAzureContentFilter({ Hate: 4, SelfHarm: 0 })
output: buildAzureContentFilter({
Hate: AzureFilterThreshold.ALLOW_SAFE_LOW_MEDIUM,
SelfHarm: AzureFilterThreshold.ALLOW_SAFE
})
}
};

Expand Down Expand Up @@ -174,7 +181,10 @@ describe('orchestration utils', () => {

it('constructs filter configuration with only input', async () => {
const filtering: FilteringModuleConfig = {
input: buildAzureContentFilter({ Hate: 4, SelfHarm: 0 })
input: buildAzureContentFilter({
Hate: AzureFilterThreshold.ALLOW_SAFE_LOW_MEDIUM,
SelfHarm: AzureFilterThreshold.ALLOW_SAFE
})
};
const expectedFilterConfig: FilteringModuleConfig = {
input: {
Expand All @@ -200,7 +210,10 @@ describe('orchestration utils', () => {

it('constructs filter configuration with only output', async () => {
const filtering: FilteringModuleConfig = {
output: buildAzureContentFilter({ Sexual: 2, Violence: 6 })
output: buildAzureContentFilter({
Sexual: AzureFilterThreshold.ALLOW_SAFE_LOW,
Violence: AzureFilterThreshold.ALLOW_ALL
})
};
const expectedFilterConfig: FilteringModuleConfig = {
output: {
Expand All @@ -227,12 +240,15 @@ describe('orchestration utils', () => {
it('constructs filter configuration with both input and output', async () => {
const filtering: FilteringModuleConfig = {
input: buildAzureContentFilter({
Hate: 4,
SelfHarm: 0,
Sexual: 2,
Violence: 6
Hate: AzureFilterThreshold.ALLOW_SAFE_LOW_MEDIUM,
SelfHarm: AzureFilterThreshold.ALLOW_SAFE,
Sexual: AzureFilterThreshold.ALLOW_SAFE_LOW,
Violence: AzureFilterThreshold.ALLOW_ALL
}),
output: buildAzureContentFilter({ Sexual: 2, Violence: 6 })
output: buildAzureContentFilter({
Sexual: AzureFilterThreshold.ALLOW_SAFE_LOW,
Violence: AzureFilterThreshold.ALLOW_ALL
})
};
const expectedFilterConfig: FilteringModuleConfig = {
input: {
Expand Down
4 changes: 2 additions & 2 deletions packages/orchestration/src/orchestration-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ import type {
Prompt,
StreamOptions,
LlmModuleConfig,
OrchestrationModuleConfig
OrchestrationModuleConfig,
AzureContentSafety
} from './orchestration-types.js';
import type {
AzureContentSafety,
GroundingModuleConfig,
InputFilteringConfig,
CompletionPostRequest,
Expand Down
Loading