Skip to content

Comments

Refine evals interfaces#355

Merged
KaQuMiQ merged 1 commit intomainfrom
feature/evals_values
Jul 4, 2025
Merged

Refine evals interfaces#355
KaQuMiQ merged 1 commit intomainfrom
feature/evals_values

Conversation

@KaQuMiQ
Copy link
Collaborator

@KaQuMiQ KaQuMiQ commented Jul 4, 2025

No description provided.

@coderabbitai
Copy link

coderabbitai bot commented Jul 4, 2025

Walkthrough

This change refactors type parameterization across several modules, primarily removing or simplifying the generic Value or Result type parameters from classes and functions related to evaluation results and instruction refinement. The value field is removed from result classes, and their signatures are updated accordingly in src/draive/evaluation/suite.py. Similar adjustments are made in src/draive/helpers/instruction_refinement.py, where all related type annotations and generics are unified or simplified. New merging class methods are added to result classes for combining multiple results. Minor updates include type annotation changes and import adjustments. The project version is incremented in pyproject.toml.

Possibly related PRs

  • Update evals interface #353: Refactors type parameterization and method signatures in evaluation suite results and instruction refinement, directly related to the removal and modification of generic type parameters in evaluation and refinement interfaces.

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e5b6a7c and f4fb632.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (5)
  • pyproject.toml (1 hunks)
  • src/draive/evaluation/generator.py (2 hunks)
  • src/draive/evaluation/scenario.py (3 hunks)
  • src/draive/evaluation/suite.py (11 hunks)
  • src/draive/helpers/instruction_refinement.py (14 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.py`: Follow Ruff import ordering (standard library, third party, local) Us...

**/*.py: Follow Ruff import ordering (standard library, third party, local)
Use Python 3.12+ type features (type unions with |, generic syntax)
Use base and abstract types like Sequence or Iterable instead of concrete types
Use custom exceptions for specific errors
Format code with Ruff
Run linters (Ruff + Bandit + Pyright strict mode)

📄 Source: CodeRabbit Inference Engine (CLAUDE.md)

List of files the instruction was applied to:

  • src/draive/evaluation/generator.py
  • src/draive/evaluation/scenario.py
  • src/draive/helpers/instruction_refinement.py
  • src/draive/evaluation/suite.py
🧠 Learnings (6)
📓 Common learnings
Learnt from: KaQuMiQ
PR: miquido/draive#327
File: src/draive/helpers/instruction_preparation.py:28-34
Timestamp: 2025-05-28T17:41:57.460Z
Learning: The draive project uses and requires Python 3.12+, so PEP-695 generic syntax with square brackets (e.g., `def func[T: Type]()`) is valid and should be used instead of the older TypeVar approach.
Learnt from: CR
PR: miquido/draive#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T09:39:43.008Z
Learning: Applies to draive/**/*.py : Use generic state classes with type parameters for reusable data structures
Learnt from: KaQuMiQ
PR: miquido/draive#338
File: src/draive/lmm/__init__.py:1-2
Timestamp: 2025-06-16T10:28:07.434Z
Learning: The draive project requires Python 3.12+ as specified in pyproject.toml with "requires-python = ">=3.12"" and uses Python 3.12+ specific features like PEP 695 type aliases and generic syntax extensively throughout the codebase.
pyproject.toml (1)
Learnt from: KaQuMiQ
PR: miquido/draive#338
File: src/draive/lmm/__init__.py:1-2
Timestamp: 2025-06-16T10:28:07.434Z
Learning: The draive project requires Python 3.12+ as specified in pyproject.toml with "requires-python = ">=3.12"" and uses Python 3.12+ specific features like PEP 695 type aliases and generic syntax extensively throughout the codebase.
src/draive/evaluation/generator.py (4)
Learnt from: CR
PR: miquido/draive#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T09:39:43.008Z
Learning: Applies to **/*.py : Use base and abstract types like Sequence or Iterable instead of concrete types
Learnt from: CR
PR: miquido/draive#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T09:39:43.008Z
Learning: Applies to draive/**/*.{py} : ALWAYS use Sequence[T] instead of list[T], Mapping[K,V] instead of dict[K,V], and Set[T] instead of set[T] for collections in State, Config and DataModel classes
Learnt from: CR
PR: miquido/draive#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T09:39:43.008Z
Learning: Applies to draive/**/*.py : Use generic state classes with type parameters for reusable data structures
Learnt from: KaQuMiQ
PR: miquido/draive#327
File: src/draive/helpers/instruction_preparation.py:28-34
Timestamp: 2025-05-28T17:41:57.460Z
Learning: The draive project uses and requires Python 3.12+, so PEP-695 generic syntax with square brackets (e.g., `def func[T: Type]()`) is valid and should be used instead of the older TypeVar approach.
src/draive/evaluation/scenario.py (3)
Learnt from: CR
PR: miquido/draive#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T09:39:43.008Z
Learning: Applies to draive/**/*.{py} : ALWAYS use Sequence[T] instead of list[T], Mapping[K,V] instead of dict[K,V], and Set[T] instead of set[T] for collections in State, Config and DataModel classes
Learnt from: CR
PR: miquido/draive#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T09:39:43.008Z
Learning: Applies to **/*.py : Use base and abstract types like Sequence or Iterable instead of concrete types
Learnt from: CR
PR: miquido/draive#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T09:39:43.008Z
Learning: Applies to draive/**/*.py : Use Protocol and runtime_checkable for function protocols
src/draive/helpers/instruction_refinement.py (1)
Learnt from: CR
PR: miquido/draive#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T09:39:43.008Z
Learning: Applies to draive/**/*.py : Use generic state classes with type parameters for reusable data structures
src/draive/evaluation/suite.py (3)
Learnt from: CR
PR: miquido/draive#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T09:39:43.008Z
Learning: Applies to draive/**/*.py : Use generic state classes with type parameters for reusable data structures
Learnt from: CR
PR: miquido/draive#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T09:39:43.008Z
Learning: Applies to draive/**/*.{py} : ALWAYS use Sequence[T] instead of list[T], Mapping[K,V] instead of dict[K,V], and Set[T] instead of set[T] for collections in State, Config and DataModel classes
Learnt from: CR
PR: miquido/draive#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T09:39:43.008Z
Learning: Applies to draive/**/*.py : Use Protocol and runtime_checkable for function protocols
🧬 Code Graph Analysis (1)
src/draive/helpers/instruction_refinement.py (3)
src/draive/instructions/types.py (1)
  • Instruction (46-216)
src/draive/evaluation/suite.py (4)
  • evaluation_suite (508-586)
  • EvaluationSuite (249-505)
  • SuiteEvaluatorResult (98-141)
  • EvaluationSuiteCase (26-29)
src/draive/parameters/model.py (1)
  • DataModel (386-756)
🔇 Additional comments (7)
pyproject.toml (1)

8-8: LGTM!

Version bump to 0.75.1 is appropriate for these internal refactoring changes.

src/draive/evaluation/generator.py (1)

1-1: Type annotation correctly uses abstract type.

Good change to use Sequence[Parameters] instead of list[Parameters] in the return type annotation. This aligns with the project's coding guidelines to prefer abstract types over concrete ones.

Also applies to: 38-38

src/draive/evaluation/scenario.py (1)

99-115: Well-implemented result merging functionality.

The merging class method correctly combines multiple evaluation results and their metadata. Good use of as_list for efficient list operations and proper metadata merging using the existing merged_with method.

src/draive/helpers/instruction_refinement.py (1)

20-29: Type parameter simplification improves flexibility.

Good change to remove the Result: DataModel | str constraint and use an unconstrained Value type parameter. This aligns with the evaluation suite module changes and provides more flexibility without breaking existing usage.

src/draive/evaluation/suite.py (3)

32-32: Simplified type hierarchy improves maintainability.

The removal of the Value type parameter from SuiteEvaluatorCaseResult and SuiteEvaluatorResult significantly simplifies the type hierarchy. This architectural change removes the need to propagate the evaluated value type through multiple layers of results, making the API cleaner and easier to use.

Also applies to: 98-100, 144-144


192-208: Consistent implementation of result merging.

The merging method follows the same pattern as in EvaluationScenarioResult, providing a consistent API for combining evaluation results across the framework.


175-190: Removal of value field confirmed—no breaking references detected

A repository-wide search for .value access on EvaluationCaseResult, SuiteEvaluatorCaseResult, and SuiteEvaluatorResult (including tests) returned no matches outside the class definition. This change appears intentional and safe to merge.

✨ Finishing Touches
  • 📝 Generate Docstrings

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@KaQuMiQ KaQuMiQ merged commit 75babbb into main Jul 4, 2025
5 checks passed
@KaQuMiQ KaQuMiQ deleted the feature/evals_values branch July 4, 2025 12:21
This was referenced Jul 4, 2025
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