Skip to content
This repository was archived by the owner on Jan 15, 2025. It is now read-only.

Various fixes to bf-lu #813

Merged
vishwacsena merged 8 commits into from
May 22, 2020
Merged

Various fixes to bf-lu #813

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
b58c8d7
various fixes.
May 20, 2020
309bdad
fixes
May 20, 2020
6ada7aa
updates
May 20, 2020
23dadf6
updates
May 20, 2020
cec2eea
Merge branch 'master' of https://github.com/microsoft/botframework-cl…
May 20, 2020
d67a740
fix for luis:build issue.
May 20, 2020
fd5913e
updates for nDepth reference resolver.
May 22, 2020
4ea73f5
Update luis:build to create result using luis()
May 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion packages/lu/src/parser/lubuild/builder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const retCode = require('./../utils/enums/CLI-errors')
const exception = require('./../utils/exception')
const LuisBuilderVerbose = require('./../luis/luisCollate')
const LuisBuilder = require('./../luis/luisBuilder')
const Luis = require('./../luis/luis')
const LUOptions = require('./../lu/luOptions')
const Content = require('./../lu/lu')
const recognizerType = require('./../utils/enums/recognizertypes')
Expand Down Expand Up @@ -55,9 +56,11 @@ export class Builder {

let fileContent = ''
let result
let luisObj
try {
result = await LuisBuilderVerbose.build(luFiles, true, fileCulture)
fileContent = result.parseToLuContent()
luisObj = new Luis(result)
fileContent = luisObj.parseToLuContent()
} catch (err) {
if (err.source) {
err.text = `Invalid LU file ${err.source}: ${err.text}`
Expand Down
89 changes: 53 additions & 36 deletions packages/lu/src/parser/lufile/parseFileContents.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,10 +184,6 @@ const parseLuAndQnaWithAntlr = async function (parsedContent, fileContent, log,

validateNDepthEntities(parsedContent.LUISJsonStructure.entities, parsedContent.LUISJsonStructure.flatListOfEntityAndRoles, parsedContent.LUISJsonStructure.intents);

// This nDepth child might have been labelled, if so, remove the duplicate simple entity.
// If utterances have this child, then all parents must be included in the label
updateModelBasedOnNDepthEntities(parsedContent.LUISJsonStructure.utterances, parsedContent.LUISJsonStructure.entities);

// Update intent and entities with phrase lists if any
updateIntentAndEntityFeatures(parsedContent.LUISJsonStructure);

Expand Down Expand Up @@ -234,23 +230,30 @@ const updateModelBasedOnNDepthEntities = function(utterances, entities) {
entityFoundInMaster.push({id: idx, entityRoot: entity, path: entityPath});
}
});
let isParentLabelled = false;
entityFoundInMaster.forEach(entityInMaster => {
let splitPath = entityInMaster.path.split("/").filter(item => item.trim() !== "");
if (entityFoundInMaster.length > 1 && splitPath.length === 0 && (!entityInMaster.entityRoot.children || entityInMaster.entityRoot.children.length === 0)) {
// this child needs to be removed. Note: There can only be at most one more entity due to utterance validation rules.
entities.splice(entityInMaster.id, 1);
} else {
splitPath.reverse().forEach(parent => {
// Ensure each parent is also labelled in this utterance
let parentLabelled = utterance.entities.find(entityUtt => entityUtt.entity == parent);
if (!parentLabelled) {
const errorMsg = `Every child entity labelled in an utterance must have its parent labelled in that utterance. Parent "${parent}" for child "${entityInUtterance.entity}" is not labelled in utterance "${utterance.text}" for intent "${utterance.intent}".`;
const error = BuildDiagnostic({
message: errorMsg
});
throw (new exception(retCode.errorCode.INVALID_INPUT, error.toString(), [error]));
}
})
if (isParentLabelled === false) {
let rSplitPath = splitPath.reverse();
rSplitPath.splice(0, 1);
rSplitPath.forEach(parent => {
// Ensure each parent is also labelled in this utterance
let parentLabelled = utterance.entities.find(entityUtt => entityUtt.entity == parent);
if (!parentLabelled) {
const errorMsg = `Every child entity labelled in an utterance must have its parent labelled in that utterance. Parent "${parent}" for child "${entityInUtterance.entity}" is not labelled in utterance "${utterance.text}" for intent "${utterance.intent}".`;
const error = BuildDiagnostic({
message: errorMsg
});
throw (new exception(retCode.errorCode.INVALID_INPUT, error.toString(), [error]));
} else {
isParentLabelled = true;
}
})
}
}
})
})
Expand Down Expand Up @@ -876,37 +879,43 @@ const parseAndHandleSimpleIntentSection = function (parsedContent, luResource) {
if (entity.role) {
addItemOrRoleIfNotPresent(parsedContent.LUISJsonStructure, LUISObjNameEnum.CLOSEDLISTS, entity.entity, [entity.role.trim()]);
} else {
let errorMsg = `${entity.entity} has been defined as a LIST entity type. It cannot be explicitly included in a labelled utterance unless the label includes a role.`;
let error = BuildDiagnostic({
message: errorMsg,
context: utteranceAndEntities.context
});

throw (new exception(retCode.errorCode.INVALID_INPUT, error.toString(), [error]));
if (!isChildEntity(entity, entitiesFound)) {
Copy link
Contributor

@munozemilio munozemilio May 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This operation is repetitive and can be a function that receives the error message

let errorMsg = `${entity.entity} has been defined as a LIST entity type. It cannot be explicitly included in a labelled utterance unless the label includes a role.`;
let error = BuildDiagnostic({
message: errorMsg,
context: utteranceAndEntities.context
});

throw (new exception(retCode.errorCode.INVALID_INPUT, error.toString(), [error]));
}
}
} else if (prebuiltExists !== undefined) {
if (entity.role) {
addItemOrRoleIfNotPresent(parsedContent.LUISJsonStructure, LUISObjNameEnum.PREBUILT, entity.entity, [entity.role.trim()]);
} else {
let errorMsg = `${entity.entity} has been defined as a PREBUILT entity type. It cannot be explicitly included in a labelled utterance unless the label includes a role.`;
let error = BuildDiagnostic({
message: errorMsg,
context: utteranceAndEntities.context
});

throw (new exception(retCode.errorCode.INVALID_INPUT, error.toString(), [error]));
if (!isChildEntity(entity, entitiesFound)) {
let errorMsg = `${entity.entity} has been defined as a PREBUILT entity type. It cannot be explicitly included in a labelled utterance unless the label includes a role.`;
let error = BuildDiagnostic({
message: errorMsg,
context: utteranceAndEntities.context
});

throw (new exception(retCode.errorCode.INVALID_INPUT, error.toString(), [error]));
}
}
} else if (regexExists !== undefined) {
if (entity.role) {
addItemOrRoleIfNotPresent(parsedContent.LUISJsonStructure, LUISObjNameEnum.REGEX, entity.entity, [entity.role.trim()]);
} else {
let errorMsg = `${entity.entity} has been defined as a Regex entity type. It cannot be explicitly included in a labelled utterance unless the label includes a role.`;
let error = BuildDiagnostic({
message: errorMsg,
context: utteranceAndEntities.context
});

throw (new exception(retCode.errorCode.INVALID_INPUT, error.toString(), [error]));
if (!isChildEntity(entity, entitiesFound)) {
let errorMsg = `${entity.entity} has been defined as a Regex entity type. It cannot be explicitly included in a labelled utterance unless the label includes a role.`;
let error = BuildDiagnostic({
message: errorMsg,
context: utteranceAndEntities.context
});

throw (new exception(retCode.errorCode.INVALID_INPUT, error.toString(), [error]));
}
}
} else if (patternAnyExists !== undefined) {
if (entity.value != '') {
Expand Down Expand Up @@ -981,6 +990,11 @@ const parseAndHandleSimpleIntentSection = function (parsedContent, luResource) {
}
}

const isChildEntity = function(entity, entitiesFound) {
// return true if the current entity is contained within a parent bounding label
return (entitiesFound || []).find(item => item.entity !== entity.entity && entity.startPos >= item.startPos && entity.endPos <= item.endPos) === undefined ? false : true;
}

/**
* Helper function to get entity type based on a name match
* @param {String} entityName name of entity to look up type for.
Expand Down Expand Up @@ -1161,6 +1175,7 @@ const handleNDepthEntity = function(parsedContent, entityName, entityRoles, enti
const SPACEASTABS = 4;
addItemOrRoleIfNotPresent(parsedContent.LUISJsonStructure, LUISObjNameEnum.ENTITIES, entityName, entityRoles);
let rootEntity = parsedContent.LUISJsonStructure.entities.find(item => item.name == entityName);
rootEntity.explicitlyAdded = true;
let defLine = line.start.line;
let baseTabLevel = 0;
let entityIdxByLevel = [];
Expand Down Expand Up @@ -1640,6 +1655,8 @@ const parseAndHandleEntitySection = function (parsedContent, luResource, log, lo
} else if (entityType.toLowerCase() === 'simple') {
// add this to entities if it doesnt exist
addItemOrRoleIfNotPresent(parsedContent.LUISJsonStructure, LUISObjNameEnum.ENTITIES, entityName, entityRoles);
let rootEntity = parsedContent.LUISJsonStructure.entities.find(item => item.name == entityName);
rootEntity.explicitlyAdded = true;
} else if (entityType.endsWith('=')) {
// is this qna maker alterations list?
if (entityType.includes(PARSERCONSTS.QNAALTERATIONS)) {
Expand Down
25 changes: 18 additions & 7 deletions packages/lu/src/parser/lufile/simpleIntentSection.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,24 @@ class SimpleIntentSection {
}

for (const normalIntentStr of parseTree.intentDefinition().intentBody().normalIntentBody().normalIntentString()) {
let utteranceAndEntities = visitor.visitNormalIntentStringContext(normalIntentStr);
utteranceAndEntities.context = normalIntentStr;
utteranceAndEntitiesMap.push(utteranceAndEntities);
utteranceAndEntities.errorMsgs.forEach(errorMsg => errors.push(BuildDiagnostic({
message: errorMsg,
context: normalIntentStr
})))
let utteranceAndEntities;
try {
utteranceAndEntities = visitor.visitNormalIntentStringContext(normalIntentStr);
}
catch (err) {
errors.push(BuildDiagnostic({
message: "Invalid utterance definition found. Did you miss a '{' or '}'?",
context: normalIntentStr
}))
Copy link
Contributor

@feich-ms feich-ms May 21, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only this error type will be thrown out from visitNormalIntentStringContext? How about throw this error inside visitNormalIntentStringContext function?

};
if (utteranceAndEntities !== undefined) {
utteranceAndEntities.context = normalIntentStr;
utteranceAndEntitiesMap.push(utteranceAndEntities);
utteranceAndEntities.errorMsgs.forEach(errorMsg => errors.push(BuildDiagnostic({
message: errorMsg,
context: normalIntentStr
})));
}
}
}

Expand Down
7 changes: 7 additions & 0 deletions packages/lu/src/parser/luis/luis.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ class Luis {

parseToLuContent(){
helpers.checkAndUpdateVersion(this)
helpers.cleanUpExplicitEntityProperty(this)
return luConverter(this)
}

Expand All @@ -54,6 +55,12 @@ const initialize = function(instance, LuisJSON) {
if (Object.keys(LuisJSON).includes(regexEntities)) instance[regexEntities] = LuisJSON[regexEntities];

settingsAndTokenizerCheck(instance, LuisJSON)

initializeEntities(instance)
}

const initializeEntities = function (instance) {
(instance.entities || []).forEach(e => e.explicitlyAdded = true)
}

const settingsAndTokenizerCheck = function(instance, LuisJSON) {
Expand Down
73 changes: 73 additions & 0 deletions packages/lu/src/parser/luis/luisCollate.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const Luis = require('./luis')
const helpers = require('./../utils/helpers')
const mergeLuFiles = require('./../lu/luMerger').Build
const exception = require('./../utils/exception')
const retCode = require('../utils/enums/CLI-errors')

/**
* Builds a Luis instance from a Lu list.
Expand Down Expand Up @@ -41,6 +42,7 @@ const collate = function(luisList) {
let blob = luisList[i]
mergeResults(blob, luisObject, LUISObjNameEnum.INTENT);
mergeResults(blob, luisObject, LUISObjNameEnum.ENTITIES);
mergeNDepthEntities(blob.entities, luisObject.entities);
mergeResults_closedlists(blob, luisObject, LUISObjNameEnum.CLOSEDLISTS);
mergeResults(blob, luisObject, LUISObjNameEnum.PATTERNANYENTITY);
mergeResultsWithHash(blob, luisObject, LUISObjNameEnum.UTTERANCE, hashTable);
Expand All @@ -52,6 +54,7 @@ const collate = function(luisList) {
buildPatternAny(blob, luisObject)
}
helpers.checkAndUpdateVersion(luisObject)
helpers.cleanUpExplicitEntityProperty(luisObject)
cleanupEntities(luisObject)
return luisObject
}
Expand Down Expand Up @@ -102,7 +105,77 @@ const mergeResultsWithHash = function (blob, finalCollection, type, hashTable) {
}
});
}
const mergeNDepthEntities = function (blob, finalCollection) {
let nDepthInBlob = (blob || []).filter(x => x.children !== undefined && Array.isArray(x.children) && x.children.length !== 0);
if (nDepthInBlob === undefined) return;
nDepthInBlob.forEach(item => {
let itemExistsInFinal = (finalCollection || []).find(x => x.name == item.name);
if (itemExistsInFinal === undefined) {
finalCollection.push(item);
} else {
// de-dupe and merge roles
(item.roles || []).forEach(r => {
if (itemExistsInFinal.roles === undefined) {
itemExistsInFinal.roles = [r];
} else {
if (!itemExistsInFinal.roles.includes(r)) {
itemExistsInFinal.roles.push(r);
}
}
})
// de-dupe and merge children
if (item.children !== undefined && Array.isArray(item.children) && item.children.length !== 0) {
recursivelyMergeChildrenAndFeatures(item.children, itemExistsInFinal.children)
}
}
})
}

const recursivelyMergeChildrenAndFeatures = function(srcChildren, tgtChildren) {
if (tgtChildren === undefined || !Array.isArray(tgtChildren) || tgtChildren.length === 0) {
tgtChildren = srcChildren;
return;
}
(srcChildren || []).forEach(item => {
// find child in tgt
let itemExistsInFinal = (tgtChildren || []).find(x => x.name == item.name);
if (itemExistsInFinal === undefined) {
tgtChildren.push(item);
} else {
// merge features
if (item.features !== undefined && item.features.length !== 0) {
// merge and verify type
let typeForFinalItem = (itemExistsInFinal.features || []).find(t => t.isRequired == true);
let typeForItem = (item.features || []).find(t1 => t1.isRequired == true);
if (typeForFinalItem !== undefined) {
if (typeForItem !== undefined) {
if (typeForFinalItem.modelName !== typeForItem.modelName) {
throw new exception(retCode.errorCode.INVALID_REGEX_ENTITY, `Child entity ${item.name} does not have consistent type definition. Please verify all definitions for this entity.`)
}
}
}
item.features.forEach(f => {
let featureInFinal = (itemExistsInFinal.features || []).find(itFea => {
return ((itFea.featureName !== undefined && itFea.featureName == f.featureName) ||
(itFea.modelName !== undefined && itFea.modelName == f.modelName))
});
if (featureInFinal === undefined) {
itemExistsInFinal.features.push(f);
} else {
// throw if isRequired is not the same.
if (featureInFinal.isRequired !== f.isRequired) {
throw new exception(retCode.errorCode.INVALID_REGEX_ENTITY, `Feature ${f.featureName} does not have consistent definition for entity ${item.name}. Please verify all definitions for this feature for this entity.`)
}
}
})
}
// de-dupe and merge children
if (item.children !== undefined && Array.isArray(item.children) && item.children.length !== 0) {
recursivelyMergeChildrenAndFeatures(item.children, itemExistsInFinal.children)
}
}
})
}
/**
* Helper function to merge item if it does not already exist
*
Expand Down
Loading