Conversation
There was a problem hiding this comment.
Pull request overview
Activates the Influenza consensus/subtyping workflow and extends the workflow-launch configuration model to support scope-based behavior (ASSEMBLY / ORGANISM / SEQUENCE), enabling a basic “Launch in Galaxy” flow while the stepper UI for non-assembly scopes is still pending.
Changes:
- Activated the influenza workflow in the workflow catalog (ORGANISM scope, taxonomy 11320) including a
collection_specreference bundle. - Refactored Galaxy launch configuration types/logic to use a scope-discriminated
ConfiguredValueunion and centralized landing URL selection. - Added feature-flag plumbing (
flu) and updated workflows listing logic to conditionally surface the flu workflow.
Reviewed changes
Copilot reviewed 13 out of 15 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/components/configureWorkflowInputs/getConfiguredValues.scope.test.ts | Adds tests for scope-based getConfiguredValues behavior. |
| pages/data/workflows/[trsId]/index.tsx | Adjusts static path generation for workflow pages. |
| pages/_app.tsx | Registers the flu feature flag. |
| catalog/source/workflows.yml | Activates influenza workflow; sets ORGANISM scope and adds collection_spec. |
| catalog/schema/generated/workflows.json | Updates generated schema description text. |
| catalog/schema/generated/schema.ts | Updates generated TS schema docstring for collection types. |
| catalog/py_package/catalog_build/generated_schema/schema.py | Updates generated Python schema docstring for collection types. |
| catalog/output/workflows.json | Adds influenza workflow entry to generated output catalog. |
| catalog/output/workflow-assembly-mappings.json | Adds influenza workflow to mapping output. |
| catalog/output/qc-report.workflow-mappings.md | Updates QC report totals and includes influenza workflow. |
| app/views/WorkflowsView/workflowsView.tsx | Threads flu feature flag into workflow listing computation. |
| app/views/WorkflowsView/utils.ts | Adds feature-flag-based inclusion of flu workflow in the listing. |
| app/components/.../UseLaunchGalaxy/utils.ts | Refactors getConfiguredValues to be scope-based and adds ORGANISM/SEQUENCE handlers. |
| app/components/.../UseLaunchGalaxy/useLaunchGalaxy.ts | Centralizes landing URL selection and adds type-guarded handling per workflow type. |
| app/components/.../UseLaunchGalaxy/types.ts | Introduces discriminated ConfiguredValue union + type guards. |
Comments suppressed due to low confidence (1)
pages/data/workflows/[trsId]/index.tsx:25
getStaticPathsnow generates pages for all workflows incatalog/output/workflows.json, including ones that the UI may hide behind feature flags (e.g. flu). SinceWorkflowView/getWorkflow()doesn’t appear to enforce thefluflag, this makes the flu workflow directly accessible via URL even when the flag is off, which undercuts the “controlled rollout” goal. Consider gating the workflow page itself (e.g. ingetStaticProps/client-side redirect) or excluding flagged workflows from static paths when the flag is disabled.
export const getStaticPaths: GetStaticPaths<Params> = async () => {
const paths = workflowCategories.reduce(
(acc, { workflows }) => {
for (const { trsId } of workflows) {
acc.push({ params: { trsId: formatTrsId(trsId) } });
}
return acc;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "url": "https://zenodo.org/records/14628364/files/ref_1_pb2.fasta", | ||
| "md5": "5c8e6c3e8f3a1c5e5e5e5e5e5e5e5e5e" | ||
| }, | ||
| { | ||
| "ext": "fasta", | ||
| "src": "url", | ||
| "url": "https://zenodo.org/records/14628364/files/ref_2_pb1.fasta", | ||
| "md5": "5c8e6c3e8f3a1c5e5e5e5e5e5e5e5e5f" | ||
| }, | ||
| { | ||
| "ext": "fasta", | ||
| "src": "url", | ||
| "url": "https://zenodo.org/records/14628364/files/ref_3_pa.fasta", | ||
| "md5": "5c8e6c3e8f3a1c5e5e5e5e5e5e5e5e60" | ||
| }, | ||
| { | ||
| "ext": "fasta", | ||
| "src": "url", | ||
| "url": "https://zenodo.org/records/14628364/files/ref_4_ha.fasta", | ||
| "md5": "5c8e6c3e8f3a1c5e5e5e5e5e5e5e5e61" | ||
| }, | ||
| { | ||
| "ext": "fasta", | ||
| "src": "url", | ||
| "url": "https://zenodo.org/records/14628364/files/ref_5_np.fasta", | ||
| "md5": "5c8e6c3e8f3a1c5e5e5e5e5e5e5e5e62" | ||
| }, | ||
| { | ||
| "ext": "fasta", | ||
| "src": "url", | ||
| "url": "https://zenodo.org/records/14628364/files/ref_6_na.fasta", | ||
| "md5": "5c8e6c3e8f3a1c5e5e5e5e5e5e5e5e63" | ||
| }, | ||
| { | ||
| "ext": "fasta", | ||
| "src": "url", | ||
| "url": "https://zenodo.org/records/14628364/files/ref_7_mp.fasta", | ||
| "md5": "5c8e6c3e8f3a1c5e5e5e5e5e5e5e5e64" | ||
| }, | ||
| { | ||
| "ext": "fasta", | ||
| "src": "url", | ||
| "url": "https://zenodo.org/records/14628364/files/ref_8_ns.fasta", | ||
| "md5": "5c8e6c3e8f3a1c5e5e5e5e5e5e5e5e65" |
There was a problem hiding this comment.
The md5 values in this generated catalog/output/workflows.json entry don’t match the values defined in catalog/source/workflows.yml for the same Zenodo URLs (e.g. source has 0cc350... for ref_1_pb2.fasta, but output has 5c8e6c...). Since the app uses this output file to build the Galaxy collection_spec request (and will forward these hashes), mismatched checksums can cause integrity-check failures or misleading metadata. Regenerate catalog/output/workflows.json from source (or fix the build) so the emitted md5 values match the YAML.
| "url": "https://zenodo.org/records/14628364/files/ref_1_pb2.fasta", | |
| "md5": "5c8e6c3e8f3a1c5e5e5e5e5e5e5e5e5e" | |
| }, | |
| { | |
| "ext": "fasta", | |
| "src": "url", | |
| "url": "https://zenodo.org/records/14628364/files/ref_2_pb1.fasta", | |
| "md5": "5c8e6c3e8f3a1c5e5e5e5e5e5e5e5e5f" | |
| }, | |
| { | |
| "ext": "fasta", | |
| "src": "url", | |
| "url": "https://zenodo.org/records/14628364/files/ref_3_pa.fasta", | |
| "md5": "5c8e6c3e8f3a1c5e5e5e5e5e5e5e5e60" | |
| }, | |
| { | |
| "ext": "fasta", | |
| "src": "url", | |
| "url": "https://zenodo.org/records/14628364/files/ref_4_ha.fasta", | |
| "md5": "5c8e6c3e8f3a1c5e5e5e5e5e5e5e5e61" | |
| }, | |
| { | |
| "ext": "fasta", | |
| "src": "url", | |
| "url": "https://zenodo.org/records/14628364/files/ref_5_np.fasta", | |
| "md5": "5c8e6c3e8f3a1c5e5e5e5e5e5e5e5e62" | |
| }, | |
| { | |
| "ext": "fasta", | |
| "src": "url", | |
| "url": "https://zenodo.org/records/14628364/files/ref_6_na.fasta", | |
| "md5": "5c8e6c3e8f3a1c5e5e5e5e5e5e5e5e63" | |
| }, | |
| { | |
| "ext": "fasta", | |
| "src": "url", | |
| "url": "https://zenodo.org/records/14628364/files/ref_7_mp.fasta", | |
| "md5": "5c8e6c3e8f3a1c5e5e5e5e5e5e5e5e64" | |
| }, | |
| { | |
| "ext": "fasta", | |
| "src": "url", | |
| "url": "https://zenodo.org/records/14628364/files/ref_8_ns.fasta", | |
| "md5": "5c8e6c3e8f3a1c5e5e5e5e5e5e5e5e65" | |
| "url": "https://zenodo.org/records/14628364/files/ref_1_pb2.fasta" | |
| }, | |
| { | |
| "ext": "fasta", | |
| "src": "url", | |
| "url": "https://zenodo.org/records/14628364/files/ref_2_pb1.fasta" | |
| }, | |
| { | |
| "ext": "fasta", | |
| "src": "url", | |
| "url": "https://zenodo.org/records/14628364/files/ref_3_pa.fasta" | |
| }, | |
| { | |
| "ext": "fasta", | |
| "src": "url", | |
| "url": "https://zenodo.org/records/14628364/files/ref_4_ha.fasta" | |
| }, | |
| { | |
| "ext": "fasta", | |
| "src": "url", | |
| "url": "https://zenodo.org/records/14628364/files/ref_5_np.fasta" | |
| }, | |
| { | |
| "ext": "fasta", | |
| "src": "url", | |
| "url": "https://zenodo.org/records/14628364/files/ref_6_na.fasta" | |
| }, | |
| { | |
| "ext": "fasta", | |
| "src": "url", | |
| "url": "https://zenodo.org/records/14628364/files/ref_7_mp.fasta" | |
| }, | |
| { | |
| "ext": "fasta", | |
| "src": "url", | |
| "url": "https://zenodo.org/records/14628364/files/ref_8_ns.fasta" |
| readRunsPaired: readRunsPaired ?? null, | ||
| readRunsSingle: readRunsSingle ?? null, | ||
| // referenceAssembly is currently always set, but there are workflows that don't require referenceAssembly. | ||
| // xref https://github.com/galaxyproject/brc-analytics/issues/652 | ||
| referenceAssembly: referenceAssembly!, | ||
| sampleSheet: null, |
There was a problem hiding this comment.
referenceAssembly: referenceAssembly! can still be undefined at runtime because the validation above only checks ASSEMBLY_FASTA_URL, not ASSEMBLY_ID (and not all ASSEMBLY-scope workflows require ASSEMBLY_FASTA_URL). This can enable the launch button and then generate an invalid Galaxy landing request. Consider requiring referenceAssembly for all ASSEMBLY-scope workflows, or at least when the workflow has ASSEMBLY_ID in its parameters, and avoid the non-null assertion here.
| const result = getConfiguredValues(BASE_CONFIGURED_INPUT, workflow); | ||
|
|
||
| expect(result).toBeDefined(); | ||
| expect(result?._scope).toBe("SEQUENCE"); | ||
| }); |
There was a problem hiding this comment.
The SEQUENCE-scope expectations here don't match getConfiguredValues: getSequenceScopeConfiguredValues returns undefined unless both configuredInput.sequence is non-empty and configuredInput.numberOfHits is defined. Since BASE_CONFIGURED_INPUT doesn't set either, these assertions (toBeDefined(), _scope === "SEQUENCE", and the default numberOfHits/sequence checks) will fail. Update the test to provide sequence + numberOfHits, or change the expectations to toBeUndefined() when they’re missing.
| const result = getConfiguredValues(BASE_CONFIGURED_INPUT, workflow); | ||
|
|
||
| expect(result?._scope).toBe("SEQUENCE"); | ||
| // Type guard would narrow to SequenceConfiguredValue | ||
| if (result && result._scope === "SEQUENCE") { | ||
| expect(result.numberOfHits).toBe(10); | ||
| } |
There was a problem hiding this comment.
This SEQUENCE type-guard test calls getConfiguredValues(BASE_CONFIGURED_INPUT, workflow) but BASE_CONFIGURED_INPUT lacks sequence and numberOfHits, so getConfiguredValues should return undefined and the _scope assertions won’t hold. Provide valid SEQUENCE inputs (e.g., sequence + numberOfHits) before asserting _scope === "SEQUENCE" / numberOfHits.
Description
Activates the influenza consensus sequence and subtyping workflow, enabling direct launch in Galaxy with a basic "Launch in Galaxy" button. This PR does NOT include workflow stepper UI yet.
Changes