[LEADS-225] Align lint checks with LSC#245
Open
xmican10 wants to merge 1 commit into
Open
Conversation
Contributor
|
Warning Review limit reached
More reviews will be available in 1 minute and 35 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 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (73)
<review_stack_artifact> </review_stack_artifact> 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
xmican10
force-pushed
the
LEADS-225-align-lint-check-rules-with-LSC
branch
from
c2bb02b to
e084620
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
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
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (73)
Makefilelsc_agent_eval/src/lsc_agent_eval/core/agent_goal_eval/evaluator.pylsc_agent_eval/src/lsc_agent_eval/core/agent_goal_eval/results.pypyproject.tomlscript/compare_evaluations.pyscript/run_multi_provider_eval.pysrc/generate_answers/generate_answers.pysrc/lightspeed_evaluation/core/api/client.pysrc/lightspeed_evaluation/core/llm/__init__.pysrc/lightspeed_evaluation/core/llm/custom.pysrc/lightspeed_evaluation/core/llm/litellm_patch.pysrc/lightspeed_evaluation/core/metrics/custom/custom.pysrc/lightspeed_evaluation/core/metrics/custom/tool_eval.pysrc/lightspeed_evaluation/core/metrics/geval.pysrc/lightspeed_evaluation/core/metrics/script.pysrc/lightspeed_evaluation/core/models/__init__.pysrc/lightspeed_evaluation/core/models/agents.pysrc/lightspeed_evaluation/core/models/data.pysrc/lightspeed_evaluation/core/models/quality.pysrc/lightspeed_evaluation/core/models/statistics.pysrc/lightspeed_evaluation/core/models/summary.pysrc/lightspeed_evaluation/core/output/data_persistence.pysrc/lightspeed_evaluation/core/output/generator.pysrc/lightspeed_evaluation/core/output/statistics.pysrc/lightspeed_evaluation/core/output/visualization.pysrc/lightspeed_evaluation/core/script/manager.pysrc/lightspeed_evaluation/core/storage/__init__.pysrc/lightspeed_evaluation/core/storage/config.pysrc/lightspeed_evaluation/core/storage/protocol.pysrc/lightspeed_evaluation/core/storage/sql_storage.pysrc/lightspeed_evaluation/core/system/ssl_certifi.pysrc/lightspeed_evaluation/core/system/validator.pysrc/lightspeed_evaluation/pipeline/evaluation/evaluator.pysrc/lightspeed_evaluation/pipeline/evaluation/judges.pysrc/lightspeed_evaluation/pipeline/evaluation/pipeline.pysrc/lightspeed_evaluation/runner/evaluation.pytests/script/test_compare_evaluations.pytests/script/test_run_multi_provider_eval.pytests/unit/core/api/conftest.pytests/unit/core/api/test_client.pytests/unit/core/api/test_client_infer.pytests/unit/core/api/test_streaming_parser.pytests/unit/core/config/test_models.pytests/unit/core/llm/conftest.pytests/unit/core/llm/test_custom.pytests/unit/core/llm/test_llm_manager.pytests/unit/core/llm/test_token_tracker.pytests/unit/core/metrics/conftest.pytests/unit/core/metrics/custom/test_custom.pytests/unit/core/metrics/custom/test_tool_eval.pytests/unit/core/metrics/test_geval.pytests/unit/core/models/test_api_additional.pytests/unit/core/models/test_quality.pytests/unit/core/models/test_summary.pytests/unit/core/models/test_system.pytests/unit/core/output/conftest.pytests/unit/core/output/test_final_coverage.pytests/unit/core/output/test_generator.pytests/unit/core/output/test_statistics.pytests/unit/core/output/test_statistics_api.pytests/unit/core/script/test_manager.pytests/unit/core/script/test_manager_additional.pytests/unit/core/storage/test_composite_and_factory.pytests/unit/core/storage/test_protocol.pytests/unit/core/storage/test_sql_storage.pytests/unit/core/system/test_loader.pytests/unit/core/system/test_setup.pytests/unit/core/system/test_ssl_certifi.pytests/unit/core/system/test_validator.pytests/unit/pipeline/evaluation/conftest.pytests/unit/pipeline/evaluation/test_evaluator.pytests/unit/pipeline/evaluation/test_processor.pytests/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
xmican10
force-pushed
the
LEADS-225-align-lint-check-rules-with-LSC
branch
3 times, most recently
from
6dbbb3c to
8a375d2
Compare
Contributor
There was a problem hiding this comment.
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
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (75)
.github/workflows/shellcheck.yaml.gitignoreMakefilelsc_agent_eval/src/lsc_agent_eval/core/agent_goal_eval/evaluator.pylsc_agent_eval/src/lsc_agent_eval/core/agent_goal_eval/results.pypyproject.tomlscript/compare_evaluations.pyscript/run_multi_provider_eval.pysrc/generate_answers/generate_answers.pysrc/lightspeed_evaluation/core/api/client.pysrc/lightspeed_evaluation/core/llm/__init__.pysrc/lightspeed_evaluation/core/llm/custom.pysrc/lightspeed_evaluation/core/llm/litellm_patch.pysrc/lightspeed_evaluation/core/metrics/custom/custom.pysrc/lightspeed_evaluation/core/metrics/custom/tool_eval.pysrc/lightspeed_evaluation/core/metrics/geval.pysrc/lightspeed_evaluation/core/metrics/script.pysrc/lightspeed_evaluation/core/models/__init__.pysrc/lightspeed_evaluation/core/models/agents.pysrc/lightspeed_evaluation/core/models/data.pysrc/lightspeed_evaluation/core/models/quality.pysrc/lightspeed_evaluation/core/models/statistics.pysrc/lightspeed_evaluation/core/models/summary.pysrc/lightspeed_evaluation/core/output/data_persistence.pysrc/lightspeed_evaluation/core/output/generator.pysrc/lightspeed_evaluation/core/output/statistics.pysrc/lightspeed_evaluation/core/output/visualization.pysrc/lightspeed_evaluation/core/script/manager.pysrc/lightspeed_evaluation/core/storage/__init__.pysrc/lightspeed_evaluation/core/storage/config.pysrc/lightspeed_evaluation/core/storage/protocol.pysrc/lightspeed_evaluation/core/storage/sql_storage.pysrc/lightspeed_evaluation/core/system/ssl_certifi.pysrc/lightspeed_evaluation/core/system/validator.pysrc/lightspeed_evaluation/pipeline/evaluation/evaluator.pysrc/lightspeed_evaluation/pipeline/evaluation/judges.pysrc/lightspeed_evaluation/pipeline/evaluation/pipeline.pysrc/lightspeed_evaluation/runner/evaluation.pytests/script/test_compare_evaluations.pytests/script/test_run_multi_provider_eval.pytests/unit/core/api/conftest.pytests/unit/core/api/test_client.pytests/unit/core/api/test_client_infer.pytests/unit/core/api/test_streaming_parser.pytests/unit/core/config/test_models.pytests/unit/core/llm/conftest.pytests/unit/core/llm/test_custom.pytests/unit/core/llm/test_llm_manager.pytests/unit/core/llm/test_token_tracker.pytests/unit/core/metrics/conftest.pytests/unit/core/metrics/custom/test_custom.pytests/unit/core/metrics/custom/test_tool_eval.pytests/unit/core/metrics/test_geval.pytests/unit/core/models/test_api_additional.pytests/unit/core/models/test_quality.pytests/unit/core/models/test_summary.pytests/unit/core/models/test_system.pytests/unit/core/output/conftest.pytests/unit/core/output/test_final_coverage.pytests/unit/core/output/test_generator.pytests/unit/core/output/test_statistics.pytests/unit/core/output/test_statistics_api.pytests/unit/core/script/test_manager.pytests/unit/core/script/test_manager_additional.pytests/unit/core/storage/test_composite_and_factory.pytests/unit/core/storage/test_protocol.pytests/unit/core/storage/test_sql_storage.pytests/unit/core/system/test_loader.pytests/unit/core/system/test_setup.pytests/unit/core/system/test_ssl_certifi.pytests/unit/core/system/test_validator.pytests/unit/pipeline/evaluation/conftest.pytests/unit/pipeline/evaluation/test_evaluator.pytests/unit/pipeline/evaluation/test_processor.pytests/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
xmican10
force-pushed
the
LEADS-225-align-lint-check-rules-with-LSC
branch
4 times, most recently
from
8a375d2 to
2df9c28
Compare
xmican10
force-pushed
the
LEADS-225-align-lint-check-rules-with-LSC
branch
from
2df9c28 to
0834bc5
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Type of change
Tools used to create PR
Identify any AI code assistants used in this PR (for transparency and review context)
Related Tickets & Documents
Checklist before requesting a review
Testing
Summary by CodeRabbit
Bug Fixes
Chores
Tests