Align remaining direct-score drift with the validator

Two more validator/direct-score contract drifts from review:

- allowed_max_steps <= 0 silently disabled the budget assertion on
  the direct-score path (no violation, max_steps_touched permanently
  False).  Report the validator's wording (allowed_max_steps must be
  >= 1) and keep the budget assertion disabled only alongside the
  explicit violation.
- whitespace-only expected_tools / expected_outcomes elements passed
  the malformed-element filter because it used not t while the
  validator uses not t.strip(): a whitespace tool polluted the
  hit-rate denominator and a whitespace outcome fell into the
  unknown-tag violation type.  Both filters now use the validator's
  exact predicate.
- allow_optional_tools wording aligned to the validator (must be a
  boolean) - caught by the new parity test.

Add the owner-requested parity regression test: for each malformed
golden shape the validator rejects, the direct-score path must
surface the same issue wording in its violations.

Author: subaoyan16

evals/agent_trajectory/metrics.py

@@ -520,12 +520,15 @@ def compute_trajectory_metrics(
     structure contract as :func:`validate_golden_sample` on the loader path:
     malformed parts are reported as violations and excluded from scoring
     instead of silently reshaping the result.  Malformed ``expected_tools`` /
-    ``expected_outcomes`` elements are dropped with an explicit violation,
-    and a pinned ``expected_guarded_stock`` is only honoured for the coherent
-    pairing the validator requires (``guarded_retry`` declared, and a stock
-    different from ``golden.stock_code`` after canonicalization) — an
-    unpaired pin is reported and disabled rather than silently erasing
-    wrong-stock reporting.
+    ``expected_outcomes`` elements — non-strings, empty strings and
+    whitespace-only strings, the validator's exact predicate — are dropped
+    with an explicit violation, a non-positive ``allowed_max_steps`` is
+    reported with the validator's wording (and the budget assertion stays
+    disabled), and a pinned ``expected_guarded_stock`` is only honoured for
+    the coherent pairing the validator requires (``guarded_retry`` declared,
+    and a stock different from ``golden.stock_code`` after
+    canonicalization) — an unpaired pin is reported and disabled rather than
+    silently erasing wrong-stock reporting.
     """
     used_tools: List[str] = []
     key_counts: Dict[tuple, int] = {}
@@ -564,8 +567,10 @@ def compute_trajectory_metrics(
     # below (mirroring validate_golden_sample, which rejects them at load
     # time) and only the valid names take part in scoring.
     if isinstance(golden.expected_tools, list):
-        expected_tools_malformed = [t for t in golden.expected_tools if not isinstance(t, str) or not t]
-        expected = [t for t in golden.expected_tools if isinstance(t, str) and t]
+        # Same element predicate as validate_golden_sample(): whitespace-only
+        # strings count as malformed too, not just falsy ones.
+        expected_tools_malformed = [t for t in golden.expected_tools if not isinstance(t, str) or not t.strip()]
+        expected = [t for t in golden.expected_tools if isinstance(t, str) and t.strip()]
     else:
         # Defend against hand-edited samples passing a bare string:
         # validation rejects it at load time, but scoring must not misparse
@@ -718,7 +723,7 @@ def compute_trajectory_metrics(
     # permissive, and a non-integer step limit must not crash the comparison.
     optional_allowed = golden.allow_optional_tools
     if not isinstance(optional_allowed, bool):
-        violations.append("allow_optional_tools is not a boolean")
+        violations.append("allow_optional_tools must be a boolean")
         optional_allowed = False
     if optional_tools_used and not optional_allowed:
         violations.append(f"optional tools used but not allowed: {', '.join(optional_tools_used)}")
@@ -730,13 +735,16 @@ def compute_trajectory_metrics(
         violations.append("expected_outcomes must be a list of outcome tags")
         outcomes: List[str] = []
     else:
-        malformed_outcomes = [t for t in golden.expected_outcomes if not isinstance(t, str) or not t]
+        # Same element predicate as validate_golden_sample(): whitespace-only
+        # strings are malformed too, so they never fall through to the
+        # unknown-tag check or the required-outcome set.
+        malformed_outcomes = [t for t in golden.expected_outcomes if not isinstance(t, str) or not t.strip()]
         # Malformed outcome elements are reported (same wording as
         # validate_golden_sample) instead of silently dropping the
         # requirement they tried to declare.
         if malformed_outcomes:
             violations.append("expected_outcomes must contain only non-empty strings")
-        outcomes = [t for t in golden.expected_outcomes if isinstance(t, str) and t]
+        outcomes = [t for t in golden.expected_outcomes if isinstance(t, str) and t.strip()]
         if len(set(outcomes)) != len(outcomes):
             violations.append("expected_outcomes contains duplicate tags")
             outcomes = list(dict.fromkeys(outcomes))
@@ -789,6 +797,11 @@ def compute_trajectory_metrics(
     if isinstance(limit, bool) or not isinstance(limit, int):
         violations.append("allowed_max_steps is not an integer")
         limit = 0
+    elif limit < 1:
+        # Validator wording for the same field: a non-positive limit must
+        # not silently disable the budget assertion on the direct-score path.
+        violations.append("allowed_max_steps must be >= 1")
+        limit = 0
     max_steps_touched = bool(max_step and limit > 0 and max_step >= limit)
     if max_steps_touched:
         violations.append(f"trajectory reached allowed_max_steps ({golden.allowed_max_steps})")

tests/test_agent_trajectory_metrics.py

@@ -190,7 +190,7 @@ def test_non_bool_allow_optional_tools_scores_strictly(self):
             log,
             _golden(expected_tools=["get_realtime_quote"], allow_optional_tools="false"),
         )
-        assert "allow_optional_tools is not a boolean" in m.violations
+        assert "allow_optional_tools must be a boolean" in m.violations
         assert "optional tools used but not allowed: search_stock_news" in m.violations
 
     def test_duplicate_expected_tools_scored_as_unique_names(self):
@@ -217,13 +217,24 @@ class TestDirectConstructionContract:
     def test_malformed_expected_tools_elements_are_reported(self):
         # Review counter-example: ['get_realtime_quote', ''] must not
         # silently collapse into a one-tool golden — the malformed element
-        # is reported, only valid names take part in scoring.
+        # is reported, only valid names take part in scoring.  Whitespace-
+        # only elements follow the validator's predicate (t.strip()).
         log = [_entry(tool="get_realtime_quote", arguments={"stock_code": "600519"})]
         for malformed in (["get_realtime_quote", ""], ["get_realtime_quote", 1]):
             m = compute_trajectory_metrics(log, _golden(expected_tools=malformed))
             assert m.expected_hit_rate == 1.0
             assert "expected_tools must contain only non-empty strings" in m.violations
 
+    def test_whitespace_only_expected_tool_does_not_pollute_scoring(self):
+        # Review counter-example: '   ' must not enter the hit-rate
+        # denominator nor show up as a missing tool — it is malformed.
+        log = [_entry(tool="get_realtime_quote", arguments={"stock_code": "600519"})]
+        m = compute_trajectory_metrics(log, _golden(expected_tools=["get_realtime_quote", "   "]))
+        assert m.expected_total == 1
+        assert m.expected_hit_rate == 1.0
+        assert m.missing_expected == []
+        assert "expected_tools must contain only non-empty strings" in m.violations
+
     def test_malformed_expected_outcomes_elements_are_reported(self):
         # Review counter-example: expected_outcomes=[1, ''] must not
         # silently drop the requirement — the malformed elements are
@@ -234,6 +245,59 @@ def test_malformed_expected_outcomes_elements_are_reported(self):
             assert "expected_outcomes must contain only non-empty strings" in m.violations
             assert "expected outcomes not observed" not in " ".join(m.violations)
 
+    def test_whitespace_only_outcome_gets_structural_violation(self):
+        # Review counter-example: '   ' must be reported as a malformed
+        # element, not misrouted into the unknown-tag violation type.
+        log = [_entry(tool="get_realtime_quote", arguments={"stock_code": "600519"})]
+        m = compute_trajectory_metrics(log, _golden(expected_outcomes=["   "]))
+        assert "expected_outcomes must contain only non-empty strings" in m.violations
+        assert not any("unknown expected outcome tags" in v for v in m.violations)
+
+    def test_non_positive_allowed_max_steps_reported_in_direct_score(self):
+        # Review counter-example: allowed_max_steps=0 / -3 must report the
+        # validator's wording instead of silently disabling the budget
+        # assertion, no matter how many steps the trajectory takes.
+        log = [_entry(tool="get_realtime_quote", arguments={"stock_code": "600519"}, step=s) for s in range(1, 100)]
+        for limit in (0, -3):
+            m = compute_trajectory_metrics(log, _golden(allowed_max_steps=limit))
+            assert "allowed_max_steps must be >= 1" in m.violations
+            assert m.max_steps_touched is False
+
+    def test_direct_score_mirrors_validator_structure_contract(self):
+        # The owner-requested parity: for each malformed golden shape the
+        # validator rejects, the direct-score path must surface the same
+        # issue wording in its violations.
+        log = [_entry(tool="get_realtime_quote", arguments={"stock_code": "600519"})]
+        cases = [
+            (dict(expected_tools=["get_realtime_quote", "   "]), "expected_tools must contain only non-empty strings"),
+            (dict(expected_tools=["get_realtime_quote", ""]), "expected_tools must contain only non-empty strings"),
+            (dict(expected_outcomes=["   "]), "expected_outcomes must contain only non-empty strings"),
+            (dict(expected_outcomes=[1, ""]), "expected_outcomes must contain only non-empty strings"),
+            (dict(expected_outcomes="guarded"), "expected_outcomes must be a list of outcome tags"),
+            (dict(allowed_max_steps=0), "allowed_max_steps must be >= 1"),
+            (dict(allowed_max_steps=-3), "allowed_max_steps must be >= 1"),
+            (dict(allow_optional_tools="false"), "allow_optional_tools must be a boolean"),
+            (
+                dict(expected_guarded_stock="600036"),
+                "expected_guarded_stock requires guarded_retry in expected_outcomes",
+            ),
+            (
+                dict(
+                    stock_code="600036",
+                    expected_tools=["get_stock_info"],
+                    expected_outcomes=["guarded_retry"],
+                    expected_guarded_stock="SH600036",
+                ),
+                "expected_guarded_stock must name a different stock than stock_code after canonicalization (it names the out-of-scope call)",
+            ),
+        ]
+        for overrides, issue in cases:
+            golden = _golden(**overrides)
+            validator_issues = validate_golden_sample(golden)
+            assert any(issue in i for i in validator_issues), (issue, overrides, validator_issues)
+            m = compute_trajectory_metrics(log, golden)
+            assert any(issue in v for v in m.violations), (issue, overrides, m.violations)
+
     def test_unpaired_guarded_stock_reported_and_exemption_disabled(self):
         # Review counter-example: expected_guarded_stock without guarded_retry
         # is a malformed sample; it must be reported and must not erase the