From 23b8ff54a0912fbbc1e7dd52e47eb192a23945d1 Mon Sep 17 00:00:00 2001 From: Luca Belli <129434630+sator-labs@users.noreply.github.com> Date: Wed, 22 Jul 2026 16:56:35 -0700 Subject: [PATCH] feat: wire --rubrics flag to RubricConfig.load_bundle() --- README.md | 2 +- judge.py | 18 +++- run_pipeline.py | 7 +- tests/fixtures/rubric_manifest_multi_row.json | 5 + tests/integration/test_pipeline.py | 6 +- tests/unit/judge/test_judge_cli.py | 92 +++++++++++++++++-- 6 files changed, 115 insertions(+), 15 deletions(-) create mode 100644 tests/fixtures/rubric_manifest_multi_row.json diff --git a/README.md b/README.md index d2b909dc..ff7d27d6 100644 --- a/README.md +++ b/README.md @@ -223,7 +223,7 @@ This will generate conversations under `output//conversations/` by defa | `-c` | `--conversation` | Path to a single conversation file to judge (mutually exclusive with `--folder`) | | `-j` | `--judge-model` | Model(s) to use for judging (required). Format: `model` or `model:count` for multiple instances. Can specify multiple: `--judge-model model1 model2:3`. Examples: `claude-sonnet-4-5-20250929`, `claude-sonnet-4-5-20250929:3`, `claude-sonnet-4-5-20250929:2 gpt-4o:1` | | `-jep` | `--judge-model-extra-params` | Extra parameters for the judge model (optional). Examples: `temperature=0.7,max_tokens=1000`. Default: `temperature=0` (unless overridden) | -| `-r` | `--rubrics` | Rubric file(s) to use (default: `data/rubric.tsv`) | +| `-r` | `--rubrics` | Rubric bundle manifest(s) to use (default: `data/rubric_manifest.json`). Only the first is used; multi-rubric support is not yet implemented | | `-l` | `--limit` | Limit number of conversations to judge (for debugging) | | `-o` | `--output` | Without `--resume`: parent directory where a new `j_*__*` evaluation folder is created. Default: `/evaluations/` when `-f` is a nested generation run with `conversations/`; otherwise `evaluations/` at the repo root (a notice is printed). With `--resume`: the existing `j_*` evaluation folder itself. | | | `--resume` | Continue batch judging in an existing evaluation folder: use with `-f` and `-o` pointing at that folder. Skips `(conversation, judge, instance)` jobs whose `.tsv` already exists, then rebuilds `results.csv` from all TSVs there. Not supported with `-c` / `--conversation`. | diff --git a/judge.py b/judge.py index 6796022a..2095c1d0 100644 --- a/judge.py +++ b/judge.py @@ -51,8 +51,12 @@ def get_parser() -> argparse.ArgumentParser: "--rubrics", "-r", nargs="+", - default=["data/rubric.tsv"], - help="Rubric file(s) to use (default: data/rubric.tsv)", + default=["data/rubric_manifest.json"], + help=( + "Rubric bundle manifest(s) to use " + "(default: data/rubric_manifest.json). " + "Only the first is used; multi-rubric support is not yet implemented." + ), ) # model @@ -167,9 +171,17 @@ async def main(args) -> Optional[str]: models_str = ", ".join(f"{model}x{count}" for model, count in judge_models.items()) print(f"🎯 LLM Judge | Models: {models_str}") + if len(args.rubrics) > 1: + print( + f"Warning: multiple rubrics passed ({args.rubrics}); " + f"multi-rubric support is not yet implemented, " + f"using only the first: {args.rubrics[0]}", + file=sys.stderr, + ) + # Load rubric configuration once at startup print("📚 Loading rubric configuration...") - rubric_config = await RubricConfig.load(rubric_folder="data") + rubric_config = await RubricConfig.load_bundle(args.rubrics[0]) if args.conversation: # Single conversation with first judge model (single instance) diff --git a/run_pipeline.py b/run_pipeline.py index 25aa7c90..da932004 100644 --- a/run_pipeline.py +++ b/run_pipeline.py @@ -398,8 +398,11 @@ def parse_arguments(): parser.add_argument( "--rubrics", nargs="+", - default=["data/rubric.tsv"], - help="Rubric file(s) to use for evaluation (default: data/rubric.tsv)", + default=["data/rubric_manifest.json"], + help=( + "Rubric bundle manifest(s) to use for evaluation " + "(default: data/rubric_manifest.json)" + ), ) # Optional arguments for scoring diff --git a/tests/fixtures/rubric_manifest_multi_row.json b/tests/fixtures/rubric_manifest_multi_row.json new file mode 100644 index 00000000..8195a8b1 --- /dev/null +++ b/tests/fixtures/rubric_manifest_multi_row.json @@ -0,0 +1,5 @@ +{ + "rubric_file": "rubric_multi_row.tsv", + "rubric_prompt_beginning_file": "rubric_prompt_beginning.txt", + "question_prompt_file": "question_prompt.txt" +} diff --git a/tests/integration/test_pipeline.py b/tests/integration/test_pipeline.py index b41d321b..3226c374 100644 --- a/tests/integration/test_pipeline.py +++ b/tests/integration/test_pipeline.py @@ -68,7 +68,7 @@ def pipeline_args(): judge_per_judge=False, judge_limit=None, judge_verbose_workers=False, - rubrics=["data/rubric.tsv"], + rubrics=["data/rubric_manifest.json"], resume_generate=False, resume_judge=False, skip_risk_analysis=False, @@ -531,7 +531,7 @@ def test_run_id_passed_to_generate(self, pipeline_args): def test_rubrics_argument_exists(self, pipeline_args): """Test that rubrics argument exists in pipeline args.""" assert hasattr(pipeline_args, "rubrics") - assert pipeline_args.rubrics == ["data/rubric.tsv"] # Default value + assert pipeline_args.rubrics == ["data/rubric_manifest.json"] # Default value def test_rubrics_passed_to_judge(self, pipeline_args): """Test that rubrics are correctly passed to judge args.""" @@ -624,7 +624,7 @@ def test_parse_arguments_defaults_for_new_args(self): # Check defaults assert args.run_id is None - assert args.rubrics == ["data/rubric.tsv"] + assert args.rubrics == ["data/rubric_manifest.json"] assert args.conversation_output == "output" assert args.judge_output is None diff --git a/tests/unit/judge/test_judge_cli.py b/tests/unit/judge/test_judge_cli.py index 0dc2285c..47df7d2c 100644 --- a/tests/unit/judge/test_judge_cli.py +++ b/tests/unit/judge/test_judge_cli.py @@ -57,7 +57,7 @@ def test_defaults(self): """Optional args have expected defaults.""" parser = get_parser() args = parser.parse_args(["-f", "folder", "-j", "gpt-4o"]) - assert args.rubrics == ["data/rubric.tsv"] + assert args.rubrics == ["data/rubric_manifest.json"] assert args.output is None assert args.limit is None assert args.max_concurrent is None @@ -144,13 +144,15 @@ async def test_main_single_conversation_calls_judge_single(self): new_callable=AsyncMock, ) as judge_single, ): - RubricConfig.load = AsyncMock(return_value="rubric_config") + RubricConfig.load_bundle = AsyncMock(return_value="rubric_config") ConversationData.load = AsyncMock(return_value="conversation_data") LLMJudge.return_value = "judge_instance" result = await main(args) - RubricConfig.load.assert_called_once_with(rubric_folder="data") + RubricConfig.load_bundle.assert_called_once_with( + "data/rubric_manifest.json" + ) ConversationData.load.assert_called_once_with("conv.txt") LLMJudge.assert_called_once_with( judge_model="gpt-4o", @@ -200,13 +202,15 @@ async def test_main_folder_calls_judge_conversations(self): new_callable=AsyncMock, ) as judge_convos, ): - RubricConfig.load = AsyncMock(return_value="rubric_config") + RubricConfig.load_bundle = AsyncMock(return_value="rubric_config") load_convos.return_value = [] judge_convos.return_value = ([], "evaluations/run1_timestamp") result = await main(args) - RubricConfig.load.assert_called_once_with(rubric_folder="data") + RubricConfig.load_bundle.assert_called_once_with( + "data/rubric_manifest.json" + ) expected_dir, _, _ = resolve_conversation_input("conversations/run1") load_convos.assert_called_once_with(expected_dir, limit=10) judge_convos.assert_awaited_once() @@ -251,7 +255,7 @@ async def test_main_folder_resume_uses_output_folder(self, tmp_path: Path): _judge_script, "judge_conversations", new_callable=AsyncMock ) as judge_convos, ): - RubricConfig.load = AsyncMock(return_value="rubric_config") + RubricConfig.load_bundle = AsyncMock(return_value="rubric_config") load_convos.return_value = [] judge_convos.return_value = ([], str(eval_folder)) @@ -262,3 +266,79 @@ async def test_main_folder_resume_uses_output_folder(self, tmp_path: Path): assert call_kw["output_folder"] == str(eval_folder) assert call_kw["resume"] is True assert result == str(eval_folder) + + @pytest.mark.asyncio + async def test_main_loads_distinct_rubric_bundles_end_to_end(self): + """--rubrics actually drives which bundle is loaded, not a hardcoded one. + + Uses the real RubricConfig.load_bundle() against two different test + fixture manifests and checks the parsed rubrics differ, proving the + flag is live rather than a no-op. + """ + parser = get_parser() + + async def load_rubric_config(rubrics_arg): + args = parser.parse_args( + ["-f", "conversations/run1", "-j", "gpt-4o", "-r", rubrics_arg] + ) + with ( + patch.object( + _judge_script, "load_conversations", new_callable=AsyncMock + ) as load_convos, + patch.object( + _judge_script, "judge_conversations", new_callable=AsyncMock + ) as judge_convos, + patch.object(_judge_script, "RubricConfig") as RubricConfigMock, + ): + from judge.rubric_config import RubricConfig as RealRubricConfig + + RubricConfigMock.load_bundle = RealRubricConfig.load_bundle + load_convos.return_value = [] + judge_convos.return_value = ([], "evaluations/run1_timestamp") + + await main(args) + return judge_convos.await_args[1]["rubric_config"] + + simple = await load_rubric_config("tests/fixtures/rubric_manifest_simple.json") + multi_row = await load_rubric_config( + "tests/fixtures/rubric_manifest_multi_row.json" + ) + + assert simple.question_order != multi_row.question_order + + @pytest.mark.asyncio + async def test_main_warns_on_multiple_rubrics(self, capsys): + """Passing multiple --rubrics values warns and uses only the first.""" + parser = get_parser() + args = parser.parse_args( + [ + "-f", + "conversations/run1", + "-j", + "gpt-4o", + "-r", + "tests/fixtures/rubric_manifest_simple.json", + "tests/fixtures/rubric_manifest_multi_row.json", + ] + ) + with ( + patch.object(_judge_script, "RubricConfig") as RubricConfig, + patch.object( + _judge_script, "load_conversations", new_callable=AsyncMock + ) as load_convos, + patch.object( + _judge_script, "judge_conversations", new_callable=AsyncMock + ) as judge_convos, + ): + RubricConfig.load_bundle = AsyncMock(return_value="rubric_config") + load_convos.return_value = [] + judge_convos.return_value = ([], "evaluations/run1_timestamp") + + await main(args) + + RubricConfig.load_bundle.assert_called_once_with( + "tests/fixtures/rubric_manifest_simple.json" + ) + captured = capsys.readouterr() + assert "Warning" in captured.err + assert "rubric_manifest_simple.json" in captured.err