Medical RAG Pipeline with Multi-Hop Search#44
Conversation
- Added `MemoryPipeline` and `PipelineStage` base classes for modular RAG flows. - Implemented `MedicalRagPipeline` with stages for normalization, decomposition, hybrid retrieval, re-ranking, and citation. - Created `MedicalPromptBuilder` for medical-specific prompts and safety disclaimers in Spanish. - Added comprehensive unit and integration tests in `test/medical_rag_pipeline_test.dart`. - Exported new components in `lib/isar_agent_memory.dart`.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a medical-specific RAG pipeline, including stages for query normalization, decomposition, hybrid retrieval, and evidence citation. Feedback focuses on improving the robustness of citation matching to avoid false positives, optimizing performance by pre-compiling regex patterns and parallelizing retrieval tasks, and enhancing multi-language support by removing hardcoded language triggers in the decomposition stage.
| for (var i = 0; i < context.retrievedNodes.length; i++) { | ||
| final node = context.retrievedNodes[i].node; | ||
| final citation = '[${i + 1}]'; | ||
| if (context.generatedResponse!.contains(citation)) { | ||
| cited.add(node); | ||
| } | ||
| } |
There was a problem hiding this comment.
The citation matching logic using String.contains is prone to false positives. For example, a check for [1] will return true if the response contains [10], [11], etc. This results in incorrect evidence attribution. Using a regular expression with a negative lookahead for digits ensures that only the exact citation is matched.
| for (var i = 0; i < context.retrievedNodes.length; i++) { | |
| final node = context.retrievedNodes[i].node; | |
| final citation = '[${i + 1}]'; | |
| if (context.generatedResponse!.contains(citation)) { | |
| cited.add(node); | |
| } | |
| } | |
| for (var i = 0; i < context.retrievedNodes.length; i++) { | |
| final node = context.retrievedNodes[i].node; | |
| final citation = '[${i + 1}]'; | |
| // Use regex with negative lookahead to ensure exact match (e.g., avoid matching [1] in [10]) | |
| final pattern = RegExp(RegExp.escape(citation) + r'(?!\d)'); | |
| if (context.generatedResponse!.contains(pattern)) { | |
| cited.add(node); | |
| } | |
| } |
| @override | ||
| Future<RagContext> process(RagContext context) async { | ||
| String normalized = context.currentQuery; | ||
| abbreviations.forEach((abbrev, expansion) { | ||
| // Simple regex-like replacement for whole words | ||
| final regex = RegExp('\\b$abbrev\\b', caseSensitive: false); | ||
| normalized = normalized.replaceAll(regex, expansion); | ||
| }); | ||
| context.currentQuery = normalized; | ||
| return context; | ||
| } | ||
| } |
There was a problem hiding this comment.
Creating a new RegExp instance for every abbreviation on every call to process is inefficient. Since the abbreviations are static, these patterns should be pre-compiled to improve performance, especially if this stage is called frequently.
static final Map<String, RegExp> _regexCache = abbreviations.map(
(k, v) => MapEntry(k, RegExp(r'\b' + k + r'\b', caseSensitive: false)),
);
@override
Future<RagContext> process(RagContext context) async {
String normalized = context.currentQuery;
_regexCache.forEach((abbrev, regex) {
normalized = normalized.replaceAll(regex, abbreviations[abbrev]!);
});
context.currentQuery = normalized;
return context;
}| context.currentQuery.contains(' y ') || | ||
| context.currentQuery.contains(' e ')) { |
There was a problem hiding this comment.
The decomposition logic relies on hardcoded Spanish conjunctions (' y ', ' e '). This limits the pipeline's effectiveness in other languages, despite other parts of the code attempting to support English. Consider making these triggers language-aware.
| for (final q in context.decomposedQueries) { | ||
| final results = await memoryGraph.hybridSearch(q, topK: 5, alpha: 0.5); | ||
| for (final res in results) { | ||
| final existing = allResults[res.node.id]; | ||
| if (existing == null || res.score > existing.score) { | ||
| allResults[res.node.id] = res; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The sub-queries generated during decomposition are processed sequentially. This can lead to high latency in the retrieval stage. Using Future.wait to execute these searches in parallel would significantly improve performance.
| for (final q in context.decomposedQueries) { | |
| final results = await memoryGraph.hybridSearch(q, topK: 5, alpha: 0.5); | |
| for (final res in results) { | |
| final existing = allResults[res.node.id]; | |
| if (existing == null || res.score > existing.score) { | |
| allResults[res.node.id] = res; | |
| } | |
| } | |
| } | |
| final searchFutures = context.decomposedQueries.map( | |
| (q) => memoryGraph.hybridSearch(q, topK: 5, alpha: 0.5), | |
| ); | |
| final resultsList = await Future.wait(searchFutures); | |
| for (final results in resultsList) { | |
| for (final res in results) { | |
| final existing = allResults[res.node.id]; | |
| if (existing == null || res.score > existing.score) { | |
| allResults[res.node.id] = res; | |
| } | |
| } | |
| } |
|
CORRECCION DE SCOPE: Por favor elimina el archivo medical_rag_pipeline.dart del PR. En su lugar crea lib/src/rag/pipeline_hooks.dart con interfaces genéricas:
El PR puede usar MemoryPipeline como base, pero sin lógica médica hardcodeada. |
|
SCOPE ISSUE: Este PR tiene lógica médica que NO pertenece a isar_agent_memory (package genérico). LO QUE HAY QUE ELIMINAR:
LO QUE DEBE QUEDAR:
La lógica médica específica (RAG pipeline, prompt builder, abbreviations) debe vivir en OrionHealth, no aquí. Por favor: elimina los archivos médicos y actualiza el PR. |
|
SCOPE VIOLATION: Este PR tiene logica medica hardcodeada que NO pertenece a un package generico. Por favor crea nuevo PR solo con memory_pipeline.dart generico. |
1 participant
Implemented a specialized medical RAG pipeline to address complex medical questions and multi-hop retrieval.
Key features:
All new components are thoroughly tested and follow the existing design patterns in the repository.
Fixes #39
PR created automatically by Jules for task 11294999718864016196 started by @iberi22