Skip to content

Commit aa67790

Browse files
jmchiltonclaude
andcommitted
Fail fast on unknown collection_type instead of silent fallbacks.
- collection_runtime_discriminator: raise ValueError for unknown types - _runtime_model_for_collection_type: remove nested-type fallback to generic models - py_type_internal_json: raise on unrecognized declared collection_type - _validate_collection_runtime_dict: raise instead of trying list/generic models - Add tests for discriminator routing, unknown type rejection Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 5395ad5 commit aa67790

4 files changed

Lines changed: 50 additions & 24 deletions

File tree

lib/galaxy/tool_util_models/parameters.py

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -831,9 +831,13 @@ def collection_runtime_discriminator(v: Any) -> str:
831831
return "nested_list"
832832
else:
833833
return "nested_record"
834-
else:
835-
# Unknown type - default to list (matches historical behavior)
834+
elif not ct:
835+
# Missing collection_type — data isn't a runtime collection dict.
836+
# Route to list so Pydantic fails validation with a clear schema error
837+
# rather than a discriminator error.
836838
return "list"
839+
else:
840+
raise ValueError(f"Unknown collection_type for runtime discrimination: '{ct}'")
837841

838842

839843
CollectionRuntimeDiscriminated: Type = cast(
@@ -1129,13 +1133,6 @@ def _runtime_model_for_collection_type(self, ct: str) -> tuple:
11291133
model = build_collection_model_for_type(ct)
11301134
if model is not None:
11311135
return (model, ct)
1132-
if ":" in ct:
1133-
# Fallback to generic nested models for unknown nested types
1134-
first_segment = ct.split(":")[0]
1135-
if first_segment in ("list", "sample_sheet"):
1136-
return (DataCollectionNestedListRuntime, "nested_list")
1137-
else:
1138-
return (DataCollectionNestedRecordRuntime, "nested_record")
11391136
return (None, None)
11401137

11411138
@property
@@ -1176,8 +1173,7 @@ def py_type_internal_json(self) -> Type:
11761173
if model:
11771174
return optional_if_needed(model, self.optional)
11781175

1179-
# Fallback for unrecognized types - use full discriminated union
1180-
return optional_if_needed(CollectionRuntimeDiscriminated, self.optional)
1176+
raise ValueError(f"Unknown collection_type for runtime model: '{self.collection_type}'")
11811177

11821178
def pydantic_template(self, state_representation: StateRepresentationT) -> DynamicModelInformation:
11831179
if state_representation in ["request", "relaxed_request"]:

lib/galaxy/tools/runtime.py

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,6 @@
1717
from galaxy.tool_util_models.parameters import (
1818
build_collection_model_for_type,
1919
DataCollectionInternalJsonBase,
20-
DataCollectionListRuntime,
21-
DataCollectionNestedListRuntime,
22-
DataCollectionNestedRecordRuntime,
2320
DataCollectionRequestInternal,
2421
DataInternalJson,
2522
DataRequestInternalDereferencedT,
@@ -185,15 +182,7 @@ def _validate_collection_runtime_dict(raw: dict[str, Any]) -> DataCollectionInte
185182
model = build_collection_model_for_type(ct)
186183
if model is not None:
187184
return model.model_validate(raw)
188-
# Fallback for unknown types
189-
if ":" in ct:
190-
first_segment = ct.split(":")[0]
191-
if first_segment in ("list", "sample_sheet"):
192-
return DataCollectionNestedListRuntime.model_validate(raw)
193-
else:
194-
return DataCollectionNestedRecordRuntime.model_validate(raw)
195-
# Unknown single type - try list (historical behavior)
196-
return DataCollectionListRuntime.model_validate(raw)
185+
raise ValueError(f"Cannot build runtime model for collection_type: '{ct}'")
197186

198187

199188
def _build_collection_runtime_dict(

test/unit/app/tools/test_runtime.py

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,10 @@
1515
DataCollectionSampleSheetRuntime,
1616
DataInternalJson,
1717
)
18-
from galaxy.tools.runtime import collection_to_runtime
18+
from galaxy.tools.runtime import (
19+
_validate_collection_runtime_dict,
20+
collection_to_runtime,
21+
)
1922

2023

2124
@pytest.fixture
@@ -226,3 +229,17 @@ def test_collection_to_runtime_deeply_nested(mock_adapt_dataset):
226229
inner = middle.elements[0]
227230
assert isinstance(inner, DataCollectionPairedRuntime)
228231
assert inner.elements.forward.class_ == "File"
232+
233+
234+
def test_validate_collection_runtime_dict_rejects_unknown_leaf():
235+
"""Unknown leaf collection_type raises ValueError."""
236+
raw = {"class": "Collection", "name": "x", "collection_type": "banana", "tags": [], "elements": []}
237+
with pytest.raises(ValueError, match="Cannot build runtime model"):
238+
_validate_collection_runtime_dict(raw)
239+
240+
241+
def test_validate_collection_runtime_dict_rejects_unknown_nested():
242+
"""Unknown nested collection_type raises ValueError."""
243+
raw = {"class": "Collection", "name": "x", "collection_type": "list:banana", "tags": [], "elements": []}
244+
with pytest.raises(ValueError, match="Cannot build runtime model"):
245+
_validate_collection_runtime_dict(raw)

test/unit/tool_util/test_runtime.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
from galaxy.tool_util_models.parameters import (
55
build_collection_model_for_type,
6+
collection_runtime_discriminator,
67
DataCollectionListRuntime,
78
DataCollectionPairedRuntime,
89
DataCollectionRecordRuntime,
@@ -249,3 +250,26 @@ def test_dynamic_model_json_schema_deeply_nested():
249250
defs = schema.get("$defs", {})
250251
assert len(defs) >= 2
251252
assert any("list_paired" in k for k in defs.keys())
253+
254+
255+
def test_collection_runtime_discriminator_known_types():
256+
"""Known leaf and nested types route correctly."""
257+
assert collection_runtime_discriminator({"collection_type": "list"}) == "list"
258+
assert collection_runtime_discriminator({"collection_type": "paired"}) == "paired"
259+
assert collection_runtime_discriminator({"collection_type": "record"}) == "record"
260+
assert collection_runtime_discriminator({"collection_type": "paired_or_unpaired"}) == "paired_or_unpaired"
261+
assert collection_runtime_discriminator({"collection_type": "sample_sheet"}) == "sample_sheet"
262+
assert collection_runtime_discriminator({"collection_type": "list:paired"}) == "nested_list"
263+
assert collection_runtime_discriminator({"collection_type": "record:paired"}) == "nested_record"
264+
265+
266+
def test_collection_runtime_discriminator_rejects_unknown():
267+
"""Unknown collection_type raises ValueError instead of silently defaulting."""
268+
with pytest.raises(ValueError, match="Unknown collection_type"):
269+
collection_runtime_discriminator({"collection_type": "banana"})
270+
271+
272+
def test_collection_runtime_discriminator_missing_routes_to_list():
273+
"""Missing/empty collection_type routes to list for Pydantic schema rejection."""
274+
assert collection_runtime_discriminator({"collection_type": ""}) == "list"
275+
assert collection_runtime_discriminator({}) == "list"

0 commit comments

Comments
 (0)