fix: remove invalid dummy path from gov_rag base_path and move to config#390
fix: remove invalid dummy path from gov_rag base_path and move to config#390ARYANPATEL-BIT wants to merge 2 commits intokubeedge:mainfrom
Conversation
Signed-off-by: Aryan Patel <aryan.patel7291@gmail.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: ARYANPATEL-BIT The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/assign @MooreZheng |
There was a problem hiding this comment.
Code Review
This pull request makes the base_path for GovernmentRAG configurable and adds validation to ensure it is explicitly provided. However, the current implementation in BaseModel still provides a dummy path as a default value, which bypasses the intended validation. Furthermore, the review identified several critical issues in BaseModel, including potential race conditions due to concurrent modification of the self.rag instance, logic errors in how RAG instances are reused across different query types, and a missing all_locations attribute that will cause runtime errors.
examples/government_rag/singletask_learning_bench/testalgorithms/basemodel.py
Outdated
Show resolved
Hide resolved
examples/government_rag/singletask_learning_bench/testalgorithms/basemodel.py
Outdated
Show resolved
Hide resolved
examples/government_rag/singletask_learning_bench/testalgorithms/basemodel.py
Outdated
Show resolved
Hide resolved
examples/government_rag/singletask_learning_bench/testalgorithms/basemodel.py
Outdated
Show resolved
Hide resolved
… defaults Signed-off-by: Aryan Patel <aryan.patel7291@gmail.com>
Description
This PR resolves an issue in
gov_rag.pywhere theGovernmentRAGclass was initialized with an invalid, hardcoded dummy path:acting as the default
base_path.This was an antipattern that silently masked missing parameter errors, causing confusing
FileNotFoundErrorand runtime crashes deeper down the execution stack when a path wasn't provided.Changes Made
gov_rag.py
base_pathparameter signature.base_pathtoNone.ValueError("base_path must be explicitly provided")ifbase_pathis not provided during instantiation.Example updated signature:
basemodel.py
GovernmentRAGinstantiations to explicitly inject the required path using the Ianvs configuration context, instead of relying on the library default.This change shifts the responsibility of path resolution to the configuration layer, making failures explicit at the call site.
Related Issues
Fixes #<YOUR_ISSUE_NUMBER_HERE>
(Don’t forget to replace this with your actual GitHub issue number before submitting the PR.)
Checklist
-s)##Fixes: #389