Skip to content

chore: replace pending print statements with logger#266

Merged
asamal4 merged 1 commit into
lightspeed-core:mainfrom
asamal4:print-to-logging
Jun 27, 2026
Merged

chore: replace pending print statements with logger#266
asamal4 merged 1 commit into
lightspeed-core:mainfrom
asamal4:print-to-logging

Conversation

@asamal4

@asamal4 asamal4 commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

Description

(Release Readiness) chore: replace pending print statement with logger

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

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

  • Bug Fixes
    • Replaced console output with application logging for startup and initialization messages.
    • Improved evaluation data saving feedback by logging success and failure states instead of printing to stdout.
    • Confirmed the updated logging behavior with test coverage changes.

@coderabbitai

coderabbitai Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Startup and persistence messages were switched from print to module logging. The DeepEval initialization test now checks captured logs instead of stdout.

Changes

Logging output updates

Layer / File(s) Summary
Initialization messages
src/lightspeed_evaluation/core/llm/deepeval.py, src/lightspeed_evaluation/core/metrics/custom/custom.py, tests/unit/core/llm/test_deepeval_manager.py
DeepEval and custom metrics initialization now log their startup messages, and the DeepEval test captures the logger output.
Persistence logging
src/lightspeed_evaluation/core/output/data_persistence.py
Evaluation data persistence now uses module logging for success and failure reporting while keeping the same return behavior.

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 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 refactor of replacing print statements with logging across the PR.
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.

🧹 Nitpick comments (1)
src/lightspeed_evaluation/core/output/data_persistence.py (1)

71-72: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Log the traceback on save failures.

logger.error(..., e) loses the stack trace, and this function immediately returns None, so the root cause is hard to recover later. Prefer logger.exception(...) (or exc_info=True) here.

Suggested change
-    except (OSError, yaml.YAMLError) as e:
-        logger.error("Failed to save amended evaluation data: %s", e)
+    except (OSError, yaml.YAMLError):
+        logger.exception("Failed to save amended evaluation data")
         return None
🤖 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/output/data_persistence.py` around lines 71 -
72, The save failure logging in the exception handler for the amended evaluation
data should preserve the traceback instead of only formatting the exception
object. Update the exception path in the data persistence flow to use
logger.exception or logger.error with exc_info=True so the stack trace is
emitted when the write/save operation fails, keeping the existing failure
message and the surrounding try/except behavior intact.
🤖 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.

Nitpick comments:
In `@src/lightspeed_evaluation/core/output/data_persistence.py`:
- Around line 71-72: The save failure logging in the exception handler for the
amended evaluation data should preserve the traceback instead of only formatting
the exception object. Update the exception path in the data persistence flow to
use logger.exception or logger.error with exc_info=True so the stack trace is
emitted when the write/save operation fails, keeping the existing failure
message and the surrounding try/except behavior intact.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a4e39b1b-e7a0-4abe-89e7-cc7c80a6b028

📥 Commits

Reviewing files that changed from the base of the PR and between 6ff47a6 and 16ca2d8.

📒 Files selected for processing (4)
  • src/lightspeed_evaluation/core/llm/deepeval.py
  • src/lightspeed_evaluation/core/metrics/custom/custom.py
  • src/lightspeed_evaluation/core/output/data_persistence.py
  • tests/unit/core/llm/test_deepeval_manager.py

@asamal4 asamal4 merged commit 1595389 into lightspeed-core:main Jun 27, 2026
17 checks passed
@asamal4 asamal4 changed the title chore: replace pending print statement with logger chore: replace pending print statements with logger Jun 27, 2026
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.

1 participant