Conversation
|
MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅ |
Summary of ChangesHello @arekay-nv, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on a significant refactor and cleanup effort by integrating Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Pull request overview
This PR enables mypy type checking support across the codebase by adding type annotations and fixing existing type-related issues. The changes address mypy errors by adding proper type hints, using Literal types where appropriate, and adding type: ignore comments for known msgspec limitations.
Key changes:
- Added mypy configuration to pyproject.toml and pre-commit hooks
- Fixed type annotations throughout the codebase (function signatures, variable declarations, etc.)
- Added type: ignore comments for msgspec.Struct call-arg issues
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| .pre-commit-config.yaml | Added mypy pre-commit hook for automated type checking |
| pyproject.toml | Added mypy configuration with ignore_missing_imports setting |
| src/inference_endpoint/utils/logging.py | Added Literal type for style parameter validation |
| src/inference_endpoint/utils/dataset_utils.py | Added type annotation for token_leng_counts dictionary |
| src/inference_endpoint/utils/init.py | Changed division to floor division for integer type consistency |
| src/inference_endpoint/sglang/types.py | Added type: ignore comments for msgspec.Struct call-arg warnings |
| src/inference_endpoint/openai/types.py | Added type: ignore comments for msgspec.Struct call-arg warnings |
| src/inference_endpoint/metrics/reporter.py | Changed .sum() to sum() for better type inference |
| src/inference_endpoint/metrics/recorder.py | Fixed type annotations for Path and string handling |
| src/inference_endpoint/metrics/metric.py | Added type hints for optional parameters and assertions |
| src/inference_endpoint/load_generator/session.py | Added type annotations for thread and sample_uuid_map |
| src/inference_endpoint/load_generator/scheduler.py | Fixed random.Random default parameter instantiation |
| src/inference_endpoint/load_generator/sample.py | Improved type hint for hook callback parameter |
| src/inference_endpoint/load_generator/load_generator.py | Updated return type and docstring for next method |
| src/inference_endpoint/endpoint_client/zmq_utils.py | Added UnionType support and fixed zmq error handling |
| src/inference_endpoint/endpoint_client/worker.py | Added comprehensive type annotations and assertions throughout worker code |
| src/inference_endpoint/endpoint_client/http_sample_issuer.py | Added assertion for loop existence |
| src/inference_endpoint/endpoint_client/http_client.py | Added type annotations and assertions for event loop handling |
| src/inference_endpoint/dataset_manager/factory.py | Fixed dataset format type handling with assertions |
| src/inference_endpoint/dataset_manager/dataset.py | Changed return type from instance to type class |
| src/inference_endpoint/core/types.py | Added type: ignore comments and new helper method for response output |
| src/inference_endpoint/config/schema.py | Fixed type annotations for parser and timeout fields |
| src/inference_endpoint/config/rulesets/mlcommons/rules.py | Added proper type hints for optional fields and metric handling |
| src/inference_endpoint/commands/probe.py | Refactored response_output handling to use new helper method |
| src/inference_endpoint/commands/benchmark.py | Added type annotations and improved type safety with assertions |
Comments suppressed due to low confidence (1)
src/inference_endpoint/config/rulesets/mlcommons/rules.py:1
- The expression
d.parser or Noneis redundant - if d.parser is falsy, it's already None. Simply use[d.parser for d in config.datasets]instead.
# SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request does a great job of enabling mypy and fixing a large number of typing-related issues throughout the codebase. The changes significantly improve type safety and code clarity. I've identified a couple of areas where the implementation can be further improved for correctness and to reduce code duplication. Overall, this is a solid step towards a more robust and maintainable codebase.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 29 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tests/performance/endpoint_client/test_http_client_performance_single.py
Outdated
Show resolved
Hide resolved
tests/performance/endpoint_client/test_http_client_performance_single.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 30 out of 30 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tests/performance/endpoint_client/test_http_client_performance_single.py
Outdated
Show resolved
Hide resolved
tests/performance/endpoint_client/test_http_client_performance_single.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 30 out of 30 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tests/performance/endpoint_client/test_http_client_performance_single.py
Show resolved
Hide resolved
- Added mypy configuration to pyproject.toml - Enabled pre-commit hooks for type checking - Added type annotations across endpoint_client, commands, and load_generator modules - Fixed type-related issues throughout the codebase - Configured mypy to run as warning in pre-commit This change improves code quality and catches type errors during development.
arekay-nv
force-pushed
the
arekay/add_mypy_support
branch
from
5da98f4 to
ea5495c
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 29 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Rashid Kaleem <230885705+arekay-nv@users.noreply.github.com>
Signed-off-by: Rashid Kaleem <230885705+arekay-nv@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 32 out of 32 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tests/performance/endpoint_client/test_http_client_performance_single.py
Show resolved
Hide resolved
Signed-off-by: Rashid Kaleem <230885705+arekay-nv@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 43 out of 43 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tests/performance/endpoint_client/test_http_client_performance_single.py
Show resolved
Hide resolved
Signed-off-by: Rashid Kaleem <230885705+arekay-nv@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 43 out of 43 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 43 out of 43 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Rashid Kaleem <230885705+arekay-nv@users.noreply.github.com>
Signed-off-by: Rashid Kaleem <230885705+arekay-nv@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 44 out of 44 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Rashid Kaleem <230885705+arekay-nv@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 44 out of 44 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Rashid Kaleem <230885705+arekay-nv@users.noreply.github.com>
Fixes and enables mypy support for better type checking.
Type of change
Related issues
Testing
Checklist