feat: add post-generation citation verifier with offline Quran corpus~ - #46
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
💤 Files with no reviewable changes (3)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughChangesThe PR adds a bundled Quran corpus and citation verifier, integrates configurable citation checking and strict correction into the FastAPI chat endpoint, updates session and response contracts, and expands CI with repository linting, syntax checks, and offline pytest coverage. Citation verification
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant Client
participant ChatEndpoint
participant GeminiChatSession
participant CitationVerifier
participant QuranCorpus
Client->>ChatEndpoint: Submit chat message
ChatEndpoint->>GeminiChatSession: Generate response
GeminiChatSession-->>ChatEndpoint: Return generated text
ChatEndpoint->>CitationVerifier: Extract and verify citations
CitationVerifier->>QuranCorpus: Look up referenced ayah
QuranCorpus-->>CitationVerifier: Return ayah data
CitationVerifier-->>ChatEndpoint: Return verification results
ChatEndpoint-->>Client: Return text and citation metadata
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 31-40: Add an explicit least-privilege permissions block near the
workflow or job definition in the CI workflow, granting only contents read
access. Keep the existing checkout, syntax-check, and pytest steps unchanged.
In `@data/quran_uthmani.json`:
- Around line 2-16: The quran_uthmani corpus is incomplete and must contain the
full dataset. Populate surahs with metadata for all 114 surahs and ayat with
Arabic and English text for every ayah, including references such as 2:1, 3:1,
and the complete 2:255 text; add an offline coverage test that detects missing
or partial surah and ayah entries.
In `@main.py`:
- Around line 155-161: Update delete_chat to authenticate the caller and verify
that the requested chat_id belongs to that caller before removing it from
sessions. Reject unauthorized ownership mismatches without deleting the session,
while preserving the existing success response for authorized deletions and 404
behavior for nonexistent sessions.
- Around line 92-104: Update the chat handler to use Gemini’s asynchronous send
method instead of blocking send_message, and wrap the call with a timeout and
exception handling that returns controlled HTTP errors. Before accessing
sessions via chat_id, enforce the same ownership/authentication check
established for session storage so clients cannot attach to another user’s
session.
- Around line 54-55: Replace the global sessions store with authenticated,
owner-scoped session handling: require a validated caller identity, associate
each chat session with that identity, and reject access when ownership does not
match. Remove the shared fallback “default” chat key by generating or requiring
a per-caller session identifier, and add TTL/eviction cleanup so inactive
sessions cannot accumulate indefinitely.
- Around line 42-44: Update the ChatRequest model’s message field to enforce
basic Pydantic validation: reject empty or whitespace-only messages and impose a
reasonable maximum length before requests reach the Gemini API. Keep chat_id
behavior unchanged and use the model’s existing validation conventions.
- Around line 32-51: Extend CitationVerificationResult with optional similarity
and correct_text fields matching the verifier output, then update the
formatted_results construction to pass both values through for Quran citations.
Preserve existing behavior when either value is absent and continue exposing the
remaining citation fields unchanged.
- Around line 82-89: Update run_strict_corrective_loop to remove the unused f
prefix from the static first string literal, and replace the synchronous
chat_session.send_message call with an awaited chat_session.send_message_async
call so the corrective loop remains fully asynchronous.
In `@requirements.txt`:
- Line 7: Pin the pytest dependency in requirements.txt to a specific version,
matching the existing pinned-dependency format and the project’s supported test
environment.
In `@test/test_verifier.py`:
- Line 2: Remove the unused pytest import from test_verifier.py, leaving the
remaining test code unchanged.
In `@verifier.py`:
- Around line 88-100: Update the quote comparison flow around
calculate_similarity to detect Arabic input and compare its normalized text
against ayah_data["arabic"], using exact normalized matching for Arabic
quotations. Preserve the existing fuzzy comparison against corpus_english for
non-Arabic translations, and add regression coverage for both exact and altered
Arabic quotes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8ca06fad-c02f-4e09-b486-2a72158b379f
📒 Files selected for processing (7)
.github/workflows/ci.ymlcorpus.pydata/quran_uthmani.jsonmain.pyrequirements.txttest/test_verifier.pyverifier.py
| run: flake8 . --max-line-length=120 --ignore=E501,W503 | ||
|
|
||
| - name: Check syntax | ||
| run: python -m py_compile main.py | ||
| run: | | ||
| python -m py_compile main.py | ||
| python -m py_compile corpus.py | ||
| python -m py_compile verifier.py | ||
|
|
||
| - name: Run offline unit tests | ||
| run: pytest No newline at end of file |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Restrict the workflow permissions.
The job currently relies on GitHub’s default token permissions. Add an explicit least-privilege block, such as contents: read, near the workflow or job definition; checkout does not require broader write access.
jobs:
lint-and-test:
+ permissions:
+ contents: read📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| run: flake8 . --max-line-length=120 --ignore=E501,W503 | |
| - name: Check syntax | |
| run: python -m py_compile main.py | |
| run: | | |
| python -m py_compile main.py | |
| python -m py_compile corpus.py | |
| python -m py_compile verifier.py | |
| - name: Run offline unit tests | |
| run: pytest | |
| jobs: | |
| lint-and-test: | |
| permissions: | |
| contents: read |
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 10-40: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 31 - 40, Add an explicit
least-privilege permissions block near the workflow or job definition in the CI
workflow, granting only contents read access. Keep the existing checkout,
syntax-check, and pytest steps unchanged.
Source: Linters/SAST tools
| "surahs": { | ||
| "1": { "name": "Al-Fatiha", "ayahs_count": 7 }, | ||
| "2": { "name": "Al-Baqarah", "ayahs_count": 286 }, | ||
| "114": { "name": "An-Nas", "ayahs_count": 6 } | ||
| }, | ||
| "ayat": { | ||
| "1:1": { | ||
| "arabic": "بِسْمِ اللَّهِ الرَّحْمَٰنِ الرَّحِيمِ", | ||
| "english": "In the name of Allah, the Entirely Merciful, the Especially Merciful." | ||
| }, | ||
| "2:255": { | ||
| "arabic": "اللَّهُ لَا إِلَٰهَ إِلَّا هُوَ الْحَيُّ الْقَيُّومُ", | ||
| "english": "Allah - there is no deity except Him, the Ever-Living, the Sustainer of all existence." | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Bundle the complete Quran corpus, not a fixture subset.
Only 3 surahs and 2 ayahs are present. Valid references such as 2:1 or 3:1 will be marked nonexistent/mismatched, and the supplied 2:255 text is incomplete. Ship all surah metadata and per-ayah Arabic/English text, with an offline coverage test preventing partial datasets.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@data/quran_uthmani.json` around lines 2 - 16, The quran_uthmani corpus is
incomplete and must contain the full dataset. Populate surahs with metadata for
all 114 surahs and ayat with Arabic and English text for every ayah, including
references such as 2:1, 3:1, and the complete 2:255 text; add an offline
coverage test that detects missing or partial surah and ayah entries.
| class CitationVerificationResult(BaseModel): | ||
| source: str # "quran" | "hadith" | ||
| surah: Optional[int] = None | ||
| ayah: Optional[int] = None | ||
| collection: Optional[str] = None | ||
| number: Optional[str] = None | ||
| status: str # "verified" | "mismatch" | "unverified" | "not_quoted" | ||
| reason: Optional[str] = None | ||
|
|
||
|
|
||
| class ChatRequest(BaseModel): | ||
| prompt: str | ||
| chat_id: Optional[str] = None | ||
| context: Optional[str] = None # Additional context for specific queries | ||
| message: str | ||
| chat_id: Optional[str] = "default" | ||
|
|
||
|
|
||
| class ChatResponse(BaseModel): | ||
| response: str | ||
| text: str | ||
| chat_id: str | ||
| history: List[Message] | ||
|
|
||
|
|
||
| def get_safety_settings(): | ||
| return [ | ||
| { | ||
| "category": "HARM_CATEGORY_HARASSMENT", | ||
| "threshold": "BLOCK_MEDIUM_AND_ABOVE" | ||
| }, | ||
| { | ||
| "category": "HARM_CATEGORY_HATE_SPEECH", | ||
| "threshold": "BLOCK_MEDIUM_AND_ABOVE" | ||
| }, | ||
| { | ||
| "category": "HARM_CATEGORY_SEXUALLY_EXPLICIT", | ||
| "threshold": "BLOCK_MEDIUM_AND_ABOVE" | ||
| }, | ||
| { | ||
| "category": "HARM_CATEGORY_DANGEROUS_CONTENT", | ||
| "threshold": "BLOCK_MEDIUM_AND_ABOVE" | ||
| } | ||
| ] | ||
| citations_verified: bool = True | ||
| verification_results: List[CitationVerificationResult] = [] |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
CitationVerificationResult drops similarity/correct_text from the verifier's output.
verify_quran_citation in verifier.py returns similarity for VERIFIED/MISMATCH and correct_text for MISMATCH, but this model only exposes source, surah, ayah, collection, number, status, reason. Since formatted_results (lines 134-145) builds CitationVerificationResult explicitly field-by-field, the correct Quran text and similarity score are silently dropped before reaching the client — undermining the PR's goal of surfacing per-citation verification data (the client can be told a quote is wrong but never shown the correct one).
🩹 Proposed fix
class CitationVerificationResult(BaseModel):
source: str # "quran" | "hadith"
surah: Optional[int] = None
ayah: Optional[int] = None
collection: Optional[str] = None
number: Optional[str] = None
status: str # "verified" | "mismatch" | "unverified" | "not_quoted"
reason: Optional[str] = None
+ similarity: Optional[float] = None
+ correct_text: Optional[str] = None CitationVerificationResult(
source=res["source"],
surah=res.get("surah"),
ayah=res.get("ayah"),
collection=res.get("collection"),
number=res.get("number"),
status=res["status"],
reason=res.get("reason"),
+ similarity=res.get("similarity"),
+ correct_text=res.get("correct_text"),
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| class CitationVerificationResult(BaseModel): | |
| source: str # "quran" | "hadith" | |
| surah: Optional[int] = None | |
| ayah: Optional[int] = None | |
| collection: Optional[str] = None | |
| number: Optional[str] = None | |
| status: str # "verified" | "mismatch" | "unverified" | "not_quoted" | |
| reason: Optional[str] = None | |
| class ChatRequest(BaseModel): | |
| prompt: str | |
| chat_id: Optional[str] = None | |
| context: Optional[str] = None # Additional context for specific queries | |
| message: str | |
| chat_id: Optional[str] = "default" | |
| class ChatResponse(BaseModel): | |
| response: str | |
| text: str | |
| chat_id: str | |
| history: List[Message] | |
| def get_safety_settings(): | |
| return [ | |
| { | |
| "category": "HARM_CATEGORY_HARASSMENT", | |
| "threshold": "BLOCK_MEDIUM_AND_ABOVE" | |
| }, | |
| { | |
| "category": "HARM_CATEGORY_HATE_SPEECH", | |
| "threshold": "BLOCK_MEDIUM_AND_ABOVE" | |
| }, | |
| { | |
| "category": "HARM_CATEGORY_SEXUALLY_EXPLICIT", | |
| "threshold": "BLOCK_MEDIUM_AND_ABOVE" | |
| }, | |
| { | |
| "category": "HARM_CATEGORY_DANGEROUS_CONTENT", | |
| "threshold": "BLOCK_MEDIUM_AND_ABOVE" | |
| } | |
| ] | |
| citations_verified: bool = True | |
| verification_results: List[CitationVerificationResult] = [] | |
| class CitationVerificationResult(BaseModel): | |
| source: str # "quran" | "hadith" | |
| surah: Optional[int] = None | |
| ayah: Optional[int] = None | |
| collection: Optional[str] = None | |
| number: Optional[str] = None | |
| status: str # "verified" | "mismatch" | "unverified" | "not_quoted" | |
| reason: Optional[str] = None | |
| similarity: Optional[float] = None | |
| correct_text: Optional[str] = None | |
| class ChatRequest(BaseModel): | |
| message: str | |
| chat_id: Optional[str] = "default" | |
| class ChatResponse(BaseModel): | |
| text: str | |
| chat_id: str | |
| citations_verified: bool = True | |
| verification_results: List[CitationVerificationResult] = [] |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@main.py` around lines 32 - 51, Extend CitationVerificationResult with
optional similarity and correct_text fields matching the verifier output, then
update the formatted_results construction to pass both values through for Quran
citations. Preserve existing behavior when either value is absent and continue
exposing the remaining citation fields unchanged.
| class ChatRequest(BaseModel): | ||
| prompt: str | ||
| chat_id: Optional[str] = None | ||
| context: Optional[str] = None # Additional context for specific queries | ||
| message: str | ||
| chat_id: Optional[str] = "default" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Add basic validation on ChatRequest.message.
message has no length/non-empty constraints, so an empty or arbitrarily large prompt is forwarded straight to the Gemini API on every request. As per path instructions, missing Pydantic validation on request bodies should be flagged.
🩹 Proposed fix
-class ChatRequest(BaseModel):
- message: str
- chat_id: Optional[str] = "default"
+class ChatRequest(BaseModel):
+ message: str = Field(..., min_length=1, max_length=4000)
+ chat_id: Optional[str] = "default"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| class ChatRequest(BaseModel): | |
| prompt: str | |
| chat_id: Optional[str] = None | |
| context: Optional[str] = None # Additional context for specific queries | |
| message: str | |
| chat_id: Optional[str] = "default" | |
| class ChatRequest(BaseModel): | |
| message: str = Field(..., min_length=1, max_length=4000) | |
| chat_id: Optional[str] = "default" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@main.py` around lines 42 - 44, Update the ChatRequest model’s message field
to enforce basic Pydantic validation: reject empty or whitespace-only messages
and impose a reasonable maximum length before requests reach the Gemini API.
Keep chat_id behavior unchanged and use the model’s existing validation
conventions.
Source: Path instructions
| # In-memory session store for demo purposes | ||
| sessions: Dict[str, Any] = {} |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Unauthenticated, unbounded in-memory session store.
sessions is keyed only by client-supplied chat_id with no ownership check, so any caller who knows/guesses a chat_id can continue or read someone else's conversation (IDOR). Worse, any request without an explicit chat_id falls back to the shared "default" key (line 97), mixing conversations from different anonymous callers into one session. There's also no TTL/eviction, so sessions grows unbounded for the life of the process.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@main.py` around lines 54 - 55, Replace the global sessions store with
authenticated, owner-scoped session handling: require a validated caller
identity, associate each chat session with that identity, and reject access when
ownership does not match. Remove the shared fallback “default” chat key by
generating or requiring a per-caller session identifier, and add TTL/eviction
cleanup so inactive sessions cannot accumulate indefinitely.
| @app.post("/chat", response_model=ChatResponse) | ||
| async def chat(request: ChatRequest): | ||
| try: | ||
| logger.info(f"Received chat request: {request.prompt[:100]}...") | ||
|
|
||
| # Get or create chat session | ||
| chat_id = request.chat_id or str(uuid.uuid4()) | ||
| if chat_id not in active_chats: | ||
| logger.info(f"Creating new chat session: {chat_id}") | ||
| model = genai.GenerativeModel( | ||
| 'gemini-2.5-flash-preview-05-20', | ||
| safety_settings=get_safety_settings() | ||
| ) | ||
| # Initialize chat without sending context message | ||
| active_chats[chat_id] = model.start_chat(history=[]) | ||
|
|
||
| chat = active_chats[chat_id] | ||
|
|
||
| # Prepare the prompt with context if provided | ||
| full_prompt = request.prompt | ||
| if request.context: | ||
| full_prompt = f"Context: {request.context}\n\nQuestion: {ISLAMIC_CONTEXT, request.prompt}" | ||
|
|
||
| # Send message and get response | ||
| logger.info("Sending message to chat...") | ||
| response = chat.send_message( | ||
| full_prompt, | ||
| generation_config={ | ||
| "temperature": 0.7, | ||
| "top_p": 0.8, | ||
| "top_k": 40, | ||
| "max_output_tokens": 2048, | ||
| } | ||
| ) | ||
| if not GEMINI_API_KEY: | ||
| raise HTTPException(status_code=500, detail="GEMINI_API_KEY is not configured.") | ||
|
|
||
| chat_id = request.chat_id or "default" | ||
| if chat_id not in sessions: | ||
| model = get_model() | ||
| sessions[chat_id] = model.start_chat(history=[]) | ||
|
|
||
| if not response.text: | ||
| logger.error("Empty response received from model") | ||
| raise HTTPException(status_code=500, detail="Empty response from AI model") | ||
|
|
||
| # Get chat history | ||
| history = [] | ||
| for message in chat.history: | ||
| try: | ||
| if hasattr(message, 'parts') and message.parts: | ||
| content = message.parts[0].text if hasattr(message.parts[0], 'text') else str(message.parts[0]) | ||
| else: | ||
| content = str(message) | ||
|
|
||
| history.append(Message( | ||
| role="user" if message.role == "user" else "model", | ||
| content=content | ||
| )) | ||
| except Exception as e: | ||
| logger.warning(f"Error processing message in history: {str(e)}") | ||
| continue | ||
|
|
||
| logger.info("Chat response generated successfully") | ||
| chat_session = sessions[chat_id] | ||
| response = chat_session.send_message(request.message) | ||
| response_text = response.text |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | 🏗️ Heavy lift
Blocking Gemini call and unauthenticated chat_id lookup in the request path.
Two concerns here:
chat_session.send_message(request.message)(line 103) is a blocking, synchronous network call inside anasync defhandler — it stalls the whole event loop, so a single slow Gemini response freezes every other in-flight request regardless ofchat_id. Preferawait chat_session.send_message_async(...).sessions[chat_id](lines 98-102) is accessed purely by client-suppliedchat_idwith no ownership check — same IDOR concern as the store declaration; any client can attach to/continue another user's session by supplying theirchat_id.
There's also no timeout or exception handling around the Gemini call — an API error, rate-limit, or network hang propagates as an unhandled exception (or hangs indefinitely) rather than a controlled error response.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@main.py` around lines 92 - 104, Update the chat handler to use Gemini’s
asynchronous send method instead of blocking send_message, and wrap the call
with a timeout and exception handling that returns controlled HTTP errors.
Before accessing sessions via chat_id, enforce the same ownership/authentication
check established for session storage so clients cannot attach to another user’s
session.
Source: Path instructions
| @app.delete("/chat/{chat_id}") | ||
| async def delete_chat(chat_id: str): | ||
| try: | ||
| if chat_id in active_chats: | ||
| del active_chats[chat_id] | ||
| logger.info(f"Deleted chat session: {chat_id}") | ||
| return {"message": "Chat session deleted successfully"} | ||
| return {"message": "Chat session not found"} | ||
| except Exception as e: | ||
| error_msg = f"❌ Error deleting chat: {str(e)}" | ||
| logger.error(error_msg) | ||
| raise HTTPException(status_code=500, detail=error_msg) | ||
|
|
||
| if __name__ == "__main__": | ||
| import uvicorn | ||
| logger.info("Starting server...") | ||
| uvicorn.run(app, host="0.0.0.0", port=8000) | ||
| if chat_id in sessions: | ||
| del sessions[chat_id] | ||
| return {"status": "success", "message": f"Session {chat_id} deleted."} | ||
| raise HTTPException(status_code=404, detail="Session not found.") | ||
|
|
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Same IDOR concern: any client can delete any session.
DELETE /chat/{chat_id} deletes whatever session matches the path parameter with no check that the caller owns that chat_id, letting one user terminate another's conversation just by knowing/guessing the ID.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@main.py` around lines 155 - 161, Update delete_chat to authenticate the
caller and verify that the requested chat_id belongs to that caller before
removing it from sessions. Reject unauthorized ownership mismatches without
deleting the session, while preserving the existing success response for
authorized deletions and 404 behavior for nonexistent sessions.
| google-generativeai==0.8.3 | ||
| pydantic==2.5.3 | ||
| stellar-sdk>=12.0.0 | ||
| pytest No newline at end of file |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- requirements.txt ---\n'
nl -ba requirements.txt | sed -n '1,80p'
printf '\n--- test/test_verifier.py ---\n'
nl -ba test/test_verifier.py | sed -n '1,200p'
printf '\n--- other pinned dependencies in requirements.txt ---\n'
grep -nE '^[A-Za-z0-9_.-]+(==|>=|<=|~=|!=)' requirements.txt || true
printf '\n--- pytest mentions in repo ---\n'
rg -n --hidden --glob '!**/.git/**' '\bpytest\b' .Repository: Deen-Bridge/dnb-ai
Length of output: 247
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- requirements.txt ---'
nl -ba requirements.txt | sed -n '1,120p'
echo
echo '--- test/test_verifier.py ---'
nl -ba test/test_verifier.py | sed -n '1,220p'
echo
echo '--- pinned dependencies in requirements.txt ---'
grep -nE '^[A-Za-z0-9_.-]+(==|>=|<=|~=|!=)' requirements.txt || true
echo
echo '--- pytest mentions in repo ---'
rg -n --hidden --glob '!**/.git/**' '\bpytest\b' .Repository: Deen-Bridge/dnb-ai
Length of output: 221
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- requirements.txt ---'
cat -n requirements.txt | sed -n '1,120p'
echo
echo '--- test/test_verifier.py ---'
cat -n test/test_verifier.py | sed -n '1,220p'
echo
echo '--- pinned dependencies in requirements.txt ---'
grep -nE '^[A-Za-z0-9_.-]+(==|>=|<=|~=|!=)' requirements.txt || true
echo
echo '--- pytest mentions in repo ---'
rg -n --hidden --glob '!**/.git/**' '\bpytest\b' .Repository: Deen-Bridge/dnb-ai
Length of output: 4210
Pin pytest in requirements.txt:7.
This file pins the other dependencies, so leaving the test dependency floating makes local and CI test environments drift between runs.
Proposed fix
-pytest
+pytest==<approved-version>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@requirements.txt` at line 7, Pin the pytest dependency in requirements.txt to
a specific version, matching the existing pinned-dependency format and the
project’s supported test environment.
Source: Path instructions
| # 2. Check quote similarity (English translation) | ||
| corpus_english = ayah_data.get("english", "") if ayah_data else "" | ||
| similarity = calculate_similarity(quote, corpus_english) | ||
|
|
||
| # Threshold of 0.70 accounts for variations across translation editions | ||
| if similarity >= 0.70: | ||
| return { | ||
| "source": "quran", | ||
| "surah": surah, | ||
| "ayah": ayah, | ||
| "status": VerificationStatus.VERIFIED, | ||
| "similarity": round(similarity, 2) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Verify Arabic quotes against the Arabic corpus text.
Arabic quotations are always compared with corpus_english, so even an exact normalized Arabic quote is marked mismatch. Detect Arabic input and compare normalized Arabic text against ayah_data["arabic"]; retain fuzzy English matching for translations. Add exact- and altered-Arabic regression cases.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@verifier.py` around lines 88 - 100, Update the quote comparison flow around
calculate_similarity to detect Arabic input and compare its normalized text
against ayah_data["arabic"], using exact normalized matching for Arabic
quotations. Preserve the existing fuzzy comparison against corpus_english for
non-Arabic translations, and add regression coverage for both exact and altered
Arabic quotes.
|
@David-282 fix CI and conflict too, so I can review and merge asap |
|
@David-282 CI is still failing, please fix, am ready to merge asap |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
main.py (1)
106-113: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThe top-level verification flag can falsely report trust.
main.py#L106-L113: do not setcitations_verified=Truewhen verification is disabled.main.py#L117-L132: includeUNVERIFIEDresults, and define handling forNOT_QUOTED, when calculating the aggregate status and strict-mode corrections.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@main.py` around lines 106 - 113, The citation verification flow must not report trust when verification is disabled. In main.py lines 106-113, change the off-mode ChatResponse citations_verified value to indicate unverified citations; in main.py lines 117-132, include UNVERIFIED results in aggregate status calculation and explicitly define how NOT_QUOTED affects the aggregate status and strict-mode corrections.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@main.py`:
- Around line 106-113: The citation verification flow must not report trust when
verification is disabled. In main.py lines 106-113, change the off-mode
ChatResponse citations_verified value to indicate unverified citations; in
main.py lines 117-132, include UNVERIFIED results in aggregate status
calculation and explicitly define how NOT_QUOTED affects the aggregate status
and strict-mode corrections.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 934a8a5e-4342-4704-b4bb-6bcb3e3bc725
📒 Files selected for processing (3)
corpus.pymain.pytest/test_verifier.py
🚧 Files skipped from review as they are similar to previous changes (2)
- corpus.py
- test/test_verifier.py
|
Kindly review, I've fixed the cl issue |
2 participants
Closes #40
Description
Implements a post-generation citation verification layer for the Deen Bridge AI service. This prevents the model from serving fabricated or mismatched Quran/Hadith references by validating citations against a bundled Quran corpus and enforcing an honest verification policy.
Key Changes
data/quran_uthmani.json&corpus.py):corpus.pyas a centralized accessor exposingget_ayah_count,get_ayah, and a stubhas_hadith_corpusfor future RAG ([Enhancement] Retrieval-augmented generation over authenticated Quran and Hadith sources #24) integration.verifier.py):difflib.SequenceMatcherto compare generated quotes against corpus text.unverifiedstatus when no corpus is available).main.py):ISLAMIC_CONTEXTsystem prompt with citation accuracy directives.CITATION_VERIFYmodes (off,annotate,strict).strictmode when citation mismatches occur.ChatResponsemodel withcitations_verifiedboolean and per-citationverification_results.tests/test_verifier.py&.github/workflows/ci.yml):flake8 .), compile modules, and executepytest.How Has This Been Tested?
pytest(all passing, 0 external API calls).CITATION_VERIFY=annotatemode with sample chat responses containing valid and fabricated citations.CITATION_VERIFY=offpreserves legacy behavior verbatim.Related Issues
Summary by CodeRabbit
/chatnow acceptsmessage/chat_idand returnstext,citations_verified, andverification_results, with optional strict correction behavior.pytestunit tests.