From 899ca3a7d50f89e1bab1aa707d132b97bbd8c99a Mon Sep 17 00:00:00 2001
From: Lars Bogner <ich@larsbogner.de>
Date: Thu, 13 Aug 2026 15:40:47 +0200
Subject: [PATCH] Validate stage2_model.autoregressive.order in validate_config
 (gitea #30)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

order was documented as single-valued ("energy_desc" only, placeholder for a
future alternative ordering) but validate_config only checked its siblings
history/teacher_forcing, so e.g. order = "energy_asc" was silently accepted
and trained as if it were energy_desc. Add the missing check alongside the
other two, gated the same way (only meaningful under
stage2_model.decoder = "autoregressive"). Also updates the stale reason
string on the pre-existing _KNOWN_UNUSED allow-list entry for this key in
tests/test_config_consumed_keys.py, since half of it ("validate_config ...
never [checks] order") is no longer true after this fix — the key stays
allow-listed because validate_config itself isn't in that test's
build/train/rollout consumer whitelist.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 giant/config.py                    |  7 +++++++
 tests/test_config.py               | 30 ++++++++++++++++++++++++++++--
 tests/test_config_consumed_keys.py |  5 +++--
 3 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/giant/config.py b/giant/config.py
index 24b2850..3c58ba8 100644
--- a/giant/config.py
+++ b/giant/config.py
@@ -1287,6 +1287,13 @@ def validate_config(cfg: dict) -> None:
         )
 
     if _get_path(cfg, "stage2_model.decoder") == "autoregressive":
+        order = _get_path(cfg, "stage2_model.autoregressive.order")
+        if order != "energy_desc":
+            raise ValueError(
+                f"stage2_model.autoregressive.order = {order!r} — must be "
+                "'energy_desc' (the only implemented ordering; see "
+                "AutoregressiveConfig.order's docstring)"
+            )
         history = _get_path(cfg, "stage2_model.autoregressive.history")
         if history not in ("markov", "attention"):
             raise ValueError(f"stage2_model.autoregressive.history = {history!r} — must be 'markov' or 'attention'")
diff --git a/tests/test_config.py b/tests/test_config.py
index 400410e..a678583 100644
--- a/tests/test_config.py
+++ b/tests/test_config.py
@@ -747,6 +747,31 @@ def test_validate_config_ar_default_markov_always_passes():
     gconfig.validate_config(cfg)  # must not raise
 
 
+def test_validate_config_ar_order_energy_desc_passes():
+    """'energy_desc' is the only implemented order — must not raise."""
+    cfg = _cfg_with(
+        **{
+            "stage2_model.decoder": "autoregressive",
+            "stage2_model.autoregressive.order": "energy_desc",
+        }
+    )
+    gconfig.validate_config(cfg)  # must not raise
+
+
+def test_validate_config_ar_order_invalid_value_rejected():
+    cfg = _cfg_with(
+        **{
+            "stage2_model.decoder": "autoregressive",
+            "stage2_model.autoregressive.order": "energy_asc",
+        }
+    )
+    try:
+        gconfig.validate_config(cfg)
+        assert False, "expected ValueError"
+    except ValueError as e:
+        assert "order" in str(e)
+
+
 def test_validate_config_ar_history_attention_passes():
     """v0.3.0 step 7 implements history='attention' — must not raise."""
     cfg = _cfg_with(
@@ -800,11 +825,12 @@ def test_validate_config_ar_teacher_forcing_invalid_value_rejected():
 
 
 def test_validate_config_ar_checks_skipped_under_one_shot():
-    """history/teacher_forcing values that would fail under AR are irrelevant
-    (and unchecked) when decoder='one_shot'."""
+    """order/history/teacher_forcing values that would fail under AR are
+    irrelevant (and unchecked) when decoder='one_shot'."""
     cfg = _cfg_with(
         **{
             "stage2_model.decoder": "one_shot",
+            "stage2_model.autoregressive.order": "bogus",
             "stage2_model.autoregressive.history": "attention",
             "stage2_model.autoregressive.teacher_forcing": "scheduled",
         }
diff --git a/tests/test_config_consumed_keys.py b/tests/test_config_consumed_keys.py
index e21a216..095f9d2 100644
--- a/tests/test_config_consumed_keys.py
+++ b/tests/test_config_consumed_keys.py
@@ -61,8 +61,9 @@ _KNOWN_UNUSED = {
         "validation — see Issue 16 for the real implementation"
     ),
     "stage2_model.autoregressive.order": (
-        "issues.md Issue 4 — validate_config checks history/teacher_forcing "
-        "but never order, and nothing reads it either"
+        "gitea #30 — validate_config now checks order is 'energy_desc', but "
+        "nothing in the build/train/rollout consumer whitelist reads the "
+        "value itself since it's still single-valued"
     ),
 }