Skip to content

Commit

Permalink
Remove inference_id field if no inference endpoint is selected (#205660)
Browse files Browse the repository at this point in the history
## Summary

Currently, the `semantic_text` field supports a default `inference_id`,
meaning users are not required to explicitly select an inference
endpoint during mapping. However, a bug has been identified: if the
`Select inference Id` popover is not opened, the `inference_id` field
property remains as an empty string. This causes Elasticsearch (ES) to
throw an error, as it requires a value to be present if the property is
defined.

To address this issue, the proposed solution is to remove the
`inference_id` property from the `semantic_text` field during field
mapping if its value is empty.

### Screen Recording


https://github.com/user-attachments/assets/e8d8d471-7ff2-493e-8872-e42838579d44

---------

Co-authored-by: Matthew Kime <[email protected]>
  • Loading branch information
Samiul-TheSoccerFan and mattkime authored Jan 20, 2025
1 parent 0f67c78 commit c8e0408
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,14 @@ describe('Mappings editor: core', () => {
let getMappingsEditorData = getMappingsEditorDataFactory(onChangeHandler);
let testBed: MappingsEditorTestBed;
const appDependencies = {
core: { application: {} },
docLinks: {
links: {
inferenceManagement: {
inferenceAPIDocumentation: 'https://abc.com/inference-api-create',
},
},
},
plugins: {
ml: { mlApi: {} },
},
Expand Down Expand Up @@ -474,6 +482,40 @@ describe('Mappings editor: core', () => {
expect(data).toEqual(updatedMappings);
});

test('updates mapping without inference id for semantic_text field', async () => {
let updatedMappings = { ...defaultMappings };

const {
find,
actions: { addField },
component,
} = testBed;

/**
* Mapped fields
*/
await act(async () => {
find('addFieldButton').simulate('click');
jest.advanceTimersByTime(0); // advance timers to allow the form to validate
});
component.update();

const newField = { name: 'someNewField', type: 'semantic_text' };
await addField(newField.name, newField.type);

updatedMappings = {
...updatedMappings,
properties: {
...updatedMappings.properties,
[newField.name]: { reference_field: 'address.city', type: 'semantic_text' },
},
};

({ data } = await getMappingsEditorData(component));

expect(data).toEqual(updatedMappings);
});

describe('props.indexMode sets the correct default value of _source field', () => {
it("defaults to 'stored' with 'standard' index mode prop", async () => {
await act(async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ export const CreateField = React.memo(function CreateFieldComponent({
}
};

const { createInferenceEndpoint, handleSemanticText } = useSemanticText({
const { createInferenceEndpoint } = useSemanticText({
form,
setErrorsInTrainedModelDeployment,
ml,
Expand All @@ -130,8 +130,9 @@ export const CreateField = React.memo(function CreateFieldComponent({
const { isValid, data } = await form.submit();

if (isValid && !clickOutside) {
if (isSemanticTextField(data)) {
handleSemanticText(data);
if (isSemanticTextField(data) && !data.inference_id) {
const { inference_id: inferenceId, ...rest } = data;
dispatch({ type: 'field.add', value: rest });
} else {
dispatch({ type: 'field.add', value: data });
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -293,11 +293,11 @@ function FieldListItemComponent(
</EuiBadge>
</EuiFlexItem>

{isSemanticText && (
{isSemanticText && source.inference_id ? (
<EuiFlexItem grow={false}>
<EuiBadge color="hollow">{source.inference_id as string}</EuiBadge>
</EuiFlexItem>
)}
) : null}

{isShadowed && (
<EuiFlexItem grow={false}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -689,7 +689,7 @@ export const getAllFieldTypesFromState = (allFields: Fields): DataType[] => {
};

export function isSemanticTextField(field: Partial<Field>): field is SemanticTextField {
return Boolean(field.inference_id && field.type === 'semantic_text');
return field.type === 'semantic_text';
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ export const DetailsPageMappingsContent: FunctionComponent<{
.map((field) => field.inference_id)
.filter(
(inferenceId: string) =>
inferenceId &&
inferenceToModelIdMap?.[inferenceId].trainedModelId && // third-party inference models don't have trainedModelId
!inferenceToModelIdMap?.[inferenceId].isDeployed &&
!isInferencePreconfigured(inferenceId)
Expand Down

0 comments on commit c8e0408

Please sign in to comment.