diff --git a/docs/policy-reference.md b/docs/policy-reference.md index 0a8ab11..97abe88 100644 --- a/docs/policy-reference.md +++ b/docs/policy-reference.md @@ -699,19 +699,21 @@ configured default behavior. } ``` -Resolution order is deterministic: - -1. repo + action -2. repo + intent -3. repo default -4. global action -5. global intent -6. global default -7. no override - -Each route is an object with optional `model` and `thinking` fields. A route can -set only one field; the bridge appends only the flags that are configured on the -selected route. Supported `thinking` values are `off`, `minimal`, `low`, +Matching routes are composed in deterministic order, from broad defaults to +specific overrides: + +1. global default +2. repo default +3. global intent, complexity, then action +4. repo intent, complexity, then action + +Each route is an object with optional `model` and `thinking` fields. Every +matching route overrides only the fields it defines, so a repo default such as +`{"thinking": "xhigh"}` keeps the inherited model and still allows global +action or complexity rules to select cheaper settings. Repo-specific action, +complexity, and intent rules remain the final and most specific overrides. The +bridge appends only the flags configured in the composed result. Supported +`thinking` values are `off`, `minimal`, `low`, `medium`, `high`, `xhigh`, `adaptive`, and `max`; invalid values fail policy load with a clear error. Use provider-qualified model IDs such as `openai/gpt-5.4-mini`; bare model names can resolve to a different provider diff --git a/src/github_agent_bridge/policy.py b/src/github_agent_bridge/policy.py index d2c317d..0060cea 100644 --- a/src/github_agent_bridge/policy.py +++ b/src/github_agent_bridge/policy.py @@ -60,6 +60,14 @@ def summary(self) -> str: parts.append(f"thinking={self.thinking}") return " ".join(parts) if parts else "OpenClaw default model route" + def overlay(self, override: ModelRoute | None) -> ModelRoute: + if override is None: + return self + return ModelRoute( + model=override.model if override.model is not None else self.model, + thinking=override.thinking if override.thinking is not None else self.thinking, + ) + @dataclass(frozen=True) class RepoModelRoutes: @@ -370,19 +378,14 @@ def model_route_for( if complexity_key not in ALLOWED_COMPLEXITIES: complexity_key = "substantive" repo_routes = self.model_routes.by_repo.get(repo_key) + route = ModelRoute().overlay(self.model_routes.default) if repo_routes: - route = ( - repo_routes.by_action.get(action_key) - or repo_routes.by_complexity.get(complexity_key) - or repo_routes.by_intent.get(intent_key) - or repo_routes.default - ) - if route: - return route - return ( - self.model_routes.by_action.get(action_key) - or self.model_routes.by_complexity.get(complexity_key) - or self.model_routes.by_intent.get(intent_key) - or self.model_routes.default - or ModelRoute() - ) + route = route.overlay(repo_routes.default) + route = route.overlay(self.model_routes.by_intent.get(intent_key)) + route = route.overlay(self.model_routes.by_complexity.get(complexity_key)) + route = route.overlay(self.model_routes.by_action.get(action_key)) + if repo_routes: + route = route.overlay(repo_routes.by_intent.get(intent_key)) + route = route.overlay(repo_routes.by_complexity.get(complexity_key)) + route = route.overlay(repo_routes.by_action.get(action_key)) + return route diff --git a/tests/test_policy.py b/tests/test_policy.py index d914928..240d257 100644 --- a/tests/test_policy.py +++ b/tests/test_policy.py @@ -211,6 +211,83 @@ def test_policy_from_file_loads_model_routes_and_resolution_order(tmp_path): assert route.thinking == "low" +def test_model_routes_compose_partial_repo_default_with_global_specific_routes(tmp_path): + policy_file = tmp_path / "policy.json" + policy_file.write_text( + """{ + "modelRoutes": { + "default": {"model": "default-model", "thinking": "medium"}, + "byAction": { + "sync_after_merge": {"model": "sync-model", "thinking": "low"} + }, + "byComplexity": { + "mechanical": {"model": "mechanical-model", "thinking": "minimal"} + }, + "byRepo": { + "gisce/github-agent-bridge": { + "default": {"thinking": "xhigh"} + } + } + } + }""" + ) + + policy = Policy.from_file(policy_file) + + route = policy.model_route_for( + "gisce/github-agent-bridge", "reply_comment", "work_allowed" + ) + assert route.model == "default-model" + assert route.thinking == "xhigh" + + route = policy.model_route_for( + "gisce/github-agent-bridge", "sync_after_merge", "work_allowed" + ) + assert route.model == "sync-model" + assert route.thinking == "low" + + route = policy.model_route_for( + "gisce/github-agent-bridge", "reply_comment", "work_allowed", "mechanical" + ) + assert route.model == "mechanical-model" + assert route.thinking == "minimal" + + +def test_model_routes_compose_matching_rules_field_by_field(tmp_path): + policy_file = tmp_path / "policy.json" + policy_file.write_text( + """{ + "modelRoutes": { + "default": {"model": "default-model", "thinking": "medium"}, + "byIntent": { + "review_only": {"model": "review-model"} + }, + "byComplexity": { + "mechanical": {"thinking": "low"} + }, + "byAction": { + "reply_comment": {"model": "comment-model"} + }, + "byRepo": { + "gisce/erp": { + "byAction": { + "reply_comment": {"thinking": "high"} + } + } + } + } + }""" + ) + + policy = Policy.from_file(policy_file) + route = policy.model_route_for( + "gisce/erp", "reply_comment", "review_only", "mechanical" + ) + + assert route.model == "comment-model" + assert route.thinking == "high" + + def test_policy_from_file_rejects_invalid_model_route_thinking(tmp_path): policy_file = tmp_path / "policy.json" policy_file.write_text('{"modelRoutes": {"byAction": {"sync_after_merge": {"thinking": "turbo"}}}}')