Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #43 +/- ##
==========================================
- Coverage 54.36% 52.42% -1.94%
==========================================
Files 136 143 +7
Lines 8218 8709 +491
Branches 1657 1792 +135
==========================================
+ Hits 4468 4566 +98
- Misses 3381 3749 +368
- Partials 369 394 +25
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
using schema-salad https://github.com/common-workflow-language/schema_salad/releases/tag/8.9.20251102115403 and https://github.com/common-workflow-language/cwl-v1.2/blob/92865343ab98d82373f12cf585c5f76ca2f26011/extensions.yml schema-salad-tool --codegen typescript https://github.com/common-workflow-language/cwl-v1.2/raw/codegen/extensions.yml --codegen-package cwl-ts-auto
|
@alexiswl Can you help fix these errors with generating JSON schema from the CWL TypeScript schema? https://github.com/common-workflow-lab/cwl-ts-auto/actions/runs/20676746678/job/59365385211?pr=43 |
| */ | ||
| export interface CommandLineBindableProperties { | ||
|
|
||
| extensionFields?: Internal.Dictionary<any> |
There was a problem hiding this comment.
I tried that, but then there are other validation issues: https://github.com/common-workflow-lab/cwl-ts-auto/actions/runs/20676696133/job/59365273384
The removal is legitimate, as the class is now abstract..
There was a problem hiding this comment.
class is now abstract
Yes but the object constructor still expects the extension fields, such as https://github.com/common-workflow-lab/cwl-ts-auto/blob/regen_20260103/src/CommandLineBindable.ts#L88
So you either need to either remove the extension fields from those types of calls, OR update the constructor type to add in the extension fields type (which was previously just in the CommandLineBindable properties), like so
constructor ({loadingOptions, extensionFields, inputBinding} : {loadingOptions?: LoadingOptions} & Internal.CommandLineBindableProperties) {
to
constructor ({loadingOptions, extensionFields, inputBinding} : {loadingOptions?: LoadingOptions} & Internal.Dictionary<any> & Internal.CommandLineBindableProperties) {
I tried that, but then there are other validation issues
Yep because id is now set to 'mandatory' in many of the fields, such as https://github.com/common-workflow-lab/cwl-ts-auto/pull/43/changes#diff-07abac72669befd367f8ae59ac50db4ca16d87201905fb78ab737201b14488a6R27 but none of the cwl tests have 'id' in their attributes? If this is intentional, we will need to update the script that generates the cwl schema to ensure that the 'id' attribute is removed from the required fields, which shouldn't be too difficult.
There was a problem hiding this comment.
Sounds like some of the SALAD processing for maps isn't happening, either on the Typescript side or the JSON schema generation/testing side.
https://www.commonwl.org/v1.2/Workflow.html#map
For example, the id field for WorkflowStepInput (used by the in field of a workflow step) is rarely in the long form:
class: Workflow
inputs:
message: string
steps:
a_step:
run: something.cwl
in:
- id: an_input
source: message
# […]but usually written in the short form:
class: Workflow
inputs:
message: string
steps:
a_step:
run: something.cwl
in:
an_input: message
# […]There was a problem hiding this comment.
We can manually remove the 'id' fields in these sections from the json schema, that's why we generate the cwl-raw file and then the curated schema from that as the typescript isn't a perfect representation of a CWL file.
There was a problem hiding this comment.
We can manually remove the 'id' fields in these sections from the json schema, that's why we generate the cwl-raw file and then the curated schema from that as the typescript isn't a perfect representation of a CWL file.
The JSON schema should be modified to support all variations of the map syntax. I guess that information isn't present in the TypeScript objects because the loader functions take care of the variations. So the post-processing from the TypeScript to JSON Schema is going to need additional information on where to encode the map varients.
That information is in the SALAD definition of CWL.
I see at least 3 options to move forward:
A. Record the information about the map field variations during the TypeScript code generation that produced this repository. Use that information in a post-processing step after the TypeScript objects to JSON Schema generation.
or
B. Create schema-salad-tool --codegen json-schema to directly output JSON schema, taking into account all the map field varients.
or
C. Create manual patches to apply to the generated JSON Schema, updating them as the CWL 1.2 schema continues to be refined, and manually porting them for the future CWL versions.
I feel like Option B is the most future-proofed and brings value to other SALAD users, but also the most work. In some ways it won't be as difficult as the other additions of new codegen targets as we have two examples of what JSON Schema for CWL would look like.
I can assist with all of these options, but I'm not available to do any of these options entirely myself.
This looks more like Typescript errors than json schema generation errors. See https://github.com/common-workflow-lab/cwl-ts-auto/pull/43/changes#r2660063670 |
2 participants
using schema-salad https://github.com/common-workflow-language/schema_salad/releases/tag/8.9.20251102115403
and https://github.com/common-workflow-language/cwl-v1.2/blob/92865343ab98d82373f12cf585c5f76ca2f26011/extensions.yml
schema-salad-tool --codegen typescript https://github.com/common-workflow-language/cwl-v1.2/raw/codegen/extensions.yml --codegen-package cwl-ts-autoschema-salad updates since the last regeneration include
CWL v1.2 schema refinements since the last regen
CommandLineBindableshould not be concrete common-workflow-language/cwl-v1.2#301