feat(docs): Improve knowledgebase adherence#1059
Conversation
github-actions
Bot
changed the title
📝 WalkthroughWalkthroughThis PR adds a knowledge-base groundedness metric to v2 evaluations. File-search results, including filenames, flow into per-row judging and trace comments. Metric applicability, N/A handling, top-match formatting, persistence, tests, and evaluation documentation are updated. ChangesKnowledge-base groundedness evaluation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ResponsesAPI
participant FastEvaluation
participant JudgeRow
participant TraceStorage
ResponsesAPI->>FastEvaluation: return file_search_call.results
FastEvaluation->>JudgeRow: pass generated_answer and retrieved_chunks
JudgeRow->>FastEvaluation: return applicable metric scores and reasoning
FastEvaluation->>TraceStorage: persist scores and formatted top matches
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
AkhileshNegi
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
backend/app/crud/evaluations/fast.py (1)
1058-1071: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftDon't block KB judging on missing
ground_truth(backend/app/crud/evaluations/fast.py:944-1071).
judgeablefilters out rows that lackground_truth, and the earlierUNSCOREABLE_EMPTY_GROUND_TRUTHbranch marks them unscoreable before_judge_rowsruns. That preventsknowledge_basefrom scoring rows that have a generated answer and retrieved chunks, even though itsrequired_inputsdon’t includeground_truth. Let per-metric applicability insidejudge_rowdecide; only the ground-truth metric should require the reference answer.🤖 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 `@backend/app/crud/evaluations/fast.py` around lines 1058 - 1071, The evaluation flow currently excludes judgeable rows without ground_truth before _judge_rows runs. Update the filtering and earlier UNSCOREABLE_EMPTY_GROUND_TRUTH handling in the surrounding evaluation logic so rows with generated_output and retrieved knowledge-base chunks proceed to _judge_rows; leave applicability decisions to judge_row, where only ground-truth-dependent metrics require the reference answer.
🧹 Nitpick comments (3)
backend/app/tests/crud/evaluations/test_fast_judge.py (2)
817-819: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the duplicated assertion.
Line 819 repeats the
assert "include" not in base_paramscheck from Line 817. Harmless, but it reads like a copy-paste leftover.♻️ Drop the redundant line
assert "include" not in base_params assert "tool_choice" not in base_params - assert "include" not in base_params🤖 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 `@backend/app/tests/crud/evaluations/test_fast_judge.py` around lines 817 - 819, Remove the duplicated `assert "include" not in base_params` assertion in the test’s base-parameter assertions, keeping the single existing check and the `tool_choice` assertion unchanged.
482-488: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd missing return type annotations.
_openai_responsehas no return annotation, and the test methods inTestResponsesChunkCapture,TestKnowledgeBaseScoring, andTestFileSearchIncludeParamlack-> None(unlikeTestFormatTopKbMatchesin the same file). As per coding guidelines: "Use type hints on every function parameter and return value."♻️ Annotate the helper return type
-def _openai_response(): +def _openai_response() -> SimpleNamespace: return SimpleNamespace(🤖 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 `@backend/app/tests/crud/evaluations/test_fast_judge.py` around lines 482 - 488, Add return type annotations to the _openai_response helper and every unannotated test method in TestResponsesChunkCapture, TestKnowledgeBaseScoring, and TestFileSearchIncludeParam, using -> None for test methods and the appropriate response type for _openai_response.Source: Coding guidelines
backend/app/crud/evaluations/fast.py (1)
1160-1171: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the KB "N/A" reason strings into named constants.
"Knowledge base not queried."and"Knowledge base score unavailable for this row."are inline literals, while this same file already extracts sibling reason strings (UNSCOREABLE_EMPTY_OUTPUT,UNSCOREABLE_EMPTY_GROUND_TRUTH,JUDGE_FAILED_REASON) into named constants. As per coding guidelines: "Do not use magic values in code; extract repeated literals into constants,Enummembers, or settings."♻️ Proposed fix
+KB_NOT_QUERIED_REASON = "Knowledge base not queried." +KB_SCORE_UNAVAILABLE_REASON = "Knowledge base score unavailable for this row." + ... if not sorted_chunks: - reason = "Knowledge base not queried." + reason = KB_NOT_QUERIED_REASON else: - reason = "Knowledge base score unavailable for this row." + reason = KB_SCORE_UNAVAILABLE_REASON🤖 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 `@backend/app/crud/evaluations/fast.py` around lines 1160 - 1171, Extract the two KB reason strings used in the is_kb branch into named module-level constants, following the existing UNSCOREABLE_EMPTY_OUTPUT, UNSCOREABLE_EMPTY_GROUND_TRUTH, and JUDGE_FAILED_REASON pattern. Update the empty sorted_chunks and missing-score branches to reference those constants without changing their behavior.Source: Coding guidelines
🤖 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 `@backend/app/crud/evaluations/fast.py`:
- Around line 1058-1071: The evaluation flow currently excludes judgeable rows
without ground_truth before _judge_rows runs. Update the filtering and earlier
UNSCOREABLE_EMPTY_GROUND_TRUTH handling in the surrounding evaluation logic so
rows with generated_output and retrieved knowledge-base chunks proceed to
_judge_rows; leave applicability decisions to judge_row, where only
ground-truth-dependent metrics require the reference answer.
---
Nitpick comments:
In `@backend/app/crud/evaluations/fast.py`:
- Around line 1160-1171: Extract the two KB reason strings used in the is_kb
branch into named module-level constants, following the existing
UNSCOREABLE_EMPTY_OUTPUT, UNSCOREABLE_EMPTY_GROUND_TRUTH, and
JUDGE_FAILED_REASON pattern. Update the empty sorted_chunks and missing-score
branches to reference those constants without changing their behavior.
In `@backend/app/tests/crud/evaluations/test_fast_judge.py`:
- Around line 817-819: Remove the duplicated `assert "include" not in
base_params` assertion in the test’s base-parameter assertions, keeping the
single existing check and the `tool_choice` assertion unchanged.
- Around line 482-488: Add return type annotations to the _openai_response
helper and every unannotated test method in TestResponsesChunkCapture,
TestKnowledgeBaseScoring, and TestFileSearchIncludeParam, using -> None for test
methods and the appropriate response type for _openai_response.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5288d079-38ff-43c4-9549-dc26d56f6d65
📒 Files selected for processing (9)
backend/app/crud/evaluations/fast.pybackend/app/crud/evaluations/judge.pybackend/app/crud/evaluations/score.pybackend/app/models/response.pybackend/app/services/response/response.pybackend/app/tests/crud/evaluations/test_fast_judge.pybackend/app/tests/crud/evaluations/test_judge.pybackend/app/tests/services/response/response/test_generate_response.pydocs/wiki/modules/evaluations.md
1 participant
Issue
Closes #1045
Summary
A new LLM-judged evaluation metric — "Adherence to Knowledge Base" — that measures whether an AI answer is actually grounded in the knowledge-base chunks that were retrieved to produce it. In short: hallucination detection.
Checklist
Before submitting a pull request, please ensure that you mark these task.
fastapi run --reload app/main.pyordocker compose upin the repository root and test.Summary by CodeRabbit
New Features
Documentation