From 598c2b92a249f2605787fd4ad5bf2bcec41153cb Mon Sep 17 00:00:00 2001
From: ageorge156 <ageorge156@gmail.com>
Date: Tue, 1 Sep 2026 11:10:18 +0800
Subject: [PATCH] fix: type-check factor set definition ids (#16)

---
 src/quant_engine/factor_contracts.py | 10 ++++--
 tests/test_factor_contracts.py       | 50 ++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/src/quant_engine/factor_contracts.py b/src/quant_engine/factor_contracts.py
index 61331ac..78d38f1 100644
--- a/src/quant_engine/factor_contracts.py
+++ b/src/quant_engine/factor_contracts.py
@@ -2324,8 +2324,14 @@ class FactorSetRef:
             _fail(ContractErrorCode.INPUT_CLOSURE_VIOLATION, "$.dataset_snapshot_id", "external envelope mismatch")
         if item["foundation_id"] != foundation.foundation_id or item["pit_cutoff"] != foundation.pit_cutoff:
             _fail(ContractErrorCode.INPUT_CLOSURE_VIOLATION, "$.foundation_id", "external Foundation mismatch")
-        raw_definition_ids = _array(item["definition_ids"], "$.definition_ids", minimum=1, unique=True)
-        if set(raw_definition_ids) != {definition.definition_id for definition in definitions}:
+        raw_definition_ids = _array(item["definition_ids"], "$.definition_ids", minimum=1)
+        definition_ids = [
+            _string(definition_id, f"$.definition_ids[{index}]")
+            for index, definition_id in enumerate(raw_definition_ids)
+        ]
+        if len(definition_ids) != len(set(definition_ids)):
+            _fail(ContractErrorCode.INVALID_VALUE, "$.definition_ids", "items must be unique")
+        if set(definition_ids) != {definition.definition_id for definition in definitions}:
             _fail(ContractErrorCode.INPUT_CLOSURE_VIOLATION, "$.definition_ids", "external definitions mismatch")
         raw_views = _array(item["selected_view_ref_ids"], "$.selected_view_ref_ids", minimum=1, unique=True)
         raw_bindings = _array(item["input_bindings"], "$.input_bindings", minimum=1, unique=True)
diff --git a/tests/test_factor_contracts.py b/tests/test_factor_contracts.py
index f86c883..dcacad8 100644
--- a/tests/test_factor_contracts.py
+++ b/tests/test_factor_contracts.py
@@ -810,6 +810,56 @@ def test_external_snapshot_definition_and_view_references_cannot_be_substituted(
     )
 
 
+@pytest.mark.parametrize(
+    "invalid_definition_id",
+    [
+        {"unexpected": "object"},
+        ["array"],
+        42,
+        True,
+        None,
+    ],
+)
+def test_factor_set_ref_definition_ids_reject_non_string_types(
+    invalid_definition_id: Any,
+) -> None:
+    factor_set = _factor_set()
+    definition = _golden_definition()
+    value = factor_set.to_dict()
+    value["definition_ids"] = [definition.definition_id, invalid_definition_id]
+    _reidentify(value, "factor_set_id", "rhfactorsetv1:sha256:")
+    snapshot, foundation = _snapshot_and_foundation()
+
+    with pytest.raises(FactorContractError) as error:
+        FactorSetRef.from_json(
+            canonical_json_bytes(value),
+            definitions=(definition,),
+            dataset_snapshot=snapshot,
+            foundation=foundation,
+        )
+
+    _assert_error(error, ContractErrorCode.TYPE_ERROR, "$.definition_ids[1]")
+
+
+def test_factor_set_ref_definition_ids_still_reject_duplicate_strings() -> None:
+    factor_set = _factor_set()
+    definition = _golden_definition()
+    value = factor_set.to_dict()
+    value["definition_ids"] = [definition.definition_id, definition.definition_id]
+    _reidentify(value, "factor_set_id", "rhfactorsetv1:sha256:")
+    snapshot, foundation = _snapshot_and_foundation()
+
+    with pytest.raises(FactorContractError) as error:
+        FactorSetRef.from_json(
+            canonical_json_bytes(value),
+            definitions=(definition,),
+            dataset_snapshot=snapshot,
+            foundation=foundation,
+        )
+
+    _assert_error(error, ContractErrorCode.INVALID_VALUE, "$.definition_ids")
+
+
 def test_factor_set_parent_requires_exact_identity_and_correlation() -> None:
     parent = _factor_set()
     child_arguments = _factor_set_arguments()