Skip to content

Commit

Permalink
fix: LEAP-897: Fix incorrect usage of name attr in visual tags (#5745)
Browse files Browse the repository at this point in the history
  • Loading branch information
juliosgarbi authored Apr 24, 2024
1 parent 0619295 commit fce3668
Show file tree
Hide file tree
Showing 10 changed files with 82 additions and 47 deletions.
4 changes: 2 additions & 2 deletions docs/source/templates/generative-llm-ranker.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,13 @@ The `LLM Ranker` template includes the following labeling interface in XML forma
<View>
<View style="display: flex; align-items: center; font-size: 1em;">
<View style="margin: 0.5em 0.5em 0 0;">
<Header name="task_header" value="Task: " style="font-size: 1em;"/>
<Header value="Task: " style="font-size: 1em;"/>
</View>
<Text name="task" value="Drag and rank the given AI model responses based on their relevance to the prompt and the level of perceived bias."/>
</View>
<View style="display: flex; align-items: center; box-shadow: 2px 2px 5px #999; padding: 10px; border-radius: 5px; background-color: #E0E0E0; font-size: 1.25em;">
<View style="margin: 0 1em 0 0">
<Header name="prompt_header" value="Prompt: " />
<Header value="Prompt: " />
</View>
<Text name="prompt" value="$prompt"/>
</View>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@ config: |
<View>
<View style="display: flex; align-items: center; font-size: 1em;">
<View style="margin: 0.5em 0.5em 0 0;">
<Header name="task_header" value="Task: " style="font-size: 1em;"/>
<Header value="Task: " style="font-size: 1em;"/>
</View>
<Text name="task" value="$task"/>
</View>
<View style="display: flex; align-items: center; box-shadow: 2px 2px 5px #999; padding: 10px; border-radius: 5px; background-color: #E0E0E0; font-size: 1.25em;">
<View style="margin: 0 1em 0 0">
<Header name="prompt_header" value="Prompt: " />
<Header value="Prompt: " />
</View>
<Text name="prompt" value="$prompt"/>
</View>
Expand Down
2 changes: 1 addition & 1 deletion label_studio/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ def project_dialog():
label = """<View>
<TextEditor>
<Text name="dialog" value="$dialog"></Text>
<Header name="header" value="Your answer is:"></Header>
<Header value="Your answer is:"></Header>
<TextArea name="answer"></TextArea>
</TextEditor>
</View>"""
Expand Down
8 changes: 4 additions & 4 deletions web/dist/apps/labelstudio/version.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"message": "Merge branch 'develop' into 'fb-leap-1029'",
"commit": "637c44e92b0b8a984491df28b995be64156e7b0f",
"date": "2024-04-24T12:36:14.000Z",
"branch": "fb-leap-1029"
"message": "Merge branch 'develop' into 'fb-leap-897'",
"commit": "e7fa27482ff61ddee2c633bbeb76fd9acd098eae",
"date": "2024-04-24T14:50:35.000Z",
"branch": "fb-leap-897"
}
8 changes: 4 additions & 4 deletions web/dist/libs/datamanager/version.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"message": "Merge branch 'develop' into 'fb-leap-1029'",
"commit": "637c44e92b0b8a984491df28b995be64156e7b0f",
"date": "2024-04-24T12:36:14.000Z",
"branch": "fb-leap-1029"
"message": "Merge branch 'develop' into 'fb-leap-897'",
"commit": "e7fa27482ff61ddee2c633bbeb76fd9acd098eae",
"date": "2024-04-24T14:50:35.000Z",
"branch": "fb-leap-897"
}
2 changes: 1 addition & 1 deletion web/dist/libs/editor/main.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion web/dist/libs/editor/main.js.map

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions web/dist/libs/editor/version.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"message": "Merge branch 'develop' into 'fb-leap-1029'",
"commit": "637c44e92b0b8a984491df28b995be64156e7b0f",
"date": "2024-04-24T12:36:14.000Z",
"branch": "fb-leap-1029"
"message": "Merge branch 'develop' into 'fb-leap-897'",
"commit": "e7fa27482ff61ddee2c633bbeb76fd9acd098eae",
"date": "2024-04-24T14:50:35.000Z",
"branch": "fb-leap-897"
}
20 changes: 20 additions & 0 deletions web/libs/editor/src/core/DataValidator/ConfigValidator.js
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,21 @@ const validateParentTag = (element, model) => {
return errorBuilder.parentTagUnexpected(model.name, 'parent', element.tagName, model.properties.parentTypes);
};

/**
* Validates if visual tags have name attribute
* @param {Object} element
*/
const validateVisualTags = (element) => {
const visualTags = ['Collapse', 'Filter', 'Header', 'Style', 'View'];
const { tagName } = element;

if (visualTags.includes(tagName) && element.name) {
return errorBuilder.generalError(`Attribute <b>name</b> is not allowed for tag <b>${tagName}</b>.`);
}

return null;
}

/**
* Validate other tag attributes other than name and toName
* @param {Object} child
Expand Down Expand Up @@ -301,6 +316,11 @@ export class ConfigValidator {

if (parentValidation !== null) validationResult.push(parentValidation);

// Validate visual tags
const visualTagsValidation = validateVisualTags(child);

if (visualTagsValidation !== null) validationResult.push(visualTagsValidation);

validationResult.push(...validatePerRegion(child));

validationResult.push(...validateAttributes(child, model, propertiesToSkip));
Expand Down
71 changes: 43 additions & 28 deletions web/libs/editor/src/core/__tests__/ConfigValidator.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,42 +4,37 @@ import '../../tags/object/Image';
import '../../tags/object/RichText';
import '../../tags/control/RectangleLabels';
import '../../tags/control/Label';
import '../../tags/control/Choices';
import '../../tags/control/Choice';
import '../../tags/visual/Header';
import { ConfigValidator } from "../DataValidator/ConfigValidator";

// IMPORTANT NOTE
// all tests are disabled because they wait `validation` in the root component
// and this doesn't happen, may be that was some old behavior
// @todo fix this

it.skip('Should fail if a tag referenced by toName doesn\'t exist', () => {
const result = Tree.treeToModel(`
it('Should fail if a tag referenced by toName doesn\'t exist', () => {
const result = ConfigValidator.validate(Tree.treeToModel(`
<View>
<Image name="img1" value="$image"></Image>
<RectangleLabels name="tag" toName="img" fillOpacity="0.5" strokeWidth="5">
<Label value="Planet"></Label>
<Label value="Moonwalker" background="blue"></Label>
</RectangleLabels>
</View>
`);

const errorItem = result.validation[0];
`, {}));

expect(errorItem.error).toBe('ERR_TAG_NOT_FOUND');
expect(result[0].error).toBe('ERR_TAG_NOT_FOUND');
});

it.skip('Should fail if a tag referenced by toName is not image', () => {
const result = Tree.treeToModel(`
it('Should fail if a tag referenced by toName is not image', () => {
const result = ConfigValidator.validate(Tree.treeToModel(`
<View>
<HyperText name="img" value="$text"></HyperText>
<RectangleLabels name="tag" toName="img" fillOpacity="0.5" strokeWidth="5">
<Label value="Planet"></Label>
<Label value="Moonwalker" background="blue"></Label>
</RectangleLabels>
</View>
`);

const errorItem = result.validation[0];
`, {}));

expect(errorItem.error).toBe('ERR_TAG_UNSUPPORTED');
expect(result[0].error).toBe('ERR_TAG_UNSUPPORTED');
});

it.skip('Should fail if tag lacks mandatory attribute toName', () => {
Expand All @@ -58,34 +53,54 @@ it.skip('Should fail if tag lacks mandatory attribute toName', () => {
expect(errorItem.error).toBe('ERR_REQUIRED');
});

it.skip('Should fail if opacity attribute is out of range', () => {
const result = Tree.treeToModel(`
it('Should fail if opacity attribute is out of range', () => {
const result = ConfigValidator.validate(Tree.treeToModel(`
<View>
<Image name="img" value="$image"></Image>
<RectangleLabels name="tag" toName="img" fillOpacity="-1" strokeWidth="5">
<Label value="Planet"></Label>
<Label value="Moonwalker" background="blue"></Label>
</RectangleLabels>
</View>
`);
`, {}));

const errorItem = result.validation[0];

expect(errorItem.error).toBe('ERR_BAD_TYPE');
expect(result[0].error).toBe('ERR_BAD_TYPE');
});

it.skip('Should fail if color is not a proper CSS color', () => {
const result = Tree.treeToModel(`
it('Should fail if color is not a proper CSS color', () => {
const result = ConfigValidator.validate(Tree.treeToModel(`
<View>
<Image name="img" value="$image"></Image>
<RectangleLabels name="tag" toName="img" fillOpacity="0.6" strokeWidth="5">
<Label value="Planet"></Label>
<Label value="Moonwalker" background="verywrongcolor"></Label>
</RectangleLabels>
</View>
`);
`, {}));

expect(result[0].error).toBe('ERR_BAD_TYPE');
});

const errorItem = result.validation[0];

expect(errorItem.error).toBe('ERR_BAD_TYPE');
it('Should fail if visual tags have name attribute', () => {
const result = ConfigValidator.validate(Tree.treeToModel(`
<View>
<Header name="w3"/>
<Text name="text1" value="Did the agent follow up to ensure that both parties were satisfied with the outcome and understood the resolution"/>
<Image name="image" value="$image" zoom="true"/>
<Choices name="choice" toName="label" choice="single">
<Choice value="Yes"/>
<Choice value="No"/>
</Choices>
<PolygonLabels name="label" toName="image"
strokeWidth="3" pointSize="small"
opacity="0.9">
<Label value="Airplane" background="red"/>
<Label value="Car" background="blue"/>
</PolygonLabels>
</View>
`, {}));

expect(result[0].error).toBe('ERR_GENERAL');
expect(result[0].value).toBe('Attribute <b>name</b> is not allowed for tag <b>Header</b>.');
});

0 comments on commit fce3668

Please sign in to comment.