From 02151d0afa51d8d83264656b99c2ab4d6f093560 Mon Sep 17 00:00:00 2001 From: Jyong Date: Wed, 19 Aug 2026 03:03:20 -0400 Subject: [PATCH] fix(knowledge-fs): correct quality retest workflow --- ...uality-control-database-repository.test.ts | 5 + .../quality-control-database-repository.ts | 2 +- .../new-rag/__tests__/quality-page.spec.tsx | 121 ++++-------- .../__tests__/retrieval-test-page.spec.tsx | 38 +++- web/features/new-rag/quality/quality-page.tsx | 133 ++++--------- web/features/new-rag/retrieval-test-page.tsx | 185 +++++++++++------- web/i18n/ar-TN/dataset.json | 1 + web/i18n/de-DE/dataset.json | 1 + web/i18n/en-US/dataset.json | 1 + web/i18n/es-ES/dataset.json | 1 + web/i18n/fa-IR/dataset.json | 1 + web/i18n/fr-FR/dataset.json | 1 + web/i18n/hi-IN/dataset.json | 1 + web/i18n/id-ID/dataset.json | 1 + web/i18n/it-IT/dataset.json | 1 + web/i18n/ja-JP/dataset.json | 1 + web/i18n/ko-KR/dataset.json | 1 + web/i18n/lo-LA/dataset.json | 1 + web/i18n/nl-NL/dataset.json | 1 + web/i18n/pl-PL/dataset.json | 1 + web/i18n/pt-BR/dataset.json | 1 + web/i18n/ro-RO/dataset.json | 1 + web/i18n/ru-RU/dataset.json | 1 + web/i18n/sl-SI/dataset.json | 1 + web/i18n/th-TH/dataset.json | 1 + web/i18n/tr-TR/dataset.json | 1 + web/i18n/uk-UA/dataset.json | 1 + web/i18n/vi-VN/dataset.json | 1 + web/i18n/zh-Hans/dataset.json | 1 + web/i18n/zh-Hant/dataset.json | 1 + 30 files changed, 257 insertions(+), 251 deletions(-) diff --git a/knowledge-fs/packages/api/src/quality-control-database-repository.test.ts b/knowledge-fs/packages/api/src/quality-control-database-repository.test.ts index a9fef1fec31..223df07a469 100644 --- a/knowledge-fs/packages/api/src/quality-control-database-repository.test.ts +++ b/knowledge-fs/packages/api/src/quality-control-database-repository.test.ts @@ -2141,6 +2141,11 @@ describe("database quality-control repository", () => { await expect( repositoryFor().repository.updateBadCase({ ...base, status: "fixed" }), ).rejects.toThrow("Invalid bad-case transition open -> fixed"); + await expect( + repositoryFor({ badCase: { ...badCaseRow(), status: "fixed" } }).repository.updateBadCase( + base, + ), + ).resolves.toMatchObject({ revision: 2, status: "dismissed" }); await expect( repositoryFor().repository.updateBadCase({ ...base, status: "replaying" }), ).rejects.toThrow("requires a replay run"); diff --git a/knowledge-fs/packages/api/src/quality-control-database-repository.ts b/knowledge-fs/packages/api/src/quality-control-database-repository.ts index f924a883dbb..2d580aa423a 100644 --- a/knowledge-fs/packages/api/src/quality-control-database-repository.ts +++ b/knowledge-fs/packages/api/src/quality-control-database-repository.ts @@ -2058,7 +2058,7 @@ function mapHistoryEvent(row: DatabaseRow): QualityHistoryEvent { function validateBadCaseTransition(from: QualityBadCaseState, to: QualityBadCaseState) { const allowed: Readonly> = { dismissed: ["open"], - fixed: ["open", "replaying"], + fixed: ["dismissed", "open", "replaying"], open: ["dismissed", "replaying"], replaying: ["dismissed", "fixed", "open"], }; diff --git a/web/features/new-rag/__tests__/quality-page.spec.tsx b/web/features/new-rag/__tests__/quality-page.spec.tsx index b5bff8226ee..feb842567fb 100644 --- a/web/features/new-rag/__tests__/quality-page.spec.tsx +++ b/web/features/new-rag/__tests__/quality-page.spec.tsx @@ -8,7 +8,6 @@ import { QualityPage } from '../quality/quality-page' const serviceMock = vi.hoisted(() => ({ bulkImport: vi.fn(), createGolden: vi.fn(), - createReplay: vi.fn(), deleteGolden: vi.fn(), getBadCase: vi.fn(), getBadCases: vi.fn(), @@ -52,7 +51,6 @@ vi.mock('@/service/client', () => ({ traceReference: { get: serviceMock.getTraceReference }, }, }, - replayRuns: { post: serviceMock.createReplay }, }, }, }, @@ -857,71 +855,33 @@ describe('QualityPage', () => { ).not.toBeInTheDocument() }) - it('reuses the replay idempotency key after a partial failure', async () => { - let linked = false - let rejectReplayPatch = true - serviceMock.getBadCase.mockImplementation(async () => ({ - created_at: '2026-07-28T00:00:00Z', - id: 'bad-1', - question: 'Refund after activation', - reason: 'coverage gap', - replay_run_id: null, - revision: linked ? 2 : 1, - status: 'open', - tags: linked ? ['billing', 'golden-question:golden-2'] : ['billing'], - updated_at: '2026-07-28T00:00:00Z', - })) - serviceMock.updateBadCase.mockImplementation( - async (input: { body: { status: string; tags?: string[] } }) => { - if (input.body.status === 'open') { - linked = true - return { - ...(await serviceMock.getBadCase()), - revision: 2, - tags: input.body.tags, - } - } - if (rejectReplayPatch) { - rejectReplayPatch = false - throw new Error('response lost') - } - return { - ...(await serviceMock.getBadCase()), - replay_run_id: 'replay-1', - revision: 3, - status: 'replaying', - } - }, - ) - serviceMock.createReplay.mockResolvedValue({ id: 'replay-1', revision: 1, state: 'queued' }) + it('opens the source trace in retrieval test and requests one retest', async () => { navigationMock.tab = 'bad-cases' const user = userEvent.setup() renderPage() await screen.findByText('Refund after activation') - const replay = async () => { - await user.click( - screen.getByRole('button', { - name: /dataset\.newKnowledge\.qualityPage\.questionActions/, - }), - ) - await user.click( - await screen.findByRole('menuitem', { - name: 'dataset.newKnowledge.qualityPage.replay', - }), - ) - } - await replay() - await waitFor(() => expect(serviceMock.createReplay).toHaveBeenCalledTimes(1)) - await replay() - await waitFor(() => expect(serviceMock.createReplay).toHaveBeenCalledTimes(2)) + await user.click( + screen.getByRole('button', { + name: /dataset\.newKnowledge\.qualityPage\.questionActions/, + }), + ) + await user.click( + await screen.findByRole('menuitem', { + name: 'dataset.newKnowledge.qualityPage.replay', + }), + ) - expect(serviceMock.createGolden).toHaveBeenCalledTimes(2) - const firstHeaders = serviceMock.createReplay.mock.calls[0]?.[0].headers - expect(serviceMock.createReplay.mock.calls[1]?.[0].headers).toEqual(firstHeaders) + await waitFor(() => + expect(routerMock.push).toHaveBeenCalledWith( + '/datasets/new/space-1/retrieval?retest=trace-42&trace=trace-42', + ), + ) + expect(serviceMock.createGolden).not.toHaveBeenCalled() + expect(serviceMock.updateBadCase).not.toHaveBeenCalled() }) - it('replaces a stale golden-question link before replaying a bad case', async () => { + it('creates a golden question and dismisses its source bad case', async () => { serviceMock.getBadCase.mockResolvedValue({ created_at: '2026-07-28T00:00:00Z', id: 'bad-1', @@ -941,20 +901,12 @@ describe('QualityPage', () => { tags: ['billing'], updated_at: '2026-07-29T00:00:00Z', }) - serviceMock.updateBadCase - .mockResolvedValueOnce({ - ...(await serviceMock.getBadCase()), - revision: 2, - tags: ['billing', 'golden-question:replacement-golden'], - }) - .mockResolvedValueOnce({ - ...(await serviceMock.getBadCase()), - replay_run_id: 'replay-1', - revision: 3, - status: 'replaying', - tags: ['billing', 'golden-question:replacement-golden'], - }) - serviceMock.createReplay.mockResolvedValue({ id: 'replay-1', revision: 1, state: 'queued' }) + serviceMock.updateBadCase.mockResolvedValue({ + ...(await serviceMock.getBadCase()), + revision: 2, + status: 'dismissed', + tags: ['billing'], + }) navigationMock.tab = 'bad-cases' const user = userEvent.setup() renderPage() @@ -967,20 +919,21 @@ describe('QualityPage', () => { ) await user.click( await screen.findByRole('menuitem', { - name: 'dataset.newKnowledge.qualityPage.replay', + name: 'dataset.newKnowledge.qualityPage.toGolden', }), ) - - await waitFor(() => - expect(serviceMock.createReplay).toHaveBeenCalledWith( - expect.objectContaining({ - body: { golden_question_ids: ['replacement-golden'] }, - }), - ), + await user.type( + screen.getByPlaceholderText('dataset.newKnowledge.qualityPage.annotationPlaceholder'), + 'Expected answer', ) + await user.click( + screen.getByRole('button', { name: 'dataset.newKnowledge.qualityPage.promote' }), + ) + + await waitFor(() => expect(serviceMock.createGolden).toHaveBeenCalledTimes(1)) expect(serviceMock.createGolden.mock.calls[0]?.[0]).toEqual({ body: { - annotation: 'coverage gap', + annotation: 'Expected answer', expected_evidence_ids: [], match_policy: 'all', question: 'Refund after activation', @@ -992,8 +945,8 @@ describe('QualityPage', () => { expect(serviceMock.updateBadCase.mock.calls[0]?.[0]).toEqual({ body: { expected_revision: 1, - status: 'open', - tags: ['billing', 'golden-question:replacement-golden'], + status: 'dismissed', + tags: ['billing'], }, params: { bad_case_id: 'bad-1', control_space_id: 'space-1' }, }) diff --git a/web/features/new-rag/__tests__/retrieval-test-page.spec.tsx b/web/features/new-rag/__tests__/retrieval-test-page.spec.tsx index 81707241ce9..79d7bf20ea5 100644 --- a/web/features/new-rag/__tests__/retrieval-test-page.spec.tsx +++ b/web/features/new-rag/__tests__/retrieval-test-page.spec.tsx @@ -1488,11 +1488,17 @@ describe('RetrievalTestPage', () => { }) expect(makeBadCaseButton).toHaveClass('bg-components-button-secondary-bg') await user.click(makeBadCaseButton) + expect(apiMock.createBadCase).not.toHaveBeenCalled() + await user.click( + await screen.findByRole('menuitem', { + name: 'dataset.newKnowledge.qualityPage.reasonValues.lowScore', + }), + ) await waitFor(() => expect(apiMock.createBadCase).toHaveBeenCalledWith({ body: { - reason: 'retrieval-miss', + reason: 'low-score', tags: ['retrieval-test'], trace_id: 'trace-1', }, @@ -1609,6 +1615,11 @@ describe('RetrievalTestPage', () => { name: 'dataset.newKnowledge.retrievalTest.makeBadCase', }), ) + await user.click( + await screen.findByRole('menuitem', { + name: 'dataset.newKnowledge.qualityPage.reasonValues.retrievalMiss', + }), + ) await waitFor(() => expect(apiMock.createBadCase).toHaveBeenCalledWith({ @@ -1622,6 +1633,31 @@ describe('RetrievalTestPage', () => { ) }) + it('runs a one-shot retest from a linked production trace', async () => { + apiMock.traceDetail = { + completed: true, + created_at: '2026-07-01T00:00:00.000Z', + id: 'trace-old', + mode: 'deep', + profile: {}, + query: 'Retest the refund exception', + scores: {}, + stages: [], + } + const { onUrlUpdate } = renderPage({ + searchParams: '?trace=trace-old&retest=trace-old', + }) + + await waitFor(() => + expect(apiMock.queryAdmission).toHaveBeenCalledWith({ + body: { mode: 'deep', query: 'Retest the refund exception' }, + params: { control_space_id: 'space-1' }, + }), + ) + expect(apiMock.queryAdmission).toHaveBeenCalledTimes(1) + expect(onUrlUpdate).toHaveBeenCalled() + }) + it('opens retrieval evidence through its logical document instead of its asset', async () => { apiMock.traces = [ { diff --git a/web/features/new-rag/quality/quality-page.tsx b/web/features/new-rag/quality/quality-page.tsx index f7123f2d4e0..0bbe5926488 100644 --- a/web/features/new-rag/quality/quality-page.tsx +++ b/web/features/new-rag/quality/quality-page.tsx @@ -24,7 +24,7 @@ import { import { Popover, PopoverContent, PopoverTrigger } from '@langgenius/dify-ui/popover' import { toast } from '@langgenius/dify-ui/toast' import { useInfiniteQuery, useMutation, useQueryClient } from '@tanstack/react-query' -import { useRef, useState } from 'react' +import { useState } from 'react' import { useTranslation } from 'react-i18next' import Badge from '@/app/components/base/badge' import Loading from '@/app/components/base/loading' @@ -44,23 +44,15 @@ const emptyDraft: GoldenQuestionDraft = { const goldenLinkPrefix = 'golden-question:' const pageSize = 50 -function createIdempotencyKey() { - return `quality-replay-${ - globalThis.crypto?.randomUUID?.() ?? `${Date.now()}-${Math.random().toString(36).slice(2)}` - }` -} - function visibleTags(tags: string[]) { return tags.filter((tag) => !tag.startsWith(goldenLinkPrefix)) } -function linkedGoldenQuestionId(tags: string[]) { - return tags.find((tag) => tag.startsWith(goldenLinkPrefix))?.slice(goldenLinkPrefix.length) -} - function Reason({ question, reason, tags }: { question?: string; reason: string; tags: string[] }) { const { t } = useTranslation('dataset') const normalized = reason.toLowerCase() + if (normalized === 'low-score' || (normalized.includes('low') && normalized.includes('score'))) + return t(($) => $['newKnowledge.qualityPage.reasonValues.lowScore']) if (normalized.includes('outdated')) return t(($) => $['newKnowledge.qualityPage.reasonValues.outdatedContent']) if ( @@ -148,8 +140,6 @@ export function QualityPage({ knowledgeSpaceId }: { knowledgeSpaceId: string }) const [dialogSubmitting, setDialogSubmitting] = useState(false) const [importOpen, setImportOpen] = useState(false) const [pendingBadCaseId, setPendingBadCaseId] = useState() - const replayIdempotencyKeysRef = useRef(new Map()) - const pendingGoldenQuestionIdsRef = useRef(new Map()) const [dialog, setDialog] = useState< | { key: string; mode: 'create'; value: GoldenQuestionDraft } | { id: string; key: string; mode: 'edit'; value: GoldenQuestionDraft } @@ -227,52 +217,6 @@ export function QualityPage({ knowledgeSpaceId }: { knowledgeSpaceId: string }) params: { bad_case_id: badCaseId, control_space_id: knowledgeSpaceId }, }) - const ensureLinkedGoldenQuestion = async ( - item: KnowledgeFsBadCaseResponse, - draft: GoldenQuestionDraft, - ) => { - const current = await getBadCase(item.id) - const linkedId = linkedGoldenQuestionId(current.tags) - - let goldenQuestionId = pendingGoldenQuestionIdsRef.current.get(item.id) - if (!goldenQuestionId) { - const created = await createGoldenMutation.mutateAsync({ - body: { - ...goldenQuestionPayload(draft), - source_bad_case_id: item.id, - }, - params: { control_space_id: knowledgeSpaceId }, - }) - goldenQuestionId = created.id - pendingGoldenQuestionIdsRef.current.set(item.id, goldenQuestionId) - } - if (linkedId === goldenQuestionId) { - pendingGoldenQuestionIdsRef.current.delete(item.id) - return { badCase: current, goldenQuestionId } - } - - try { - const badCase = - await consoleClient.knowledgeFs.spaces.byControlSpaceId.quality.badCases.byBadCaseId.patch({ - body: { - expected_revision: current.revision, - status: current.status, - tags: [...visibleTags(current.tags), `${goldenLinkPrefix}${goldenQuestionId}`], - }, - params: { bad_case_id: current.id, control_space_id: knowledgeSpaceId }, - }) - pendingGoldenQuestionIdsRef.current.delete(item.id) - return { badCase, goldenQuestionId } - } catch (error) { - const refreshed = await getBadCase(item.id).catch(() => undefined) - if (refreshed && linkedGoldenQuestionId(refreshed.tags) === goldenQuestionId) { - pendingGoldenQuestionIdsRef.current.delete(item.id) - return { badCase: refreshed, goldenQuestionId } - } - throw error - } - } - const submitDialog = async (draft: GoldenQuestionDraft) => { if (!dialog) return setDialogError(undefined) @@ -291,9 +235,29 @@ export function QualityPage({ knowledgeSpaceId }: { knowledgeSpaceId: string }) }) toast.success(t(($) => $['newKnowledge.qualityPage.createdToast'])) } else { - const badCase = badCases.find((item) => item.id === dialog.id) - if (!badCase) throw new Error('Bad case is unavailable') - await ensureLinkedGoldenQuestion(badCase, draft) + const badCase = await getBadCase(dialog.id) + await createGoldenMutation.mutateAsync({ + body: { + ...goldenQuestionPayload(draft), + source_bad_case_id: badCase.id, + }, + params: { control_space_id: knowledgeSpaceId }, + }) + try { + await consoleClient.knowledgeFs.spaces.byControlSpaceId.quality.badCases.byBadCaseId.patch( + { + body: { + expected_revision: badCase.revision, + status: 'dismissed', + tags: visibleTags(badCase.tags), + }, + params: { bad_case_id: badCase.id, control_space_id: knowledgeSpaceId }, + }, + ) + } catch (error) { + const refreshed = await getBadCase(dialog.id).catch(() => undefined) + if (refreshed?.status !== 'dismissed') throw error + } toast.success(t(($) => $['newKnowledge.qualityPage.promotedToast'])) } await invalidateQuality() @@ -341,43 +305,18 @@ export function QualityPage({ knowledgeSpaceId }: { knowledgeSpaceId: string }) const replayBadCase = async (item: KnowledgeFsBadCaseResponse) => { setPendingBadCaseId(item.id) try { - const { badCase, goldenQuestionId } = await ensureLinkedGoldenQuestion(item, { - annotation: item.reason, - expectedEvidenceIds: [], - matchPolicy: 'all', - question: item.question ?? '', - tags: visibleTags(item.tags), - }) - let idempotencyKey = replayIdempotencyKeysRef.current.get(item.id) - if (!idempotencyKey) { - idempotencyKey = createIdempotencyKey() - replayIdempotencyKeysRef.current.set(item.id, idempotencyKey) - } - const replay = - await consoleClient.knowledgeFs.spaces.byControlSpaceId.quality.replayRuns.post({ - body: { golden_question_ids: [goldenQuestionId] }, - headers: { 'Idempotency-Key': idempotencyKey }, - params: { control_space_id: knowledgeSpaceId }, - }) - try { - await consoleClient.knowledgeFs.spaces.byControlSpaceId.quality.badCases.byBadCaseId.patch({ - body: { - expected_revision: badCase.revision, - replay_run_id: replay.id, - status: 'replaying', - tags: badCase.tags, + const reference = + await consoleClient.knowledgeFs.spaces.byControlSpaceId.quality.badCases.byBadCaseId.traceReference.get( + { + params: { bad_case_id: item.id, control_space_id: knowledgeSpaceId }, }, - params: { bad_case_id: badCase.id, control_space_id: knowledgeSpaceId }, - }) - } catch (error) { - const current = await getBadCase(item.id).catch(() => undefined) - if (current?.replay_run_id !== replay.id) throw error - } - replayIdempotencyKeysRef.current.delete(item.id) - await invalidateQuality() - toast.success(t(($) => $['newKnowledge.qualityPage.replayStartedToast'])) + ) + const search = new URLSearchParams({ + retest: reference.trace_id, + trace: reference.trace_id, + }) + router.push(`${newKnowledgeRetrievalTestPath(knowledgeSpaceId)}?${search.toString()}`) } catch { - await invalidateQuality() toast.error(t(($) => $.unknownError)) } finally { setPendingBadCaseId(undefined) diff --git a/web/features/new-rag/retrieval-test-page.tsx b/web/features/new-rag/retrieval-test-page.tsx index b03b68f676c..235f964a006 100644 --- a/web/features/new-rag/retrieval-test-page.tsx +++ b/web/features/new-rag/retrieval-test-page.tsx @@ -17,11 +17,17 @@ import type { ResearchTaskProgressEvent } from './services/research-task-events' import type { MarkdownProps } from '@/app/components/base/markdown' import { Button } from '@langgenius/dify-ui/button' import { cn } from '@langgenius/dify-ui/cn' +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuTrigger, +} from '@langgenius/dify-ui/dropdown-menu' import { toast } from '@langgenius/dify-ui/toast' import { matchesKeyboardEvent } from '@tanstack/react-hotkeys' import { skipToken, useInfiniteQuery, useQuery, useQueryClient } from '@tanstack/react-query' import { parseAsString, useQueryStates } from 'nuqs' -import { useCallback, useEffect, useMemo, useRef, useState } from 'react' +import { useCallback, useEffect, useEffectEvent, useMemo, useRef, useState } from 'react' import { useTranslation } from 'react-i18next' import { Markdown } from '@/app/components/base/markdown' import { Link as MarkdownLink } from '@/app/components/base/markdown-blocks' @@ -75,6 +81,8 @@ type ComposerDraft = { type QualityDecision = 'bad-case' | 'golden' +type BadCaseReason = 'low-score' | 'retrieval-miss' + type ResearchExpansionState = Partial> type GoldenQuestionPromotion = { @@ -83,8 +91,6 @@ type GoldenQuestionPromotion = { value: GoldenQuestionDraft } -const retrievalTestBadCaseReason = 'retrieval-miss' - const researchStageOrder = ['planning', 'retrieving', 'analyzing', 'generating'] as const type ResearchStage = (typeof researchStageOrder)[number] const runRetrievalHotkey = 'Mod+Enter' satisfies Hotkey @@ -114,6 +120,11 @@ function timeValue(value: number) { return value < 10_000_000_000 ? value * 1000 : value } +function normalizedRetrievalTestMode(mode?: string): RetrievalTestMode { + if (mode === 'deep' || mode === 'research') return mode + return 'fast' +} + function formatRecordTime(value: number) { return new Intl.DateTimeFormat(undefined, { day: 'numeric', @@ -736,14 +747,16 @@ function QualityActions({ badCaseAvailable, decision, noResults, - onDecision, + onBadCase, + onGolden, pending, qualityHref, }: { badCaseAvailable: boolean decision?: QualityDecision noResults?: boolean - onDecision: (decision: QualityDecision) => Promise + onBadCase: (reason: BadCaseReason) => Promise + onGolden: () => void pending?: boolean qualityHref: string }) { @@ -776,22 +789,30 @@ function QualityActions({ return (
{badCaseAvailable && ( - + + } + > + + {t(($) => $['newKnowledge.retrievalTest.makeBadCase'])} + + + void onBadCase('low-score')}> + {t(($) => $['newKnowledge.qualityPage.reasonValues.lowScore'])} + + void onBadCase('retrieval-miss')}> + {t(($) => $['newKnowledge.qualityPage.reasonValues.retrievalMiss'])} + + + )} {!noResults && (