feat: add opt-in verbose_logs to surface GEval judge criteria, steps,… - #301
feat: add opt-in verbose_logs to surface GEval judge criteria, steps,…#301x86girl wants to merge 4 commits into
Conversation
… and rubrics GEval metrics ask the judge LLM to generate evaluation steps and rubrics internally, but these were never exposed in the output — making it hard to understand how a score was derived. This adds a `verbose: true` flag to any GEval metric config that captures the judge's criteria, evaluation steps, and rubric from DeepEval's verbose_logs and propagates them through the full pipeline to the JSON/CSV output. Changes: - GEvalConfig: new `verbose` bool field (default False), parsed from metadata - GEvalHandler: capture `metric.verbose_logs` at turn and conversation level - DeepEvalMetrics: propagate last_verbose_logs from the GEval handler - MetricResult: new `verbose_logs` optional field, inherited by EvaluationResult - MetricsEvaluator: read handler verbose_logs after evaluation, attach to result - Serializer: include verbose_logs in JSON output only when non-None - constants: add verbose_logs to SUPPORTED_CSV_COLUMNS - system.yaml: document the verbose option in metrics_metadata example Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
WalkthroughGEval gains opt-in verbose logging from metadata configuration through handler capture, judge and evaluation result propagation, JSON serialization, CSV support, and SQL persistence. Tests cover configuration defaults, turn and conversation capture, reset behavior, propagation, serialization, and schema coverage. ChangesGEval verbose log propagation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant GEvalConfig
participant GEvalHandler
participant DeepEvalMetrics
participant JudgeOrchestrator
participant result_to_json_dict
participant EvaluationResultDB
GEvalConfig->>GEvalHandler: provide verbose configuration
GEvalHandler->>GEvalHandler: capture metric.verbose_logs
DeepEvalMetrics->>GEvalHandler: read last_verbose_logs
JudgeOrchestrator->>DeepEvalMetrics: evaluate judge
JudgeOrchestrator->>JudgeOrchestrator: aggregate verbose_logs
result_to_json_dict->>result_to_json_dict: include verbose_logs
EvaluationResultDB->>EvaluationResultDB: store verbose_logs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/lightspeed_evaluation/pipeline/evaluation/evaluator.py`:
- Around line 230-236: Update the GEval/DeepEval verbose-log propagation in the
evaluator to use logs from the handlers created by _create_handler_for_judge
rather than self.handlers[framework]. Carry the selected or aggregated panel
logs through JudgeOrchestrator and assign them to MetricResult for the
corresponding panel run, avoiding stale primary-handler state. Add a test
covering verbose-log propagation from a panel-created handler.
🪄 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: 51c7c3aa-fd25-42c7-8778-aa22a8d6ec65
📒 Files selected for processing (13)
config/system.yamlsrc/lightspeed_evaluation/core/constants.pysrc/lightspeed_evaluation/core/metrics/deepeval.pysrc/lightspeed_evaluation/core/metrics/geval.pysrc/lightspeed_evaluation/core/models/data.pysrc/lightspeed_evaluation/core/models/llm.pysrc/lightspeed_evaluation/core/output/serializers.pysrc/lightspeed_evaluation/pipeline/evaluation/evaluator.pytests/unit/core/metrics/test_geval.pytests/unit/core/models/test_data.pytests/unit/core/models/test_system.pytests/unit/core/output/test_final_coverage.pytests/unit/pipeline/evaluation/test_evaluator.py
asamal4
left a comment
There was a problem hiding this comment.
Thanks !!
Please address coderabbit's comment. and PTAL my comments..
Thread safety — shared mutable state between write and read
Judge panel path — verbose logs not captured
Only GEval — but verbose logs are available on all DeepEval metrics
Only file backends — missing from SQL, MLflow, Langfuse
Inconsistent JSON schema — conditionally omitted unlike other optional fields
| "agent_latency": r.agent_latency, | ||
| "tokens_per_second": r.tokens_per_second, | ||
| } | ||
| if r.verbose_logs is not None: |
There was a problem hiding this comment.
Every other optional field in this dict (judge_scores, time_to_first_token, etc.) is always present, set to None when absent.
There was a problem hiding this comment.
Fixed. verbose_logs is now always included in the JSON dict as null when not set, consistent with how judge_scores, time_to_first_token and other optional fields are handled.
| turn_data=scope.turn_data, | ||
| is_conversation=scope.is_conversation, | ||
| ) | ||
| self.last_verbose_logs = self.geval_handler.last_verbose_logs |
There was a problem hiding this comment.
it's a DeepEval-wide feature, this code limit it to geval only, standard deepeval metric wont capture the verbose log.
There was a problem hiding this comment.
Acknowledged. The current implementation captures verbose logs from GEval metrics only. Extending to all DeepEval metrics is a valid scope expansion that would be better addressed in a follow up PR to keep this change focused.
xmican10
left a comment
There was a problem hiding this comment.
Thanks! I noticed that the PR description mentions CSV export support but I don't see a test covering that path. Is it tested somewhere I missed?
…, DB column, JSON schema - Move verbose_logs capture from evaluator (shared handler state) into JudgeOrchestrator._evaluate_single_judge (per-judge handler, no race) - Propagate verbose_logs through JudgeScore → MetricResult (panel path works) - Add verbose_logs column to EvaluationResultDB (SQL storage) - Always include verbose_logs in JSON output (consistent with other optional fields) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The DB schema test was missing the verbose_logs column added in the previous commit, causing CI to fail on all Python versions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/lightspeed_evaluation/core/storage/sql_storage.py`:
- Line 78: Update initialize() to apply an additive migration that adds the
nullable verbose_logs column to existing result databases before validating the
table schema. Preserve strict validation after migration and ensure the
migration is safe for databases where the column already exists; add coverage
for upgrading a pre-existing database without raising StorageError.
🪄 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: 18cb156d-ee34-40b3-b666-231bee5684d6
📒 Files selected for processing (6)
src/lightspeed_evaluation/core/models/data.pysrc/lightspeed_evaluation/core/output/serializers.pysrc/lightspeed_evaluation/core/storage/sql_storage.pysrc/lightspeed_evaluation/pipeline/evaluation/judges.pytests/unit/core/output/test_final_coverage.pytests/unit/core/storage/test_sql_storage.py
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lightspeed_evaluation/core/models/data.py
Existing databases created before this PR are missing the verbose_logs column and would fail initialize() schema validation. Add a migration step that runs ALTER TABLE ADD COLUMN for any missing nullable columns before validation, and refresh the inspector cache afterward. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/lightspeed_evaluation/core/storage/sql_storage.py`:
- Around line 173-193: Update the schema initialization flow around
`_migrate_missing_columns` and `_validate_evaluation_results_schema` to
preflight missing non-nullable columns before applying any ALTER TABLE
statements. Reject non-result or structurally incompatible `evaluation_results`
tables first, and only run `_migrate_missing_columns` when all required ORM
columns are present; preserve nullable-column migration for compatible legacy
tables.
🪄 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: 9315fb3c-ce26-470b-a913-f4709e5134c5
📒 Files selected for processing (1)
src/lightspeed_evaluation/core/storage/sql_storage.py
| def _migrate_missing_columns(self, db_inspector: Any, table_name: str) -> None: | ||
| """Add missing nullable columns to an existing table. | ||
|
|
||
| Databases created before new columns were added would fail strict | ||
| validation. This method applies safe additive migrations (nullable | ||
| columns only) so that older databases keep working. | ||
| """ | ||
| reflected = {col["name"] for col in db_inspector.get_columns(table_name)} | ||
| orm_columns = EvaluationResultDB.__table__.columns | ||
| for col in orm_columns: | ||
| if col.name not in reflected and col.nullable: | ||
| col_type = col.type.compile(dialect=self._engine.dialect) | ||
| with self._engine.connect() as conn: | ||
| conn.execute( | ||
| text( | ||
| f"ALTER TABLE {table_name} " | ||
| f"ADD COLUMN {col.name} {col_type}" | ||
| ) | ||
| ) | ||
| conn.commit() | ||
| logger.info("Migrated: added column '%s' to %s", col.name, table_name) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Validate incompatible schemas before mutating them.
This adds nullable columns before _validate_evaluation_results_schema() detects missing required columns. A non-result or structurally incompatible evaluation_results table is therefore modified and then rejected. Preflight missing non-nullable ORM columns before this migration loop; only migrate a table that is otherwise compatible.
🤖 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 `@src/lightspeed_evaluation/core/storage/sql_storage.py` around lines 173 -
193, Update the schema initialization flow around `_migrate_missing_columns` and
`_validate_evaluation_results_schema` to preflight missing non-nullable columns
before applying any ALTER TABLE statements. Reject non-result or structurally
incompatible `evaluation_results` tables first, and only run
`_migrate_missing_columns` when all required ORM columns are present; preserve
nullable-column migration for compatible legacy tables.
|
@x86girl is this PR ready ? There is a open coderabbit comment. |
Currently, lightspeed-evaluation does not report when creating rubrics using a Judge (LLM).
For correctness or consistency, for example, it asks the judge to define the steps to get this rubric, and lightspeed-evaluation is
not reporting anywhere how it has been done.
This PR create a new opt-in verbose_logs arg to surface GEval judge criteria, steps, and rubrics.
Summary by CodeRabbit
Nonewhen verbose mode is disabled.