laserninja commented on code in PR #12900: URL: https://github.com/apache/gravitino/pull/12900#discussion_r4267243456
########## web-v2/web/src/lib/utils/semanticMetadata.js: ########## @@ -0,0 +1,88 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { cloneDeep, isEqual } from 'lodash-es' + +const editableProperties = { + model: [], + dataset: ['description'], + field: ['label', 'description', 'datatype'], + metric: ['description', 'datatype'], + relationship: ['from', 'to', 'fromColumns', 'toColumns'] +} + +/** Finds the selected entity without flattening dataset-scoped field names. */ +export const getSemanticMetadataEntity = (definition, target) => { + if (target.kind === 'model') return definition + if (target.kind === 'field') { + return definition.datasets + .find(dataset => dataset.name === target.datasetName) + ?.fields?.find(field => field.name === target.name) + } + + return definition[`${target.kind}s`]?.find(entity => entity.name === target.name) +} + +/** Supplies form values while keeping the original wire representation separate. */ +export const semanticMetadataFormValues = entity => ({ + ...cloneDeep(entity), + dialects: cloneDeep(entity.expression?.dialects || []), + contextFormat: typeof entity.aiContext === 'string' ? 'text' : 'structured', + contextText: typeof entity.aiContext === 'string' ? entity.aiContext : '', + instructions: typeof entity.aiContext === 'string' ? entity.aiContext : entity.aiContext?.instructions || '', + synonyms: cloneDeep(entity.aiContext?.synonyms || []), + examples: cloneDeep(entity.aiContext?.examples || []) +}) + +/** Applies only edited form values, preserving every other entity and extension in the definition. */ +export const updateSemanticMetadata = (definition, target, values) => { + const updated = cloneDeep(definition) + const entity = getSemanticMetadataEntity(updated, target) + if (!entity) throw new Error('The selected metadata no longer exists. Reload the model and try again.') + const initial = semanticMetadataFormValues(entity) + for (const key of editableProperties[target.kind]) { + if (!isEqual(initial[key], values[key])) entity[key] = values[key] ?? null Review Comment: Fixed in 2e2eb9cca. Both editors now use a clearable selector containing the ten wire values accepted by SemanticDTOUtils.DataTypeDeserializer. Clearing sends datatype: null, and definition validation rejects unsupported values before submission, including in the JSON editor. Added regression tests for field/metric selection, clearing, and invalid types; verified both clear-and-save flows in Chromium with mocked APIs. ########## web-v2/web/src/app/catalogs/rightContent/SemanticMetadataEditor.js: ########## @@ -0,0 +1,246 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { useState } from 'react' +import { Alert, Button, Form, Input, Modal, Select, Space, Typography } from 'antd' +import { isEqual } from 'lodash-es' +import { useAppDispatch } from '@/lib/hooks/useStore' +import { updateSemanticModel } from '@/lib/store/metalakes' +import { validateSemanticModelDefinition } from '@/lib/utils/semanticModel' +import { + getSemanticMetadataEntity, + semanticMetadataFormValues, + updateSemanticMetadata +} from '@/lib/utils/semanticMetadata' + +export default function SemanticMetadataEditor({ + definition, + target, + metalake, + catalog, + schema, + semanticModel, + onClose, + onSaved +}) { + const dispatch = useAppDispatch() + const [form] = Form.useForm() + const [saving, setSaving] = useState(false) + const [errors, setErrors] = useState([]) + + // Keep the editing snapshot stable through list/detail refreshes while the dialog is open. + const [original] = useState(definition) + const entity = getSemanticMetadataEntity(original, target) + const [initial] = useState(() => semanticMetadataFormValues(entity)) + const contextFormat = Form.useWatch('contextFormat', form) || initial.contextFormat + const hasExpression = ['field', 'metric'].includes(target.kind) + const title = target.kind === 'model' ? 'Edit Model AI Context' : `Edit ${target.kind} ${target.name}` + + const save = async () => { + try { + await form.validateFields() + } catch { + return + } + const updated = updateSemanticMetadata(original, target, form.getFieldsValue(true)) + const problems = validateSemanticModelDefinition(updated) + setErrors(problems) + if (problems.length) return + if (isEqual(original, updated)) { + onClose() + + return + } + setSaving(true) + try { + const result = await dispatch( + updateSemanticModel({ + metalake, + catalog, + schema, + semanticModel, + data: { updates: [{ '@type': 'replaceDefinition', definition: updated }] } + }) + ) + if (result.error || result.payload?.err || !result.payload) { + setErrors(['Unable to save metadata. Your edits are retained; please try again.']) + + return + } + await onSaved(result.payload) + onClose() + } finally { + setSaving(false) + } + } + + return ( + <Modal + open + title={title} + width={760} + onCancel={onClose} + onOk={save} + okText='Save' + confirmLoading={saving} + cancelButtonProps={{ disabled: saving }} + closable={!saving} + maskClosable={!saving} + keyboard={!saving} + > + <div className='overflow-auto pr-2' style={{ maxHeight: '65vh' }}> + {errors.length > 0 && ( + <Alert + className='mb-3' + type='error' + showIcon + message='Could not save metadata' + description={ + <ul className='m-0 pl-5'> + {errors.map(error => ( + <li key={error}>{error}</li> + ))} + </ul> + } + /> + )} + <Form form={form} initialValues={initial} layout='vertical' disabled={saving} name='semanticMetadata'> + {target.kind === 'field' && ( + <Form.Item name='label' label='Label'> + <Input /> + </Form.Item> + )} + {['dataset', 'field', 'metric'].includes(target.kind) && ( + <Form.Item name='description' label='Description'> + <Input.TextArea autoSize={{ minRows: 2, maxRows: 6 }} /> + </Form.Item> + )} + {hasExpression && ( + <> + <Form.Item name='datatype' label='Data type'> + <Input /> + </Form.Item> + <Typography.Title level={5}>Dialect expressions</Typography.Title> + <Form.List name='dialects'> + {(fields, { add, remove }) => ( + <> + {fields.map(({ key, name, ...rest }) => ( + <div key={key} className='mb-3 rounded border border-borderColor p-3'> + <Form.Item + {...rest} + name={[name, 'dialect']} + label='Dialect' + rules={[{ required: true, whitespace: true }]} + > + <Input /> + </Form.Item> + <Form.Item + {...rest} + name={[name, 'expression']} + label='SQL expression' + rules={[{ required: true, whitespace: true }]} + > + <Input.TextArea className='font-mono' autoSize={{ minRows: 3, maxRows: 8 }} /> + </Form.Item> + <Button onClick={() => remove(name)} aria-label={`Remove dialect ${name + 1}`}> + Remove dialect + </Button> + </div> + ))} + <Button className='mb-3' onClick={() => add({ dialect: '', expression: '' })}> + Add dialect + </Button> + </> + )} + </Form.List> + </> + )} + {target.kind === 'relationship' && ( + <> + <Space align='start' className='w-full'> + {['from', 'to'].map(side => ( + <Form.Item + key={side} + name={side} + label={side === 'from' ? 'From dataset' : 'To dataset'} + rules={[{ required: true }]} + > + <Select + className='min-w-48' + options={original.datasets.map(dataset => ({ value: dataset.name, label: dataset.name }))} + /> + </Form.Item> + ))} + </Space> + {['fromColumns', 'toColumns'].map(side => ( + <Form.Item key={side} name={side} label={side === 'fromColumns' ? 'From columns' : 'To columns'}> + <Select mode='tags' /> + </Form.Item> + ))} + </> + )} + <Typography.Title level={5}>AI Context</Typography.Title> + <Form.Item name='contextFormat' label='Guidance format'> + <Select + options={[ + { value: 'text', label: 'Text', disabled: !!entity.aiContext && typeof entity.aiContext === 'object' }, + { value: 'structured', label: 'Structured' } + ]} + onChange={value => { + if (value === 'structured' && initial.contextFormat === 'text') + form.setFieldValue('instructions', form.getFieldValue('contextText')) Review Comment: Fixed in 2e2eb9cca. The structured draft is initialized from the current text only on the first conversion. Subsequent format switches preserve both drafts, including edited instructions and examples. Added a regression test that switches twice and checks the saved definition, and verified the reported metric-editor flow in Chromium with mocked APIs. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
