Fix: nlp sample populate entities #973
Conversation
|
Warning Rate limit exceeded@abdou6666 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 14 minutes and 44 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThe changes integrate NLP entity extraction into the chat workflow and enhance the NLP sample management in both backend and frontend. On the backend, the Chat module now imports the NLP module and registers the NLP sample model. The ChatService is updated to utilize the NlpSampleService, invoking a new method to upgrade samples with detected entities after message creation. The NlpSampleService introduces an Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ChatService
participant NlpSampleService
participant NlpSampleEntityService
participant NlpLanguageModel
User->>ChatService: Send message
ChatService->>ChatService: Create message
ChatService->>NlpSampleService: upgradeSampleWithEntities(entities, message)
NlpSampleService->>NlpSampleService: Check message.text
NlpSampleService->>NlpSampleService: Separate 'language' entity
NlpSampleService->>NlpSampleService: Find sample by message.text and type 'inbox'
alt Sample found
NlpSampleService->>NlpSampleEntityService: storeSampleEntities(entities, sample)
NlpSampleService->>NlpLanguageModel: Find language by code
alt Language found
NlpSampleService->>NlpSampleService: Update sample type to 'train' and set language
else Language not found
NlpSampleService->>NlpSampleService: Log warning
end
else Sample not found
NlpSampleService->>NlpSampleService: Return
end
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
api/src/nlp/services/nlp-sample.service.ts (1)
309-349: Well-implemented method for upgrading NLP samples with entities.This method effectively handles the process of upgrading inbox samples with detected entities and language information. The implementation includes:
- Proper validation with early returns when message text is missing
- Clean separation of language entity from other entities
- Appropriate sample lookup and entity storage
- Conditional language assignment with fallback handling
This addresses the PR's core issue of missing entities in NLP samples.
Consider enhancing error handling with more detailed logging, especially for the sample entity storage operation:
async upgradeSampleWithEntities( entities: NLU.ParseEntity[], createdMessage: Message, ) { if (!('text' in createdMessage.message)) { this.logger.warn('Received message without text attribute'); return; } const inferredLanguage = entities.find((e) => e.entity === 'language'); const entitiesWithoutLanguage = entities.filter( (e) => e.entity !== 'language', ); const foundSample = await this.repository.findOne({ text: createdMessage.message.text, type: 'inbox', }); if (!foundSample) { + this.logger.debug(`No inbox sample found for text: "${createdMessage.message.text.substring(0, 50)}..."`); return; } + try { await this.nlpSampleEntityService.storeSampleEntities( foundSample, entitiesWithoutLanguage, ); + this.logger.debug(`Successfully stored ${entitiesWithoutLanguage.length} entities for sample ID: ${foundSample.id}`); + } catch (error) { + this.logger.error(`Failed to store sample entities: ${error.message}`, error.stack); + return; + } const language = await this.languageService.findOne( { code: inferredLanguage?.value }, undefined, { _id: 1 }, ); if (!language) { this.logger.warn('Unable to find inferred language', inferredLanguage); } + try { await this.repository.updateOne(foundSample.id, { type: 'train', ...(language && { language: language.id }), }); + this.logger.debug(`Successfully upgraded sample ${foundSample.id} to train type${language ? ` with language ${language.id}` : ''}`); + } catch (error) { + this.logger.error(`Failed to update sample: ${error.message}`, error.stack); + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
api/src/chat/chat.module.ts(2 hunks)api/src/chat/services/chat.service.ts(4 hunks)api/src/nlp/services/nlp-sample.service.spec.ts(0 hunks)api/src/nlp/services/nlp-sample.service.ts(2 hunks)frontend/src/components/nlp/components/NlpSampleForm.tsx(2 hunks)frontend/src/components/nlp/components/NlpTrainForm.tsx(4 hunks)frontend/src/components/nlp/index.tsx(2 hunks)
💤 Files with no reviewable changes (1)
- api/src/nlp/services/nlp-sample.service.spec.ts
🧰 Additional context used
🧬 Code Graph Analysis (2)
api/src/chat/chat.module.ts (1)
api/src/nlp/schemas/nlp-sample.schema.ts (1)
NlpSampleModel(79-82)
api/src/nlp/services/nlp-sample.service.ts (2)
api/src/helper/types.ts (1)
ParseEntity(18-24)api/src/extensions/channels/web/types.ts (1)
Message(221-221)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: API-Tests
- GitHub Check: Frontend Tests
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (14)
frontend/src/components/nlp/components/NlpSampleForm.tsx (2)
32-33: LGTM: Good loading state managementThe extraction of loading state from the
useUpdatehook with proper naming improves user experience by allowing the UI to reflect mutation status.
66-70: LGTM: Correctly propagating loading statePassing the
isUpdatingSampleloading state to theNlpDatasetSamplecomponent enables proper UI feedback during sample updates.frontend/src/components/nlp/index.tsx (2)
65-65: LGTM: Improved hook destructuring with descriptive namingRenaming
mutatetocreateSampleand extracting theisLoadingstate improves code readability and enables proper loading state management.
94-97: LGTM: Consistent loading state propagationPassing the loading state to the
NlpDatasetSamplecomponent ensures consistent UI feedback during sample creation operations, aligning with the update functionality in other components.api/src/chat/chat.module.ts (2)
19-20: LGTM: Proper imports for NLP integrationAdding imports for
NlpModuleandNlpSampleModelenables the required integration between chat and NLP functionality.
67-67: LGTM: Correct module registration for NLP integrationRegistering the
NlpSampleModelin Mongoose and includingNlpModulein the imports provides the necessary foundation for NLP entity extraction in the chat workflow.Also applies to: 74-74
api/src/chat/services/chat.service.ts (3)
32-32: LGTM: Clean dependency injection of NlpSampleServiceProperly importing and injecting the NlpSampleService follows the application's dependency injection pattern and enables the NLP integration.
Also applies to: 50-50
147-154: LGTM: Effective implementation of entity extractionThis change correctly extracts NLP entities from the event, checks for their existence, and calls the NlpSampleService to upgrade samples with the detected entities, addressing the core issue.
369-371:✅ Verification successful
Verify event emission flow change
The emission of the 'hook:chatbot:received' event was moved in the execution flow. Please verify that this change doesn't affect any dependencies that might rely on the specific order of events.
Run this script to check for other listeners of this event:
🏁 Script executed:
#!/bin/bash # Search for other listeners of the 'hook:chatbot:received' event rg -A 3 -B 3 "hook:chatbot:received" --type ts --type js --glob "!**/chat.service.ts"Length of output: 85
Safe to relocate ‘hook:chatbot:received’ emission—no action needed
A repository-wide search found no subscribers or handlers for this event outside ofchat.service.ts. Moving the emit call won’t impact any internal dependencies.frontend/src/components/nlp/components/NlpTrainForm.tsx (4)
52-53: Good addition of loading state handling.The new
isMutationLoadingprop allows the component to be aware of mutation operations (create/update) in progress, which improves user experience by preventing multiple submissions while a request is processing.Also applies to: 58-59
101-101: LGTM - Language field is now tracked for validation.Adding the language field to the watched values is necessary for proper form validation.
171-180: Improved validation logic with clearer structure.Breaking down the validation logic into semantic variable names improves readability and maintainability. The new composite boolean
shouldDisableValidateButtonmakes it explicit when the button should be disabled, including validation for language field that was previously missing.
458-458: Clean implementation of button disabled state.Using the composite boolean
shouldDisableValidateButtonis much cleaner than an inline condition and properly prevents submissions when a mutation is in progress or when form data is invalid.api/src/nlp/services/nlp-sample.service.ts (1)
19-19: LGTM - Proper import for ParseEntity type.The import of NLU from helper types is needed for the new method.
abdou6666
requested review from
medchedli,
IkbelTalebHssan,
yassinedorbozgithub and
MohamedAliBouhaouala
Motivation
This PR fixes the issue where
nlp-samplemissing entities when nlp is enabled .Fixes #974
Type of change:
Please delete options that are not relevant.
Checklist:
Summary by CodeRabbit
New Features
Improvements
Style