[WIP] refactor(strategy-executor): implement Strategy and Executor patterns #69

ployts · 2026-01-15T12:49:56Z

OpenJudge Version

0.2.0

Description

This PR implements the Strategy and Executor patterns as part of a major architectural refactoring of the OpenJudge framework. The changes separate concerns between "what to test" (atomic evaluation) and "how to test" (evaluation strategy), and abstract resource scheduling and execution management to dedicated controllers.

Checklist

Please check the following items before code is ready to be reviewed.

Code has been formatted with pre-commit run --all-files command
All tests are passing
Docstrings are in Google style
Related documentation has been updated (e.g. links, examples, etc.)
Code is ready for review

- Refactored Grader to separate "what to test" (atomic evaluation) from "how to test" (strategy) - Enhanced controller abstraction for resource scheduling and execution management - Updated all grader implementations to align with new architecture - Modified runner to delegate execution control to dedicated controllers - Updated tests to reflect new pattern implementation

gemini-code-assist · 2026-01-15T12:51:47Z

Summary of Changes

Hello @ployts, I'm Gemini Code Assist¹! 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 introduces a significant architectural refactoring to the evaluation framework by integrating the Strategy and Executor design patterns. This change enhances modularity and flexibility by clearly delineating the evaluation logic from its execution and resource management. The update streamlines how graders are configured and run, making the system more adaptable to different testing methodologies and resource constraints.

Highlights

Architectural Refactoring: The core evaluation logic has been refactored to implement Strategy and Executor patterns, separating 'what to test' from 'how to test' and abstracting resource management.
Grader Interface Update: The BaseGrader class now includes strategy and mapper parameters in its constructor, and its public aevaluate method now orchestrates calls to an internal _aevaluate method via a controller and strategy.
New Strategy and Controller Modules: Introduced openjudge/strategy and openjudge/runner/controller modules, defining abstract base classes for evaluation strategies and execution controllers, along with concrete implementations like DirectStrategy, MajorityVoteStrategy, and LocalController.
GradingRunner Simplification: The GradingRunner has been simplified by removing the GraderConfig dataclass and directly accepting BaseGrader instances. It now leverages the new controller system for concurrency management.
Internal Method Renaming: Numerous aevaluate methods across various grader implementations have been renamed to _aevaluate, indicating their new role as internal, atomic evaluation logic.

🧠 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 Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request introduces a significant and well-executed architectural refactoring, implementing the Strategy and Executor (Controller) patterns. The changes effectively separate concerns, making the framework more modular and extensible. The new strategy and controller modules are cleanly implemented, and the updates across the various grader classes and tests are consistent with the new design. My review includes a couple of suggestions to further enhance the new architecture by making the default strategy more explicit in the BaseGrader and addressing a minor logic issue in the RelevanceGrader. Overall, this is a very positive step forward for the codebase.

gemini-code-assist · 2026-01-15T12:55:01Z

openjudge/graders/base_grader.py

        self.name = name
        self.mode = mode
        self.description = description
+        self.strategy = strategy


To make the default strategy more explicit and simplify the aevaluate method, consider initializing self.strategy with DirectStrategy if it's None. This avoids a conditional check later on and makes the code's intent clearer.

You'll need to add the following import at the top of the file:

from openjudge.strategy import DirectStrategy

Suggested change

self.strategy = strategy

self.strategy = strategy or DirectStrategy()

gemini-code-assist · 2026-01-15T12:55:02Z

openjudge/graders/base_grader.py

+        if self.strategy:
+            return await self.strategy.execute(managed_fn, **data)
+        else:
+            return await managed_fn(**data)


If you apply the suggestion to initialize self.strategy with a default DirectStrategy in __init__, this conditional logic is no longer needed. This simplifies the aevaluate method and removes the need to handle the None case for self.strategy.

return await self.strategy.execute(managed_fn, **data)

gemini-code-assist · 2026-01-15T12:55:02Z

openjudge/graders/common/relevance.py

+            # Check if the result is an error
+            if isinstance(result, GraderError):
+                return result


The super()._aevaluate() call, which resolves to LLMGrader._aevaluate, is designed to return a GraderScore or GraderRank object, or raise an exception on failure. It does not return GraderError instances directly. Therefore, this check for GraderError will never be true and can be considered dead code. Exceptions are caught by the except block below. This block can be safely removed.

weizhang25 · 2026-01-15T17:31:04Z

openjudge/strategy/base.py

@@ -0,0 +1,53 @@
+# -*- coding: utf-8 -*-
+"""Base class for evaluation strategies.


avoild using file name "base.py". Name it as base_strategy.py. Make the file name self-explainable.

-> base_evaluation_strategy.py

weizhang25 · 2026-01-15T17:32:16Z

openjudge/strategy/base.py

+from typing import Any, Awaitable, Callable
+
+
+class BaseStrategy(ABC):


BaseEvaluationExecutionStrategy, or something similar. BaseStrategy is too general to understand its purpose.

-> BaseEvaluationStrategy

weizhang25 · 2026-01-15T17:33:23Z

openjudge/strategy/__init__.py

@@ -0,0 +1,8 @@
+# -*- coding: utf-8 -*-
+"""Strategy module for evaluation workflows."""


The folder name should be renamed to something like eval_execution_strategy.

-> evaluation_strategy

weizhang25 · 2026-01-15T17:34:14Z

openjudge/strategy/direct_strategy.py

+from openjudge.strategy.base import BaseStrategy
+
+
+class DirectStrategy(BaseStrategy):


class LocallExecutionStrategy.

-> LocalEvaluationStrategy

weizhang25 · 2026-01-15T17:38:19Z

openjudge/strategy/majority_vote_strategy.py

+from openjudge.strategy.base import BaseStrategy
+
+
+class MajorityVoteStrategy(BaseStrategy):


nitpick. vote -> voting. class MajorityVotingStrategy

代码改动太多，先删除具体实现

weizhang25 · 2026-01-15T17:41:09Z

openjudge/strategy/majority_vote_strategy.py

+
+        # For now, return the first result as a placeholder
+        # In a real implementation, this would aggregate the results appropriately
+        return results[0]


Better remove this voting strategy from this PR, if you don't plan to correctly implement.
Also this PR is too large. Consider adding concrete strategy classes in a separate PR. Let this CR only focus on the core structure change.

同上，删除具体实现

weizhang25 · 2026-01-15T17:49:27Z

openjudge/runner/controller/base.py

@@ -0,0 +1,49 @@
+# -*- coding: utf-8 -*-
+"""Base class for execution controllers.


avoid using "base.py", too generic. base_controller.py

-> base_resource_executor.py

weizhang25 · 2026-01-15T17:54:41Z

openjudge/runner/controller/base.py

+R = TypeVar("R")
+
+
+class BaseController(ABC):


On a second thought. The name Controller is vague. This class refers to computational resource allocation, and using the resource to run callable. It's more like a concept of "ComputationResourceWrapper".

What to "Control"? The word control is kind of misleading..

-> BaseResourceExecutor

weizhang25 · 2026-01-16T01:52:53Z

openjudge/graders/base_grader.py

+        Called by the Runner to inject the controller.
+        """
+        # 1. Apply mapper if available to transform input data
+        data = parse_data_with_mapper(kwargs, self.mapper)


为什么要在grader内部生成data？而不是以前在外部生成

主要是考虑GraderConfig还有必要吗，所有mapper跟Grader一对一绑定，跟当前的strategy一样

weizhang25 · 2026-01-16T01:53:54Z

openjudge/graders/base_grader.py

+        # 2. Wrap the atomic evaluation task to submit to the controller
+        async def managed_fn(**runtime_kwargs):
+            # Submit to Controller for execution
+            if controller is None:


为什么不强制用local controller兜底？

这个是给到测试使用的，测试可能不需要资源，直接跑就行

weizhang25 · 2026-01-16T01:54:34Z

openjudge/graders/base_grader.py

+
+        # 3. Execute the strategy
+        # The strategy receives a function with resource management capabilities
+        if self.strategy:


同理，为什么不强制direct strategy兜底？

同上，给到测试使用的边界情况，不过这里默认是有strategy初始化，可以修改

weizhang25 · 2026-01-16T01:58:27Z

openjudge/runner/grading_runner.py

    def __init__(
        self,
-        grader_configs: Dict[str, GraderConfig | BaseGrader | Tuple[BaseGrader, Dict[str, str] | Callable | None]],
+        graders: Dict[str, "BaseGrader"],


应该考虑在runner的_arun()里，对每一个data record，创建一个grader 实例，避免一个grader 实例在asyncio里同时处理多条数据记录。这会带来结果错误。

在当前的使用下，去通过config初始化实例确实没问题，但是如果涉及icl引入动态fewshot，多个示例化内存就不高了，这个就必须高代码解决，无法配置化了

ployts changed the title ~~refactor(strategy-executor): implement Strategy and Executor patterns~~ [WIP] refactor(strategy-executor): implement Strategy and Executor patterns Jan 15, 2026

gemini-code-assist bot reviewed Jan 15, 2026

View reviewed changes

weizhang25 reviewed Jan 15, 2026

View reviewed changes

weizhang25 reviewed Jan 16, 2026

View reviewed changes

ployts closed this Jan 16, 2026

	self.strategy = strategy
	self.strategy = strategy or DirectStrategy()

		@@ -0,0 +1,53 @@
		# -- coding: utf-8 --
		"""Base class for evaluation strategies.

		from typing import Any, Awaitable, Callable


		class BaseStrategy(ABC):

		@@ -0,0 +1,8 @@
		# -- coding: utf-8 --
		"""Strategy module for evaluation workflows."""

		from openjudge.strategy.base import BaseStrategy


		class DirectStrategy(BaseStrategy):

		from openjudge.strategy.base import BaseStrategy


		class MajorityVoteStrategy(BaseStrategy):

		@@ -0,0 +1,49 @@
		# -- coding: utf-8 --
		"""Base class for execution controllers.

[WIP] refactor(strategy-executor): implement Strategy and Executor patterns #69

[WIP] refactor(strategy-executor): implement Strategy and Executor patterns #69

Uh oh!

Conversation

ployts commented Jan 15, 2026

OpenJudge Version

Description

Checklist

Uh oh!

gemini-code-assist bot commented Jan 15, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ployts Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

ployts Jan 16, 2026 •

edited

Loading