From abde3900459195fce46ac36de5d2ba6fc072f75a Mon Sep 17 00:00:00 2001 From: Zwitschi Date: Tue, 10 Nov 2026 09:12:26 +0100 Subject: [PATCH] feat(tools): every image judged by two models, every generated texture drawn by two checkpoints A single vision model has been wrong in both directions on this project - it missed real defects and disputed frames that were correct - so agreement between different model families is worth more than one confident opinion. judge_screenshot.py now runs two models by default (openai/gpt-4o-mini + qwen/qwen3.7-flash, both served on OpenRouter; claude-3.5-haiku is gone, which the first attempt discovered with a 404). combine() reconciles them: agreement carries the verdict with the mean confidence, disagreement reports 'uncertain' and keeps both reasons rather than letting the more confident model win. --model still forces a single judgement, and every line prints which model said what. comfyui_generate.py generates the same prompt and seed with two checkpoints, scores both by high-frequency detail (mean absolute Laplacian, which catches blurry or flat results), keeps the loser as __.png with its sidecar, and moves the winner onto --out. --alt-model restores the single run. Tests cover the reconciliation - agreement, disagreement, unknown verdicts, and no verdicts at all - and the suite is at 103. --- Tools/README.md | 4 +- Tools/comfyui_generate.py | 61 ++++++++++++++++++++++++++++++ Tools/judge_screenshot.py | 63 +++++++++++++++++++++++++++---- Tools/tests/test_judge_combine.py | 40 ++++++++++++++++++++ 4 files changed, 158 insertions(+), 10 deletions(-) create mode 100644 Tools/tests/test_judge_combine.py diff --git a/Tools/README.md b/Tools/README.md index 2d1a317..b7da761 100644 --- a/Tools/README.md +++ b/Tools/README.md @@ -69,7 +69,7 @@ Geographic tools live in [`Geo/tools/`](../Geo/README.md) and the Blender-side b | Tool | Purpose | | --- | --- | | `render_screenshots.py` / `render_screenshots.sh` | Rebuilds the review map, runs the game, captures the shot list into `Screenshots/` | -| `judge_screenshot.py` | Vision-model pre-check: judges a screenshot against an expected-scene description (`--image` + `--expect`, or `--shots` for every shot in the README). Reads `OPENROUTER_API_KEY` from `.env` (ignored). Optional, advisory only — the model is unreliable on this content and never replaces a human verdict. | +| `judge_screenshot.py` | Vision-model pre-check: judges a screenshot against an expected-scene description (`--image` + `--expect`, or `--shots` for every shot in the README). Reads `OPENROUTER_API_KEY` from `.env` (ignored). Judged by **two models by default** (`--models a,b`), reconciled: agreement carries the verdict, disagreement reports `uncertain` with both reasons. Optional, advisory only — never replaces a human verdict. | `--image` + `--expect`; `--model` for a single judgement | | `visual_loop.py` | The reference-vs-render loop. `--verify` gates each capture against its expectation in `visual_targets.json` (blank/short/wrong-size/tone-less shots fail); `--compare` measures luminance, contrast, edge density, palette and 3×3 composition against the reference photograph and turns the deltas into modelling actions naming the skill to consult (`blender-modeler`, `archviz`, `materials`, `lookdev`, `camera-cinematography`); `--iteration N` writes the artefacts under `Art/reference/iterations/N/`, which is **gitignored until the owner confirms a shot**. | | `visual_targets.json` | Per-shot expectation and reference photograph for `visual_loop.py`. Every entry must be a demo-map shot. | @@ -94,7 +94,7 @@ Geographic tools live in [`Geo/tools/`](../Geo/README.md) and the Blender-side b | --- | --- | | `input_emulation.py` | Pure-stdlib `ctypes`/`fcntl` driver that creates a virtual keyboard+mouse on `/dev/uinput`. The desktop treats it as real hardware, so it proves whether OS input actually reaches the game. Needs `/dev/uinput` writable (ACL on this host). | | `input_test.py` | Injects a scripted sequence into the focused window: `--sequence move:2,look:3,jump:1,reset:1,pause:1,click`. | -| `comfyui_generate.py` | Local ComfyUI driver: list checkpoints, run txt2img through the HTTP API, and write a provenance sidecar (model, prompt, seed, full graph). Local, human-triggered. | `models`; `txt2img --prompt ... --out ` | `python3 Tools/comfyui_generate.py models` | +| `comfyui_generate.py` | Local ComfyUI driver: list checkpoints, run txt2img through the HTTP API, and write a provenance sidecar (model, prompt, seed, full graph). **Two checkpoints by default**: both are generated from the same prompt and seed, scored by high-frequency detail, and the better one takes `--out` while the loser is kept beside it. Local, human-triggered. | `models`; `txt2img --prompt ... --out [--alt-model ...]` | `python3 Tools/comfyui_generate.py models` | | `comfyui_character_blueprint.py` | Generates a character blueprint sheet - front, side, back and head views plus a contact sheet - from one description, with fixed seeds and a JSON record. | `--desc`, `--out`, `--model`, `--size` | `python3 Tools/comfyui_character_blueprint.py --desc "lean pedestrian, navy tee, jeans" --out Art/characters/blueprints` | | `character_match.py` | Compares a rendered character against a blueprint: figure aspect, fill and coverage deltas, mean CIE76 colour distance, dominant-palette distance, and a side-by-side montage. | `--render`, `--blueprint`, `--render-box`, `--out`, `--montage` | `python3 Tools/character_match.py --render Screenshots/demo/review-demo-third-person-front.png --blueprint Art/characters/blueprints/front.png --render-box 0.42,0.46,0.56,0.97 --montage /tmp/gigline/match.png` | | `generate_character_textures.py` | Procedural skin-pore and hair-strand textures with sidecars; no model involved. | `--out` | `python3 Tools/generate_character_textures.py --out Art/textures/generated` | diff --git a/Tools/comfyui_generate.py b/Tools/comfyui_generate.py index 268c0e9..34d770b 100644 --- a/Tools/comfyui_generate.py +++ b/Tools/comfyui_generate.py @@ -40,6 +40,26 @@ import urllib.request HOST = os.environ.get("COMFYUI_HOST", "http://127.0.0.1:8188") DEFAULT_MODEL = "juggernautXL_ragnarok.safetensors" +# A second checkpoint by default: one model's texture is a lottery ticket, two is a comparison. Both are +# generated from the identical prompt and seed, then scored, and only the winner takes the output path. +DEFAULT_ALT_MODEL = "sd_xl_base_1.0.safetensors" + + +def score_image(path): + """A deterministic, offline 'how much detail did this texture actually get' score. + + Mean absolute Laplacian: high-frequency energy. Generated textures that are blurry or flat score low, + which is the failure this comparison exists to catch. Pillow is used only here, so a host without it + still generates and simply skips scoring. + """ + try: + from PIL import Image, ImageFilter + except ImportError: # pragma: no cover - host without Pillow + return None + with Image.open(path) as image: + grey = image.convert("L").filter(ImageFilter.FIND_EDGES) + pixels = list(grey.getdata()) + return sum(pixels) / max(1, len(pixels)) def api(path: str) -> dict: @@ -156,6 +176,42 @@ def generate(args) -> int: +def generate_pair(args) -> int: + """Generate the same prompt with two checkpoints and keep the better one at `--out`. + + Both are written beside the output as `__.png` with their own sidecars, so the losing + candidate is kept for inspection instead of thrown away. The winner is chosen by `score_image()`; when + Pillow is missing or the scores tie, the primary model keeps the output and the run says so. + """ + import dataclasses + + stem, ext = os.path.splitext(args.out) + candidates = [] + for label, model in (("primary", args.model), ("alternate", args.alt_model)): + out = f"{stem}__{model.replace('/', '_')[:40]}{ext}" + sub_args = dataclasses.replace(args, model=model, out=out) + if generate(sub_args) != 0: + print(f"{label} model {model} failed; continuing with what worked") + continue + candidates.append((model, out, score_image(out))) + if not candidates: + return 1 + scored = [c for c in candidates if c[2] is not None] + if not scored: + print(f"no scoring available; keeping {candidates[0][1]} as the output") + winner = candidates[0] + else: + winner = max(scored, key=lambda c: c[2]) + for model, path, score in candidates: + print(f" {model}: {score if score is None else round(score, 2)} {path}") + os.replace(winner[1], args.out) + sidecar = winner[1] + ".json" + if os.path.exists(sidecar): + os.replace(sidecar, args.out + ".json") + print(f"chose {winner[0]} -> {args.out}") + return 0 + + def main(argv=None) -> int: argv = sys.argv[1:] if argv is None else argv ap = argparse.ArgumentParser(description=__doc__, @@ -166,6 +222,9 @@ def main(argv=None) -> int: t.add_argument("--prompt", required=True) t.add_argument("--negative", default="text, watermark, signature, blurry, low quality") t.add_argument("--model", default=DEFAULT_MODEL) + t.add_argument("--alt-model", default=DEFAULT_ALT_MODEL, + help=f"second checkpoint generated from the same prompt and seed, then compared " + f"(default {DEFAULT_ALT_MODEL}); pass an empty string to generate one image only") t.add_argument("--size", default="1024x1024") t.add_argument("--seed", type=int, default=None) t.add_argument("--steps", type=int, default=28) @@ -181,6 +240,8 @@ def main(argv=None) -> int: for name in checkpoints(): print(name) return 0 + if args.alt_model and args.alt_model != args.model: + return generate_pair(args) return generate(args) diff --git a/Tools/judge_screenshot.py b/Tools/judge_screenshot.py index 1f0e556..7992dd9 100755 --- a/Tools/judge_screenshot.py +++ b/Tools/judge_screenshot.py @@ -35,6 +35,39 @@ import requests API_URL = "https://openrouter.ai/api/v1/chat/completions" DEFAULT_MODEL = "openai/gpt-4o-mini" +# Two models by default. A single vision model is advisory and has been wrong in both directions on this +# project, so every image is judged twice by different families and the pair is reconciled below. +DEFAULT_MODELS = ( + "openai/gpt-4o-mini", + "qwen/qwen3.7-flash", +) + + +def combine(judgements): + """Reconcile two or more model judgements into one verdict. + + Agreement carries the verdict and the mean confidence. Disagreement is reported as `uncertain` with + both reasons kept, because picking the more confident model would hide exactly the case worth looking + at - the models seeing different things. + """ + usable = [j for j in judgements if j.get("verdict") in ("yes", "no", "uncertain")] + if not usable: + return {"verdict": "unknown", "confidence": 0.0, + "reason": "no usable verdicts", "models": {}} + verdicts = {j["verdict"] for j in usable} + by_model = {j.get("model", "?"): {"verdict": j.get("verdict"), + "confidence": j.get("confidence", 0.0), + "reason": j.get("reason", "")} for j in usable} + if len(verdicts) == 1: + verdict = verdicts.pop() + confidence = sum(j.get("confidence", 0.0) for j in usable) / len(usable) + reason = usable[0].get("reason", "") + else: + verdict = "uncertain" + confidence = min(j.get("confidence", 0.0) for j in usable) + reason = "models disagree: " + " | ".join( + f"{j.get('model')}={j.get('verdict')}: {j.get('reason', '')[:120]}" for j in usable) + return {"verdict": verdict, "confidence": confidence, "reason": reason, "models": by_model} # Project root = parent of the Tools/ directory the script lives in. PROJECT_ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), "..")) @@ -87,6 +120,7 @@ def build_messages(image_path, expect): def judge(api_key, image_path, expect, model, timeout=60): + """One model's judgement. `combine()` reconciles several of these.""" payload = { "model": model, "messages": build_messages(image_path, expect), @@ -109,7 +143,9 @@ def judge(api_key, image_path, expect, model, timeout=60): m = re.search(r"\{.*\}", content, re.S) if not m: sys.exit(f"error: model returned non-JSON:\n{content[:500]}") - return json.loads(m.group(0)) + verdict = json.loads(m.group(0)) + verdict["model"] = model + return verdict def parse_shots_from_targets(): @@ -144,19 +180,24 @@ def main(): g.add_argument("--shots", action="store_true", help="evaluate every review shot listed in Screenshots/README.md") ap.add_argument("--expect", help="what the shot is expected to show (required with --image)") - ap.add_argument("--model", default=DEFAULT_MODEL, help=f"OpenRouter model (default {DEFAULT_MODEL})") + ap.add_argument("--models", default=",".join(DEFAULT_MODELS), + help=f"comma-separated OpenRouter models; every image is judged by all of them " + f"and the results reconciled (default {','.join(DEFAULT_MODELS)})") + ap.add_argument("--model", default=None, + help="single model, overriding --models (one judgement, no reconciliation)") ap.add_argument("--verbose", action="store_true") ap.add_argument("--timeout", type=int, default=60) args = ap.parse_args() api_key = load_api_key() + models = [args.model] if args.model else [m.strip() for m in args.models.split(",") if m.strip()] if args.shots: shots = parse_shots_from_targets() if not shots: sys.exit("error: no shots found in Tools/visual_targets.json") n = len(shots) - print(f"Judging {n} screenshots with {args.model} ...") + print(f"Judging {n} screenshots with {' + '.join(models)} ...") ok = 0 for i, (fname, expect) in enumerate(shots, 1): path = os.path.join(PROJECT_ROOT, fname) @@ -164,10 +205,11 @@ def main(): print(f"[{i}/{n}] {fname}: MISSING (skip)") continue try: - res = judge(api_key, path, expect, args.model, args.timeout) + judged = [judge(api_key, path, expect, model, args.timeout) for model in models] except SystemExit as e: print(f"[{i}/{n}] {fname}: ERROR {e}") continue + res = combine(judged) if len(judged) > 1 else judged[0] verdict = res.get("verdict", "unknown") conf = res.get("confidence", 0.0) reason = res.get("reason", "") @@ -181,10 +223,15 @@ def main(): if not args.expect: ap.error("--expect is required with --image") - res = judge(api_key, args.image, args.expect, args.model, args.timeout) - print(json.dumps(res, indent=2) if args.verbose - else f"verdict={res.get('verdict')} confidence={res.get('confidence')} " - f"reason={res.get('reason', '')}") + judged = [judge(api_key, args.image, args.expect, model, args.timeout) for model in models] + res = combine(judged) if len(judged) > 1 else judged[0] + if args.verbose: + print(json.dumps(res, indent=2)) + else: + print(f"verdict={res.get('verdict')} confidence={res.get('confidence')} " + f"reason={res.get('reason', '')}") + for model, detail in res.get("models", {}).items(): + print(f" {model}: {detail['verdict']} ({detail['confidence']:.2f})") if __name__ == "__main__": diff --git a/Tools/tests/test_judge_combine.py b/Tools/tests/test_judge_combine.py new file mode 100644 index 0000000..205eb0e --- /dev/null +++ b/Tools/tests/test_judge_combine.py @@ -0,0 +1,40 @@ +"""Tests for the multi-model reconciliation in judge_screenshot.combine().""" + +import unittest + +from judge_screenshot import combine + + +class CombineTests(unittest.TestCase): + def test_agreement_takes_the_verdict_and_mean_confidence(self): + result = combine([ + {"model": "a", "verdict": "yes", "confidence": 0.8, "reason": "ok"}, + {"model": "b", "verdict": "yes", "confidence": 0.6, "reason": "fine"}, + ]) + self.assertEqual(result["verdict"], "yes") + self.assertAlmostEqual(result["confidence"], 0.7) + + def test_disagreement_becomes_uncertain_and_keeps_both_reasons(self): + result = combine([ + {"model": "a", "verdict": "yes", "confidence": 0.9, "reason": "looks right"}, + {"model": "b", "verdict": "no", "confidence": 0.4, "reason": "missing banner"}, + ]) + self.assertEqual(result["verdict"], "uncertain") + self.assertAlmostEqual(result["confidence"], 0.4) + self.assertIn("a=yes", result["reason"]) + self.assertIn("b=no", result["reason"]) + + def test_unknown_verdicts_are_ignored(self): + result = combine([ + {"model": "a", "verdict": "no", "confidence": 0.5, "reason": "x"}, + {"model": "b", "verdict": "garbage", "confidence": 0.9, "reason": "y"}, + ]) + self.assertEqual(result["verdict"], "no") + + def test_no_usable_verdicts(self): + result = combine([{"model": "a", "verdict": "garbage"}]) + self.assertEqual(result["verdict"], "unknown") + + +if __name__ == "__main__": + unittest.main()