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

Commit ab2ff4c

Browse files
Chris McConnellfeich-ms
Chris McConnell
and
feich-ms
authored
Fix bug where luis:convert would incorrectly generate an error message (#1129)
* Fix #1128 * Remove debugger * Add test cases * fix merge intent don't merge features bug and adjust test cases * further fix and adjust test cases Co-authored-by: Fei Chen <[email protected]>
1 parent 5f35a0e commit ab2ff4c

File tree

6 files changed

+491
-59
lines changed

6 files changed

+491
-59
lines changed

‎packages/lu/src/parser/luis/luisCollate.js‎

Lines changed: 60 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ const retCode = require('../utils/enums/CLI-errors')
1616
* @returns {Luis} new Luis instance
1717
* @throws {exception} Throws on errors. exception object includes errCode and text.
1818
*/
19-
const build = async function(luArray, verbose, luis_culture, luSearchFn) {
19+
const build = async function (luArray, verbose, luis_culture, luSearchFn) {
2020
let mergedContent = await mergeLuFiles(luArray, verbose, luis_culture, luSearchFn)
2121
let parsedLUISList = mergedContent.LUISContent.filter(item => item.includeInCollate)
2222
if (parsedLUISList.length === 0) return new Luis()
@@ -33,12 +33,12 @@ const build = async function(luArray, verbose, luis_culture, luSearchFn) {
3333
* @param {Luis} luisObject Luis instances to collate with
3434
* @throws {exception} Throws on errors. exception object includes errCode and text.
3535
*/
36-
const collate = function(luisList) {
36+
const collate = function (luisList) {
3737
if (luisList.length === 0) return
3838
let luisObject = new Luis(luisList[0])
3939
let hashTable = {};
4040
initializeHash(luisObject, hashTable)
41-
for(let i = 1; i < luisList.length; i++) {
41+
for (let i = 1; i < luisList.length; i++) {
4242
let blob = luisList[i]
4343
mergeResults(blob, luisObject, LUISObjNameEnum.INTENT);
4444
mergeResults(blob, luisObject, LUISObjNameEnum.ENTITIES);
@@ -60,11 +60,11 @@ const collate = function(luisList) {
6060
}
6161

6262
module.exports = {
63-
collate,
63+
collate,
6464
build
6565
}
6666

67-
const cleanupEntities = function(luisObject) {
67+
const cleanupEntities = function (luisObject) {
6868
let consolidatedList = [];
6969
luisObject.composites.forEach(item => consolidatedList.push(item));
7070
luisObject.closedLists.forEach(item => consolidatedList.push(item));
@@ -74,12 +74,12 @@ const cleanupEntities = function(luisObject) {
7474
luisObject.entities.forEach((item, idx) => {
7575
if (consolidatedList.find(e => e.name == item.name) !== undefined) idxToRemove.push(idx);
7676
})
77-
idxToRemove.sort((a, b) => a-b).forEach(idx => luisObject.entities.splice(idx, 1))
77+
idxToRemove.sort((a, b) => a - b).forEach(idx => luisObject.entities.splice(idx, 1))
7878
delete luisObject.onAmbiguousLabels;
7979
}
8080

8181
const mergeResultsWithHash = function (blob, finalCollection, type, hashTable) {
82-
if (blob[type] === undefined || blob[type].length === 0) {
82+
if (blob[type] === undefined || blob[type].length === 0) {
8383
return
8484
}
8585
blob[type].forEach(function (blobItem) {
@@ -95,14 +95,14 @@ const mergeResultsWithHash = function (blob, finalCollection, type, hashTable) {
9595
type !== LUISObjNameEnum.PATTERNS &&
9696
type !== LUISObjNameEnum.UTTERANCE &&
9797
item.name === blobItem.name) {
98-
// merge roles
99-
(blobItem.roles || []).forEach(blobRole => {
100-
if (item.roles &&
101-
!item.roles.includes(blobRole)) {
102-
item.roles.push(blobRole);
103-
}
104-
});
105-
}
98+
// merge roles
99+
(blobItem.roles || []).forEach(blobRole => {
100+
if (item.roles &&
101+
!item.roles.includes(blobRole)) {
102+
item.roles.push(blobRole);
103+
}
104+
});
105+
}
106106
}
107107
});
108108
}
@@ -126,17 +126,17 @@ const mergeNDepthEntities = function (blob, finalCollection) {
126126
})
127127
// de-dupe and merge children
128128
if (item.children !== undefined && Array.isArray(item.children) && item.children.length !== 0) {
129+
if (itemExistsInFinal && itemExistsInFinal.children === undefined) {
130+
itemExistsInFinal.children = []
131+
itemExistsInFinal.explicitlyAdded = item.explicitlyAdded;
132+
}
129133
recursivelyMergeChildrenAndFeatures(item.children, itemExistsInFinal.children)
130134
}
131135
}
132136
})
133137
}
134138

135-
const recursivelyMergeChildrenAndFeatures = function(srcChildren, tgtChildren) {
136-
if (tgtChildren === undefined || !Array.isArray(tgtChildren) || tgtChildren.length === 0) {
137-
tgtChildren = srcChildren;
138-
return;
139-
}
139+
const recursivelyMergeChildrenAndFeatures = function (srcChildren, tgtChildren) {
140140
(srcChildren || []).forEach(item => {
141141
// find child in tgt
142142
let itemExistsInFinal = (tgtChildren || []).find(x => x.name == item.name);
@@ -157,8 +157,8 @@ const recursivelyMergeChildrenAndFeatures = function(srcChildren, tgtChildren) {
157157
}
158158
item.features.forEach(f => {
159159
let featureInFinal = (itemExistsInFinal.features || []).find(itFea => {
160-
return ((itFea.featureName !== undefined && itFea.featureName == f.featureName) ||
161-
(itFea.modelName !== undefined && itFea.modelName == f.modelName))
160+
return ((itFea.featureName !== undefined && itFea.featureName == f.featureName) ||
161+
(itFea.modelName !== undefined && itFea.modelName == f.modelName))
162162
});
163163
if (featureInFinal === undefined) {
164164
itemExistsInFinal.features.push(f);
@@ -172,6 +172,9 @@ const recursivelyMergeChildrenAndFeatures = function(srcChildren, tgtChildren) {
172172
}
173173
// de-dupe and merge children
174174
if (item.children !== undefined && Array.isArray(item.children) && item.children.length !== 0) {
175+
if (itemExistsInFinal && itemExistsInFinal.children === undefined) {
176+
itemExistsInFinal.children = []
177+
}
175178
recursivelyMergeChildrenAndFeatures(item.children, itemExistsInFinal.children)
176179
}
177180
}
@@ -186,7 +189,7 @@ const recursivelyMergeChildrenAndFeatures = function(srcChildren, tgtChildren) {
186189
* @returns {void} Nothing
187190
*/
188191
const mergeResults = function (blob, finalCollection, type) {
189-
if (blob[type] === undefined || blob[type].length === 0) {
192+
if (blob[type] === undefined || blob[type].length === 0) {
190193
return
191194
}
192195
blob[type].forEach(function (blobItem) {
@@ -200,24 +203,41 @@ const mergeResults = function (blob, finalCollection, type) {
200203
if (deepEqual(finalCollection[type][fIndex], blobItem)) {
201204
itemExists = true;
202205
break;
203-
}
206+
}
207+
208+
if ((type === LUISObjNameEnum.INTENT || type === LUISObjNameEnum.ENTITIES) && finalCollection[type][fIndex].name === blobItem.name) {
209+
itemExists = true;
210+
(blobItem.features || []).forEach(blobFeature => {
211+
if (finalCollection[type][fIndex].features === undefined) {
212+
finalCollection[type][fIndex].features = [];
213+
}
214+
if (!finalCollection[type][fIndex].features.find(
215+
(feature) =>
216+
(feature.modelName && feature.modelName === blobFeature.modelName) ||
217+
(feature.featureName && feature.featureName === blobFeature.featureName))) {
218+
finalCollection[type][fIndex].features.push(blobFeature);
219+
}
220+
});
221+
222+
if (type === LUISObjNameEnum.INTENT) break;
223+
}
204224

205225
// if item name matches, merge roles if available for everything other than intent
206-
if (type === LUISObjNameEnum.INTENT ||
207-
type === LUISObjNameEnum.PATTERNS ||
226+
if (type === LUISObjNameEnum.INTENT ||
227+
type === LUISObjNameEnum.PATTERNS ||
208228
type === LUISObjNameEnum.UTTERANCE ||
209229
finalCollection[type][fIndex].name !== blobItem.name) {
210-
continue;
230+
continue;
211231
}
212-
232+
213233
itemExists = true;
214234
(blobItem.roles || []).forEach(blobRole => {
215-
if (finalCollection[type][fIndex].roles &&
235+
if (finalCollection[type][fIndex].roles &&
216236
!finalCollection[type][fIndex].roles.includes(blobRole)) {
217-
finalCollection[type][fIndex].roles.push(blobRole);
237+
finalCollection[type][fIndex].roles.push(blobRole);
218238
}
219239
});
220-
240+
221241
}
222242
if (!itemExists) {
223243
finalCollection[type].push(blobItem);
@@ -267,7 +287,7 @@ const mergeResults_closedlists = function (blob, finalCollection, type) {
267287
});
268288
}
269289

270-
const buildRegex = function(blob, FinalLUISJSON){
290+
const buildRegex = function (blob, FinalLUISJSON) {
271291
// do we have regex entities here?
272292
if (blob.regex_entities === undefined || blob.regex_entities.length === 0) {
273293
return
@@ -293,7 +313,7 @@ const buildRegex = function(blob, FinalLUISJSON){
293313
})
294314
}
295315

296-
const buildPrebuiltEntities = function(blob, FinalLUISJSON){
316+
const buildPrebuiltEntities = function (blob, FinalLUISJSON) {
297317
// do we have prebuiltEntities here?
298318
if (blob.prebuiltEntities === undefined || blob.prebuiltEntities.length === 0) {
299319
return
@@ -318,7 +338,7 @@ const buildPrebuiltEntities = function(blob, FinalLUISJSON){
318338
});
319339
}
320340

321-
const buildModelFeatures = function(blob, FinalLUISJSON){
341+
const buildModelFeatures = function (blob, FinalLUISJSON) {
322342
// Find what scope to use in blob
323343
let blobScope = blob.model_features || blob.phraselists || [];
324344
if (blobScope.length === 0) return;
@@ -343,7 +363,7 @@ const buildModelFeatures = function(blob, FinalLUISJSON){
343363
});
344364
}
345365

346-
const buildComposites = function(blob, FinalLUISJSON){
366+
const buildComposites = function (blob, FinalLUISJSON) {
347367
if (blob.composites === undefined) return;
348368
// do we have composites? collate them correctly
349369
(blob.composites || []).forEach(composite => {
@@ -367,7 +387,7 @@ const buildComposites = function(blob, FinalLUISJSON){
367387
});
368388
}
369389

370-
const buildPatternAny = function(blob, FinalLUISJSON){
390+
const buildPatternAny = function (blob, FinalLUISJSON) {
371391
if (blob.patternAnyEntities === undefined) return;
372392
// do we have pattern.any entities here?
373393
(blob.patternAnyEntities || []).forEach(patternAny => {
@@ -385,15 +405,15 @@ const buildPatternAny = function(blob, FinalLUISJSON){
385405
let listEntityInMaster = FinalLUISJSON.closedLists.find(item => item.name == patternAny.name);
386406
let regexEntityInMaster = FinalLUISJSON.regex_entities.find(item => item.name == patternAny.name);
387407
let prebuiltInMaster = FinalLUISJSON.prebuiltEntities.find(item => item.name == patternAny.name);
388-
if (!simpleEntityInMaster &&
408+
if (!simpleEntityInMaster &&
389409
!compositeInMaster &&
390410
!listEntityInMaster &&
391411
!regexEntityInMaster &&
392412
!prebuiltInMaster) {
393413
if (patternAnyInMaster) {
394414
(patternAny.roles || []).forEach(role => !patternAnyInMaster.roles.includes(role) ? patternAnyInMaster.roles.push(role) : undefined);
395415
} else {
396-
FinalLUISJSON.patternAnyEntities.push(patternAny);
416+
FinalLUISJSON.patternAnyEntities.push(patternAny);
397417
}
398418
} else {
399419
// remove the pattern.any from master if another entity type has this name.
@@ -404,10 +424,10 @@ const buildPatternAny = function(blob, FinalLUISJSON){
404424
})
405425
}
406426

407-
const initializeHash = function(LuisJSON, hashTable = undefined) {
427+
const initializeHash = function (LuisJSON, hashTable = undefined) {
408428
for (let prop in LuisJSON) {
409429
if (hashTable !== undefined && (prop === LUISObjNameEnum.UTTERANCE || prop === LUISObjNameEnum.PATTERNS)) {
410430
(LuisJSON[prop] || []).forEach(item => hashTable[helpers.hashCode(JSON.stringify(item))] = item)
411431
}
412-
}
432+
}
413433
}

‎packages/lu/test/commands/luis/convert.test.js‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,10 @@ describe('luis:convert', () => {
105105
await assertToJSON('./../../fixtures/examples/13.lu', './../../fixtures/verified/13.json', '13')
106106
})
107107

108+
it('luis:convert hierarchical entities defined after labels parsed correctly', async () => {
109+
await assertToJSON('./../../fixtures/examples/newEntityIncludes.lu', './../../fixtures/verified/newEntityIncludes.json')
110+
})
111+
108112
it('Parse to LU instance', async () => {
109113
let luFile = `
110114
@ ml test

‎packages/lu/test/fixtures/examples/newEntityIncludes.lu‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
[labels](./test/fixtures/examples/newEntityWithFeaturesLabels.lu)
2+
[definitions](./test/fixtures/examples/newEntityWithFeatures.lu)

‎packages/lu/test/fixtures/examples/newEntityWithFeaturesLabels.lu‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
# GetUserProfile
2+
- I am {@userProfile = {@userAge = 36}} years old
3+
- My first name is {@userProfile = {@userName = {@firstName = vishwac}}}
4+
- My last name is {@userProfile = {@userName = {@lastName = kannan}}}
5+
- First of {@userProfile = {@userName = {@firstName = vishwac}}}
6+
- Age of {@userProfile = {@userAge = 36}}

0 commit comments

Comments
 (0)