Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
91 changes: 91 additions & 0 deletions api/src/nlp/controllers/nlp-sample.controller.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -453,4 +453,95 @@ describe('NlpSampleController', () => {
expect(result).toEqual({ count: 0 });
});
});

describe('message', () => {
it('should filter out intent values that are not user-defined', async () => {
// Create a user-defined intent value
const intentEntity = await nlpEntityService.findOne({ name: 'intent' });
await nlpValueService.create({
entity: intentEntity!.id,
value: 'greeting',
expressions: [],
});

// Mock the NLU helper to return both user-defined and inferred intents
const mockPredict = jest.fn().mockResolvedValue({
entities: [
{
entity: 'intent',
value: 'greeting', // User-defined
confidence: 0.95,
},
{
entity: 'intent',
value: 'greetings_goodevening', // Not user-defined (inferred by Ludwig)
confidence: 0.99,
},
{
entity: 'language',
value: 'en',
confidence: 0.98,
},
],
});

jest
.spyOn(nlpSampleController['helperService'], 'getDefaultHelper')
.mockResolvedValue({
predict: mockPredict,
} as any);

const result = await nlpSampleController.message('Good evening');

// Should only include the user-defined intent value
expect(result.entities).toHaveLength(2); // greeting intent + language
expect(result.entities).toEqual(
expect.arrayContaining([
expect.objectContaining({
entity: 'intent',
value: 'greeting',
}),
expect.objectContaining({
entity: 'language',
value: 'en',
}),
]),
);
// Should NOT include the inferred intent
expect(result.entities).not.toEqual(
expect.arrayContaining([
expect.objectContaining({
value: 'greetings_goodevening',
}),
]),
);
});

it('should include all entities when they are user-defined', async () => {
const mockPredict = jest.fn().mockResolvedValue({
entities: [
{
entity: 'intent',
value: 'greeting',
confidence: 0.95,
},
{
entity: 'language',
value: 'en',
confidence: 0.98,
},
],
});

jest
.spyOn(nlpSampleController['helperService'], 'getDefaultHelper')
.mockResolvedValue({
predict: mockPredict,
} as any);

const result = await nlpSampleController.message('Hello');

expect(result.entities).toHaveLength(2);
});
});
});
91 changes: 67 additions & 24 deletions api/src/nlp/controllers/nlp-sample.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,31 +9,31 @@
import { Readable } from 'stream';

import {
BadRequestException,
Body,
Controller,
Delete,
Get,
HttpCode,
InternalServerErrorException,
NotFoundException,
Param,
Patch,
Post,
Query,
Res,
StreamableFile,
UploadedFile,
UseInterceptors,
BadRequestException,
Body,
Controller,
Delete,
Get,
HttpCode,
InternalServerErrorException,
NotFoundException,
Param,
Patch,
Post,
Query,
Res,
StreamableFile,
UploadedFile,
UseInterceptors,
} from '@nestjs/common';
import { FileInterceptor } from '@nestjs/platform-express';
import { CsrfCheck } from '@tekuconcept/nestjs-csrf';
import { Response } from 'express';
import { z } from 'zod';

import {
NlpValueMatchPattern,
nlpValueMatchPatternSchema,
NlpValueMatchPattern,
nlpValueMatchPatternSchema,
} from '@/chat/schemas/types/pattern';
import { HelperService } from '@/helper/helper.service';
import { HelperType } from '@/helper/types';
Expand All @@ -50,15 +50,16 @@ import { TFilterQuery } from '@/utils/types/filter.types';

import { NlpSampleDto, TNlpSampleDto } from '../dto/nlp-sample.dto';
import {
NlpSample,
NlpSampleFull,
NlpSamplePopulate,
NlpSampleStub,
NlpSample,
NlpSampleFull,
NlpSamplePopulate,
NlpSampleStub,
} from '../schemas/nlp-sample.schema';
import { NlpSampleState } from '../schemas/types';
import { NlpEntityService } from '../services/nlp-entity.service';
import { NlpSampleEntityService } from '../services/nlp-sample-entity.service';
import { NlpSampleService } from '../services/nlp-sample.service';
import { NlpValueService } from '../services/nlp-value.service';

@UseInterceptors(CsrfInterceptor)
@Controller('nlpsample')
Expand All @@ -73,6 +74,7 @@ export class NlpSampleController extends BaseController<
private readonly nlpSampleService: NlpSampleService,
private readonly nlpSampleEntityService: NlpSampleEntityService,
private readonly nlpEntityService: NlpEntityService,
private readonly nlpValueService: NlpValueService,
private readonly languageService: LanguageService,
private readonly helperService: HelperService,
) {
Expand Down Expand Up @@ -204,15 +206,56 @@ export class NlpSampleController extends BaseController<

/**
* Analyzes the input text using the NLP service and returns the parsed result.
* Filters out entities and values that are not defined in the user's Hexabot configuration.
*
* @param text - The input text to be analyzed.
*
* @returns The result of the NLP parsing process.
* @returns The result of the NLP parsing process with only user-defined entities.
*/
@Get('message')
async message(@Query('text') text: string) {
const helper = await this.helperService.getDefaultHelper(HelperType.NLU);
return helper.predict(text);
const prediction = await helper.predict(text);

// Get all user-defined entities and their values
const nlpMap = await this.nlpEntityService.getNlpMap();

// Filter entities to only include those that exist in user's configuration
const filteredEntities = await Promise.all(
prediction.entities.map(async (entity) => {
const nlpEntity = nlpMap.get(entity.entity);

// If entity doesn't exist in user's configuration, exclude it
if (!nlpEntity) {
return null;
}

// For trait entities (like intent), check if the value exists
if (nlpEntity.lookups?.includes('trait')) {
// Get all values for this entity
const entityValues = await this.nlpValueService.find({
entity: nlpEntity.id,
});

// Check if the predicted value exists in user-defined values
const valueExists = entityValues.some(
(v) => v.value === entity.value,
);

// If value doesn't exist, exclude this entity
if (!valueExists) {
return null;
}
}

return entity;
}),
);
Comment on lines +224 to +253
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Potential N+1 query performance issue in entity filtering.

The filtering logic calls this.nlpValueService.find({ entity: nlpEntity.id }) inside a Promise.all(prediction.entities.map(...)), which means for each trait entity in the prediction, a separate database query is executed. If predictions contain many trait entities, this could cause performance degradation.

Consider optimizing by:

  1. Pre-fetching all entity values in a single query before the loop
  2. Building a Map<entityId, Set> for O(1) lookups

Apply this optimization:

     const nlpMap = await this.nlpEntityService.getNlpMap();
+
+    // Pre-fetch all values for trait entities in one query
+    const traitEntityIds = Array.from(nlpMap.values())
+      .filter((entity) => entity.lookups?.includes('trait'))
+      .map((entity) => entity.id);
+    
+    const allEntityValues = traitEntityIds.length > 0
+      ? await this.nlpValueService.find({
+          entity: { $in: traitEntityIds },
+        })
+      : [];
+    
+    // Build a lookup map: entityId -> Set of valid values
+    const entityValueMap = new Map<string, Set<string>>();
+    for (const value of allEntityValues) {
+      if (!entityValueMap.has(value.entity)) {
+        entityValueMap.set(value.entity, new Set());
+      }
+      entityValueMap.get(value.entity)!.add(value.value);
+    }
 
     const filteredEntities = await Promise.all(
       prediction.entities.map(async (entity) => {
         const nlpEntity = nlpMap.get(entity.entity);
 
         if (!nlpEntity) {
           return null;
         }
 
         if (nlpEntity.lookups?.includes('trait')) {
-          const entityValues = await this.nlpValueService.find({
-            entity: nlpEntity.id,
-          });
-
-          const valueExists = entityValues.some(
-            (v) => v.value === entity.value,
-          );
+          const validValues = entityValueMap.get(nlpEntity.id);
+          const valueExists = validValues?.has(entity.value) ?? false;
 
           if (!valueExists) {
             return null;
           }
         }
 
         return entity;
       }),
     );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const filteredEntities = await Promise.all(
prediction.entities.map(async (entity) => {
const nlpEntity = nlpMap.get(entity.entity);
// If entity doesn't exist in user's configuration, exclude it
if (!nlpEntity) {
return null;
}
// For trait entities (like intent), check if the value exists
if (nlpEntity.lookups?.includes('trait')) {
// Get all values for this entity
const entityValues = await this.nlpValueService.find({
entity: nlpEntity.id,
});
// Check if the predicted value exists in user-defined values
const valueExists = entityValues.some(
(v) => v.value === entity.value,
);
// If value doesn't exist, exclude this entity
if (!valueExists) {
return null;
}
}
return entity;
}),
);
const nlpMap = await this.nlpEntityService.getNlpMap();
// Pre-fetch all values for trait entities in one query
const traitEntityIds = Array.from(nlpMap.values())
.filter((entity) => entity.lookups?.includes('trait'))
.map((entity) => entity.id);
const allEntityValues = traitEntityIds.length > 0
? await this.nlpValueService.find({
entity: { $in: traitEntityIds },
})
: [];
// Build a lookup map: entityId -> Set of valid values
const entityValueMap = new Map<string, Set<string>>();
for (const value of allEntityValues) {
if (!entityValueMap.has(value.entity)) {
entityValueMap.set(value.entity, new Set());
}
entityValueMap.get(value.entity)!.add(value.value);
}
const filteredEntities = await Promise.all(
prediction.entities.map(async (entity) => {
const nlpEntity = nlpMap.get(entity.entity);
// If entity doesn't exist in user's configuration, exclude it
if (!nlpEntity) {
return null;
}
// For trait entities (like intent), check if the value exists
if (nlpEntity.lookups?.includes('trait')) {
const validValues = entityValueMap.get(nlpEntity.id);
const valueExists = validValues?.has(entity.value) ?? false;
// If value doesn't exist, exclude this entity
if (!valueExists) {
return null;
}
}
return entity;
}),
);
🤖 Prompt for AI Agents
In api/src/nlp/controllers/nlp-sample.controller.ts around lines 224 to 253, the
current loop issues a DB query per trait entity causing an N+1 problem; instead,
before the Promise.all loop collect all nlpEntity ids that have lookups
including 'trait', fetch all values for those entities in a single call (e.g.,
this.nlpValueService.find({ entity: { $in: [...] } })), build a Map<entityId,
Set<value>> for O(1) membership checks, and then inside the mapping use the
precomputed Map to decide whether to return the entity or null without any
additional DB calls.


// Remove null entries (filtered out entities)
return {
entities: filteredEntities.filter((e) => e !== null),
};
}

/**
Expand Down