From 39c31ef46cc48fc03bf0d8a61e2a40516af615a3 Mon Sep 17 00:00:00 2001 From: Jyong Date: Wed, 19 Aug 2026 02:40:20 -0400 Subject: [PATCH] fix(knowledge-fs): clarify golden question evidence editing --- api/knowledge-fs-contract.lock.json | 4 +- api/services/knowledge_fs/data_facade.py | 13 +- api/services/knowledge_fs/product_dto.py | 21 +- .../services/test_knowledge_fs_data_facade.py | 41 +++- .../services/test_knowledge_fs_product_dto.py | 14 ++ ...6-08-19-golden-question-evidence-editor.md | 29 +++ ...-question-handlers-branch-coverage.test.ts | 50 ++++- .../api/src/golden-question-handlers.ts | 36 +++- ...knowledge-space-golden-question-schemas.ts | 25 ++- .../api/console/knowledge-fs/types.gen.ts | 7 +- .../api/console/knowledge-fs/zod.gen.ts | 7 +- .../search-input/__tests__/index.spec.tsx | 11 ++ .../components/base/search-input/index.tsx | 4 +- .../new-rag/__tests__/quality-page.spec.tsx | 102 +++++++++- .../__tests__/retrieval-test-page.spec.tsx | 1 - .../quality/golden-question-dialog.tsx | 186 ++++++++++++------ web/features/new-rag/quality/quality-page.tsx | 6 - web/features/new-rag/quality/types.ts | 1 - web/features/new-rag/retrieval-test-page.tsx | 2 - 19 files changed, 465 insertions(+), 95 deletions(-) create mode 100644 knowledge-fs/.harness/changes/2026-08-19-golden-question-evidence-editor.md diff --git a/api/knowledge-fs-contract.lock.json b/api/knowledge-fs-contract.lock.json index 9bbcf467cad..562f6bf87ea 100644 --- a/api/knowledge-fs-contract.lock.json +++ b/api/knowledge-fs-contract.lock.json @@ -1,7 +1,7 @@ { "schemaVersion": 5, - "subtreeTree": "8546a4070c6c6815883bee1fc681ec2af3c36b28", - "openapiSha256": "37c8bdd6a6e7696aae3b336b0de215577ecda45535b1c2eb02d7a337b7399a95", + "subtreeTree": "ec8579ba2dcd26f230c3a046c13c7c71b22889ae", + "openapiSha256": "5a1057572e2ba2442808296e3ef10cf39236de5b9f8fd58508e111ca9778f787", "capabilityV2AuthManifestSha256": "fc0a47e23cce12544882f0298522b4933002e892b84ce1815df7e81d36a7a0c7", "capabilityV2AuthTestVectorSha256": "ae0de37b1ff05c40f905cf17a7b410d8971acacf64db07d5ee3d6fecfa559ce3", "productOperationManifestSha256": "f936926be46ec84e828f993fa5536a92add52b46348f03c0c9f6e1414812f846", diff --git a/api/services/knowledge_fs/data_facade.py b/api/services/knowledge_fs/data_facade.py index 68a5e28bfc9..ad0ba6d7879 100644 --- a/api/services/knowledge_fs/data_facade.py +++ b/api/services/knowledge_fs/data_facade.py @@ -1977,13 +1977,22 @@ class KnowledgeFSDataFacade: control_space_id=control_space_id, operation_id="matchGoldenQuestionEvidence", payload=KnowledgeFSGoldenQuestionEvidenceMatchRemotePayload( - evidence_texts=[payload.evidence], + evidence_texts=[payload.evidence] if payload.evidence else None, minimum_similarity=payload.minimum_similarity, + node_ids=payload.node_ids or None, top_k=payload.top_k, ), ) if not isinstance(raw, dict): raise KnowledgeFSProductRemoteError("KnowledgeFS returned an invalid evidence match response") + if payload.node_ids: + return KnowledgeFSGoldenQuestionEvidenceMatchResponse.model_validate( + { + "candidates": raw.get("resolvedEvidence", []), + "evidence": "", + "matched": False, + } + ) items = raw.get("items") if not isinstance(items, list) or len(items) != 1 or not isinstance(items[0], dict): raise KnowledgeFSProductRemoteError("KnowledgeFS returned an invalid evidence match response") @@ -1991,7 +2000,7 @@ class KnowledgeFSDataFacade: return KnowledgeFSGoldenQuestionEvidenceMatchResponse.model_validate( { "candidates": item.get("candidates", []), - "evidence": item.get("evidenceText", payload.evidence), + "evidence": item.get("evidenceText", payload.evidence or ""), "matched": item.get("matched", False), } ) diff --git a/api/services/knowledge_fs/product_dto.py b/api/services/knowledge_fs/product_dto.py index 0dfc9bb8db1..31aae62180c 100644 --- a/api/services/knowledge_fs/product_dto.py +++ b/api/services/knowledge_fs/product_dto.py @@ -3182,8 +3182,9 @@ class KnowledgeFSGoldenQuestionListResponse(ResponseModel): class KnowledgeFSGoldenQuestionEvidenceMatchPayload(BaseModel): - evidence: str = Field(min_length=1, max_length=8_000) + evidence: str | None = Field(default=None, min_length=1, max_length=8_000) minimum_similarity: float = Field(default=0.7, ge=0, le=1) + node_ids: list[str] = Field(default_factory=list, max_length=50) top_k: int = Field(default=5, ge=1, le=10) model_config = ConfigDict(extra="forbid") @@ -3193,10 +3194,22 @@ class KnowledgeFSGoldenQuestionEvidenceMatchPayload(BaseModel): def strip_evidence(cls, value: object) -> object: return value.strip() if isinstance(value, str) else value + @field_validator("node_ids") + @classmethod + def normalize_node_ids(cls, value: list[str]) -> list[str]: + return list(dict.fromkeys(item.strip() for item in value if item.strip())) + + @model_validator(mode="after") + def require_one_lookup(self) -> KnowledgeFSGoldenQuestionEvidenceMatchPayload: + if bool(self.evidence) == bool(self.node_ids): + raise ValueError("Provide exactly one of evidence or node_ids") + return self + class KnowledgeFSGoldenQuestionEvidenceMatchRemotePayload(BaseModel): - evidence_texts: list[str] = Field(serialization_alias="evidenceTexts") + evidence_texts: list[str] | None = Field(default=None, serialization_alias="evidenceTexts") minimum_similarity: float = Field(serialization_alias="minimumSimilarity") + node_ids: list[str] | None = Field(default=None, serialization_alias="nodeIds") top_k: int = Field(serialization_alias="topK") model_config = ConfigDict(extra="forbid", serialize_by_alias=True) @@ -3206,8 +3219,8 @@ class KnowledgeFSGoldenQuestionEvidenceCandidateResponse(ResponseModel): document_asset_id: str = Field(validation_alias=AliasChoices("document_asset_id", "documentAssetId")) node_id: str = Field(validation_alias=AliasChoices("node_id", "nodeId")) page_number: int | None = Field(default=None, validation_alias=AliasChoices("page_number", "pageNumber")) - projection_id: str = Field(validation_alias=AliasChoices("projection_id", "projectionId")) - score: float = Field(ge=0, le=1) + projection_id: str | None = Field(default=None, validation_alias=AliasChoices("projection_id", "projectionId")) + score: float | None = Field(default=None, ge=0, le=1) section_path: list[str] = Field(validation_alias=AliasChoices("section_path", "sectionPath")) text: str diff --git a/api/tests/unit_tests/services/test_knowledge_fs_data_facade.py b/api/tests/unit_tests/services/test_knowledge_fs_data_facade.py index 763b8fb4b5b..739c13a0cf8 100644 --- a/api/tests/unit_tests/services/test_knowledge_fs_data_facade.py +++ b/api/tests/unit_tests/services/test_knowledge_fs_data_facade.py @@ -1665,7 +1665,9 @@ def test_golden_question_facade_matches_evidence_and_forwards_csv_as_one_batch() "matchGoldenQuestionEvidence", "bulkImportGoldenQuestions", ] - assert interactive.call_args_list[0].kwargs["payload"].model_dump(mode="json", by_alias=True) == { + assert interactive.call_args_list[0].kwargs["payload"].model_dump( + mode="json", by_alias=True, exclude_none=True + ) == { "evidenceTexts": ["Refund policy"], "minimumSimilarity": 0.7, "topK": 5, @@ -1687,6 +1689,43 @@ def test_golden_question_facade_matches_evidence_and_forwards_csv_as_one_batch() } +def test_golden_question_facade_resolves_saved_evidence_ids_without_a_search_term() -> None: + facade = KnowledgeFSDataFacade(broker=MagicMock(), remote=MagicMock()) + interactive = MagicMock( + return_value={ + "items": [], + "resolvedEvidence": [ + { + "documentAssetId": "document-1", + "nodeId": "node-1", + "pageNumber": 2, + "sectionPath": ["Permissions", "Roles"], + "text": "Only workspace owners can change permissions.", + } + ], + } + ) + + with patch.object(facade, "_interactive", interactive): + result = facade.match_golden_question_evidence( + tenant_id="tenant-1", + account_id="account-1", + control_space_id="control-1", + payload=KnowledgeFSGoldenQuestionEvidenceMatchPayload(node_ids=[" node-1 ", "node-1"]), + ) + + assert result.evidence == "" + assert result.matched is False + assert result.candidates[0].node_id == "node-1" + assert result.candidates[0].projection_id is None + assert result.candidates[0].score is None + assert interactive.call_args.kwargs["payload"].model_dump(mode="json", by_alias=True, exclude_none=True) == { + "minimumSimilarity": 0.7, + "nodeIds": ["node-1"], + "topK": 5, + } + + @pytest.mark.parametrize( "remote_response", [None, True, 1, 1.5, [], "invalid", {}, {"items": []}, {"items": [None]}], diff --git a/api/tests/unit_tests/services/test_knowledge_fs_product_dto.py b/api/tests/unit_tests/services/test_knowledge_fs_product_dto.py index 5a1f5e62b13..074c76573ba 100644 --- a/api/tests/unit_tests/services/test_knowledge_fs_product_dto.py +++ b/api/tests/unit_tests/services/test_knowledge_fs_product_dto.py @@ -20,6 +20,7 @@ from services.knowledge_fs.product_dto import ( KnowledgeFSDocumentReindexPayload, KnowledgeFSDocumentReindexResponse, KnowledgeFSFindQuery, + KnowledgeFSGoldenQuestionEvidenceMatchPayload, KnowledgeFSGoldenQuestionPayload, KnowledgeFSGoldenQuestionResponse, KnowledgeFSGrepQuery, @@ -66,6 +67,19 @@ from services.knowledge_fs.product_dto import ( ) +def test_golden_question_evidence_match_requires_exactly_one_lookup_mode() -> None: + assert KnowledgeFSGoldenQuestionEvidenceMatchPayload(evidence=" permissions ").evidence == "permissions" + assert KnowledgeFSGoldenQuestionEvidenceMatchPayload(node_ids=[" node-1 ", "node-1", "node-2"]).node_ids == [ + "node-1", + "node-2", + ] + + with pytest.raises(ValidationError, match="Provide exactly one of evidence or node_ids"): + KnowledgeFSGoldenQuestionEvidenceMatchPayload() + with pytest.raises(ValidationError, match="Provide exactly one of evidence or node_ids"): + KnowledgeFSGoldenQuestionEvidenceMatchPayload(evidence="permissions", node_ids=["node-1"]) + + def test_settings_response_serializes_rerank_plugin_id_with_its_public_alias() -> None: response = KnowledgeFSSettingsResponse.model_validate( { diff --git a/knowledge-fs/.harness/changes/2026-08-19-golden-question-evidence-editor.md b/knowledge-fs/.harness/changes/2026-08-19-golden-question-evidence-editor.md new file mode 100644 index 00000000000..a2b4782e01e --- /dev/null +++ b/knowledge-fs/.harness/changes/2026-08-19-golden-question-evidence-editor.md @@ -0,0 +1,29 @@ +# Golden Question Evidence Editor + +## Summary + +- Separated the durable Golden Question evidence selection from the transient text used to search + for candidate evidence. +- Added exact, permission-filtered node resolution to the existing evidence-match operation so an + editor can render the passages represented by persisted `expectedEvidenceIds`. +- Kept semantic evidence matching unchanged for interactive searches while allowing callers to + request exact nodes with `nodeIds`. +- Stopped new Console create and update flows from persisting the evidence search text as Golden + Question metadata. + +## User-visible behavior + +- Editing a Golden Question displays every selected passage, its section path, and an individual + remove action instead of only showing the selected count. +- The evidence search field starts empty, clears after a successful lookup, and never becomes part + of the Golden Question payload. +- Saved node IDs that are no longer visible or no longer exist remain removable and are shown as + stale instead of silently disappearing from the selection. + +## Regression contract + +- Exact evidence resolution preserves the requested node order and applies candidate permission + filtering before returning text. +- Semantic matching and retrieval-test promotion continue to expose selectable candidates. +- Create and update requests persist `expectedEvidenceIds` and `matchPolicy`, but omit transient + evidence search text. diff --git a/knowledge-fs/packages/api/src/golden-question-handlers-branch-coverage.test.ts b/knowledge-fs/packages/api/src/golden-question-handlers-branch-coverage.test.ts index d73b6845203..471d60045ae 100644 --- a/knowledge-fs/packages/api/src/golden-question-handlers-branch-coverage.test.ts +++ b/knowledge-fs/packages/api/src/golden-question-handlers-branch-coverage.test.ts @@ -1,4 +1,4 @@ -import type { GoldenQuestion } from "@knowledge/core"; +import { type GoldenQuestion, KnowledgeNodeSchema } from "@knowledge/core"; import { describe, expect, it, vi } from "vitest"; import { encodeGoldenQuestionCursor } from "./cursor-utils"; @@ -16,6 +16,7 @@ import { deleteGoldenQuestionRoute, getGoldenQuestionRoute, listGoldenQuestionsRoute, + matchGoldenQuestionEvidenceRoute, updateGoldenQuestionRoute, } from "./golden-question-routes"; import { KnowledgeFsValidationError } from "./knowledge-fs-errors"; @@ -111,6 +112,48 @@ describe("golden-question handler branch coverage", () => { } }); + it("resolves the exact visible evidence nodes without running semantic matching", async () => { + const node = KnowledgeNodeSchema.parse({ + artifactHash: "f".repeat(64), + documentAssetId: ASSET_ID, + endOffset: 26, + id: NODE_ID, + kind: "chunk", + knowledgeSpaceId: SPACE_ID, + metadata: {}, + parseArtifactId: "018f0d60-7a49-7cc2-9c1b-5b36f18f2c46", + permissionScope: [], + sourceLocation: { + endOffset: 26, + pageNumber: 2, + sectionPath: ["Permissions", "Roles"], + startOffset: 0, + }, + startOffset: 0, + text: "Only workspace owners can change permissions.", + }); + const fixture = goldenFixture({ + body: { nodeIds: [NODE_ID] }, + nodes: [node], + }); + + expect(await fixture.invoke(matchGoldenQuestionEvidenceRoute)).toEqual({ + body: { + items: [], + resolvedEvidence: [ + { + documentAssetId: ASSET_ID, + nodeId: NODE_ID, + pageNumber: 2, + sectionPath: ["Permissions", "Roles"], + text: "Only workspace owners can change permissions.", + }, + ], + }, + status: 200, + }); + }); + it("gets existing questions and hides absent rows", async () => { expect(await goldenFixture().invoke(getGoldenQuestionRoute)).toMatchObject({ body: { id: QUESTION_ID }, @@ -325,6 +368,7 @@ interface GoldenFixtureOptions { readonly keySpaceId?: string; readonly listError?: Error; readonly listResult?: { readonly items: GoldenQuestion[]; readonly nextCursor?: unknown }; + readonly nodes?: readonly unknown[]; readonly permissionError?: Error; readonly query?: Record; readonly space?: unknown; @@ -380,7 +424,9 @@ function goldenFixture(options: GoldenFixtureOptions = {}) { get: vi.fn(async () => (options.asset === undefined ? { metadata: {} } : options.asset)), } as never, authorization: { authorize } as never, - nodes: { getManyByIdsAcrossGenerations: vi.fn(async () => []) } as never, + nodes: { + getManyByIdsAcrossGenerations: vi.fn(async () => options.nodes ?? []), + } as never, now: () => "2026-07-14T12:00:00.000Z", questions: questions as never, spaces: { diff --git a/knowledge-fs/packages/api/src/golden-question-handlers.ts b/knowledge-fs/packages/api/src/golden-question-handlers.ts index 5342469b136..20a42ce5e78 100644 --- a/knowledge-fs/packages/api/src/golden-question-handlers.ts +++ b/knowledge-fs/packages/api/src/golden-question-handlers.ts @@ -145,12 +145,44 @@ export function registerGoldenQuestionHandlers({ knowledgeSpaceId, now, }); + const body = context.req.valid("json"); + if (body.nodeIds) { + const resolvedNodes = await nodes.getManyByIdsAcrossGenerations({ + ids: uniqueStrings(body.nodeIds), + knowledgeSpaceId, + }); + const nodesById = new Map( + resolvedNodes + .filter((node) => candidatePermissionAllowsNode(node, permission.candidateGrants)) + .map((node) => [node.id, node]), + ); + return context.json( + { + items: [], + resolvedEvidence: body.nodeIds.flatMap((nodeId) => { + const node = nodesById.get(nodeId); + if (!node) return []; + return [ + { + documentAssetId: node.documentAssetId, + nodeId: node.id, + ...(node.sourceLocation.pageNumber === undefined + ? {} + : { pageNumber: node.sourceLocation.pageNumber }), + sectionPath: [...node.sourceLocation.sectionPath], + text: node.text, + }, + ]; + }), + }, + 200, + ); + } if (!evidenceMatcher) { return context.json({ error: "Golden question evidence matching is unavailable" }, 503); } - const body = context.req.valid("json"); const matches = await evidenceMatcher.match({ - evidenceTexts: body.evidenceTexts, + evidenceTexts: body.evidenceTexts ?? [], knowledgeSpaceId, minimumSimilarity: body.minimumSimilarity, permissionScope: permission.candidateGrants, diff --git a/knowledge-fs/packages/api/src/knowledge-space-golden-question-schemas.ts b/knowledge-fs/packages/api/src/knowledge-space-golden-question-schemas.ts index 2d4d92b13c4..0f4f7ad2b1e 100644 --- a/knowledge-fs/packages/api/src/knowledge-space-golden-question-schemas.ts +++ b/knowledge-fs/packages/api/src/knowledge-space-golden-question-schemas.ts @@ -10,6 +10,7 @@ import { const MAX_GOLDEN_QUESTION_ANNOTATION_EVIDENCE = 50; export const MAX_GOLDEN_QUESTION_BULK_IMPORT_ROWS = 500; export const MAX_GOLDEN_QUESTION_EVIDENCE_MATCH_TEXTS = 500; +export const MAX_GOLDEN_QUESTION_EXPECTED_EVIDENCE_IDS = 50; const DEFAULT_LIST_LIMIT = 100; const BoundedListLimitSchema = z.preprocess( (value) => (value === undefined ? DEFAULT_LIST_LIMIT : value), @@ -148,11 +149,20 @@ export const MatchGoldenQuestionEvidenceSchema = z evidenceTexts: z .array(GoldenQuestionEvidenceTextSchema) .min(1) - .max(MAX_GOLDEN_QUESTION_EVIDENCE_MATCH_TEXTS), + .max(MAX_GOLDEN_QUESTION_EVIDENCE_MATCH_TEXTS) + .optional(), minimumSimilarity: GoldenQuestionMinimumSimilaritySchema, + nodeIds: z + .array(z.string().uuid()) + .min(1) + .max(MAX_GOLDEN_QUESTION_EXPECTED_EVIDENCE_IDS) + .optional(), topK: GoldenQuestionEvidenceTopKSchema, }) - .strict(); + .strict() + .refine((value) => Boolean(value.evidenceTexts) !== Boolean(value.nodeIds), { + message: "Provide exactly one of evidenceTexts or nodeIds", + }); export const GoldenQuestionEvidenceCandidateSchema = z .object({ @@ -174,9 +184,20 @@ export const GoldenQuestionEvidenceMatchSchema = z }) .strict(); +export const GoldenQuestionResolvedEvidenceSchema = z + .object({ + documentAssetId: z.string().uuid(), + nodeId: z.string().uuid(), + pageNumber: z.number().int().positive().optional(), + sectionPath: z.array(z.string()), + text: z.string(), + }) + .strict(); + export const MatchGoldenQuestionEvidenceResponseSchema = z .object({ items: z.array(GoldenQuestionEvidenceMatchSchema), + resolvedEvidence: z.array(GoldenQuestionResolvedEvidenceSchema).optional(), }) .strict(); diff --git a/packages/contracts/generated/api/console/knowledge-fs/types.gen.ts b/packages/contracts/generated/api/console/knowledge-fs/types.gen.ts index 80780929e07..52fc4ef165c 100644 --- a/packages/contracts/generated/api/console/knowledge-fs/types.gen.ts +++ b/packages/contracts/generated/api/console/knowledge-fs/types.gen.ts @@ -415,8 +415,9 @@ export type KnowledgeFsGoldenQuestionBulkImportResponse = { } export type KnowledgeFsGoldenQuestionEvidenceMatchPayload = { - evidence: string + evidence?: string | null minimum_similarity?: number + node_ids?: Array top_k?: number } @@ -1590,8 +1591,8 @@ export type KnowledgeFsGoldenQuestionEvidenceCandidateResponse = { document_asset_id: string node_id: string page_number?: number | null - projection_id: string - score: number + projection_id?: string | null + score?: number | null section_path: Array text: string } diff --git a/packages/contracts/generated/api/console/knowledge-fs/zod.gen.ts b/packages/contracts/generated/api/console/knowledge-fs/zod.gen.ts index 40d8aa8b170..6b57bff6ecd 100644 --- a/packages/contracts/generated/api/console/knowledge-fs/zod.gen.ts +++ b/packages/contracts/generated/api/console/knowledge-fs/zod.gen.ts @@ -213,8 +213,9 @@ export const zKnowledgeFsGoldenQuestionListResponse = z.object({ * KnowledgeFSGoldenQuestionEvidenceMatchPayload */ export const zKnowledgeFsGoldenQuestionEvidenceMatchPayload = z.object({ - evidence: z.string().min(1).max(8000), + evidence: z.string().min(1).max(8000).nullish(), minimum_similarity: z.number().gte(0).lte(1).optional().default(0.7), + node_ids: z.array(z.string()).max(50).optional(), top_k: z.int().gte(1).lte(10).optional().default(5), }) @@ -1521,8 +1522,8 @@ export const zKnowledgeFsGoldenQuestionEvidenceCandidateResponse = z.object({ document_asset_id: z.string(), node_id: z.string(), page_number: z.int().nullish(), - projection_id: z.string(), - score: z.number().gte(0).lte(1), + projection_id: z.string().nullish(), + score: z.number().gte(0).lte(1).nullish(), section_path: z.array(z.string()), text: z.string(), }) diff --git a/web/app/components/base/search-input/__tests__/index.spec.tsx b/web/app/components/base/search-input/__tests__/index.spec.tsx index 445819f89b4..7d0115d57e3 100644 --- a/web/app/components/base/search-input/__tests__/index.spec.tsx +++ b/web/app/components/base/search-input/__tests__/index.spec.tsx @@ -73,6 +73,17 @@ describe('SearchInput', () => { screen.queryByRole('button', { name: 'common.operation.clear' }), ).not.toBeInTheDocument() }) + + it('forwards keyboard events so forms can own the search action', async () => { + const onKeyDown = vi.fn() + const user = userEvent.setup() + render( {}} onKeyDown={onKeyDown} />) + + await user.click(screen.getByRole('searchbox')) + await user.keyboard('{Enter}') + + expect(onKeyDown).toHaveBeenCalledWith(expect.objectContaining({ key: 'Enter' })) + }) }) describe('Interaction', () => { diff --git a/web/app/components/base/search-input/index.tsx b/web/app/components/base/search-input/index.tsx index 6a50f304ac5..ad07de85d9d 100644 --- a/web/app/components/base/search-input/index.tsx +++ b/web/app/components/base/search-input/index.tsx @@ -13,7 +13,7 @@ type SearchInputProps = { className?: string } & Pick< InputGroupInputProps, - 'aria-describedby' | 'aria-label' | 'autoFocus' | 'disabled' | 'name' + 'aria-describedby' | 'aria-label' | 'autoFocus' | 'disabled' | 'name' | 'onKeyDown' > export function SearchInput({ @@ -25,6 +25,7 @@ export function SearchInput({ name = 'query', autoFocus, disabled, + onKeyDown, 'aria-describedby': ariaDescribedBy, 'aria-label': ariaLabel, }: SearchInputProps) { @@ -89,6 +90,7 @@ export function SearchInput({ // oxlint-disable-next-line jsx-a11y/no-autofocus autoFocus={autoFocus} enterKeyHint="search" + onKeyDown={onKeyDown} /> { expect(serviceMock.createGolden.mock.calls[0]?.[0]).toEqual({ body: { annotation: 'Expected answer', - evidence_text: '', expected_evidence_ids: [], match_policy: 'all', question: 'New question', @@ -263,7 +262,6 @@ describe('QualityPage', () => { expect(serviceMock.createGolden.mock.calls[0]?.[0]).toEqual({ body: { annotation: 'Expected answer', - evidence_text: '', expected_evidence_ids: [], match_policy: 'all', question: 'New question', @@ -274,7 +272,7 @@ describe('QualityPage', () => { ) }) - it('matches a human-readable evidence passage and stores the selected node id', async () => { + it('uses a human-readable search only to select the persisted evidence node id', async () => { serviceMock.matchEvidence.mockResolvedValue({ candidates: [ { @@ -322,7 +320,6 @@ describe('QualityPage', () => { expect(serviceMock.createGolden.mock.calls[0]?.[0]).toEqual({ body: { annotation: 'The answer must cite the refund window.', - evidence_text: 'refund within 30 days', expected_evidence_ids: ['node-1'], match_policy: 'all', question: 'When can I request a refund?', @@ -417,7 +414,6 @@ describe('QualityPage', () => { expect(serviceMock.createGolden.mock.calls[0]?.[0]).toEqual({ body: { annotation: 'The answer must cite both policies.', - evidence_text: 'refund and cancellation policy', expected_evidence_ids: ['node-1', 'node-2'], match_policy: 'any', question: 'When can I request a refund?', @@ -580,7 +576,6 @@ describe('QualityPage', () => { { body: { annotation: 'Updated expected answer', - evidence_text: '', expected_evidence_ids: [], match_policy: 'all', question: 'What is the refund policy?', @@ -596,6 +591,100 @@ describe('QualityPage', () => { ).not.toBeInTheDocument() }) + it('resolves and displays saved evidence passages while keeping the search query ephemeral', async () => { + serviceMock.getGolden.mockResolvedValue({ + data: [ + { + annotation: 'Must cite both permission rules.', + created_at: '2026-07-28T00:00:00Z', + evidence_text: '权限', + expected_evidence_ids: ['node-1', 'node-2'], + id: 'golden-1', + match_policy: 'all', + question: 'Who can change permissions?', + status: 'active', + tags: ['permissions'], + updated_at: '2026-07-28T00:00:00Z', + }, + ], + next_cursor: null, + }) + serviceMock.matchEvidence.mockImplementation( + async ({ body }: { body: { evidence?: string; node_ids?: string[] } }) => { + if (body.node_ids) { + return { + candidates: [ + { + document_asset_id: 'document-1', + node_id: 'node-1', + section_path: ['Permissions', 'Owners'], + text: 'Workspace owners can change member permissions.', + }, + { + document_asset_id: 'document-1', + node_id: 'node-2', + section_path: ['Permissions', 'Admins'], + text: 'Administrators can assign application roles.', + }, + ], + evidence: '', + matched: false, + } + } + return { candidates: [], evidence: body.evidence ?? '', matched: false } + }, + ) + serviceMock.updateGolden.mockResolvedValue({}) + const user = userEvent.setup() + renderPage() + + await screen.findByText('Who can change permissions?') + await user.click( + screen.getByRole('button', { + name: /dataset\.newKnowledge\.qualityPage\.questionActions/, + }), + ) + await user.click( + await screen.findByRole('menuitem', { name: 'dataset.newKnowledge.qualityPage.edit' }), + ) + + expect(await screen.findByText('Workspace owners can change member permissions.')).toBeVisible() + expect(screen.getByText('Administrators can assign application roles.')).toBeVisible() + expect(serviceMock.matchEvidence.mock.calls[0]?.[0]).toEqual({ + body: { node_ids: ['node-1', 'node-2'] }, + params: { control_space_id: 'space-1' }, + }) + + const search = screen.getByRole('searchbox', { + name: 'dataset.newKnowledge.qualityPage.findEvidence', + }) + expect(search).toHaveValue('') + await user.type(search, '权限') + await user.click( + screen.getByRole('button', { name: 'dataset.newKnowledge.qualityPage.findEvidence' }), + ) + await waitFor(() => expect(search).toHaveValue('')) + expect(screen.getByText('Workspace owners can change member permissions.')).toBeVisible() + expect(screen.getByText('Administrators can assign application roles.')).toBeVisible() + + await user.click(screen.getByRole('button', { name: 'dataset.newKnowledge.qualityPage.save' })) + await waitFor(() => + expect(serviceMock.updateGolden).toHaveBeenCalledWith( + { + body: { + annotation: 'Must cite both permission rules.', + expected_evidence_ids: ['node-1', 'node-2'], + match_policy: 'all', + question: 'Who can change permissions?', + tags: ['permissions'], + }, + params: { control_space_id: 'space-1', question_id: 'golden-1' }, + }, + expect.anything(), + ), + ) + }) + it('resolves the protected trace reference before navigating', async () => { const user = userEvent.setup() navigationMock.tab = 'bad-cases' @@ -892,7 +981,6 @@ describe('QualityPage', () => { expect(serviceMock.createGolden.mock.calls[0]?.[0]).toEqual({ body: { annotation: 'coverage gap', - evidence_text: '', expected_evidence_ids: [], match_policy: 'all', question: 'Refund after activation', 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 be8088d023a..81707241ce9 100644 --- a/web/features/new-rag/__tests__/retrieval-test-page.spec.tsx +++ b/web/features/new-rag/__tests__/retrieval-test-page.spec.tsx @@ -1567,7 +1567,6 @@ describe('RetrievalTestPage', () => { expect(apiMock.createGolden).toHaveBeenCalledWith({ body: { annotation: 'The answer must cite the retrieved useEffect evidence.', - evidence_text: '', expected_evidence_ids: ['chunk-1'], match_policy: 'all', question: 'What is useEffect?', diff --git a/web/features/new-rag/quality/golden-question-dialog.tsx b/web/features/new-rag/quality/golden-question-dialog.tsx index 5ad9201d688..c19df7e812d 100644 --- a/web/features/new-rag/quality/golden-question-dialog.tsx +++ b/web/features/new-rag/quality/golden-question-dialog.tsx @@ -1,6 +1,6 @@ 'use client' -import type { FormEvent } from 'react' +import type { FormEvent, KeyboardEvent } from 'react' import type { GoldenQuestionDraft, GoldenQuestionEvidenceOption } from './types' import { Button } from '@langgenius/dify-ui/button' import { Checkbox } from '@langgenius/dify-ui/checkbox' @@ -15,12 +15,14 @@ import { } from '@langgenius/dify-ui/dialog' import { Field, FieldError, FieldItem, FieldLabel } from '@langgenius/dify-ui/field' import { Fieldset, FieldsetLegend } from '@langgenius/dify-ui/fieldset' +import { IconButton } from '@langgenius/dify-ui/icon-button' import { Input } from '@langgenius/dify-ui/input' import { RadioGroup, RadioItem } from '@langgenius/dify-ui/radio' import { Textarea } from '@langgenius/dify-ui/textarea' import { useMutation } from '@tanstack/react-query' -import { useState } from 'react' +import { useCallback, useEffect, useState } from 'react' import { useTranslation } from 'react-i18next' +import { SearchInput } from '@/app/components/base/search-input' import { consoleQuery } from '@/service/client' type DialogMode = 'create' | 'edit' | 'promote' @@ -66,18 +68,44 @@ export function GoldenQuestionDialog({ pending?: boolean }) { const { t } = useTranslation('dataset') + const { t: tCommon } = useTranslation('common') const [question, setQuestion] = useState(initialValue.question) const [annotation, setAnnotation] = useState(initialValue.annotation) - const [evidenceText, setEvidenceText] = useState(initialValue.evidenceText) + const [evidenceQuery, setEvidenceQuery] = useState('') const [expectedEvidenceIds, setExpectedEvidenceIds] = useState(initialValue.expectedEvidenceIds) + const [evidenceByNodeId, setEvidenceByNodeId] = useState( + () => new Map(evidenceOptions.map((option) => [option.node_id, option])), + ) const [matchPolicy, setMatchPolicy] = useState(initialValue.matchPolicy) const [tags, setTags] = useState(initialValue.tags.join(', ')) const [questionInvalid, setQuestionInvalid] = useState(false) const [annotationInvalid, setAnnotationInvalid] = useState(false) const [matchError, setMatchError] = useState<'unavailable' | 'unknown'>() + const mergeEvidenceOptions = useCallback((options: readonly GoldenQuestionEvidenceOption[]) => { + setEvidenceByNodeId((current) => { + const next = new Map(current) + for (const option of options) next.set(option.node_id, option) + return next + }) + }, []) const matchMutation = useMutation( consoleQuery.knowledgeFs.spaces.byControlSpaceId.goldenQuestions.evidenceMatches.post.mutationOptions(), ) + const resolveMutation = useMutation({ + ...consoleQuery.knowledgeFs.spaces.byControlSpaceId.goldenQuestions.evidenceMatches.post.mutationOptions(), + onSuccess: (data) => mergeEvidenceOptions(data.candidates), + }) + const unresolvedInitialEvidenceKey = initialValue.expectedEvidenceIds + .filter((nodeId) => !evidenceByNodeId.has(nodeId)) + .join(',') + const resolveEvidence = resolveMutation.mutate + useEffect(() => { + if (!unresolvedInitialEvidenceKey) return + resolveEvidence({ + body: { node_ids: unresolvedInitialEvidenceKey.split(',') }, + params: { control_space_id: knowledgeSpaceId }, + }) + }, [knowledgeSpaceId, resolveEvidence, unresolvedInitialEvidenceKey]) const title = mode === 'create' ? t(($) => $['newKnowledge.qualityPage.createTitle']) @@ -98,7 +126,6 @@ export function GoldenQuestionDialog({ if (nextQuestionInvalid || nextAnnotationInvalid) return await onSubmit({ annotation: annotation.trim(), - evidenceText: evidenceText.trim(), expectedEvidenceIds, matchPolicy, question: question.trim(), @@ -107,24 +134,28 @@ export function GoldenQuestionDialog({ } const findEvidence = async () => { - if (!evidenceText.trim()) return + const query = evidenceQuery.trim() + if (!query) return setMatchError(undefined) try { - await matchMutation.mutateAsync({ - body: { evidence: evidenceText.trim() }, + const result = await matchMutation.mutateAsync({ + body: { evidence: query }, params: { control_space_id: knowledgeSpaceId }, }) + mergeEvidenceOptions(result.candidates) + setEvidenceQuery('') } catch (error) { setMatchError(errorStatus(error) === 503 ? 'unavailable' : 'unknown') } } - const candidatesByNodeId = new Map( - evidenceOptions.map((candidate) => [candidate.node_id, candidate]), - ) - for (const candidate of matchMutation.data?.candidates ?? []) - candidatesByNodeId.set(candidate.node_id, candidate) - const candidates = [...candidatesByNodeId.values()] + const handleEvidenceSearchKeyDown = (event: KeyboardEvent) => { + if (event.key !== 'Enter' || event.nativeEvent.isComposing) return + event.preventDefault() + void findEvidence() + } + + const searchCandidates = matchMutation.data?.candidates ?? evidenceOptions return ( @@ -181,47 +212,90 @@ export function GoldenQuestionDialog({ )} {!annotationInvalid && error && {error}} -
- - {t(($) => $['newKnowledge.qualityPage.evidence'])} -