imbajin commented on code in PR #302:
URL:
https://github.com/apache/incubator-hugegraph-ai/pull/302#discussion_r2447061965
##########
hugegraph-llm/src/hugegraph_llm/api/rag_api.py:
##########
@@ -48,6 +50,13 @@ def rag_http_api(
def rag_answer_api(req: RAGRequest):
set_graph_config(req)
+ # Basic parameter validation: empty query => 400
Review Comment:
⚠️ **Important: Input validation added but inconsistent**
Good addition of empty query validation, but the validation logic is
duplicated across multiple endpoints (`rag_answer_api`, `graph_rag_recall_api`,
`text2gremlin_api`).
**Recommendation**: Extract to a shared validator function or Pydantic
validator:
```python
# In rag_requests.py
from pydantic import field_validator
class BaseQueryRequest(BaseModel):
query: str
@field_validator('query')
@classmethod
def validate_non_empty(cls, v):
if not v or not v.strip():
raise ValueError('Query must not be empty')
return v.strip()
```
This ensures consistent validation across all endpoints and reduces code
duplication.
##########
README.md:
##########
@@ -75,42 +75,6 @@ python -m hugegraph_llm.demo.rag_demo.app
> [!NOTE]
> Examples assume you've activated the virtual environment with `source
> .venv/bin/activate`
Review Comment:
‼️ **Critical: Breaking change without deprecation period**
The README removes code examples for `RAGPipeline` and `KgBuilder` without:
1. Marking them as deprecated in the actual code
2. Providing a deprecation timeline
3. Ensuring backward compatibility
**Impact**: Existing users will experience immediate breakage upon upgrading.
**Recommendation**:
- Keep the old classes with `@deprecated` decorators that proxy to new
scheduler
- Add deprecation warnings
- Document migration path in a MIGRATION.md file
- Consider keeping old examples in a "Legacy" section with deprecation notice
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
