mirror of
https://github.com/vxcontrol/pentagi.git
synced 2026-08-24 12:06:36 +00:00
feat(knowledge): allow docType edit, partial UPDATE, and field-clear on save
Backend now accepts `docType` on `UpdateKnowledgeDocumentInput`, so the edit form was reworked end-to-end: - Form schema mirrors REST length limits (content 65536, question 2048, description 1000, codeLang 100) — GraphQL itself doesn't enforce them. - `docType` is editable on existing docs, not just on create. Switching it clears stale subtype values (`answerType`/`guideType`/`codeLang`) through `setValue` in the Select's `onValueChange`, not via effect, so freshly loaded documents keep their persisted subtype on first render. - UPDATE is now a partial payload built from RHF's `dirtyFields`: untouched optional fields are omitted, cleared fields go out as `""` so the backend wipes them (previously `'' → undefined` silently swallowed clears, leaving stale values on the server). - `KnowledgesProvider` drops the duplicate `Knowledge` interface and uses the GraphQL fragment directly; no more hand-rolled field mapping. - Both submit paths (form button and "Save & Leave" dialog) parse values through `formSchema.safeParse` so trim/normalisation is identical regardless of code path. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
co-authored by
Cursor
parent
616b864b17
commit
8da0312e9d
@@ -1,4 +1,4 @@
|
||||
import { type Control, useWatch } from 'react-hook-form';
|
||||
import { type Control, useFormContext, useWatch } from 'react-hook-form';
|
||||
|
||||
import type {
|
||||
KnowledgeAnswerType as KnowledgeAnswerTypeT,
|
||||
@@ -14,6 +14,8 @@ import { KnowledgeAnswerType, KnowledgeDocType, KnowledgeGuideType } from '@/gra
|
||||
|
||||
import type { FormValues } from './knowledge-form';
|
||||
|
||||
import { KNOWLEDGE_LIMITS } from './knowledge-form';
|
||||
|
||||
// `<Select>` option lists. Co-located with the controls they feed because no
|
||||
// other module needs them.
|
||||
const docTypeValues = [KnowledgeDocType.Answer, KnowledgeDocType.Guide, KnowledgeDocType.Code] as const;
|
||||
@@ -31,6 +33,28 @@ export const KnowledgeMetaFields = ({ control, isNew, isSaving }: KnowledgeMetaF
|
||||
// not the whole form. The full-form `useWatch` from the original code
|
||||
// re-rendered on every keystroke in the markdown editor.
|
||||
const docType = useWatch({ control, name: 'docType' });
|
||||
// `setValue` is used to clear subtype fields that no longer apply when the
|
||||
// user switches docType. We do this synchronously inside `onValueChange`
|
||||
// (rather than via `useEffect`) so that a fresh load of an existing
|
||||
// document doesn't wipe its persisted subtype on first render.
|
||||
const { setValue } = useFormContext<FormValues>();
|
||||
|
||||
const handleDocTypeChange = (next: KnowledgeDocType, fieldOnChange: (value: KnowledgeDocType) => void) => {
|
||||
fieldOnChange(next);
|
||||
const opts = { shouldDirty: true, shouldValidate: true };
|
||||
|
||||
if (next !== KnowledgeDocType.Answer) {
|
||||
setValue('answerType', undefined, opts);
|
||||
}
|
||||
|
||||
if (next !== KnowledgeDocType.Code) {
|
||||
setValue('codeLang', '', opts);
|
||||
}
|
||||
|
||||
if (next !== KnowledgeDocType.Guide) {
|
||||
setValue('guideType', undefined, opts);
|
||||
}
|
||||
};
|
||||
|
||||
return (
|
||||
<div className="flex flex-col gap-4">
|
||||
@@ -41,33 +65,29 @@ export const KnowledgeMetaFields = ({ control, isNew, isSaving }: KnowledgeMetaF
|
||||
render={({ field }) => (
|
||||
<FormItem>
|
||||
<FormLabel>Document type</FormLabel>
|
||||
{isNew ? (
|
||||
<Select
|
||||
disabled={isSaving}
|
||||
onValueChange={field.onChange}
|
||||
value={field.value}
|
||||
>
|
||||
<FormControl>
|
||||
<SelectTrigger>
|
||||
<SelectValue placeholder="Select type" />
|
||||
</SelectTrigger>
|
||||
</FormControl>
|
||||
<SelectContent>
|
||||
{docTypeValues.map((value) => (
|
||||
<SelectItem
|
||||
key={value}
|
||||
value={value}
|
||||
>
|
||||
{value}
|
||||
</SelectItem>
|
||||
))}
|
||||
</SelectContent>
|
||||
</Select>
|
||||
) : (
|
||||
<div className="border-input bg-muted/30 text-muted-foreground flex h-9 items-center rounded-md border px-3 text-sm">
|
||||
{field.value || '—'}
|
||||
</div>
|
||||
)}
|
||||
<Select
|
||||
disabled={isSaving}
|
||||
onValueChange={(value) =>
|
||||
handleDocTypeChange(value as KnowledgeDocType, field.onChange)
|
||||
}
|
||||
value={field.value}
|
||||
>
|
||||
<FormControl>
|
||||
<SelectTrigger>
|
||||
<SelectValue placeholder="Select type" />
|
||||
</SelectTrigger>
|
||||
</FormControl>
|
||||
<SelectContent>
|
||||
{docTypeValues.map((value) => (
|
||||
<SelectItem
|
||||
key={value}
|
||||
value={value}
|
||||
>
|
||||
{value}
|
||||
</SelectItem>
|
||||
))}
|
||||
</SelectContent>
|
||||
</Select>
|
||||
<FormMessage />
|
||||
</FormItem>
|
||||
)}
|
||||
@@ -151,6 +171,7 @@ export const KnowledgeMetaFields = ({ control, isNew, isSaving }: KnowledgeMetaF
|
||||
<FormControl>
|
||||
<Input
|
||||
disabled={isSaving}
|
||||
maxLength={KNOWLEDGE_LIMITS.codeLang}
|
||||
placeholder="e.g. python, go, typescript"
|
||||
{...field}
|
||||
/>
|
||||
@@ -175,6 +196,7 @@ export const KnowledgeMetaFields = ({ control, isNew, isSaving }: KnowledgeMetaF
|
||||
autoFocus={isNew}
|
||||
className="min-h-0"
|
||||
disabled={isSaving}
|
||||
maxLength={KNOWLEDGE_LIMITS.question}
|
||||
maxRows={6}
|
||||
minRows={1}
|
||||
placeholder="Short title or question this document answers"
|
||||
@@ -198,6 +220,7 @@ export const KnowledgeMetaFields = ({ control, isNew, isSaving }: KnowledgeMetaF
|
||||
{...field}
|
||||
className="min-h-0"
|
||||
disabled={isSaving}
|
||||
maxLength={KNOWLEDGE_LIMITS.description}
|
||||
maxRows={8}
|
||||
minRows={1}
|
||||
placeholder="Optional short description"
|
||||
|
||||
@@ -27,15 +27,47 @@ import { KnowledgeHeader } from './knowledge-header';
|
||||
// Schema, types, pure helpers
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
// Length limits mirror the REST validation tags on the Go side
|
||||
// (`backend/pkg/server/models/knowledge.go`). The GraphQL layer itself does
|
||||
// not enforce them, so without these the user could submit a payload that
|
||||
// later round-trips through REST and gets rejected.
|
||||
export const KNOWLEDGE_LIMITS = {
|
||||
codeLang: 100,
|
||||
content: 65536,
|
||||
description: 1000,
|
||||
question: 2048,
|
||||
} as const;
|
||||
|
||||
// Optional text fields are trimmed and length-checked but NOT collapsed to
|
||||
// `undefined` — the partial-update logic in `formValuesToUpdateInput` needs
|
||||
// to distinguish "user cleared a previously-set value" (send `""` so the
|
||||
// backend clears it) from "field was empty and untouched" (don't send at
|
||||
// all so the backend leaves it alone). Mapping `"" → undefined` here would
|
||||
// erase that signal and break the "clear an existing description" use case.
|
||||
const optionalTrimmed = (max: number, label: string) =>
|
||||
z
|
||||
.string()
|
||||
.trim()
|
||||
.max(max, { message: `${label} must be ${max} characters or fewer` })
|
||||
.optional();
|
||||
|
||||
export const formSchema = z
|
||||
.object({
|
||||
answerType: z.nativeEnum(KnowledgeAnswerType).optional(),
|
||||
codeLang: z.string().trim().optional(),
|
||||
content: z.string().trim().min(1, { message: 'Content is required' }),
|
||||
description: z.string().trim().optional(),
|
||||
codeLang: optionalTrimmed(KNOWLEDGE_LIMITS.codeLang, 'Code language'),
|
||||
content: z
|
||||
.string()
|
||||
.trim()
|
||||
.min(1, { message: 'Content is required' })
|
||||
.max(KNOWLEDGE_LIMITS.content, { message: `Content must be ${KNOWLEDGE_LIMITS.content} characters or fewer` }),
|
||||
description: optionalTrimmed(KNOWLEDGE_LIMITS.description, 'Description'),
|
||||
docType: z.nativeEnum(KnowledgeDocType),
|
||||
guideType: z.nativeEnum(KnowledgeGuideType).optional(),
|
||||
question: z.string().trim().min(1, { message: 'Question is required' }),
|
||||
question: z
|
||||
.string()
|
||||
.trim()
|
||||
.min(1, { message: 'Question is required' })
|
||||
.max(KNOWLEDGE_LIMITS.question, { message: `Question must be ${KNOWLEDGE_LIMITS.question} characters or fewer` }),
|
||||
})
|
||||
.superRefine((value, ctx) => {
|
||||
const requiredByDocType: Partial<Record<KnowledgeDocType, { field: FieldPath<FormValues>; message: string }>> =
|
||||
@@ -85,38 +117,62 @@ export const documentToFormValues = (k: KnowledgeDocumentFragmentFragment): Form
|
||||
question: k.question,
|
||||
});
|
||||
|
||||
// `description` and `codeLang` are optional and we want empty strings to map
|
||||
// to `undefined` so the backend treats them as "absent" instead of "set to ''".
|
||||
const trimmedOrUndefined = (value: null | string | undefined): string | undefined => {
|
||||
if (!value) {
|
||||
return undefined;
|
||||
}
|
||||
// react-hook-form's `dirtyFields` is a partial map of the same shape as
|
||||
// `FormValues`, with `true` for fields the user actually changed compared to
|
||||
// `defaultValues`. We project it onto the (flat) FormValues keys here.
|
||||
export type DirtyFlags = Partial<Record<keyof FormValues, boolean>>;
|
||||
|
||||
const trimmed = value.trim();
|
||||
|
||||
return trimmed.length === 0 ? undefined : trimmed;
|
||||
};
|
||||
|
||||
// Shared field projection used by both create and update payloads. The only
|
||||
// difference between the two GraphQL inputs is that `docType` is required on
|
||||
// create and immutable on update — see `formValuesTo{Create,Update}Input`
|
||||
// below.
|
||||
const formValuesToBasePayload = (values: FormValues) => ({
|
||||
answerType: values.docType === KnowledgeDocType.Answer ? values.answerType : undefined,
|
||||
codeLang: values.docType === KnowledgeDocType.Code ? trimmedOrUndefined(values.codeLang) : undefined,
|
||||
// CREATE: send all required fields and only non-empty optional fields. There
|
||||
// is no prior document to "clear", so an empty `description`/`codeLang` just
|
||||
// means "don't store anything in cmetadata for this field".
|
||||
export const formValuesToCreateInput = (values: FormValues): CreateKnowledgeDocumentInput => ({
|
||||
answerType: values.answerType,
|
||||
codeLang: values.codeLang ? values.codeLang : undefined,
|
||||
content: values.content,
|
||||
description: trimmedOrUndefined(values.description),
|
||||
guideType: values.docType === KnowledgeDocType.Guide ? values.guideType : undefined,
|
||||
description: values.description ? values.description : undefined,
|
||||
docType: values.docType,
|
||||
guideType: values.guideType,
|
||||
question: values.question,
|
||||
});
|
||||
|
||||
export const formValuesToCreateInput = (values: FormValues): CreateKnowledgeDocumentInput => ({
|
||||
...formValuesToBasePayload(values),
|
||||
docType: values.docType,
|
||||
});
|
||||
// UPDATE: send only fields the user actually edited. `content` is GraphQL-
|
||||
// required (the backend always re-embeds), so it goes through unconditionally;
|
||||
// every other field is gated by `dirty`. This way:
|
||||
// - untouched fields stay `undefined` and the backend keeps the existing value;
|
||||
// - cleared fields go out as `""` so the backend wipes them;
|
||||
// - subtype-related fields cleared by `setValue` on docType change are
|
||||
// marked dirty by the form, so they reach the backend with the right
|
||||
// "clear me" value (the backend additionally wipes mismatching subtypes
|
||||
// itself, but we mirror the user-visible state explicitly).
|
||||
export const formValuesToUpdateInput = (values: FormValues, dirty: DirtyFlags): UpdateKnowledgeDocumentInput => {
|
||||
const input: UpdateKnowledgeDocumentInput = { content: values.content };
|
||||
|
||||
export const formValuesToUpdateInput = (values: FormValues): UpdateKnowledgeDocumentInput =>
|
||||
formValuesToBasePayload(values);
|
||||
if (dirty.docType) {
|
||||
input.docType = values.docType;
|
||||
}
|
||||
|
||||
if (dirty.question) {
|
||||
input.question = values.question;
|
||||
}
|
||||
|
||||
if (dirty.description) {
|
||||
input.description = values.description ?? '';
|
||||
}
|
||||
|
||||
if (dirty.guideType) {
|
||||
input.guideType = values.guideType;
|
||||
}
|
||||
|
||||
if (dirty.answerType) {
|
||||
input.answerType = values.answerType;
|
||||
}
|
||||
|
||||
if (dirty.codeLang) {
|
||||
input.codeLang = values.codeLang ?? '';
|
||||
}
|
||||
|
||||
return input;
|
||||
};
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Form component
|
||||
@@ -132,7 +188,7 @@ interface KnowledgeFormProps {
|
||||
isNew: boolean;
|
||||
knowledge?: KnowledgeDocumentFragmentFragment | null;
|
||||
knowledgeName: null | string;
|
||||
onSubmit: (values: FormValues) => Promise<SubmitResult>;
|
||||
onSubmit: (values: FormValues, dirtyFields: DirtyFlags) => Promise<SubmitResult>;
|
||||
}
|
||||
|
||||
export const KnowledgeForm = ({ initialValues, isNew, knowledge, knowledgeName, onSubmit }: KnowledgeFormProps) => {
|
||||
@@ -157,7 +213,11 @@ export const KnowledgeForm = ({ initialValues, isNew, knowledge, knowledgeName,
|
||||
const performSave = useCallback(
|
||||
async (values: FormValues): Promise<boolean> => {
|
||||
try {
|
||||
const result = await onSubmit(values);
|
||||
// Snapshot dirty flags from the latest formState. We read it
|
||||
// here (instead of capturing into deps) so partial-update
|
||||
// logic in the page sees the same state RHF used to decide
|
||||
// `isDirty`/`canSubmit` at submit time.
|
||||
const result = await onSubmit(values, form.formState.dirtyFields as DirtyFlags);
|
||||
|
||||
// Prefer the server's view of the document — backend may have
|
||||
// trimmed/normalized fields, attached derived data, or filled
|
||||
@@ -183,7 +243,7 @@ export const KnowledgeForm = ({ initialValues, isNew, knowledge, knowledgeName,
|
||||
return false;
|
||||
}
|
||||
},
|
||||
[navigate, onSubmit, reset],
|
||||
[form, navigate, onSubmit, reset],
|
||||
);
|
||||
|
||||
// The ref below breaks an otherwise circular hook dependency:
|
||||
@@ -221,10 +281,20 @@ export const KnowledgeForm = ({ initialValues, isNew, knowledge, knowledgeName,
|
||||
return false;
|
||||
}
|
||||
|
||||
// `form.getValues()` returns raw field state (no zod transforms applied),
|
||||
// so we run it through the schema explicitly. This way the dialog path
|
||||
// produces the same trimmed/normalized values as the form-button path
|
||||
// (which gets parsed values directly from `handleSubmit`'s callback).
|
||||
const parsed = formSchema.safeParse(form.getValues());
|
||||
|
||||
if (!parsed.success) {
|
||||
return false;
|
||||
}
|
||||
|
||||
setIsSaving(true);
|
||||
|
||||
try {
|
||||
return await performSave(form.getValues());
|
||||
return await performSave(parsed.data);
|
||||
} finally {
|
||||
setIsSaving(false);
|
||||
}
|
||||
|
||||
@@ -1229,6 +1229,7 @@ export type UpdateKnowledgeDocumentInput = {
|
||||
codeLang?: InputMaybe<Scalars['String']['input']>;
|
||||
content: Scalars['String']['input'];
|
||||
description?: InputMaybe<Scalars['String']['input']>;
|
||||
docType?: InputMaybe<KnowledgeDocType>;
|
||||
guideType?: InputMaybe<KnowledgeGuideType>;
|
||||
question?: InputMaybe<Scalars['String']['input']>;
|
||||
};
|
||||
|
||||
@@ -5,6 +5,7 @@ import { Button } from '@/components/ui/button';
|
||||
import { Card, CardContent } from '@/components/ui/card';
|
||||
import { Spinner } from '@/components/ui/spinner';
|
||||
import {
|
||||
type DirtyFlags,
|
||||
documentToFormValues,
|
||||
type FormValues,
|
||||
formValuesToCreateInput,
|
||||
@@ -39,7 +40,12 @@ const Knowledge = () => {
|
||||
);
|
||||
|
||||
const handleSubmit = useCallback(
|
||||
async (values: FormValues): Promise<SubmitResult> => {
|
||||
// `values` are the zod-parsed form output (trimmed, length-validated).
|
||||
// CREATE sends a full payload; UPDATE sends only fields the user
|
||||
// actually changed (`dirtyFields`) so untouched optional fields stay
|
||||
// untouched on the backend and explicit clears (e.g. wiping an existing
|
||||
// description) reach it as `""`.
|
||||
async (values: FormValues, dirtyFields: DirtyFlags): Promise<SubmitResult> => {
|
||||
if (isNew) {
|
||||
const created = await createKnowledge(formValuesToCreateInput(values));
|
||||
|
||||
@@ -53,7 +59,7 @@ const Knowledge = () => {
|
||||
return {};
|
||||
}
|
||||
|
||||
const updated = await updateKnowledge(knowledgeId, formValuesToUpdateInput(values));
|
||||
const updated = await updateKnowledge(knowledgeId, formValuesToUpdateInput(values, dirtyFields));
|
||||
|
||||
return { document: updated ?? undefined };
|
||||
},
|
||||
|
||||
@@ -1,12 +1,13 @@
|
||||
import { createContext, type ReactNode, useCallback, useContext, useMemo } from 'react';
|
||||
import { toast } from 'sonner';
|
||||
|
||||
import type { CreateKnowledgeDocumentInput, UpdateKnowledgeDocumentInput } from '@/graphql/types';
|
||||
import type {
|
||||
CreateKnowledgeDocumentInput,
|
||||
KnowledgeDocumentFragmentFragment,
|
||||
UpdateKnowledgeDocumentInput,
|
||||
} from '@/graphql/types';
|
||||
|
||||
import {
|
||||
KnowledgeAnswerType,
|
||||
KnowledgeDocType,
|
||||
KnowledgeGuideType,
|
||||
useCreateKnowledgeDocumentMutation,
|
||||
useDeleteKnowledgeDocumentMutation,
|
||||
useKnowledgeDocumentCreatedSubscription,
|
||||
@@ -18,23 +19,12 @@ import {
|
||||
import { Log } from '@/lib/log';
|
||||
import { useUser } from '@/providers/user-provider';
|
||||
|
||||
export interface Knowledge {
|
||||
answerType?: KnowledgeAnswerType | null;
|
||||
codeLang?: null | string;
|
||||
content: string;
|
||||
description?: null | string;
|
||||
docType: KnowledgeDocType;
|
||||
flowId?: null | string;
|
||||
guideType?: KnowledgeGuideType | null;
|
||||
id: string;
|
||||
manual: boolean;
|
||||
partSize: number;
|
||||
question: string;
|
||||
subtaskId?: null | string;
|
||||
taskId?: null | string;
|
||||
totalSize: number;
|
||||
userId: string;
|
||||
}
|
||||
// The provider operates directly on the GraphQL fragment. Previously we kept a
|
||||
// hand-rolled `Knowledge` shape that mirrored the fragment field-by-field; that
|
||||
// duplication forced a manual mapping step and drifted from the schema. The
|
||||
// alias keeps the public surface (`Knowledge`) for callers while making it
|
||||
// obvious there is no extra translation layer.
|
||||
export type Knowledge = KnowledgeDocumentFragmentFragment;
|
||||
|
||||
interface KnowledgesContextValue {
|
||||
createKnowledge: (input: CreateKnowledgeDocumentInput) => Promise<Knowledge | undefined>;
|
||||
@@ -70,27 +60,7 @@ export const KnowledgesProvider = ({ children }: KnowledgesProviderProps) => {
|
||||
useKnowledgeDocumentUpdatedSubscription({ skip: !shouldFetch });
|
||||
useKnowledgeDocumentDeletedSubscription({ skip: !shouldFetch });
|
||||
|
||||
const knowledges = useMemo<Knowledge[]>(() => {
|
||||
const raw = data?.knowledgeDocuments ?? [];
|
||||
|
||||
return raw.map((d) => ({
|
||||
answerType: d.answerType ?? null,
|
||||
codeLang: d.codeLang ?? null,
|
||||
content: d.content,
|
||||
description: d.description ?? null,
|
||||
docType: d.docType,
|
||||
flowId: d.flowId ?? null,
|
||||
guideType: d.guideType ?? null,
|
||||
id: d.id,
|
||||
manual: d.manual,
|
||||
partSize: d.partSize,
|
||||
question: d.question,
|
||||
subtaskId: d.subtaskId ?? null,
|
||||
taskId: d.taskId ?? null,
|
||||
totalSize: d.totalSize,
|
||||
userId: d.userId,
|
||||
}));
|
||||
}, [data?.knowledgeDocuments]);
|
||||
const knowledges = useMemo<Knowledge[]>(() => data?.knowledgeDocuments ?? [], [data?.knowledgeDocuments]);
|
||||
|
||||
const getKnowledge = useCallback(
|
||||
(id: string): Knowledge | undefined => knowledges.find((k) => k.id === id),
|
||||
@@ -102,7 +72,7 @@ export const KnowledgesProvider = ({ children }: KnowledgesProviderProps) => {
|
||||
try {
|
||||
const { data: result } = await createKnowledgeMutation({ variables: { input } });
|
||||
|
||||
return result?.createKnowledgeDocument as Knowledge | undefined;
|
||||
return result?.createKnowledgeDocument;
|
||||
} catch (error) {
|
||||
const errorMessage = error instanceof Error ? error.message : 'Failed to create knowledge document';
|
||||
toast.error('Failed to create knowledge document', { description: errorMessage });
|
||||
@@ -118,7 +88,7 @@ export const KnowledgesProvider = ({ children }: KnowledgesProviderProps) => {
|
||||
try {
|
||||
const { data: result } = await updateKnowledgeMutation({ variables: { id, input } });
|
||||
|
||||
return result?.updateKnowledgeDocument as Knowledge | undefined;
|
||||
return result?.updateKnowledgeDocument;
|
||||
} catch (error) {
|
||||
const errorMessage = error instanceof Error ? error.message : 'Failed to update knowledge document';
|
||||
toast.error('Failed to update knowledge document', { description: errorMessage });
|
||||
|
||||
Reference in New Issue
Block a user