Skip to content

[LEADS-225] Align lint checks with LSC#245

Merged
asamal4 merged 1 commit into
lightspeed-core:mainfrom
xmican10:LEADS-225-align-lint-check-rules-with-LSC
Jun 3, 2026
Merged

[LEADS-225] Align lint checks with LSC#245
asamal4 merged 1 commit into
lightspeed-core:mainfrom
xmican10:LEADS-225-align-lint-check-rules-with-LSC

Conversation

@xmican10
Copy link
Copy Markdown
Collaborator

@xmican10 xmican10 commented May 26, 2026

Description

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 #
  • Closes #

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

    • Improved timestamp handling with timezone-aware UTC timestamps across evaluation outputs and reports
    • Enhanced API request handling with conditional conversation ID inclusion
  • Improvements

    • Strengthened code quality checks with stricter type checking rules and shell script validation
    • Added optional caching controls for proposal agent configurations
    • Updated Python syntax modernization for improved type annotations
  • Dependencies

    • Updated critical dependencies (protobuf, langsmith, grpcio, sentry-sdk, typer, and others)
    • Added websockets dependency for enhanced functionality

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 26, 2026

Review Change Stack

Warning

Review limit reached

@xmican10, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 33 minutes and 51 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: af078094-35f5-46a0-84c2-287956e4f7ce

📥 Commits

Reviewing files that changed from the base of the PR and between 27127e3 and 3021c7d.

⛔ Files ignored due to path filters (2)
  • uv-gpu.lock is excluded by !**/*.lock
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (79)
  • Makefile
  • lsc_agent_eval/src/lsc_agent_eval/core/agent_goal_eval/evaluator.py
  • lsc_agent_eval/src/lsc_agent_eval/core/agent_goal_eval/results.py
  • pyproject.toml
  • requirements-all-extras.txt
  • requirements-local-embeddings.txt
  • requirements-nlp-metrics.txt
  • requirements.txt
  • script/compare_evaluations.py
  • script/run_multi_provider_eval.py
  • src/generate_answers/generate_answers.py
  • src/lightspeed_evaluation/core/api/client.py
  • src/lightspeed_evaluation/core/llm/__init__.py
  • src/lightspeed_evaluation/core/llm/custom.py
  • src/lightspeed_evaluation/core/llm/litellm_patch.py
  • src/lightspeed_evaluation/core/metrics/custom/custom.py
  • src/lightspeed_evaluation/core/metrics/custom/tool_eval.py
  • src/lightspeed_evaluation/core/metrics/geval.py
  • src/lightspeed_evaluation/core/metrics/script.py
  • src/lightspeed_evaluation/core/models/__init__.py
  • src/lightspeed_evaluation/core/models/agents.py
  • src/lightspeed_evaluation/core/models/data.py
  • src/lightspeed_evaluation/core/models/quality.py
  • src/lightspeed_evaluation/core/models/statistics.py
  • src/lightspeed_evaluation/core/models/summary.py
  • src/lightspeed_evaluation/core/output/data_persistence.py
  • src/lightspeed_evaluation/core/output/generator.py
  • src/lightspeed_evaluation/core/output/serializers.py
  • src/lightspeed_evaluation/core/output/statistics.py
  • src/lightspeed_evaluation/core/output/visualization.py
  • src/lightspeed_evaluation/core/script/manager.py
  • src/lightspeed_evaluation/core/storage/__init__.py
  • src/lightspeed_evaluation/core/storage/config.py
  • src/lightspeed_evaluation/core/storage/protocol.py
  • src/lightspeed_evaluation/core/storage/sql_storage.py
  • src/lightspeed_evaluation/core/system/ssl_certifi.py
  • src/lightspeed_evaluation/core/system/validator.py
  • src/lightspeed_evaluation/pipeline/evaluation/__init__.py
  • src/lightspeed_evaluation/pipeline/evaluation/driver.py
  • src/lightspeed_evaluation/pipeline/evaluation/judges.py
  • src/lightspeed_evaluation/pipeline/evaluation/pipeline.py
  • src/lightspeed_evaluation/runner/evaluation.py
  • tests/script/test_compare_evaluations.py
  • tests/script/test_run_multi_provider_eval.py
  • tests/unit/core/api/conftest.py
  • tests/unit/core/api/test_client.py
  • tests/unit/core/api/test_client_infer.py
  • tests/unit/core/api/test_streaming_parser.py
  • tests/unit/core/config/test_models.py
  • tests/unit/core/llm/conftest.py
  • tests/unit/core/llm/test_custom.py
  • tests/unit/core/llm/test_llm_manager.py
  • tests/unit/core/llm/test_token_tracker.py
  • tests/unit/core/metrics/conftest.py
  • tests/unit/core/metrics/custom/test_custom.py
  • tests/unit/core/metrics/custom/test_tool_eval.py
  • tests/unit/core/metrics/test_geval.py
  • tests/unit/core/models/test_api_additional.py
  • tests/unit/core/models/test_quality.py
  • tests/unit/core/models/test_summary.py
  • tests/unit/core/models/test_system.py
  • tests/unit/core/output/conftest.py
  • tests/unit/core/output/test_final_coverage.py
  • tests/unit/core/output/test_generator.py
  • tests/unit/core/output/test_statistics.py
  • tests/unit/core/output/test_statistics_api.py
  • tests/unit/core/script/test_manager.py
  • tests/unit/core/script/test_manager_additional.py
  • tests/unit/core/storage/test_composite_and_factory.py
  • tests/unit/core/storage/test_protocol.py
  • tests/unit/core/storage/test_sql_storage.py
  • tests/unit/core/system/test_loader.py
  • tests/unit/core/system/test_setup.py
  • tests/unit/core/system/test_ssl_certifi.py
  • tests/unit/core/system/test_validator.py
  • tests/unit/pipeline/evaluation/conftest.py
  • tests/unit/pipeline/evaluation/test_evaluator.py
  • tests/unit/pipeline/evaluation/test_processor.py
  • tests/unit/runner/test_evaluation.py

Walkthrough

This PR modernizes the lightspeed-evaluation codebase across multiple dimensions: strengthening build/linting configuration with stricter type checking defaults, standardizing timestamp handling to use UTC throughout, migrating type annotations to Python 3.10+ union syntax, improving core evaluation logic for result typing and LiteLLM integration, updating pinned dependencies, and reorganizing imports for consistency.

Changes

Code Modernization & Quality Improvements

Layer / File(s) Summary
Build and linting configuration updates
Makefile, pyproject.toml
Makefile targets refined: check-types runs mypy with reduced flags, black-check moves --check before paths, pylint removes --disable=R0801 override from lsc_agent_eval check, bandit loads config from pyproject.toml. pyproject.toml strengthened with stricter mypy rules (explicit_package_bases, disallow_untyped_calls/defs/incomplete_defs), pylint source-roots and MESSAGES CONTROL section disabling R0801, pyright extraPaths, ruff line-length and expanded lint rules, and types-PyYAML dependency.
Type annotation modernization to PEP 604 syntax
src/lightspeed_evaluation/core/*, src/lightspeed_evaluation/pipeline/*, script/*, tests/unit/*
Systematically updates type annotations to use Python 3.10+ union syntax (X | Y instead of Union[X, Y]) across APIClient, LLM modules, metrics, models (agents, data, storage), script manager, and storage config. Imports Callable from collections.abc in custom LLM, tool_eval, judges modules and test utilities. Removes unused Union/Tuple imports. Adds ValidationError import in geval module for configuration error handling.
UTC timezone-aware timestamp standardization
lsc_agent_eval/src/*, script/run_multi_provider_eval.py, src/lightspeed_evaluation/core/models/summary.py, src/lightspeed_evaluation/core/output/*, src/lightspeed_evaluation/core/storage/*, tests/unit/core/storage/test_protocol.py
Replaces all naive datetime.now() with datetime.now(UTC) across evaluation result filenames, output generation, script execution timing, storage backends, and test assertions. Imports UTC constant and updates all timestamp generation to ensure consistent UTC-aware ISO-8601 formatting in results, reports, and database records.
Core evaluation and integration logic improvements
lsc_agent_eval/src/lsc_agent_eval/core/agent_goal_eval/evaluator.py, src/lightspeed_evaluation/core/llm/__init__.py, src/lightspeed_evaluation/pipeline/evaluation/pipeline.py, tests/unit/core/script/test_manager_additional.py
Imports EvalResultItem for improved result typing; explicitly types api_input as dict[str, str] and conditionally includes conversation_id only when non-None. Ensures litellm_patch module imports before lazy-import wiring to guarantee patching occurs before usage. Replaces litellm cache.disconnect getattr access with direct attribute access, casts to Callable returning Coroutine, executes via asyncio.run, and adds TypeError to ignored exceptions. Adds logging assertions (caplog) for script output verification.
Dependency version updates
requirements.txt, requirements-local-embeddings.txt, requirements-nlp-metrics.txt, requirements-all-extras.txt
Bumps multiple pinned versions: grpcio/grpcio-status 1.80.0→1.81.0, idna 3.16→3.17, langchain-protocol 0.0.15→0.0.16, langsmith 0.8.5→0.8.8, protobuf 6.33.6→7.35.0, rpds-py 0.30.0→2026.5.1, sentry-sdk 2.60.0→2.61.1, typer 0.26.1→0.26.5. Adds websockets==16.0 transitive dependency (via langsmith) to all requirements files.
Import statement organization and cleanup
src/lightspeed_evaluation/core/models/__init__.py, src/lightspeed_evaluation/core/output/*, src/lightspeed_evaluation/core/storage/__init__.py, src/lightspeed_evaluation/pipeline/evaluation/*, src/lightspeed_evaluation/runner/evaluation.py, tests/unit/core/api/*, tests/unit/core/config/*, tests/unit/core/llm/*, tests/unit/core/metrics/*, tests/unit/core/models/*, tests/unit/core/output/*, tests/unit/core/script/*, tests/unit/core/storage/*, tests/unit/core/system/*, tests/unit/pipeline/evaluation/*, tests/unit/runner/test_evaluation.py, tests/script/*
Reorganizes module-level imports for consistency: groups LLM and statistics model imports separately, moves composite storage imports before config imports, consolidates related imports into multi-line blocks, adjusts blank-line separators between import groups, reorders specific import symbols, and adjusts test method formatting (whitespace/indentation). No changes to imported names or runtime behavior.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • asamal4
  • VladimirKadlec
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

@xmican10 xmican10 force-pushed the LEADS-225-align-lint-check-rules-with-LSC branch from c2bb02b to e084620 Compare May 26, 2026 09:44
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 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/output/generator.py`:
- Line 455: The timestamp written by f.write uses
datetime.now(UTC).strftime('%Y-%m-%d %H:%M:%S') which omits timezone info;
update the call that produces the string (the datetime.now(UTC).strftime usage
in generator.py) to include an explicit timezone marker (for example use a
format with %Z or use an ISO8601 representation) so the output clearly shows
"UTC" (or equivalent offset) alongside the date/time.

In `@src/lightspeed_evaluation/pipeline/evaluation/pipeline.py`:
- Around line 275-276: The inline "# type: ignore" must be removed and the
variable `disconnect` must be given an explicit type so static checkers know
it's an async callable; change the assignment to something like `disconnect:
Callable[[], Awaitable[None]] = cache.disconnect` (import Callable, Awaitable
from typing) and then call it normally with `asyncio.run(disconnect())`;
reference `cache.disconnect` and the `disconnect` variable used with
`asyncio.run`.

In `@tests/unit/core/llm/conftest.py`:
- Line 1: Remove the module-level pylint suppression and stop directly assigning
to protected members (e.g., avoid direct assignment to _hidden_params in the
fixtures); instead, delete the "# pylint: disable=protected-access" and modify
the fixtures that touch _hidden_params to use setattr(obj, "_hidden_params",
value) or another public API to set the value; update any occurrences referenced
in tests/unit/core/llm/conftest.py (and the similar locations noted around lines
62 and 98) so no protected-member access remains.
🪄 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

Run ID: cd31889a-863b-4c25-b600-0c5d6b26191d

📥 Commits

Reviewing files that changed from the base of the PR and between 53f91bc and e084620.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (73)
  • Makefile
  • lsc_agent_eval/src/lsc_agent_eval/core/agent_goal_eval/evaluator.py
  • lsc_agent_eval/src/lsc_agent_eval/core/agent_goal_eval/results.py
  • pyproject.toml
  • script/compare_evaluations.py
  • script/run_multi_provider_eval.py
  • src/generate_answers/generate_answers.py
  • src/lightspeed_evaluation/core/api/client.py
  • src/lightspeed_evaluation/core/llm/__init__.py
  • src/lightspeed_evaluation/core/llm/custom.py
  • src/lightspeed_evaluation/core/llm/litellm_patch.py
  • src/lightspeed_evaluation/core/metrics/custom/custom.py
  • src/lightspeed_evaluation/core/metrics/custom/tool_eval.py
  • src/lightspeed_evaluation/core/metrics/geval.py
  • src/lightspeed_evaluation/core/metrics/script.py
  • src/lightspeed_evaluation/core/models/__init__.py
  • src/lightspeed_evaluation/core/models/agents.py
  • src/lightspeed_evaluation/core/models/data.py
  • src/lightspeed_evaluation/core/models/quality.py
  • src/lightspeed_evaluation/core/models/statistics.py
  • src/lightspeed_evaluation/core/models/summary.py
  • src/lightspeed_evaluation/core/output/data_persistence.py
  • src/lightspeed_evaluation/core/output/generator.py
  • src/lightspeed_evaluation/core/output/statistics.py
  • src/lightspeed_evaluation/core/output/visualization.py
  • src/lightspeed_evaluation/core/script/manager.py
  • src/lightspeed_evaluation/core/storage/__init__.py
  • src/lightspeed_evaluation/core/storage/config.py
  • src/lightspeed_evaluation/core/storage/protocol.py
  • src/lightspeed_evaluation/core/storage/sql_storage.py
  • src/lightspeed_evaluation/core/system/ssl_certifi.py
  • src/lightspeed_evaluation/core/system/validator.py
  • src/lightspeed_evaluation/pipeline/evaluation/evaluator.py
  • src/lightspeed_evaluation/pipeline/evaluation/judges.py
  • src/lightspeed_evaluation/pipeline/evaluation/pipeline.py
  • src/lightspeed_evaluation/runner/evaluation.py
  • tests/script/test_compare_evaluations.py
  • tests/script/test_run_multi_provider_eval.py
  • tests/unit/core/api/conftest.py
  • tests/unit/core/api/test_client.py
  • tests/unit/core/api/test_client_infer.py
  • tests/unit/core/api/test_streaming_parser.py
  • tests/unit/core/config/test_models.py
  • tests/unit/core/llm/conftest.py
  • tests/unit/core/llm/test_custom.py
  • tests/unit/core/llm/test_llm_manager.py
  • tests/unit/core/llm/test_token_tracker.py
  • tests/unit/core/metrics/conftest.py
  • tests/unit/core/metrics/custom/test_custom.py
  • tests/unit/core/metrics/custom/test_tool_eval.py
  • tests/unit/core/metrics/test_geval.py
  • tests/unit/core/models/test_api_additional.py
  • tests/unit/core/models/test_quality.py
  • tests/unit/core/models/test_summary.py
  • tests/unit/core/models/test_system.py
  • tests/unit/core/output/conftest.py
  • tests/unit/core/output/test_final_coverage.py
  • tests/unit/core/output/test_generator.py
  • tests/unit/core/output/test_statistics.py
  • tests/unit/core/output/test_statistics_api.py
  • tests/unit/core/script/test_manager.py
  • tests/unit/core/script/test_manager_additional.py
  • tests/unit/core/storage/test_composite_and_factory.py
  • tests/unit/core/storage/test_protocol.py
  • tests/unit/core/storage/test_sql_storage.py
  • tests/unit/core/system/test_loader.py
  • tests/unit/core/system/test_setup.py
  • tests/unit/core/system/test_ssl_certifi.py
  • tests/unit/core/system/test_validator.py
  • tests/unit/pipeline/evaluation/conftest.py
  • tests/unit/pipeline/evaluation/test_evaluator.py
  • tests/unit/pipeline/evaluation/test_processor.py
  • tests/unit/runner/test_evaluation.py
💤 Files with no reviewable changes (2)
  • tests/unit/core/models/test_quality.py
  • src/lightspeed_evaluation/core/metrics/geval.py

Comment thread src/lightspeed_evaluation/core/output/generator.py Outdated
Comment thread src/lightspeed_evaluation/pipeline/evaluation/pipeline.py Outdated
Comment thread tests/unit/core/llm/conftest.py Outdated
@xmican10 xmican10 force-pushed the LEADS-225-align-lint-check-rules-with-LSC branch 3 times, most recently from 6dbbb3c to 8a375d2 Compare May 26, 2026 14:33
@xmican10 xmican10 marked this pull request as ready for review May 26, 2026 15:22
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 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 @.github/workflows/shellcheck.yaml:
- Line 14: In the shellcheck GitHub Actions workflow update the checkout step
that currently uses "actions/checkout@v4": pin it to a specific commit SHA
(replace the version tag with the exact commit SHA for actions/checkout) and add
"persist-credentials: false" to the checkout step configuration so the
GITHUB_TOKEN is not persisted to subsequent steps; modify the checkout step
where actions/checkout is referenced to include these two changes.

In `@src/lightspeed_evaluation/core/output/generator.py`:
- Line 1: Remove the inline "# pylint: disable=too-many-lines" at the top of
src/lightspeed_evaluation/core/output/generator.py and refactor the file to
address the file-length lint by splitting responsibilities into smaller modules
and classes: extract the main Generator/OutputGenerator class implementations
(e.g., Generator, OutputGenerator) into their own files, move large helper
functions (e.g., generate_output, render_template, evaluate_metrics,
format_results) into dedicated modules, and group related utility functions into
a new utils/helpers module so each file is under the lint threshold and the
top-level generator imports the smaller components.

In `@src/lightspeed_evaluation/pipeline/evaluation/pipeline.py`:
- Around line 276-280: The except block catching (AttributeError, RuntimeError,
OSError) around the asyncio.run(disconnect()) call should also guard against
TypeError coming from asyncio.run when a non-standard cache.disconnect is
provided; update the handler in the pipeline code where disconnect =
cast(Callable[[], Coroutine[Any, Any, object]], cache.disconnect) and
asyncio.run(disconnect()) is invoked (in the evaluation pipeline) to either add
TypeError to the caught exceptions or first verify the disconnect callable
returns an awaitable before calling asyncio.run, so unexpected third-party
litellm.cache implementations won't crash the shutdown path.
🪄 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

Run ID: f5b3d2b5-c498-411e-9dfa-7045faa69d07

📥 Commits

Reviewing files that changed from the base of the PR and between e084620 and 8a375d2.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (75)
  • .github/workflows/shellcheck.yaml
  • .gitignore
  • Makefile
  • lsc_agent_eval/src/lsc_agent_eval/core/agent_goal_eval/evaluator.py
  • lsc_agent_eval/src/lsc_agent_eval/core/agent_goal_eval/results.py
  • pyproject.toml
  • script/compare_evaluations.py
  • script/run_multi_provider_eval.py
  • src/generate_answers/generate_answers.py
  • src/lightspeed_evaluation/core/api/client.py
  • src/lightspeed_evaluation/core/llm/__init__.py
  • src/lightspeed_evaluation/core/llm/custom.py
  • src/lightspeed_evaluation/core/llm/litellm_patch.py
  • src/lightspeed_evaluation/core/metrics/custom/custom.py
  • src/lightspeed_evaluation/core/metrics/custom/tool_eval.py
  • src/lightspeed_evaluation/core/metrics/geval.py
  • src/lightspeed_evaluation/core/metrics/script.py
  • src/lightspeed_evaluation/core/models/__init__.py
  • src/lightspeed_evaluation/core/models/agents.py
  • src/lightspeed_evaluation/core/models/data.py
  • src/lightspeed_evaluation/core/models/quality.py
  • src/lightspeed_evaluation/core/models/statistics.py
  • src/lightspeed_evaluation/core/models/summary.py
  • src/lightspeed_evaluation/core/output/data_persistence.py
  • src/lightspeed_evaluation/core/output/generator.py
  • src/lightspeed_evaluation/core/output/statistics.py
  • src/lightspeed_evaluation/core/output/visualization.py
  • src/lightspeed_evaluation/core/script/manager.py
  • src/lightspeed_evaluation/core/storage/__init__.py
  • src/lightspeed_evaluation/core/storage/config.py
  • src/lightspeed_evaluation/core/storage/protocol.py
  • src/lightspeed_evaluation/core/storage/sql_storage.py
  • src/lightspeed_evaluation/core/system/ssl_certifi.py
  • src/lightspeed_evaluation/core/system/validator.py
  • src/lightspeed_evaluation/pipeline/evaluation/evaluator.py
  • src/lightspeed_evaluation/pipeline/evaluation/judges.py
  • src/lightspeed_evaluation/pipeline/evaluation/pipeline.py
  • src/lightspeed_evaluation/runner/evaluation.py
  • tests/script/test_compare_evaluations.py
  • tests/script/test_run_multi_provider_eval.py
  • tests/unit/core/api/conftest.py
  • tests/unit/core/api/test_client.py
  • tests/unit/core/api/test_client_infer.py
  • tests/unit/core/api/test_streaming_parser.py
  • tests/unit/core/config/test_models.py
  • tests/unit/core/llm/conftest.py
  • tests/unit/core/llm/test_custom.py
  • tests/unit/core/llm/test_llm_manager.py
  • tests/unit/core/llm/test_token_tracker.py
  • tests/unit/core/metrics/conftest.py
  • tests/unit/core/metrics/custom/test_custom.py
  • tests/unit/core/metrics/custom/test_tool_eval.py
  • tests/unit/core/metrics/test_geval.py
  • tests/unit/core/models/test_api_additional.py
  • tests/unit/core/models/test_quality.py
  • tests/unit/core/models/test_summary.py
  • tests/unit/core/models/test_system.py
  • tests/unit/core/output/conftest.py
  • tests/unit/core/output/test_final_coverage.py
  • tests/unit/core/output/test_generator.py
  • tests/unit/core/output/test_statistics.py
  • tests/unit/core/output/test_statistics_api.py
  • tests/unit/core/script/test_manager.py
  • tests/unit/core/script/test_manager_additional.py
  • tests/unit/core/storage/test_composite_and_factory.py
  • tests/unit/core/storage/test_protocol.py
  • tests/unit/core/storage/test_sql_storage.py
  • tests/unit/core/system/test_loader.py
  • tests/unit/core/system/test_setup.py
  • tests/unit/core/system/test_ssl_certifi.py
  • tests/unit/core/system/test_validator.py
  • tests/unit/pipeline/evaluation/conftest.py
  • tests/unit/pipeline/evaluation/test_evaluator.py
  • tests/unit/pipeline/evaluation/test_processor.py
  • tests/unit/runner/test_evaluation.py
💤 Files with no reviewable changes (2)
  • src/lightspeed_evaluation/core/metrics/geval.py
  • tests/unit/core/models/test_quality.py
✅ Files skipped from review due to trivial changes (54)
  • tests/unit/core/output/test_statistics_api.py
  • tests/unit/core/system/test_ssl_certifi.py
  • src/lightspeed_evaluation/core/models/agents.py
  • .gitignore
  • src/lightspeed_evaluation/core/llm/litellm_patch.py
  • tests/unit/core/llm/test_custom.py
  • tests/unit/core/script/test_manager.py
  • src/generate_answers/generate_answers.py
  • src/lightspeed_evaluation/core/metrics/custom/tool_eval.py
  • src/lightspeed_evaluation/core/storage/init.py
  • tests/unit/core/output/conftest.py
  • src/lightspeed_evaluation/core/metrics/custom/custom.py
  • tests/unit/core/storage/test_composite_and_factory.py
  • src/lightspeed_evaluation/core/models/quality.py
  • src/lightspeed_evaluation/runner/evaluation.py
  • tests/unit/core/config/test_models.py
  • tests/unit/core/models/test_summary.py
  • tests/unit/core/api/test_streaming_parser.py
  • tests/unit/core/storage/test_sql_storage.py
  • tests/unit/core/api/conftest.py
  • tests/unit/core/metrics/conftest.py
  • tests/unit/core/models/test_api_additional.py
  • tests/unit/pipeline/evaluation/test_evaluator.py
  • tests/unit/core/metrics/custom/test_tool_eval.py
  • tests/unit/core/api/test_client_infer.py
  • src/lightspeed_evaluation/core/llm/custom.py
  • tests/unit/core/metrics/custom/test_custom.py
  • src/lightspeed_evaluation/core/models/statistics.py
  • src/lightspeed_evaluation/core/output/visualization.py
  • tests/unit/core/output/test_generator.py
  • src/lightspeed_evaluation/core/metrics/script.py
  • src/lightspeed_evaluation/pipeline/evaluation/judges.py
  • tests/unit/core/llm/test_llm_manager.py
  • tests/unit/core/system/test_setup.py
  • src/lightspeed_evaluation/core/system/ssl_certifi.py
  • src/lightspeed_evaluation/pipeline/evaluation/evaluator.py
  • tests/unit/runner/test_evaluation.py
  • src/lightspeed_evaluation/core/output/statistics.py
  • src/lightspeed_evaluation/core/storage/config.py
  • tests/unit/core/output/test_statistics.py
  • tests/unit/core/system/test_validator.py
  • tests/unit/core/output/test_final_coverage.py
  • tests/script/test_run_multi_provider_eval.py
  • tests/unit/core/api/test_client.py
  • tests/unit/core/storage/test_protocol.py
  • script/compare_evaluations.py
  • src/lightspeed_evaluation/core/system/validator.py
  • tests/unit/pipeline/evaluation/test_processor.py
  • tests/script/test_compare_evaluations.py
  • tests/unit/core/metrics/test_geval.py
  • src/lightspeed_evaluation/core/models/init.py
  • src/lightspeed_evaluation/core/output/data_persistence.py
  • src/lightspeed_evaluation/core/api/client.py
  • tests/unit/pipeline/evaluation/conftest.py

Comment thread .github/workflows/shellcheck.yaml
Comment thread src/lightspeed_evaluation/core/output/generator.py Outdated
Comment thread src/lightspeed_evaluation/pipeline/evaluation/pipeline.py Outdated
@xmican10 xmican10 force-pushed the LEADS-225-align-lint-check-rules-with-LSC branch 5 times, most recently from 2df9c28 to 0834bc5 Compare May 27, 2026 09:07
VladimirKadlec
VladimirKadlec previously approved these changes May 28, 2026
Copy link
Copy Markdown
Member

@VladimirKadlec VladimirKadlec left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Copy Markdown
Collaborator

@asamal4 asamal4 left a comment

Choose a reason for hiding this comment

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

Thank you !!
I see a new package in pyproject - we will have to generate all lock files and requirements txt files.
make sync-lock-and-requirements

@xmican10 xmican10 force-pushed the LEADS-225-align-lint-check-rules-with-LSC branch from 0834bc5 to 27127e3 Compare June 2, 2026 10:57
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
pyproject.toml (1)

113-114: ⚡ Quick win

Avoid dual import sorting (ruff I001 vs isort)

pyproject.toml enables ruff import sorting via I001, while also defining [tool.isort]. CI/dev pre-commit runs ruff but not isort; however lsc_agent_eval/README.md instructs running uv run isort src tests, so developers can still get reformat churn when they run both.

[tool.ruff.lint]
extend-select = ["TID251", "UP006", "UP007", "UP010", "UP017", "UP035", "RUF100", "B009", "B010", "DTZ005", "D202", "I001", "PLR1733"]

Consider consolidating on a single sorter (drop I001 or remove/stop using [tool.isort], and/or update the dev instructions to use only one).

🤖 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 `@pyproject.toml` around lines 113 - 114, The pyproject enables ruff
import-sorting via the I001 rule in [tool.ruff.lint] while the project also
defines [tool.isort], causing dual-format churn; pick one approach and update
configuration and docs accordingly: either remove "I001" from extend-select in
[tool.ruff.lint] and keep [tool.isort] (and update lsc_agent_eval/README.md to
instruct running isort), or remove the [tool.isort] section and keep I001 (and
update README to run ruff --fix only); ensure the chosen tool is reflected
consistently in pyproject.toml, pre-commit/CI config, and the README.
🤖 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 `@pyproject.toml`:
- Around line 113-114: The pyproject enables ruff import-sorting via the I001
rule in [tool.ruff.lint] while the project also defines [tool.isort], causing
dual-format churn; pick one approach and update configuration and docs
accordingly: either remove "I001" from extend-select in [tool.ruff.lint] and
keep [tool.isort] (and update lsc_agent_eval/README.md to instruct running
isort), or remove the [tool.isort] section and keep I001 (and update README to
run ruff --fix only); ensure the chosen tool is reflected consistently in
pyproject.toml, pre-commit/CI config, and the README.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 318ee08d-b5e9-4c4d-b359-35e40e9806c4

📥 Commits

Reviewing files that changed from the base of the PR and between c02ee08 and 27127e3.

⛔ Files ignored due to path filters (2)
  • uv-gpu.lock is excluded by !**/*.lock
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (79)
  • Makefile
  • lsc_agent_eval/src/lsc_agent_eval/core/agent_goal_eval/evaluator.py
  • lsc_agent_eval/src/lsc_agent_eval/core/agent_goal_eval/results.py
  • pyproject.toml
  • requirements-all-extras.txt
  • requirements-local-embeddings.txt
  • requirements-nlp-metrics.txt
  • requirements.txt
  • script/compare_evaluations.py
  • script/run_multi_provider_eval.py
  • src/generate_answers/generate_answers.py
  • src/lightspeed_evaluation/core/api/client.py
  • src/lightspeed_evaluation/core/llm/__init__.py
  • src/lightspeed_evaluation/core/llm/custom.py
  • src/lightspeed_evaluation/core/llm/litellm_patch.py
  • src/lightspeed_evaluation/core/metrics/custom/custom.py
  • src/lightspeed_evaluation/core/metrics/custom/tool_eval.py
  • src/lightspeed_evaluation/core/metrics/geval.py
  • src/lightspeed_evaluation/core/metrics/script.py
  • src/lightspeed_evaluation/core/models/__init__.py
  • src/lightspeed_evaluation/core/models/agents.py
  • src/lightspeed_evaluation/core/models/data.py
  • src/lightspeed_evaluation/core/models/quality.py
  • src/lightspeed_evaluation/core/models/statistics.py
  • src/lightspeed_evaluation/core/models/summary.py
  • src/lightspeed_evaluation/core/output/data_persistence.py
  • src/lightspeed_evaluation/core/output/generator.py
  • src/lightspeed_evaluation/core/output/serializers.py
  • src/lightspeed_evaluation/core/output/statistics.py
  • src/lightspeed_evaluation/core/output/visualization.py
  • src/lightspeed_evaluation/core/script/manager.py
  • src/lightspeed_evaluation/core/storage/__init__.py
  • src/lightspeed_evaluation/core/storage/config.py
  • src/lightspeed_evaluation/core/storage/protocol.py
  • src/lightspeed_evaluation/core/storage/sql_storage.py
  • src/lightspeed_evaluation/core/system/ssl_certifi.py
  • src/lightspeed_evaluation/core/system/validator.py
  • src/lightspeed_evaluation/pipeline/evaluation/__init__.py
  • src/lightspeed_evaluation/pipeline/evaluation/driver.py
  • src/lightspeed_evaluation/pipeline/evaluation/judges.py
  • src/lightspeed_evaluation/pipeline/evaluation/pipeline.py
  • src/lightspeed_evaluation/runner/evaluation.py
  • tests/script/test_compare_evaluations.py
  • tests/script/test_run_multi_provider_eval.py
  • tests/unit/core/api/conftest.py
  • tests/unit/core/api/test_client.py
  • tests/unit/core/api/test_client_infer.py
  • tests/unit/core/api/test_streaming_parser.py
  • tests/unit/core/config/test_models.py
  • tests/unit/core/llm/conftest.py
  • tests/unit/core/llm/test_custom.py
  • tests/unit/core/llm/test_llm_manager.py
  • tests/unit/core/llm/test_token_tracker.py
  • tests/unit/core/metrics/conftest.py
  • tests/unit/core/metrics/custom/test_custom.py
  • tests/unit/core/metrics/custom/test_tool_eval.py
  • tests/unit/core/metrics/test_geval.py
  • tests/unit/core/models/test_api_additional.py
  • tests/unit/core/models/test_quality.py
  • tests/unit/core/models/test_summary.py
  • tests/unit/core/models/test_system.py
  • tests/unit/core/output/conftest.py
  • tests/unit/core/output/test_final_coverage.py
  • tests/unit/core/output/test_generator.py
  • tests/unit/core/output/test_statistics.py
  • tests/unit/core/output/test_statistics_api.py
  • tests/unit/core/script/test_manager.py
  • tests/unit/core/script/test_manager_additional.py
  • tests/unit/core/storage/test_composite_and_factory.py
  • tests/unit/core/storage/test_protocol.py
  • tests/unit/core/storage/test_sql_storage.py
  • tests/unit/core/system/test_loader.py
  • tests/unit/core/system/test_setup.py
  • tests/unit/core/system/test_ssl_certifi.py
  • tests/unit/core/system/test_validator.py
  • tests/unit/pipeline/evaluation/conftest.py
  • tests/unit/pipeline/evaluation/test_evaluator.py
  • tests/unit/pipeline/evaluation/test_processor.py
  • tests/unit/runner/test_evaluation.py
💤 Files with no reviewable changes (2)
  • src/lightspeed_evaluation/core/metrics/geval.py
  • tests/unit/core/models/test_quality.py
✅ Files skipped from review due to trivial changes (50)
  • src/lightspeed_evaluation/core/metrics/custom/tool_eval.py
  • src/generate_answers/generate_answers.py
  • tests/unit/core/storage/test_composite_and_factory.py
  • src/lightspeed_evaluation/core/output/data_persistence.py
  • src/lightspeed_evaluation/pipeline/evaluation/init.py
  • src/lightspeed_evaluation/pipeline/evaluation/driver.py
  • tests/unit/core/api/conftest.py
  • tests/unit/core/output/test_statistics_api.py
  • src/lightspeed_evaluation/core/models/quality.py
  • src/lightspeed_evaluation/core/output/visualization.py
  • tests/unit/core/system/test_setup.py
  • src/lightspeed_evaluation/core/llm/litellm_patch.py
  • tests/unit/core/system/test_ssl_certifi.py
  • tests/unit/core/output/test_final_coverage.py
  • tests/unit/core/models/test_api_additional.py
  • src/lightspeed_evaluation/runner/evaluation.py
  • tests/unit/core/llm/test_llm_manager.py
  • requirements-nlp-metrics.txt
  • tests/unit/core/models/test_summary.py
  • tests/unit/core/api/test_client_infer.py
  • tests/unit/core/metrics/test_geval.py
  • src/lightspeed_evaluation/core/system/validator.py
  • tests/unit/core/models/test_system.py
  • tests/unit/core/script/test_manager.py
  • src/lightspeed_evaluation/core/api/client.py
  • src/lightspeed_evaluation/core/output/statistics.py
  • tests/unit/core/config/test_models.py
  • tests/unit/core/metrics/custom/test_custom.py
  • src/lightspeed_evaluation/core/output/serializers.py
  • src/lightspeed_evaluation/core/models/statistics.py
  • src/lightspeed_evaluation/core/metrics/script.py
  • tests/unit/core/output/test_statistics.py
  • src/lightspeed_evaluation/core/metrics/custom/custom.py
  • tests/unit/core/api/test_streaming_parser.py
  • src/lightspeed_evaluation/core/llm/custom.py
  • src/lightspeed_evaluation/pipeline/evaluation/judges.py
  • src/lightspeed_evaluation/core/models/init.py
  • tests/unit/core/storage/test_protocol.py
  • tests/unit/core/system/test_validator.py
  • tests/unit/core/llm/test_token_tracker.py
  • tests/unit/runner/test_evaluation.py
  • tests/unit/pipeline/evaluation/test_evaluator.py
  • requirements-all-extras.txt
  • tests/script/test_compare_evaluations.py
  • src/lightspeed_evaluation/core/storage/init.py
  • script/compare_evaluations.py
  • tests/unit/core/api/test_client.py
  • tests/unit/core/output/test_generator.py
  • src/lightspeed_evaluation/core/system/ssl_certifi.py
  • tests/script/test_run_multi_provider_eval.py
🚧 Files skipped from review as they are similar to previous changes (22)
  • tests/unit/core/storage/test_sql_storage.py
  • tests/unit/core/metrics/conftest.py
  • src/lightspeed_evaluation/core/storage/protocol.py
  • lsc_agent_eval/src/lsc_agent_eval/core/agent_goal_eval/results.py
  • tests/unit/pipeline/evaluation/conftest.py
  • tests/unit/core/output/conftest.py
  • tests/unit/core/metrics/custom/test_tool_eval.py
  • src/lightspeed_evaluation/core/storage/sql_storage.py
  • tests/unit/core/llm/test_custom.py
  • src/lightspeed_evaluation/core/script/manager.py
  • src/lightspeed_evaluation/core/storage/config.py
  • src/lightspeed_evaluation/pipeline/evaluation/pipeline.py
  • tests/unit/core/llm/conftest.py
  • lsc_agent_eval/src/lsc_agent_eval/core/agent_goal_eval/evaluator.py
  • src/lightspeed_evaluation/core/models/data.py
  • tests/unit/core/system/test_loader.py
  • tests/unit/pipeline/evaluation/test_processor.py
  • script/run_multi_provider_eval.py
  • tests/unit/core/script/test_manager_additional.py
  • src/lightspeed_evaluation/core/output/generator.py
  • Makefile
  • src/lightspeed_evaluation/core/models/summary.py

@xmican10 xmican10 force-pushed the LEADS-225-align-lint-check-rules-with-LSC branch from 27127e3 to 3021c7d Compare June 2, 2026 11:23
Copy link
Copy Markdown
Collaborator

@asamal4 asamal4 left a comment

Choose a reason for hiding this comment

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

LGTM

@asamal4 asamal4 merged commit e8b5f96 into lightspeed-core:main Jun 3, 2026
17 checks passed
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