fix(skill-evals): extend default prose_fields with grading-schema.json

[HeaTTap]

Fixes #946

• Updated load_grading_schema in tools/skill-evals/src/skill_evals/runner.py to extend DEFAULT_PROSE_FIELDS rather than replacing it.
• Updated unit tests in test_runner.py and documentation in README.md.
• Removed dead grading-schema.json from evals/release-audit-report/step-2-assemble-record/fixtures/.

[justinmclean]

What it does

A grading-schema.json lists field names that get judged loosely by a model rather than having to match the expected value exactly. Before this PR that file replaced the built-in list. Now it adds to it.

Why it matters

Under the old behaviour, a step with its own schema quietly lost the built-ins. Add a reason field to that step later and it would suddenly demand an exact string match, failing for no visible cause. That's the bug in #946.

What I checked

Adding fields to the loosely-judged list makes checks weaker, so the question is whether any existing eval gets weaker. I went through all 86 schema files: for each one, worked out which built-in names it currently leaves out, then looked at every expected answer in that directory to see whether any of those names actually appears.

None do. Not one suite changes. The verbose schemas that already list every built-in become no-ops, and the short ones never had a built-in name in their expected output. So the change is inert against the current tree and only affects fields added later, which is the intent.

The deleted release-audit-report schema checks out too. It named record_markdown, and that key appears in the output spec but in none of the four expected answers, which all go through a has_record_markdown_non_empty check instead. Genuinely defunct, so deleting rather than repurposing is right.

The docstring was updated in the same hunk as the code, and the README no longer claims the old behaviour.

The README example had to change and did. It used to show {"prose_fields": ["rationale", "drop_reason"]}, which made sense when the file replaced the defaults: it narrowed the set to those two. Under the new behaviour both names are already built in, so that example is a file that does nothing. Leaving it would have taught people to write no-ops. Swapping it for a custom field name is the right call.

One thing to fix though: the replacement is an invented custom_prose_field, and the test in the same PR invents custom_prose instead. Two made-up names for the same idea, in one change. Neither reads like a field, they read like a description of the category. Every real custom field in the tree reads like data: paste_recipe, record_markdown, skip_reason, handoff_note, evidence_summary. Use paste_recipe in both places. release-verify-rc has three live instances, so a reader can grep it.

Three things to fix

A comment five lines above the fixed docstring still describes the old behaviour (runner.py:352):

# A per-fixtures-dir ``grading-schema.json`` can replace this list.

That's now backwards, and it's the first thing a reader sees when they scroll to the constant.

Four files now say nothing. These contain {"prose_fields": []}:

• good-first-issue-sweep/step-3-present-proposals
• pr-stale-sweep/step-4-compose-comment
• pr-stale-sweep/step-5-confirm
• pr-stale-sweep/step-7-recap

An empty list used to mean "be strict about everything in this step". Empty plus the built-ins is just the built-ins, so now it means nothing at all. Harmless today, for the same reason as above, but a contributor opening one will reasonably conclude the step is deliberately strict and act on it. I'd delete all four here, same as the defunct one. Worth noting this also removes the only per-step way to force exact matching, since --exact applies to the whole run. If anyone actually wants that, it needs its own setting.

The replacement test is weaker than the one it replaces (test_runner.py, test_cli_grader_mode_respects_grading_schema_override).

The old test set prose_fields to [] and checked that a differing reason caused a FAIL. That proved the schema really controlled grading, and it proved it in the direction that can catch a bug.

The new test invents a field, lists it in the schema, and checks the result is PASS with a stub grader that always says yes. It does prove something, since an exact compare of two different strings would fail. But it can't tell "the field was judged by the grader" apart from "the field was skipped entirely". If a future change silently dropped that field from comparison, this test would still pass.

The file already has a stub grader that always says no. Adding a second assertion with it, expecting a FAIL, closes the hole: a skipped field would pass, a judged field fails.

Worth a follow-up issue, not this PR

The same reasoning that condemns the old README example applies to most of the tree. Of the 85 schema files with a list, 53 contain built-in names that now do nothing, 467 redundant entries between them. Two are complete no-ops:

• pre-first-pr-check/step-2-check-categories → ["details"]
• security-issue-import-from-md/step-2-duplicate-guard → the full built-in list, verbatim

Those two plus the four empty ones are six files that now say nothing at all. Stripping the built-in names everywhere would leave each file stating exactly what it adds, which is the whole point of the new behaviour. But probably too large to fold in here.

This review was drafted by an AI-assisted tool and confirmed by a Magpie maintainer. The maintainer approving this PR has read the findings and signed off. If something feels off, please reply on the PR and a maintainer will follow up.

More on how Magpie handles maintainer review: [CONTRIBUTING.md](https://github.com/apache/magpie/blob/main/CONTRIBUTING.md)