mirror of
https://github.com/langgenius/dify.git
synced 2026-05-25 19:00:43 -04:00
chore: Filter model presets by supported parameters (#36339)
This commit is contained in:
@@ -92,9 +92,21 @@ vi.mock('../../model-selector', () => ({
|
||||
}))
|
||||
|
||||
vi.mock('../presets-parameter', () => ({
|
||||
default: ({ onSelect }: { onSelect: (id: number) => void }) => (
|
||||
<button onClick={() => onSelect(1)}>Preset 1</button>
|
||||
),
|
||||
default: ({ onSelect, supportedParameterNames }: { onSelect: (id: number) => void, supportedParameterNames?: string[] }) => {
|
||||
if (supportedParameterNames && !supportedParameterNames.includes('temperature'))
|
||||
return null
|
||||
|
||||
return <button onClick={() => onSelect(1)}>Preset 1</button>
|
||||
},
|
||||
}))
|
||||
|
||||
vi.mock('../presets-parameter-utils', () => ({
|
||||
getSupportedPresetConfig: (_toneId: number, supportedParameterNames?: string[]) => {
|
||||
if (supportedParameterNames && !supportedParameterNames.includes('temperature'))
|
||||
return {}
|
||||
|
||||
return { temperature: 0.8 }
|
||||
},
|
||||
}))
|
||||
|
||||
vi.mock('../trigger', () => ({
|
||||
@@ -194,7 +206,28 @@ describe('ModelParameterModal', () => {
|
||||
render(<ModelParameterModal {...defaultProps} />)
|
||||
fireEvent.click(screen.getByText('Open Settings'))
|
||||
fireEvent.click(screen.getByText('Preset 1'))
|
||||
expect(defaultProps.onCompletionParamsChange).toHaveBeenCalled()
|
||||
expect(defaultProps.onCompletionParamsChange).toHaveBeenCalledWith({
|
||||
...defaultProps.completionParams,
|
||||
temperature: 0.8,
|
||||
})
|
||||
})
|
||||
|
||||
it('should not render preset control when visible parameters do not support preset keys', () => {
|
||||
parameterRules = [
|
||||
{
|
||||
name: 'max_tokens',
|
||||
label: { en_US: 'Max Tokens' },
|
||||
type: 'int',
|
||||
default: 256,
|
||||
min: 1,
|
||||
max: 4096,
|
||||
},
|
||||
]
|
||||
|
||||
render(<ModelParameterModal {...defaultProps} />)
|
||||
fireEvent.click(screen.getByText('Open Settings'))
|
||||
|
||||
expect(screen.queryByText('Preset 1')).not.toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('should call setModel when model selector picks another model', () => {
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { fireEvent, render, screen } from '@testing-library/react'
|
||||
import { vi } from 'vitest'
|
||||
import PresetsParameter from '../presets-parameter'
|
||||
import { getSupportedPresetConfig } from '../presets-parameter-utils'
|
||||
|
||||
describe('PresetsParameter', () => {
|
||||
beforeEach(() => {
|
||||
@@ -47,4 +48,22 @@ describe('PresetsParameter', () => {
|
||||
|
||||
expect(onSelect).toHaveBeenCalledWith(3)
|
||||
})
|
||||
|
||||
it('should render presets when at least one preset parameter is supported', () => {
|
||||
render(<PresetsParameter onSelect={vi.fn()} supportedParameterNames={['temperature']} />)
|
||||
|
||||
expect(screen.getByRole('button', { name: /common\.modelProvider\.loadPresets/i })).toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('should not render presets when no preset parameters are supported', () => {
|
||||
render(<PresetsParameter onSelect={vi.fn()} supportedParameterNames={['max_tokens']} />)
|
||||
|
||||
expect(screen.queryByRole('button', { name: /common\.modelProvider\.loadPresets/i })).not.toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('should return only supported preset config keys', () => {
|
||||
expect(getSupportedPresetConfig(1, ['temperature'])).toEqual({
|
||||
temperature: 0.8,
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -24,7 +24,7 @@ import { useMemo, useRef, useState } from 'react'
|
||||
import { useTranslation } from 'react-i18next'
|
||||
import { ArrowNarrowLeft } from '@/app/components/base/icons/src/vender/line/arrows'
|
||||
import Loading from '@/app/components/base/loading'
|
||||
import { PROVIDER_WITH_PRESET_TONE, STOP_PARAMETER_RULE, TONE_LIST } from '@/config'
|
||||
import { PROVIDER_WITH_PRESET_TONE, STOP_PARAMETER_RULE } from '@/config'
|
||||
import { useModelParameterRules } from '@/service/use-common'
|
||||
import {
|
||||
useTextGenerationCurrentProviderAndModelAndModelList,
|
||||
@@ -32,6 +32,7 @@ import {
|
||||
import ModelSelector from '../model-selector'
|
||||
import ParameterItem from './parameter-item'
|
||||
import PresetsParameter from './presets-parameter'
|
||||
import { getSupportedPresetConfig } from './presets-parameter-utils'
|
||||
import Trigger from './trigger'
|
||||
|
||||
export type ModelParameterModalProps = {
|
||||
@@ -90,6 +91,9 @@ const ModelParameterModal: FC<ModelParameterModalProps> = ({
|
||||
const parameterRules: ModelParameterRule[] = useMemo(() => {
|
||||
return parameterRulesData?.data || []
|
||||
}, [parameterRulesData])
|
||||
const supportedPresetParameterNames = useMemo(() => {
|
||||
return parameterRules.map(parameterRule => parameterRule.name)
|
||||
}, [parameterRules])
|
||||
|
||||
const handleParamChange = (key: string, value: ParameterValue) => {
|
||||
onCompletionParamsChange({
|
||||
@@ -125,13 +129,10 @@ const ModelParameterModal: FC<ModelParameterModalProps> = ({
|
||||
}
|
||||
|
||||
const handleSelectPresetParameter = (toneId: number) => {
|
||||
const tone = TONE_LIST.find(tone => tone.id === toneId)
|
||||
if (tone) {
|
||||
onCompletionParamsChange({
|
||||
...completionParams,
|
||||
...tone.config,
|
||||
})
|
||||
}
|
||||
onCompletionParamsChange({
|
||||
...completionParams,
|
||||
...getSupportedPresetConfig(toneId, supportedPresetParameterNames),
|
||||
})
|
||||
}
|
||||
|
||||
return (
|
||||
@@ -199,7 +200,10 @@ const ModelParameterModal: FC<ModelParameterModalProps> = ({
|
||||
<div className="flex flex-1 items-center system-sm-semibold-uppercase text-text-secondary">{t('modelProvider.parameters', { ns: 'common' })}</div>
|
||||
{
|
||||
PROVIDER_WITH_PRESET_TONE.includes(provider) && (
|
||||
<PresetsParameter onSelect={handleSelectPresetParameter} />
|
||||
<PresetsParameter
|
||||
onSelect={handleSelectPresetParameter}
|
||||
supportedParameterNames={supportedPresetParameterNames}
|
||||
/>
|
||||
)
|
||||
}
|
||||
</div>
|
||||
|
||||
@@ -0,0 +1,19 @@
|
||||
import { TONE_LIST } from '@/config'
|
||||
|
||||
export const getSupportedPresetConfig = (toneId: number, supportedParameterNames?: string[]) => {
|
||||
const tone = TONE_LIST.find(tone => tone.id === toneId)
|
||||
if (!tone?.config)
|
||||
return {}
|
||||
|
||||
if (!supportedParameterNames)
|
||||
return { ...tone.config }
|
||||
|
||||
const supportedParameterNameSet = new Set(supportedParameterNames)
|
||||
|
||||
return Object.entries(tone.config).reduce<Record<string, number>>((acc, [key, value]) => {
|
||||
if (supportedParameterNameSet.has(key))
|
||||
acc[key] = value
|
||||
|
||||
return acc
|
||||
}, {})
|
||||
}
|
||||
@@ -12,6 +12,8 @@ import { Scales02 } from '@/app/components/base/icons/src/vender/solid/FinanceAn
|
||||
import { Target04 } from '@/app/components/base/icons/src/vender/solid/general'
|
||||
import { TONE_LIST } from '@/config'
|
||||
|
||||
const PRESET_TONE_LIST = TONE_LIST.slice(0, 3)
|
||||
|
||||
const toneI18nKeyMap = {
|
||||
Creative: 'model.tone.Creative',
|
||||
Balanced: 'model.tone.Balanced',
|
||||
@@ -27,10 +29,18 @@ const TONE_ICONS: Record<number, ReactNode> = {
|
||||
|
||||
type PresetsParameterProps = {
|
||||
onSelect: (toneId: number) => void
|
||||
supportedParameterNames?: string[]
|
||||
}
|
||||
|
||||
function PresetsParameter({ onSelect }: PresetsParameterProps) {
|
||||
function PresetsParameter({ onSelect, supportedParameterNames }: PresetsParameterProps) {
|
||||
const { t } = useTranslation()
|
||||
const supportedParameterNameSet = supportedParameterNames ? new Set(supportedParameterNames) : undefined
|
||||
const visiblePresetTones = supportedParameterNameSet
|
||||
? PRESET_TONE_LIST.filter(tone => Object.keys(tone.config ?? {}).some(key => supportedParameterNameSet.has(key)))
|
||||
: PRESET_TONE_LIST
|
||||
|
||||
if (!visiblePresetTones.length)
|
||||
return null
|
||||
|
||||
return (
|
||||
<DropdownMenu>
|
||||
@@ -47,7 +57,7 @@ function PresetsParameter({ onSelect }: PresetsParameterProps) {
|
||||
<span className="ml-0.5 i-ri-arrow-down-s-line h-3.5 w-3.5" />
|
||||
</DropdownMenuTrigger>
|
||||
<DropdownMenuContent>
|
||||
{TONE_LIST.slice(0, 3).map(tone => (
|
||||
{visiblePresetTones.map(tone => (
|
||||
<DropdownMenuItem key={tone.id} onClick={() => onSelect(tone.id)}>
|
||||
{TONE_ICONS[tone.id]}
|
||||
{t(toneI18nKeyMap[tone.name], { ns: 'common' })}
|
||||
|
||||
@@ -75,14 +75,58 @@ vi.mock('@/config', () => ({
|
||||
|
||||
// Mock PresetsParameter component
|
||||
vi.mock('@/app/components/header/account-setting/model-provider-page/model-parameter-modal/presets-parameter', () => ({
|
||||
default: ({ onSelect }: { onSelect: (toneId: number) => void }) => (
|
||||
<div data-testid="presets-parameter">
|
||||
<button data-testid="preset-creative" onClick={() => onSelect(1)}>Creative</button>
|
||||
<button data-testid="preset-balanced" onClick={() => onSelect(2)}>Balanced</button>
|
||||
<button data-testid="preset-precise" onClick={() => onSelect(3)}>Precise</button>
|
||||
<button data-testid="preset-custom" onClick={() => onSelect(4)}>Custom</button>
|
||||
</div>
|
||||
),
|
||||
default: ({ onSelect, supportedParameterNames }: { onSelect: (toneId: number) => void, supportedParameterNames?: string[] }) => {
|
||||
const hasSupportedParameter = !supportedParameterNames || supportedParameterNames.some(name => ['temperature', 'top_p', 'presence_penalty', 'frequency_penalty'].includes(name))
|
||||
if (!hasSupportedParameter)
|
||||
return null
|
||||
|
||||
return (
|
||||
<div data-testid="presets-parameter">
|
||||
<button data-testid="preset-creative" onClick={() => onSelect(1)}>Creative</button>
|
||||
<button data-testid="preset-balanced" onClick={() => onSelect(2)}>Balanced</button>
|
||||
<button data-testid="preset-precise" onClick={() => onSelect(3)}>Precise</button>
|
||||
<button data-testid="preset-custom" onClick={() => onSelect(4)}>Custom</button>
|
||||
</div>
|
||||
)
|
||||
},
|
||||
}))
|
||||
|
||||
vi.mock('@/app/components/header/account-setting/model-provider-page/model-parameter-modal/presets-parameter-utils', () => ({
|
||||
getSupportedPresetConfig: (toneId: number, supportedParameterNames?: string[]) => {
|
||||
const toneConfigMap: Record<number, Record<string, number> | undefined> = {
|
||||
1: {
|
||||
temperature: 0.8,
|
||||
top_p: 0.9,
|
||||
presence_penalty: 0.1,
|
||||
frequency_penalty: 0.1,
|
||||
},
|
||||
2: {
|
||||
temperature: 0.5,
|
||||
top_p: 0.85,
|
||||
presence_penalty: 0.2,
|
||||
frequency_penalty: 0.3,
|
||||
},
|
||||
3: {
|
||||
temperature: 0.2,
|
||||
top_p: 0.75,
|
||||
presence_penalty: 0.5,
|
||||
frequency_penalty: 0.5,
|
||||
},
|
||||
}
|
||||
const toneConfig = toneConfigMap[toneId]
|
||||
if (!toneConfig)
|
||||
return {}
|
||||
|
||||
if (!supportedParameterNames)
|
||||
return toneConfig
|
||||
|
||||
return Object.entries(toneConfig).reduce<Record<string, number>>((acc, [key, value]) => {
|
||||
if (supportedParameterNames.includes(key))
|
||||
acc[key] = value
|
||||
|
||||
return acc
|
||||
}, {})
|
||||
},
|
||||
}))
|
||||
|
||||
// Mock ParameterItem component
|
||||
@@ -202,7 +246,7 @@ describe('LLMParamsPanel', () => {
|
||||
|
||||
it('should render PresetsParameter for openai provider', () => {
|
||||
// Arrange
|
||||
setupModelParameterRulesMock({ data: [], isPending: false })
|
||||
setupModelParameterRulesMock({ data: [createParameterRule({ name: 'temperature' })], isPending: false })
|
||||
const props = createDefaultProps({ provider: 'langgenius/openai/openai' })
|
||||
|
||||
// Act
|
||||
@@ -214,7 +258,7 @@ describe('LLMParamsPanel', () => {
|
||||
|
||||
it('should render PresetsParameter for azure_openai provider', () => {
|
||||
// Arrange
|
||||
setupModelParameterRulesMock({ data: [], isPending: false })
|
||||
setupModelParameterRulesMock({ data: [createParameterRule({ name: 'temperature' })], isPending: false })
|
||||
const props = createDefaultProps({ provider: 'langgenius/azure_openai/azure_openai' })
|
||||
|
||||
// Act
|
||||
@@ -224,6 +268,18 @@ describe('LLMParamsPanel', () => {
|
||||
expect(screen.getByTestId('presets-parameter')).toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('should not render PresetsParameter when no visible parameter supports presets', () => {
|
||||
// Arrange
|
||||
setupModelParameterRulesMock({ data: [createParameterRule({ name: 'max_tokens', type: 'int' })], isPending: false })
|
||||
const props = createDefaultProps({ provider: 'langgenius/openai/openai' })
|
||||
|
||||
// Act
|
||||
render(<LLMParamsPanel {...props} />)
|
||||
|
||||
// Assert
|
||||
expect(screen.queryByTestId('presets-parameter')).not.toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('should not render PresetsParameter for non-preset providers', () => {
|
||||
// Arrange
|
||||
setupModelParameterRulesMock({ data: [], isPending: false })
|
||||
@@ -360,7 +416,15 @@ describe('LLMParamsPanel', () => {
|
||||
it('should apply Creative preset config', () => {
|
||||
// Arrange
|
||||
const onCompletionParamsChange = vi.fn()
|
||||
setupModelParameterRulesMock({ data: [], isPending: false })
|
||||
setupModelParameterRulesMock({
|
||||
data: [
|
||||
createParameterRule({ name: 'temperature' }),
|
||||
createParameterRule({ name: 'top_p' }),
|
||||
createParameterRule({ name: 'presence_penalty' }),
|
||||
createParameterRule({ name: 'frequency_penalty' }),
|
||||
],
|
||||
isPending: false,
|
||||
})
|
||||
const props = createDefaultProps({
|
||||
provider: 'langgenius/openai/openai',
|
||||
onCompletionParamsChange,
|
||||
@@ -384,7 +448,15 @@ describe('LLMParamsPanel', () => {
|
||||
it('should apply Balanced preset config', () => {
|
||||
// Arrange
|
||||
const onCompletionParamsChange = vi.fn()
|
||||
setupModelParameterRulesMock({ data: [], isPending: false })
|
||||
setupModelParameterRulesMock({
|
||||
data: [
|
||||
createParameterRule({ name: 'temperature' }),
|
||||
createParameterRule({ name: 'top_p' }),
|
||||
createParameterRule({ name: 'presence_penalty' }),
|
||||
createParameterRule({ name: 'frequency_penalty' }),
|
||||
],
|
||||
isPending: false,
|
||||
})
|
||||
const props = createDefaultProps({
|
||||
provider: 'langgenius/openai/openai',
|
||||
onCompletionParamsChange,
|
||||
@@ -407,7 +479,15 @@ describe('LLMParamsPanel', () => {
|
||||
it('should apply Precise preset config', () => {
|
||||
// Arrange
|
||||
const onCompletionParamsChange = vi.fn()
|
||||
setupModelParameterRulesMock({ data: [], isPending: false })
|
||||
setupModelParameterRulesMock({
|
||||
data: [
|
||||
createParameterRule({ name: 'temperature' }),
|
||||
createParameterRule({ name: 'top_p' }),
|
||||
createParameterRule({ name: 'presence_penalty' }),
|
||||
createParameterRule({ name: 'frequency_penalty' }),
|
||||
],
|
||||
isPending: false,
|
||||
})
|
||||
const props = createDefaultProps({
|
||||
provider: 'langgenius/openai/openai',
|
||||
onCompletionParamsChange,
|
||||
@@ -430,7 +510,7 @@ describe('LLMParamsPanel', () => {
|
||||
it('should apply empty config for Custom preset (spreads undefined)', () => {
|
||||
// Arrange
|
||||
const onCompletionParamsChange = vi.fn()
|
||||
setupModelParameterRulesMock({ data: [], isPending: false })
|
||||
setupModelParameterRulesMock({ data: [createParameterRule({ name: 'temperature' })], isPending: false })
|
||||
const props = createDefaultProps({
|
||||
provider: 'langgenius/openai/openai',
|
||||
onCompletionParamsChange,
|
||||
@@ -444,6 +524,27 @@ describe('LLMParamsPanel', () => {
|
||||
// Assert - Custom preset has no config, so only existing params are kept
|
||||
expect(onCompletionParamsChange).toHaveBeenCalledWith({ existing: 'value' })
|
||||
})
|
||||
|
||||
it('should apply only preset config keys supported by visible parameters', () => {
|
||||
// Arrange
|
||||
const onCompletionParamsChange = vi.fn()
|
||||
setupModelParameterRulesMock({ data: [createParameterRule({ name: 'temperature' })], isPending: false })
|
||||
const props = createDefaultProps({
|
||||
provider: 'langgenius/openai/openai',
|
||||
onCompletionParamsChange,
|
||||
completionParams: { existing: 'value' },
|
||||
})
|
||||
|
||||
// Act
|
||||
render(<LLMParamsPanel {...props} />)
|
||||
fireEvent.click(screen.getByTestId('preset-creative'))
|
||||
|
||||
// Assert
|
||||
expect(onCompletionParamsChange).toHaveBeenCalledWith({
|
||||
existing: 'value',
|
||||
temperature: 0.8,
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('handleParamChange', () => {
|
||||
|
||||
@@ -10,7 +10,8 @@ import { useTranslation } from 'react-i18next'
|
||||
import Loading from '@/app/components/base/loading'
|
||||
import ParameterItem from '@/app/components/header/account-setting/model-provider-page/model-parameter-modal/parameter-item'
|
||||
import PresetsParameter from '@/app/components/header/account-setting/model-provider-page/model-parameter-modal/presets-parameter'
|
||||
import { PROVIDER_WITH_PRESET_TONE, STOP_PARAMETER_RULE, TONE_LIST } from '@/config'
|
||||
import { getSupportedPresetConfig } from '@/app/components/header/account-setting/model-provider-page/model-parameter-modal/presets-parameter-utils'
|
||||
import { PROVIDER_WITH_PRESET_TONE, STOP_PARAMETER_RULE } from '@/config'
|
||||
import { useModelParameterRules } from '@/service/use-common'
|
||||
|
||||
type Props = {
|
||||
@@ -34,15 +35,15 @@ const LLMParamsPanel = ({
|
||||
const parameterRules: ModelParameterRule[] = useMemo(() => {
|
||||
return parameterRulesData?.data || []
|
||||
}, [parameterRulesData])
|
||||
const supportedPresetParameterNames = useMemo(() => {
|
||||
return parameterRules.map(parameterRule => parameterRule.name)
|
||||
}, [parameterRules])
|
||||
|
||||
const handleSelectPresetParameter = (toneId: number) => {
|
||||
const tone = TONE_LIST.find(tone => tone.id === toneId)
|
||||
if (tone) {
|
||||
onCompletionParamsChange({
|
||||
...completionParams,
|
||||
...tone.config,
|
||||
})
|
||||
}
|
||||
onCompletionParamsChange({
|
||||
...completionParams,
|
||||
...getSupportedPresetConfig(toneId, supportedPresetParameterNames),
|
||||
})
|
||||
}
|
||||
const handleParamChange = (key: string, value: ParameterValue) => {
|
||||
onCompletionParamsChange({
|
||||
@@ -77,7 +78,10 @@ const LLMParamsPanel = ({
|
||||
<div className={cn('flex h-6 items-center system-sm-semibold text-text-secondary')}>{t('modelProvider.parameters', { ns: 'common' })}</div>
|
||||
{
|
||||
PROVIDER_WITH_PRESET_TONE.includes(provider) && (
|
||||
<PresetsParameter onSelect={handleSelectPresetParameter} />
|
||||
<PresetsParameter
|
||||
onSelect={handleSelectPresetParameter}
|
||||
supportedParameterNames={supportedPresetParameterNames}
|
||||
/>
|
||||
)
|
||||
}
|
||||
</div>
|
||||
|
||||
Reference in New Issue
Block a user