From 944d4b93e105c49a74217fa003f3aa527f5f6d92 Mon Sep 17 00:00:00 2001 From: Claude Bot Date: Thu, 2 Jul 2026 09:46:41 +0800 Subject: [PATCH] fix: design: enforce judge-must-be-stronger-than-executor across all judgment paths (#208) Closes #208 --- claude_ops.py | 72 +++++++++++++++++++-- model_registry.py | 161 ++++++++++++++++++++++++++++++++++++++++++++++ worker.py | 4 ++ 3 files changed, 231 insertions(+), 6 deletions(-) diff --git a/claude_ops.py b/claude_ops.py index a5afc33..26fdad1 100644 --- a/claude_ops.py +++ b/claude_ops.py @@ -20,7 +20,15 @@ _last_reviewer_tokens: tuple[int, int] = (0, 0) from git_ops import git, gh -from model_registry import DEFAULT_AI_MODEL, REVIEW_MODEL +from model_registry import ( + DEFAULT_AI_MODEL, + REVIEW_MODEL, + META_JUDGE_MODEL, + NoStrongerModelError, + effective_model_for_alias, + is_judge_stronger_than_executor, + judge_model_stronger_than, +) # --------------------------------------------------------------------------- # Constants @@ -273,10 +281,52 @@ def truncate_log(text: str, max_chars: int = 12000) -> str: # --------------------------------------------------------------------------- +def _select_judge_model(executor_effective_model: str | None) -> str: + """Pick the reviewer model: STRICTLY stronger than the executor (issue #208). -def _call_reviewer_api(prompt: str, review_tier: str) -> tuple[dict | None, bool]: + Defaults to ``REVIEW_MODEL`` (glm-5.2) when the executor is unknown or + sub-ceiling — glm-5.2 is strictly stronger than every executor tier below + it (glm-5-turbo, glm-4.7). When the executor is already at the judge + ceiling (e.g. the hard-tier glm-5.2), the judge escalates to + ``META_JUDGE_MODEL`` so judge > executor is never violated. + """ + if not executor_effective_model: + return REVIEW_MODEL + try: + judge = judge_model_stronger_than(executor_effective_model) + except NoStrongerModelError: + # Executor is at the meta-judge tier already — cannot go higher, but + # this branch is unreachable in practice (executors never run on the + # meta-judge). Keep REVIEW_MODEL and let the guard below catch it. + return REVIEW_MODEL + if not is_judge_stronger_than_executor(judge, executor_effective_model): + log.warning( + "judge %s not stronger than executor %s — falling back to %s", + judge, executor_effective_model, REVIEW_MODEL, + ) + return REVIEW_MODEL + if judge != REVIEW_MODEL: + log.info( + "Reviewer judge escalated to %s (executor was %s) — judge > executor", + judge, executor_effective_model, + ) + return judge + + +def _call_reviewer_api( + prompt: str, + review_tier: str, + executor_effective_model: str | None = None, +) -> tuple[dict | None, bool]: """Direct API call to reviewer model — guarantees JSON output, no multi-turn narrative. - Returns (result, was_rate_limited).""" + Returns (result, was_rate_limited). + + ``executor_effective_model`` is the model that produced the diff under + review. The reviewer is ALWAYS a strictly stronger model than that executor + (issue #208): for sub-ceiling executors this is ``REVIEW_MODEL`` (glm-5.2); + for an executor already at the judge ceiling it escalates to + ``META_JUDGE_MODEL`` so the invariant judge > executor always holds. + """ global _last_reviewer_tokens _last_reviewer_tokens = (0, 0) import urllib.request as _urllib_request @@ -285,7 +335,7 @@ def _call_reviewer_api(prompt: str, review_tier: str) -> tuple[dict | None, bool base_url = env.get("ANTHROPIC_BASE_URL", "https://api.z.ai/api/anthropic").rstrip("/") if not token: return None - model = REVIEW_MODEL # always opus for review + model = _select_judge_model(executor_effective_model) try: payload = json.dumps({ "model": model, @@ -406,6 +456,12 @@ def run_ai_review(worktree_path: str, base_branch: str, title: str, agent = "deep-reviewer" if review_tier == "deep" else "reviewer" + # The judge (reviewer) must be STRICTLY stronger than the executor that + # produced this diff (issue #208). ``model_alias`` is the executor's alias; + # resolve it to an effective model so _call_reviewer_api can pick a stronger + # judge (escalating to the meta-judge when the executor is at the ceiling). + executor_effective_model = effective_model_for_alias(model_alias) + # Build review prompt review_prompt = ( f"You are a strict code reviewer. Respond with ONLY a JSON object, no other text, " @@ -428,7 +484,9 @@ def run_ai_review(worktree_path: str, base_branch: str, title: str, # --agent reviewer does multi-turn conversation and returns narrative text, not JSON any_rate_limited = False - review_result, was_rl = _call_reviewer_api(review_prompt, review_tier) + review_result, was_rl = _call_reviewer_api( + review_prompt, review_tier, executor_effective_model, + ) any_rate_limited = any_rate_limited or was_rl if review_result is not None: return review_result, False @@ -439,7 +497,9 @@ def run_ai_review(worktree_path: str, base_branch: str, title: str, f'Review this diff. Respond with ONLY JSON: {{"approved": true, "severity": "none", "summary": "...", "findings": [], "merge_risk": "low"}}\n\n' f"Issue #{issue_number}: {title}\n\nDiff (first 5000 chars):\n{diff[:5000]}" ) - review_result, was_rl = _call_reviewer_api(simple_prompt, review_tier) + review_result, was_rl = _call_reviewer_api( + simple_prompt, review_tier, executor_effective_model, + ) any_rate_limited = any_rate_limited or was_rl if review_result is not None: return review_result, False diff --git a/model_registry.py b/model_registry.py index 234c2ff..1faed1d 100644 --- a/model_registry.py +++ b/model_registry.py @@ -15,6 +15,12 @@ ESCALATION_MODEL_ALIAS, # opus — used when retry_count >= 2 or hard/deep ESCALATION_EFFECTIVE_MODEL, # glm-5.2 — maps to escalation alias HIGH_PEAK_FALLBACK_MODEL, # glm-5-turbo — lighter fallback during peak + # judge-must-be-stronger-than-executor (issue #208): + judge_model_stronger_than, # pick a strictly stronger judge EFFECTIVE model + judge_alias_stronger_than, # pick a strictly stronger judge ALIAS + is_judge_stronger_than_executor, + requires_meta_judge, + META_JUDGE_MODEL, ) """ @@ -61,6 +67,12 @@ #: Fallback model during high-peak windows to avoid 429s HIGH_PEAK_FALLBACK_MODEL: str = os.environ.get("HIGH_PEAK_FALLBACK_MODEL", "glm-5-turbo") +#: Meta-judge model — strictly stronger than every executor and every judge. +#: Used to adversarially refute judge verdicts on critical paths (security, +#: self-modifying code, migrations) per issue #208 rule 3. Configurable so ops +#: can point it at whichever strongest endpoint is available. +META_JUDGE_MODEL: str = os.environ.get("META_JUDGE_MODEL", "gpt-5.5-highest") + #: Model alias for cross-repo org coherence patrol (configurable via env) ORG_COHERENCE_MODEL_ALIAS: str = os.environ.get("ORG_COHERENCE_MODEL", "opus") @@ -132,3 +144,152 @@ def get_provider_model_seed_rows(provider: str = "zai-glm") -> list[tuple]: 1, # effective_concurrency )) return rows + + +# --------------------------------------------------------------------------- +# Model strength ranking — judge-must-be-stronger-than-executor (issue #208) +# --------------------------------------------------------------------------- +# +# Principle: every layer that JUDGES another layer's work must use a strictly +# stronger model than the executor whose work it is judging. The helpers below +# encode a single strength ordering and guarantee that a selected judge is +# always stronger than its executor, so the invariant is enforced in code +# rather than relying on each call site to remember it. + +class NoStrongerModelError(RuntimeError): + """Raised when no strictly-stronger model exists for the given executor. + + The executor is already at the top of the available ladder — the caller + must escalate to the meta-judge provider (issue #208 rule 3). + """ + + +#: Strength ordering of every model that can appear as an executor or a judge. +#: Higher number = stronger. Context-window suffixes (e.g. ``[1m]``) are +#: stripped before comparison — they extend context, not reasoning strength. +#: +#: Two families are ranked on a single ladder so cross-family judgment is +#: well-defined: a gpt-5.5 judge may not review a gpt-5.5 executor (equal), +#: and nothing below glm-5.2 may act as a judge (issue #208 tier table). +MODEL_STRENGTH: dict[str, int] = { + # --- executor tier (cheapest — does the actual code writing) --- + "glm-5-turbo": 10, # high-peak fallback / lightest executor + "gpt-5.4-mini": 10, # codex docs executor + "glm-4.7": 20, # normal/docs executor + # --- judge tier (reviews / empty-diff judgment / verify-first) --- + "glm-5.2": 40, # hard executor AND default judge + "gpt-5.5": 40, # codex escalation / judge tier + # --- meta-judge tier (adversarial refute of judge verdicts) --- + META_JUDGE_MODEL: 50, +} + +#: Labels that mark a job as a critical path requiring the meta-judge rather +#: than the plain judge (issue #208 rule 3): security-sensitive PRs, +#: self-modifying code, and schema migrations. +META_JUDGE_TRIGGER_LABELS: frozenset[str] = frozenset({ + "claude-hard", "security", "self-edit", "self-fix", "migration", +}) + +#: Minimum strength a model must have to act as a judge (issue #208 tier table). +JUDGE_TIER_FLOOR: int = 40 + + +def normalize_model(model: str) -> str: + """Strip context-window / variant suffixes for strength comparison. + + ``"glm-5.2[1m]"`` → ``"glm-5.2"``. Unknown shapes pass through unchanged. + """ + if not model: + return model + # Drop a trailing ``[...]`` bracketed suffix (e.g. the 1M-context marker). + if "[" in model: + return model.split("[", 1)[0].strip() + return model.strip() + + +def model_strength(model: str) -> int: + """Return the integer strength rank of ``model``. + + Unknown models default to ``JUDGE_TIER_FLOOR`` so a novel executor can never + accidentally be judged by a weaker model — the safe direction is to + *over*-estimate the executor's strength (issue #208 fail-safe). + """ + return MODEL_STRENGTH.get(normalize_model(model), JUDGE_TIER_FLOOR) + + +def effective_model_for_alias(model_alias: str) -> str: + """Return the effective model string backing a Claude Code model alias. + + ``"opus"`` → ``"glm-5.2[1m]"``, ``"sonnet"``/``"haiku"`` → ``"glm-4.7"``. + Strings that are already effective model identifiers (e.g. ``"glm-5-turbo"``) + are returned unchanged so callers can pass either form. + """ + for cfg in MODEL_CONFIGS.values(): + if cfg["model_alias"] == model_alias: + return cfg["effective_model"] + return model_alias + + +def alias_for_effective_model(effective_model: str) -> str | None: + """Reverse lookup: the Claude Code alias for an effective model, or None.""" + norm = normalize_model(effective_model) + for cfg in MODEL_CONFIGS.values(): + if normalize_model(cfg["effective_model"]) == norm: + return cfg["model_alias"] + return None + + +def is_judge_stronger_than_executor(judge_model: str, executor_model: str) -> bool: + """True iff ``judge_model`` is STRICTLY stronger than ``executor_model``.""" + return model_strength(judge_model) > model_strength(executor_model) + + +def judge_model_stronger_than(executor_model: str) -> str: + """Return the weakest model STRICTLY stronger than ``executor_model``. + + The result is always at least ``JUDGE_TIER_FLOOR`` strength — executors are + never judged by a sub-judge-tier model (issue #208 tier table). Raises + :class:`NoStrongerModelError` if the executor is already at the meta-judge + tier (nothing stronger is configured). + """ + exec_strength = model_strength(executor_model) + # Candidate judges: every model at or above the judge tier, sorted weakest-first. + candidates = sorted( + (m for m, s in MODEL_STRENGTH.items() if s >= JUDGE_TIER_FLOOR and s > exec_strength), + key=lambda m: MODEL_STRENGTH[normalize_model(m)], + ) + if not candidates: + raise NoStrongerModelError( + f"no model strictly stronger than {executor_model!r} is configured " + f"(strength={exec_strength}); it is already at the top tier" + ) + return candidates[0] + + +def judge_alias_stronger_than(executor_alias: str) -> str: + """Return a Claude Code model alias STRICTLY stronger than ``executor_alias``. + + Maps the executor alias to its effective model, selects a strictly stronger + judge model, then maps back to the alias that invokes it via ``--model``. + Raises :class:`NoStrongerModelError` when the executor is already at the + alias ceiling (e.g. ``opus``) — the caller must then escalate to the + meta-judge provider, which accepts an arbitrary model string. + """ + executor_effective = effective_model_for_alias(executor_alias) + judge_effective = judge_model_stronger_than(executor_effective) + alias = alias_for_effective_model(judge_effective) + if alias is None: + raise NoStrongerModelError( + f"judge model {judge_effective!r} (stronger than {executor_alias!r}) " + f"has no Claude Code alias — escalate via the meta-judge provider" + ) + return alias + + +def requires_meta_judge(labels) -> bool: + """True if the job's labels mark a critical path needing the meta-judge. + + Critical paths (issue #208 rule 3): security-sensitive PRs, self-modifying + code, and schema migrations. ``labels`` may be any iterable of label names. + """ + return bool(set(META_JUDGE_TRIGGER_LABELS).intersection(labels or ())) diff --git a/worker.py b/worker.py index 964effe..6f2cb3e 100644 --- a/worker.py +++ b/worker.py @@ -59,6 +59,10 @@ def notify_crash_loop_abort(*a, **k): pass from model_registry import ( ESCALATION_MODEL_ALIAS, ESCALATION_EFFECTIVE_MODEL, HIGH_PEAK_FALLBACK_MODEL, + META_JUDGE_MODEL, + NoStrongerModelError, + judge_alias_stronger_than, + requires_meta_judge, ) # Watchdog self-fix feedback loop (issue #7): record error events so the # watchdog can detect crash loops / worktree failures / systemic errors.