[LEADS-230] fix: missing metric_metadata value in csv

[asamal4]

Description

We are supposed to store metric_metadata for each metric in CSV file. This column should contain all properties for a metric that will be used for evaluation (except metric identifier and threshold).
Currently the field doesn't contain any value, because of incorrect property name.

Type of change

• Refactor
• New feature
• Bug fix
• CVE fix
• Optimization
• Documentation Update
• Configuration Update
• Bump-up service version
• Bump-up dependent library
• Bump-up library or tool used for development (does not change the final image)
• CI configuration change
• Unit tests improvement

Tools used to create PR

Identify any AI code assistants used in this PR (for transparency and review context)

• Assisted-by: (e.g., Claude, CodeRabbit, Ollama, etc., N/A if not used)
• Generated by: (e.g., tool name and version; N/A if not used)

Related Tickets & Documents

• Related Issue # LEADS-230
• Closes # LEADS-230

Checklist before requesting a review

• I have performed a self-review of my code.
• PR has passed all pre-merge test jobs.
• If it is a core feature, I have added thorough tests.

Testing

• Please provide detailed steps to perform tests related to this code change.
• How were the fix/results from this change verified? Please provide relevant screenshots or results.

Summary by CodeRabbit

• CSV Output Format Changes
  • Reordered evaluation metrics and metadata columns in CSV exports
  • Removed execution_time, reason, and query columns from CSV output
  • Refined metadata field handling in evaluation results
  • Updated CSV generation to include improved metadata reporting in output files

[coderabbitai]

Walkthrough

The PR renames the metrics_metadata field to metric_metadata in the EvaluationResult data model and reorders CSV output columns to place metric_metadata immediately after metric_identifier. All related references across configuration, constants, evaluator logic, and tests are updated accordingly.

Changes

Cohort / File(s)	Summary
CSV Configuration and Constants config/system.yaml, src/lightspeed_evaluation/core/constants.py	Reordered CSV columns to move metric_metadata from after threshold to after metric_identifier; removed reason, execution_time, and query columns from system.yaml CSV output configuration.
Data Model src/lightspeed_evaluation/core/models/data.py	Renamed metrics_metadata: Optional[str] to metric_metadata: Optional[str] in the EvaluationResult class.
Evaluation Logic src/lightspeed_evaluation/pipeline/evaluation/evaluator.py	Updated EvaluationResult construction to use renamed metric_metadata field; added new helper method _extract_metadata_for_csv(request) to extract filtered metadata payload for CSV output.
Tests tests/unit/core/output/test_generator.py	Updated test data and assertions to use metric_metadata field; adjusted test identifiers and validation checks to align with renamed field and output configuration changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• [LEADS-182] - Add eval config data to the report #142: Modifies the same CSV metadata feature by adding and extracting metric-level metadata alongside updating CSV columns and evaluator/model fields.
• add additional fields to output for non-error scenarios #114: Modifies CSV column ordering and EvaluationResult data model (SUPPORTED_CSV_COLUMNS and models/data.py) in a related capacity.

Suggested reviewers

• VladimirKadlec

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: fixing the missing metric_metadata value in CSV output, which aligns with the core objective of this bug fix pull request.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Merge Conflict Detection	✅ Passed	✅ No merge conflicts detected when merging into main

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/lightspeed_evaluation/pipeline/evaluation/evaluator.py (1)

488-512: ⚠️ Potential issue | 🟡 Minor

Minor robustness concern: _extract_metadata_for_csv could raise inside error-handling path.

_create_error_result is called from except blocks (e.g., Line 178). If self._extract_metadata_for_csv(request) fails (e.g., get_metric_metadata raises), the original error is swallowed and replaced with an unrelated exception. Consider wrapping the call defensively:

Proposed fix

     def _create_error_result(
         self, request: EvaluationRequest, reason: str, execution_time: float
     ) -> EvaluationResult:
         """Create an ERROR result for failed evaluation."""
         turn_data = request.turn_data
+        try:
+            metadata = self._extract_metadata_for_csv(request)
+        except Exception:  # noqa: BLE001
+            metadata = None
         return EvaluationResult(
             conversation_group_id=request.conv_data.conversation_group_id,
             tag=request.conv_data.tag,
             turn_id=request.turn_id,
             metric_identifier=request.metric_identifier,
-            metric_metadata=self._extract_metadata_for_csv(request),
+            metric_metadata=metadata,
             result="ERROR",

🧹 Nitpick comments (1)

tests/unit/core/output/test_generator.py (1)

362-386: Consider asserting metric_metadata for FAIL and ERROR rows too.

The FAIL and ERROR results don't set metric_metadata, so they'll serialize as empty strings in the CSV. Adding assertions for those rows would guard against regressions (e.g., accidental default values).

assert rows[1]["metric_metadata"] == ""
assert rows[2]["metric_metadata"] == ""