[qiaochuz-nv]

What does this PR do?

physics_judge now produces the per-domain metrics its README promises (<domain>/pass@1[avg-of-k]/...). On main, /verify drops each row's domain, so no rollout carries it and both the eval run's aggregate and a later /aggregate_metrics call return only the top-level keys. Scores themselves were never affected.

Fixes NVBug 6791004. Closes #4509.

Cause

physics_judge has never defined its own verify models. It inherits LibraryJudgeMathVerifyRequest / LibraryJudgeMathVerifyResponse from math_with_judge, which declare question / expected_answer and the scoring fields but no domain, so pydantic's default extra="ignore" drops it at request validation. PhysicsJudgeResourcesServer.compute_metrics builds the breakdown only when a verify row has domain (resources_servers/physics_judge/app.py, if any("domain" in r ...)), which never happened. All of this dates from #1163, which added the server, the per-domain gate, the README text and the example rollouts (0/5 rows with domain, against 5/5 in example.jsonl). The existing per-domain unit test passed rows that already had domain straight to compute_metrics, so it never went through /verify.

Fix

• PhysicsJudgeVerifyRequest / PhysicsJudgeVerifyResponse add domain: Optional[str] = None on top of the math_with_judge models, and verify() returns the parent's result with domain carried over. A row without domain gets null, and compute_subset_metrics already skips empty subset values, so it gets no subset keys.
• data/example_rollouts.jsonl: each committed row gains its domain (from example.jsonl, paired by _ng_task_index), which is what /verify now returns. Every row is otherwise byte-identical.
• task_data.py: the module docstring said physics_judge has no request model of its own; it now names the new one.
• Tests: the shipped five rows go through the real /verify route and /aggregate_metrics (no judge call) and must come back with their domain and with <domain>/pass@1/... keys for all five domains; a row without domain must give exactly the keys of a row that has no domain field at all.

Not chosen: ConfigDict(extra="allow") on both models, as ugphysics_judge does for subject. That echoes every row field into each rollout, including the reference solution text and difficulty / answer_type / language, which nothing reads and the README doesn't promise. Not chosen either: relaxing the shared math_with_judge models, which math_with_judge itself (referenced by 26 server and benchmark configs) and three other subclasses use.

Before the fix

Gym c07c935d8 (main when reproduced; nothing under physics_judge, math_with_judge or the aggregation path changed up to bedf56450): the five shipped rows posted to the real /verify route in-process (symbolic only, no model), then the returned rows posted to /aggregate_metrics:

row 0: input domain='mechanics' -> verify domain='<absent>' reward=1.0
row 1: input domain='thermodynamics' -> verify domain='<absent>' reward=1.0
row 2: input domain='electromagnetism' -> verify domain='<absent>' reward=1.0
row 3: input domain='quantum' -> verify domain='<absent>' reward=1.0
row 4: input domain='relativity' -> verify domain='<absent>' reward=1.0
mean/reward: 1.0
domain_prefixed_keys: 0

New test on main's app.py:

>       assert [row.get("domain") for row in verified] == domains
E       AssertionError: assert [None, None, None, None, None] == ['mechanics',... 'relativity']
FAILED resources_servers/physics_judge/tests/test_app.py::TestDomainThroughVerify::test_shipped_rows_keep_domain_and_get_per_domain_metrics

QA testcase physics_judge on one H100 (the README's gym env start + gym eval run --agent physics_judge_simple_agent over the five shipped rows, Qwen/Qwen3-8B on vLLM as policy, the shipped NIM openai/gpt-oss-20b judge), Gym c1582e18d:

FAIL (389.5s): all controls and modes complete; deferred semantic failures are final FAIL: expected [], got
[{'check': 'generated persisted metrics', 'error': 'metric exists electromagnetism/pass@1/no_answer'},
 {'check': 'generated HTTP metrics', 'error': 'metric exists electromagnetism/pass@1/no_answer'}]
physics_judge: 0/1 passed in 389.5s

The saved rollouts have domain in 0/5 rows and the aggregate has 0 domain keys, with mean/reward 1.0.

After the fix

Gym 20dd937c6 (this branch, on bedf56450), same script:

row 0: input domain='mechanics' -> verify domain='mechanics' reward=1.0
row 1: input domain='thermodynamics' -> verify domain='thermodynamics' reward=1.0
row 2: input domain='electromagnetism' -> verify domain='electromagnetism' reward=1.0
row 3: input domain='quantum' -> verify domain='quantum' reward=1.0
row 4: input domain='relativity' -> verify domain='relativity' reward=1.0
mean/reward: 1.0
domain_prefixed_keys: 30
   electromagnetism/majority@1/no_answer 0.0
   electromagnetism/majority@1/symbolic_accuracy 100.0
   electromagnetism/pass@1/no_answer 0.0
   electromagnetism/pass@1/symbolic_accuracy 100.0
   ...

Same QA testcase, Gym 7feb1bf1a (this change before its rebase onto bedf56450; same diff):

PASS   230.4s  evaluate all five original physics tasks and live symbolic judge parser subset repeat controls
physics_judge: 1/1 passed in 230.4s

The saved rollouts carry domain in 5/5 rows and the persisted and HTTP aggregates both have all 30 domain keys (e.g. electromagnetism/pass@1/symbolic_accuracy 100.0, electromagnetism/pass@1/no_answer 0.0), matching the case's independent oracle.

Validation

• QA testcase physics_judge: 0/1 on main c1582e18d, 1/1 on this branch. Both runs used the QA suite with its fix for an unrelated stale routing check (the case expected --agent to restamp agent_ref as {'name': ...}, which no longer matches main since fix(grl): start GRL READMEs without the unbound gymnasium_agent and keep agent_ref on --agent #4544; that QA fix is pending in the suite), so only Gym differs between them.
• resources_servers/physics_judge/tests + resources_servers/math_with_judge/tests: 159 passed; the new domain test fails on main's app.py, the null-domain guard passes on both.
• gym env test +entrypoint=resources_servers/physics_judge +should_validate_data=true: 29 passed, data validated.
• tests/unit_tests/test_task_data.py: 401 passed.
• pre-commit run --all-files: passed, no changes.
• Rollout evidence (H100, the two QA runs above): five Qwen3-8B rollouts per run, every response completed, empty failures sidecar, no mask_sample; identical policy outputs on both commits (output tokens 1274/2145/2076/1120/1290), all five correct on the symbolic path (reward 1.0, no judge call). The case's live judge controls made 10 judge calls, 10/10 with a parseable verdict. Only domain (0/5 → 5/5 rows) and the per-domain keys (0 → 30) changed.

Compatibility

Rollouts from physics_judge gain one field, domain (null when the row has none), and aggregates gain the per-domain keys when rows have a domain. Top-level metrics and rewards are unchanged. No other server is touched.

🤖 Generated with Claude Code

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[qiaochuz-nv]

/ok to test 20dd937

[yaoyu-33]

Thanks for the clear root-cause write-up and the before/after evidence. The fix looks right to us: /verify is registered from self.verify, so the PhysicsJudgeVerifyRequest body type takes effect, and domain now reaches the rows that compute_metrics reads.

What we checked at 20dd937c6:

• Reward semantics are unchanged. _verify_answer is untouched, and PhysicsJudgeVerifyResponse(**response.model_dump(), domain=body.domain) round-trips losslessly, including judge_evaluations and mask_sample. The parent response drops undeclared fields, so there is no duplicate-keyword clash.
• Failure and null paths are covered. The judge-failsafe row is built from the subclass body, so domain survives judge failures. A null or empty domain adds no subset keys, because compute_subset_metrics skips falsy values.
• example_rollouts.jsonl only gains domain. All five rows match example.jsonl via _ng_task_index, and nothing else in them differs from main.
• The new tests are not vacuous. With main's app.py swapped in, test_shipped_rows_keep_domain_and_get_per_domain_metrics fails ([None]*5 != [...]). With the PR it passes, and the physics_judge test file runs 29 passed locally.
• No other server is affected. Nothing else subclasses PhysicsJudgeResourcesServer.
• PR hygiene: the title, body, linked issue, validation list, rollout evidence and compatibility note are all in place.

No blocking issues found. Two optional observations, no action needed for this PR:

• A non-string domain (for example 5) now returns HTTP 422 from /verify, where main silently ignored it. The shipped data uses strings, so this is probably the intended strictness.
• The judge-enabled path of the new verify() is not exercised by a test. It shares the same super().verify() call, so the risk is low.

[yaoyu-33]

Approving after the team review above: no blocking issues at the reviewed head.

[github-actions]

🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA-NeMo/Gym/actions/runs/39830708645