Skip to content

feat: add opt-in verbose_logs to surface GEval judge criteria, steps,… - #301

Open
x86girl wants to merge 4 commits into
lightspeed-core:mainfrom
x86girl:prgutier/g-eval/rubrics
Open

feat: add opt-in verbose_logs to surface GEval judge criteria, steps,…#301
x86girl wants to merge 4 commits into
lightspeed-core:mainfrom
x86girl:prgutier/g-eval/rubrics

Conversation

@x86girl

@x86girl x86girl commented Jul 29, 2026

Copy link
Copy Markdown

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

  • New Features
    • Added optional verbose mode for GEval metrics to capture detailed evaluation logs.
    • Exposed verbose logs in evaluation results, including JSON/CSV exports.
    • Persisted verbose logs in SQL-stored evaluation results (with automatic schema migration for the new field).
    • Works for both turn-level and conversation-level evaluations.
  • Bug Fixes
    • Verbose log capture now resets correctly between successive evaluations and stays None when verbose mode is disabled.

… 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>
@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

GEval 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.

Changes

GEval verbose log propagation

Layer / File(s) Summary
Verbose logging contracts
src/lightspeed_evaluation/core/models/llm.py, src/lightspeed_evaluation/core/models/data.py, src/lightspeed_evaluation/core/constants.py, config/system.yaml, tests/unit/core/models/*
GEval metadata accepts a verbose flag, result models expose optional verbose_logs, CSV columns include verbose_logs, and configuration and model tests cover the new fields.
GEval log capture
src/lightspeed_evaluation/core/metrics/geval.py, src/lightspeed_evaluation/core/metrics/deepeval.py, tests/unit/core/metrics/test_geval.py
GEval handlers propagate the verbose setting, reset previous logs, capture turn- and conversation-level logs, and expose them through DeepEvalMetrics.
Result propagation and persistence
src/lightspeed_evaluation/pipeline/evaluation/judges.py, src/lightspeed_evaluation/core/output/serializers.py, src/lightspeed_evaluation/core/storage/sql_storage.py, tests/unit/pipeline/evaluation/test_evaluator.py, tests/unit/core/output/test_final_coverage.py, tests/unit/core/storage/test_sql_storage.py
Judge scores and metric results collect verbose logs, JSON output always includes the field, SQL storage maps it to a nullable text column with missing-column migration, and evaluator, serializer, and schema tests validate the flow.

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
Loading

Suggested reviewers: bsatapat-jpg

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: optional verbose_logs support for GEval output.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 590f807 and 84eff7c.

📒 Files selected for processing (13)
  • config/system.yaml
  • src/lightspeed_evaluation/core/constants.py
  • src/lightspeed_evaluation/core/metrics/deepeval.py
  • src/lightspeed_evaluation/core/metrics/geval.py
  • src/lightspeed_evaluation/core/models/data.py
  • src/lightspeed_evaluation/core/models/llm.py
  • src/lightspeed_evaluation/core/output/serializers.py
  • src/lightspeed_evaluation/pipeline/evaluation/evaluator.py
  • tests/unit/core/metrics/test_geval.py
  • tests/unit/core/models/test_data.py
  • tests/unit/core/models/test_system.py
  • tests/unit/core/output/test_final_coverage.py
  • tests/unit/pipeline/evaluation/test_evaluator.py

Comment thread src/lightspeed_evaluation/pipeline/evaluation/evaluator.py Outdated

@asamal4 asamal4 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/lightspeed_evaluation/core/metrics/geval.py
Comment thread src/lightspeed_evaluation/pipeline/evaluation/evaluator.py Outdated
Comment thread src/lightspeed_evaluation/core/models/data.py
"agent_latency": r.agent_latency,
"tokens_per_second": r.tokens_per_second,
}
if r.verbose_logs is not None:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Every other optional field in this dict (judge_scores, time_to_first_token, etc.) is always present, set to None when absent.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's a DeepEval-wide feature, this code limit it to geval only, standard deepeval metric wont capture the verbose log.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread config/system.yaml

@xmican10 xmican10 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

x86girl and others added 2 commits July 30, 2026 12:20
…, 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>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 84eff7c and b56812d.

📒 Files selected for processing (6)
  • src/lightspeed_evaluation/core/models/data.py
  • src/lightspeed_evaluation/core/output/serializers.py
  • src/lightspeed_evaluation/core/storage/sql_storage.py
  • src/lightspeed_evaluation/pipeline/evaluation/judges.py
  • tests/unit/core/output/test_final_coverage.py
  • tests/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

Comment thread src/lightspeed_evaluation/core/storage/sql_storage.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>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between b56812d and d146d2c.

📒 Files selected for processing (1)
  • src/lightspeed_evaluation/core/storage/sql_storage.py

Comment on lines +173 to +193
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)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ 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.

@asamal4

asamal4 commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

@x86girl is this PR ready ? There is a open coderabbit comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants