Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: implement strict null checks #563

Merged
merged 13 commits into from
Jan 15, 2025
Merged
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
18 changes: 8 additions & 10 deletions api/src/attachment/controllers/attachment.controller.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import {

import { attachment, attachmentFile } from '../mocks/attachment.mock';
import { AttachmentRepository } from '../repositories/attachment.repository';
import { AttachmentModel, Attachment } from '../schemas/attachment.schema';
import { Attachment, AttachmentModel } from '../schemas/attachment.schema';
import { AttachmentService } from '../services/attachment.service';

import { AttachmentController } from './attachment.controller';
Expand Down Expand Up @@ -55,14 +55,12 @@ describe('AttachmentController', () => {
attachmentController =
module.get<AttachmentController>(AttachmentController);
attachmentService = module.get<AttachmentService>(AttachmentService);
attachmentToDelete = await attachmentService.findOne({
attachmentToDelete = (await attachmentService.findOne({
name: 'store1.jpg',
});
}))!;
});

afterAll(async () => {
await closeInMongodConnection();
});
afterAll(closeInMongodConnection);

afterEach(jest.clearAllMocks);

Expand All @@ -79,7 +77,7 @@ describe('AttachmentController', () => {
describe('Upload', () => {
it('should throw BadRequestException if no file is selected to be uploaded', async () => {
const promiseResult = attachmentController.uploadFile({
file: undefined,
file: [],
});
await expect(promiseResult).rejects.toThrow(
new BadRequestException('No file was selected'),
Expand Down Expand Up @@ -117,17 +115,17 @@ describe('AttachmentController', () => {

it('should download the attachment by id', async () => {
jest.spyOn(attachmentService, 'findOne');
const storedAttachment = await attachmentService.findOne({
const storedAttachment = (await attachmentService.findOne({
name: 'store1.jpg',
});
}))!;
const result = await attachmentController.download({
id: storedAttachment.id,
});

expect(attachmentService.findOne).toHaveBeenCalledWith(
storedAttachment.id,
);
expect(result.options).toEqual({
expect(result?.options).toEqual({
type: storedAttachment.type,
length: storedAttachment.size,
disposition: `attachment; filename="${encodeURIComponent(
Expand Down
10 changes: 8 additions & 2 deletions api/src/attachment/services/attachment.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -256,9 +256,15 @@ export class AttachmentService extends BaseService<Attachment> {
async download(
attachment: Attachment,
rootDir = config.parameters.uploadDir,
) {
): Promise<StreamableFile> {
if (this.getStoragePlugin()) {
return await this.getStoragePlugin()?.download(attachment);
const streamableFile =
await this.getStoragePlugin()?.download(attachment);
if (!streamableFile) {
throw new NotFoundException('No file was found');
}

return streamableFile;
} else {
const path = resolve(join(rootDir, attachment.location));

Expand Down
66 changes: 33 additions & 33 deletions api/src/extensions/helpers/llm-nlu/index.helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,40 +100,39 @@ export default class LlmNluHelper
*
* @returns An array of objects representing the found entities, with their `value`, `start`, and `end` positions.
*/
private findKeywordEntities(
text: string,
entity: NlpEntityFull,
): NLU.ParseEntity[] {
return entity.values
.flatMap(({ value, expressions }) => {
const allValues = [value, ...expressions];

// Filter the terms that are found in the text
return allValues
.flatMap((term) => {
const regex = new RegExp(`\\b${term}\\b`, 'g');
const matches = [...text.matchAll(regex)];

// Map matches to FoundEntity format
return matches.map((match) => ({
entity: entity.name,
value: term,
start: match.index!,
end: match.index! + term.length,
confidence: 1,
}));
})
.shift();
})
.filter((v) => !!v);
private findKeywordEntities(text: string, entity: NlpEntityFull) {
return (
entity.values
.flatMap(({ value, expressions }) => {
const allValues = [value, ...expressions];

// Filter the terms that are found in the text
return allValues
.flatMap((term) => {
const regex = new RegExp(`\\b${term}\\b`, 'g');
const matches = [...text.matchAll(regex)];

// Map matches to FoundEntity format
return matches.map((match) => ({
entity: entity.name,
value: term,
start: match.index!,
end: match.index! + term.length,
confidence: 1,
}));
})
.shift();
})
.filter((v) => !!v) || []
);
}

async predict(text: string): Promise<NLU.ParseEntities> {
const settings = await this.getSettings();
const helper = await this.helperService.getDefaultLlmHelper();
const defaultLanguage = await this.languageService.getDefaultLanguage();
// Detect language
const language = await helper.generateStructuredResponse<string>(
const language = await helper.generateStructuredResponse<string>?.(
abdou6666 marked this conversation as resolved.
Show resolved Hide resolved
`input text: ${text}`,
settings.model,
this.languageClassifierPrompt,
Expand All @@ -147,13 +146,13 @@ export default class LlmNluHelper
{
entity: 'language',
value: language || defaultLanguage.code,
confidence: undefined,
confidence: 100,
},
];
for await (const { name, doc, prompt, values } of this
.traitClassifierPrompts) {
const allowedValues = values.map(({ value }) => value);
const result = await helper.generateStructuredResponse<string>(
const result = await helper.generateStructuredResponse<string>?.(
abdou6666 marked this conversation as resolved.
Show resolved Hide resolved
`input text: ${text}`,
settings.model,
prompt,
Expand All @@ -163,12 +162,13 @@ export default class LlmNluHelper
enum: allowedValues.concat('unknown'),
},
);
const safeValue = result.toLowerCase().trim();
const value = allowedValues.includes(safeValue) ? safeValue : '';
const safeValue = result?.toLowerCase().trim();
const value =
safeValue && allowedValues.includes(safeValue) ? safeValue : '';
traits.push({
entity: name,
value,
confidence: undefined,
confidence: 100,
});
}

Expand All @@ -179,7 +179,7 @@ export default class LlmNluHelper
});
const entities = keywordEntities.flatMap((keywordEntity) =>
this.findKeywordEntities(text, keywordEntity),
);
) as NLU.ParseEntity[];

return { entities: traits.concat(entities) };
}
Expand Down
2 changes: 1 addition & 1 deletion api/src/migration/migration.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ describe('MigrationService', () => {

await service.run({
action: MigrationAction.UP,
version: null,
version: undefined,
isAutoMigrate: false,
});

Expand Down
5 changes: 3 additions & 2 deletions api/src/migration/migration.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import { Migration, MigrationDocument } from './migration.schema';
import {
MigrationAction,
MigrationName,
MigrationRunOneParams,
MigrationRunParams,
MigrationSuccessCallback,
MigrationVersion,
Expand Down Expand Up @@ -239,7 +240,7 @@ module.exports = {
*
* @returns Resolves when the migration action is successfully executed or stops if the migration already exists.
*/
private async runOne({ version, action }: MigrationRunParams) {
private async runOne({ version, action }: MigrationRunOneParams) {
// Verify DB status
const { exist, migrationDocument } = await this.verifyStatus({
version,
Expand All @@ -258,7 +259,7 @@ module.exports = {
attachmentService: this.attachmentService,
});

if (result) {
if (result && migrationDocument) {
await this.successCallback({
version,
action,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,10 @@ const migrateAttachmentMessages = async ({
type: response.headers['content-type'],
channel: {},
});
await updateAttachmentId(msg._id, attachment.id);

if (attachment) {
await updateAttachmentId(msg._id, attachment.id);
}
}
} else {
logger.warn(
Expand Down
4 changes: 4 additions & 0 deletions api/src/migration/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ export interface MigrationRunParams {
isAutoMigrate?: boolean;
}

export interface MigrationRunOneParams extends MigrationRunParams {
version: MigrationVersion;
}

export interface MigrationSuccessCallback extends MigrationRunParams {
migrationDocument: MigrationDocument;
}
Expand Down
2 changes: 1 addition & 1 deletion api/src/nlp/controllers/nlp-entity.controller.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ describe('NlpEntityController', () => {
acc.push({
...curr,
values: nlpValueFixtures.filter(
({ entity }) => parseInt(entity) === index,
({ entity }) => parseInt(entity!) === index,
) as NlpEntityFull['values'],
lookups: curr.lookups!,
builtin: curr.builtin!,
Expand Down
6 changes: 3 additions & 3 deletions api/src/nlp/controllers/nlp-value.controller.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ describe('NlpValueController', () => {
acc.push({
...curr,
entity: nlpEntityFixtures[
parseInt(curr.entity)
parseInt(curr.entity!)
] as NlpValueFull['entity'],
builtin: curr.builtin!,
expressions: curr.expressions!,
Expand All @@ -125,15 +125,15 @@ describe('NlpValueController', () => {
(acc, curr) => {
const ValueWithEntities = {
...curr,
entity: nlpEntities[parseInt(curr.entity)].id,
entity: curr.entity ? nlpEntities[parseInt(curr.entity!)].id : null,
expressions: curr.expressions!,
metadata: curr.metadata!,
builtin: curr.builtin!,
};
acc.push(ValueWithEntities);
return acc;
},
[] as TFixtures<NlpValue>[],
[] as TFixtures<NlpValueCreateDto>[],
);
expect(result).toEqualPayload(nlpValueFixturesWithEntities);
});
Expand Down
5 changes: 3 additions & 2 deletions api/src/nlp/controllers/nlp-value.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,9 @@ export class NlpValueController extends BaseController<
this.validate({
dto: createNlpValueDto,
allowedIds: {
entity: (await this.nlpEntityService.findOne(createNlpValueDto.entity))
?.id,
entity: createNlpValueDto.entity
? (await this.nlpEntityService.findOne(createNlpValueDto.entity))?.id
abdou6666 marked this conversation as resolved.
Show resolved Hide resolved
: null,
},
});
return await this.nlpValueService.create(createNlpValueDto);
Expand Down
36 changes: 33 additions & 3 deletions api/src/nlp/dto/nlp-value.dto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
* 2. All derivative works must include clear attribution to the original creator and software, Hexastack and Hexabot, in a prominent location (e.g., in the software's "About" section, documentation, and README file).
*/

import { PartialType } from '@nestjs/mapped-types';
import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger';
import {
IsArray,
Expand Down Expand Up @@ -49,11 +48,42 @@ export class NlpValueCreateDto {
@IsString()
@IsNotEmpty()
@IsObjectId({ message: 'Entity must be a valid ObjectId' })
entity: string;
entity: string | null;
}

export class NlpValueUpdateDto extends PartialType(NlpValueCreateDto) {}
export class NlpValueUpdateDto {
@ApiPropertyOptional({ description: 'Foreign ID', type: String })
@IsOptional()
@IsString()
foreign_id?: string;

@ApiPropertyOptional({ description: 'Nlp value', type: String })
@IsOptional()
@IsString()
value?: string;

@ApiPropertyOptional({
description: 'Nlp value expressions',
isArray: true,
type: Array,
})
@IsOptional()
@IsArray()
expressions?: string[];

@ApiPropertyOptional({ description: 'Nlp value entity', type: String })
@IsOptional()
@IsString()
@IsObjectId({ message: 'Entity must be a valid ObjectId' })
entity?: string | null;

@ApiPropertyOptional({ description: 'Nlp value is builtin', type: Boolean })
@IsOptional()
@IsBoolean()
builtin?: boolean;
}

export type NlpValueDto = DtoConfig<{
create: NlpValueCreateDto;
update: NlpValueUpdateDto;
}>;
2 changes: 1 addition & 1 deletion api/src/nlp/repositories/nlp-value.repository.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ describe('NlpValueRepository', () => {
const ValueWithEntities = {
...curr,
entity: nlpEntityFixtures[
parseInt(curr.entity)
parseInt(curr.entity!)
] as NlpValueFull['entity'],
builtin: curr.builtin!,
expressions: curr.expressions!,
Expand Down
2 changes: 1 addition & 1 deletion api/src/nlp/seeds/nlp-value.seed.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export class NlpValueSeeder extends BaseSeeder<
const entities = await this.nlpEntityRepository.findAll();
const modelDtos = models.map((v) => ({
...v,
entity: entities.find(({ name }) => name === v.entity)?.id,
entity: entities.find(({ name }) => name === v.entity)?.id || null,
abdou6666 marked this conversation as resolved.
Show resolved Hide resolved
}));
await this.repository.createMany(modelDtos);
return true;
Expand Down
8 changes: 2 additions & 6 deletions api/src/nlp/services/nlp-value.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,7 @@ describe('NlpValueService', () => {
nlpValues = await nlpValueRepository.findAll();
});

afterAll(async () => {
await closeInMongodConnection();
});
afterAll(closeInMongodConnection);

afterEach(jest.clearAllMocks);

Expand All @@ -94,9 +92,7 @@ describe('NlpValueService', () => {
(acc, curr) => {
const ValueWithEntities = {
...curr,
entity: nlpEntityFixtures[
parseInt(curr.entity)
] as NlpValueFull['entity'],
entity: nlpEntityFixtures[parseInt(curr.entity!)] as NlpEntity,
expressions: curr.expressions!,
metadata: curr.metadata!,
builtin: curr.builtin!,
Expand Down
Loading
Loading