From 73e449c3efcce2aa4f1fa272806401c79164c159 Mon Sep 17 00:00:00 2001 From: Stephen Zhou <38493346+hyoban@users.noreply.github.com> Date: Tue, 1 Sep 2026 20:22:46 +0800 Subject: [PATCH] fix(workflow): allow KnowledgeFS top n up to 100 --- .../base/param-item/__tests__/index.spec.tsx | 16 ++++++++++------ .../param-item/__tests__/top-k-item.spec.tsx | 6 ++++++ web/app/components/base/param-item/index.tsx | 4 +++- .../components/base/param-item/top-k-item.tsx | 15 +++++++++++---- .../__tests__/recall-settings.spec.tsx | 19 ++++++++++++++----- .../components/recall-settings.tsx | 2 ++ .../nodes/knowledge-retrieval-v2/constants.ts | 2 ++ .../knowledge-retrieval-v2/use-config.ts | 3 ++- 8 files changed, 50 insertions(+), 17 deletions(-) diff --git a/web/app/components/base/param-item/__tests__/index.spec.tsx b/web/app/components/base/param-item/__tests__/index.spec.tsx index 8c595749608..6f25959069c 100644 --- a/web/app/components/base/param-item/__tests__/index.spec.tsx +++ b/web/app/components/base/param-item/__tests__/index.spec.tsx @@ -109,15 +109,17 @@ describe('ParamItem', () => { expect(defaultProps.onChange).toHaveBeenLastCalledWith('test_param', 0.8) }) - it('should reset the textbox and slider when users clear the input', async () => { + it('should allow users to replace the minimum value with a multi-digit value', async () => { const user = userEvent.setup() const StatefulParamItem = () => { - const [value, setValue] = useState(defaultProps.value) + const [value, setValue] = useState(1) return ( { defaultProps.onChange(key, nextValue) setValue(nextValue) @@ -131,12 +133,14 @@ describe('ParamItem', () => { const input = screen.getByRole('textbox') await user.clear(input) - expect(defaultProps.onChange).toHaveBeenLastCalledWith('test_param', 0) - expect(getSlider()).toHaveAttribute('aria-valuenow', '0') + expect(input).toHaveValue('') + expect(defaultProps.onChange).not.toHaveBeenCalled() - await user.tab() + await user.type(input, '20') - expect(input).toHaveValue('0') + expect(input).toHaveValue('20') + expect(defaultProps.onChange).toHaveBeenLastCalledWith('test_param', 20) + expect(getSlider()).toHaveAttribute('aria-valuenow', '20') }) it('should clamp out-of-range text edits before updating state', async () => { diff --git a/web/app/components/base/param-item/__tests__/top-k-item.spec.tsx b/web/app/components/base/param-item/__tests__/top-k-item.spec.tsx index 6e2402af24d..d0173504f55 100644 --- a/web/app/components/base/param-item/__tests__/top-k-item.spec.tsx +++ b/web/app/components/base/param-item/__tests__/top-k-item.spec.tsx @@ -75,6 +75,12 @@ describe('TopKItem', () => { expect(input).toBeInTheDocument() }) + it('should allow a feature-specific maximum', () => { + render() + + expect(getSlider()).toHaveAttribute('max', '100') + }) + it('should render slider with max >= 5 so no scaling is applied', () => { render() const slider = getSlider() diff --git a/web/app/components/base/param-item/index.tsx b/web/app/components/base/param-item/index.tsx index c9469e81313..ff9e49049ec 100644 --- a/web/app/components/base/param-item/index.tsx +++ b/web/app/components/base/param-item/index.tsx @@ -85,7 +85,9 @@ const ParamItem: FC = ({ max={max} step={step} value={value} - onValueChange={(nextValue) => onChange(id, nextValue ?? min)} + onValueChange={(nextValue) => { + if (nextValue !== null) onChange(id, nextValue) + }} > diff --git a/web/app/components/base/param-item/top-k-item.tsx b/web/app/components/base/param-item/top-k-item.tsx index 8c23563de98..64e0a960fb4 100644 --- a/web/app/components/base/param-item/top-k-item.tsx +++ b/web/app/components/base/param-item/top-k-item.tsx @@ -11,22 +11,28 @@ type Props = Readonly<{ onChange: (key: string, value: number) => void enable: boolean disabled?: boolean + max?: number }> -const maxTopK = env.NEXT_PUBLIC_TOP_K_MAX_VALUE const VALUE_LIMIT = { default: 2, step: 1, min: 1, - max: maxTopK, } -const TopKItem: FC = ({ className, value, enable, onChange, disabled = false }) => { +const TopKItem: FC = ({ + className, + value, + enable, + onChange, + disabled = false, + max = env.NEXT_PUBLIC_TOP_K_MAX_VALUE, +}) => { const { t } = useTranslation() const handleParamChange = (key: string, value: number) => { let notOutRangeValue = Number.parseInt(value.toFixed(0)) notOutRangeValue = Math.max(VALUE_LIMIT.min, notOutRangeValue) - notOutRangeValue = Math.min(VALUE_LIMIT.max, notOutRangeValue) + notOutRangeValue = Math.min(max, notOutRangeValue) onChange(key, notOutRangeValue) } return ( @@ -36,6 +42,7 @@ const TopKItem: FC = ({ className, value, enable, onChange, disabled = fa name={t(($) => $['datasetConfig.top_k'], { ns: 'appDebug' })} tip={t(($) => $['datasetConfig.top_kTip'], { ns: 'appDebug' }) as string} {...VALUE_LIMIT} + max={max} value={value} enable={enable} disabled={disabled} diff --git a/web/app/components/workflow/nodes/knowledge-retrieval-v2/components/__tests__/recall-settings.spec.tsx b/web/app/components/workflow/nodes/knowledge-retrieval-v2/components/__tests__/recall-settings.spec.tsx index cee40b4eae5..532dec08de0 100644 --- a/web/app/components/workflow/nodes/knowledge-retrieval-v2/components/__tests__/recall-settings.spec.tsx +++ b/web/app/components/workflow/nodes/knowledge-retrieval-v2/components/__tests__/recall-settings.spec.tsx @@ -3,6 +3,7 @@ import { fireEvent, render, screen } from '@testing-library/react' import RecallSettings from '../recall-settings' const mockModelSelector = vi.hoisted(() => vi.fn()) +const mockTopKItem = vi.hoisted(() => vi.fn()) vi.mock('@/app/components/header/account-setting/model-provider-page/hooks', () => ({ useModelListAndDefaultModelAndCurrentProviderAndModel: () => ({ @@ -45,11 +46,18 @@ vi.mock('@langgenius/dify-ui/select', () => ({ })) vi.mock('@/app/components/base/param-item/top-k-item', () => ({ - default: (props: { onChange: (key: string, value: number) => void; value: number }) => ( - - ), + default: (props: { + max?: number + onChange: (key: string, value: number) => void + value: number + }) => { + mockTopKItem(props) + return ( + + ) + }, })) vi.mock('@/app/components/base/param-item/score-threshold-item', () => ({ @@ -86,6 +94,7 @@ describe('RecallSettings', () => { ) expect(screen.getByText('common.modelProvider.defaultConfig')).toBeInTheDocument() + expect(mockTopKItem).toHaveBeenLastCalledWith(expect.objectContaining({ max: 100 })) expect(mockModelSelector).toHaveBeenLastCalledWith( expect.objectContaining({ value: { provider: 'system/provider', model: 'system-rerank' }, diff --git a/web/app/components/workflow/nodes/knowledge-retrieval-v2/components/recall-settings.tsx b/web/app/components/workflow/nodes/knowledge-retrieval-v2/components/recall-settings.tsx index 0d36b837fd0..b31568c6d86 100644 --- a/web/app/components/workflow/nodes/knowledge-retrieval-v2/components/recall-settings.tsx +++ b/web/app/components/workflow/nodes/knowledge-retrieval-v2/components/recall-settings.tsx @@ -20,6 +20,7 @@ import TopKItem from '@/app/components/base/param-item/top-k-item' import { ModelTypeEnum } from '@/app/components/header/account-setting/model-provider-page/declarations' import { useModelListAndDefaultModelAndCurrentProviderAndModel } from '@/app/components/header/account-setting/model-provider-page/hooks' import { ModelSelector } from '@/app/components/header/account-setting/model-provider-page/model-selector' +import { KNOWLEDGE_RETRIEVAL_V2_TOP_N_MAX } from '../constants' const i18nPrefix = 'nodes.knowledgeRetrievalV2' const DEFAULT_SCORE_THRESHOLD = 0.5 @@ -186,6 +187,7 @@ const RecallSettings: FC = ({ value={topK} enable disabled={readonly} + max={KNOWLEDGE_RETRIEVAL_V2_TOP_N_MAX} onChange={(_, value) => onTopKChange(value)} /> { const { nodesReadOnly: readOnly } = useNodesReadOnly() @@ -91,7 +92,7 @@ const useConfig = (id: string, payload: KnowledgeRetrievalV2NodeType) => { const handleTopNChange = useCallback( (topN: number) => { - if (!Number.isInteger(topN) || topN < 1 || topN > 100) return + if (!Number.isInteger(topN) || topN < 1 || topN > KNOWLEDGE_RETRIEVAL_V2_TOP_N_MAX) return setInputs( produce(inputs, (draft) => { draft.top_n = topN