Various YAML Tool Hardening and Progress toward Tool State Goals

[jmchilton]

Extracted from #17393.

@mvdbeek this is probably too much to review in this format but it is only growing. Any ideas about stuff you'd like to see pulled out and reviewed in isolation?

I think the biggest changes to review are probably the parameter_specification.yml file and runtime.py / evaluation.py

AI Generated Summary:

Structured tool state: typed collection runtime models and YAML tool hardening

Summary

• Typed collection runtime models: Replace untyped Dict[str, DataInternalJson] collection representations with a hierarchy of Pydantic models (DataCollectionPairedRuntime, DataCollectionListRuntime, etc.) using discriminated unions routed by collection_type. A dynamic model factory (build_collection_model_for_type) generates precise models for nested types like list:paired with exact inner type validation.
• Persist validated tool state on Job: Add Job.tool_state column (with Alembic migration) to store Pydantic-validated internal tool state at execution time, enabling the new runtimeify path that converts internal state to CWL-style runtime representations without the legacy to_cwl function.
• YAML tool test case validation: YAML tools now validate test inputs against Pydantic parameter models at parse time via a new test_case_json state representation. Test inputs are flattened from structured JSON to the pipe-delimited format the test framework expects.
• Collection inputs for YAML tools: Wire collection input support (data_collection type) through the YAML tool parser, including output collections, dce source type for subcollection mapping, and comma-separated collection_type (e.g., list,paired).
• New state representations: Add job_runtime and test_case_json to the state representation enum with full parameter specification coverage for all scalar and collection types.

Details

Typed Collection Runtime Models (lib/galaxy/tool_util_models/parameters.py)

The old representation used raw dicts (Dict[str, DataInternalJson], DataCollectionPaired). This branch introduces:

• DataCollectionInternalJsonBase -- base model with class, name, collection_type, tags, and optional metadata fields (column_definitions, fields, has_single_item, columns)
• Leaf models: DataCollectionListRuntime, DataCollectionPairedRuntime, DataCollectionRecordRuntime, DataCollectionPairedOrUnpairedRuntime, DataCollectionSampleSheetRuntime
• Nested models: DataCollectionNestedListRuntime (list-like outer), DataCollectionNestedRecordRuntime (record-like outer) with recursive element unions
• CollectionRuntimeDiscriminated -- a Discriminator-based union routing by collection_type pattern
• build_collection_model_for_type() -- LRU-cached factory that recursively builds create_model() types with Literal[collection_type] and narrowed elements types. For list:paired, elements is List[DataCollectionPairedRuntime] (not a generic union).
• DataCollectionElementInternalJson -- extends DataInternalJson with element_identifier and optional columns
• DataCollectionPairedElements -- typed model requiring exactly forward and reverse fields

DataCollectionParameterModel.py_type_internal_json now routes through _runtime_model_for_collection_type(), handling single types, comma-separated types (discriminated union subsets), and unknown types (full discriminated union fallback).

Collection-to-Runtime Conversion (lib/galaxy/tools/runtime.py)

New module implementing the collection_to_runtime() pipeline:

1. setup_for_runtimeify() builds lookup dicts for HDAs, HDCAs, and DCEs from job inputs, returns adapt_dataset and adapt_collection callbacks
2. _build_collection_runtime_dict() recursively converts DatasetCollection -> raw dict, distinguishing list-like (array elements) from record-like (object elements) via is_list_like()
3. _validate_collection_runtime_dict() validates raw dict through build_collection_model_for_type() with fallback to generic nested models
4. Special metadata preserved: column_definitions (sample_sheet), fields (record), has_single_item (paired_or_unpaired), columns (sample_sheet elements)

Dynamic Model Factory for Recursive Collection Types

• build_collection_model_for_type() recursively generates precise Pydantic models for nested collection types (e.g. list:paired, list:list:paired)
• Dynamic models use Literal[collection_type] and narrowed element types -- inner type mismatches and depth mismatches are caught structurally by Pydantic
• _LEAF_COLLECTION_MODELS is the single source of truth for leaf type routing
• _collection_type_discriminator returns full collection_type strings for routing comma-separated type unions

Job Tool State Persistence

• Job.tool_state column added (JSONType, nullable)
• Alembic migration: b4d6191307a5
• ExecutionSlice carries validated_param_combination
• UserToolEvaluator.build_param_dict() uses runtimeify when job.tool_state is present, falls back to legacy to_cwl with deprecation log

YAML Tool Enhancements

• Allow YAML tools to produce output collections
• Allow collection inputs to YAML tools with JSON-based test syntax
• Added value_state_representation to distinguish test_case_xml vs test_case_json test formats
• Set minimum profile of 24.2 for YAML tools so test requests load properly
• Improved error handling when test requests are unavailable
• Fixed YAML tool source parsing to not mutate the underlying dict during serialization

State Flow

1. Tool receives request state
2. Request validated and converted to internal state
3. Internal state persisted on Job.tool_state
4. Internal state converted to runtime state with resolved paths/locations
5. Runtime state serialized for tool evaluation

Test plan

Unit tests (test/unit/tool_util/test_runtime.py)

• E2E collection-to-runtime: paired, list, record, sample_sheet, paired_or_unpaired, nested list:paired, deeply nested list:list:paired
• Invalid structure detection: paired missing reverse
• Dynamic model factory: leaf type routing, unknown type returns None, correct/wrong inner types, wrong collection_type literal, depth mismatch, JSON schema precision, LRU caching
• Record-based dynamic models: record:paired accept/reject, empty dict elements
• Unknown nested segments: list:unknown_type, record:unknown, list:list:unknown all return None

Parameter specification tests (parameter_specification.yml)

• job_runtime_valid/job_runtime_invalid for all scalar types (int, boolean, float, text, select, genomebuild, directory_uri, hidden, drill_down)
• job_runtime specs for structural types (conditionals, repeats, sections)
• Cross-type discrimination: list rejects paired data, paired rejects list data
• Comma-separated type unions: list:paired,list:list and list,list:paired with valid and invalid runtime cases
• Collection runtime specs for all leaf types plus nested types

New functional test tools

• 14 collection type tool definitions covering all leaf types, nested types, and comma-separated type unions
• 4 scalar/YAML user tool definitions (boolean, data, data_multiple, select_multiple)
• All registered in sample_tool_conf.xml

Serialization roundtrip test

• Validates YAML tool source survives to_string() -> re-parse without alteration

How to test the changes?

(Select all options that apply)

• I've included appropriate automated tests.
• This is a refactoring of components with existing test coverage.
• Instructions for manual testing are as follows:
  1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

• I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

[mvdbeek]

• Test inputs are flattened from structured JSON to the pipe-delimited format the test framework expects.

This seems very unfortunate? Is that the internal test framework outside of tool_util ? Because I'm certain we don't need to do that in the planemo / tool_util route. The API should also accept this unmodified

Other than that the description reads great, now to actually review it :)

[mvdbeek]

OK, i see it's in 4cb4723 ... we only do this for tool tests ? How much would you hate it if you had claude come up with a plan to use the nested state ? I've reviewed all commits and they look great, I wonder if I missed anything but this all makes perfect sense to me.

[mvdbeek]

Awesome, I think we should merge this!

[mvdbeek]

... but please change the JSONType column in model and migration

[jmchilton]

OK, i see it's in 4cb4723 ... we only do this for tool tests ? How much would you hate it if you had claude come up with a plan to use the nested state ? I've reviewed all commits and they look great, I wonder if I missed anything but this all makes perfect sense to me.

I think there is a balance between keeping things in a... natural, consistent format (nested) vs. not forking code between the older paths and the newer paths. It is possible I went to far toward converging things on the existing path through the code if that makes sense? I'm pretty consistently more conservative about these things than you - but I definitely share your desire to not be collapsing this junk into the flat form - I don't think that helps anyone.

Because I'm certain we don't need to do that in the planemo / tool_util route. The API should also accept this unmodified

That is a surprising claim. Does it not all just go through this same code? Are you sure that is how this works? I mean I know planemo can load tool tests from sidecare JSON/YAML file but no one does that right and that path through the code isn't really used? If there is an established path that does this and is working consistently - then I would especially like to see a way to avoid flat paths.

At any rate - eager to see the Claude research or if you have a prompt I can use my tokens and bake it into the branch.

[jmchilton]

Thanks for the detailed review - it is a relief that you're onboard with the dynamic collection type models, that work is cool but it is very extra at the same time. Do you think we should do the same thing for the tool tests? I will remove the stray comments and I want to cleanup some fallback code it left in I wasn't super happy about on my review.

[mvdbeek]

I mean I know planemo can load tool tests from sidecare JSON/YAML file but no one does that right and that path through the code isn't really used?

I don't know what I was thinking, yes, we don't use this at all, i think i got that all confused with workflow tests, which are not nested at all. The tool run API does support this though (I think).

[mvdbeek]

Sorry about the JSON suggestion, should be JSONB on postgres. I've pushed one commit to change that and one to fix up the linting, feel free to rebase or drop these.

[jmchilton]

Thanks for the fix - that makes sense to me! I'm loving pre-commit since you mentioned it the other day but I am occasionally falling into traps where it is using the wrong black version also. I guess because when I install the hook I have to do it for the whole repository so it is using a fixed version of black and doesn't match the branch. Figuring out how to get that to dynamically dispatch - maybe a wrapper or something - would be great.

[mvdbeek]

I'm loving pre-commit since you mentioned it the other day but I am occasionally falling into traps where it is using the wrong black version also.

that's interesting, i'm just symlinking the sample config, so as long as we keep the sample config updated we should get the correct version. When the sample file changes I see the new version getting installed, so it should work ...