Skip to content

fix: assistant handoff to stepper (incl. disabled launch in galaxy bug) (#1295)#1330

Open
frano-m wants to merge 7 commits into
mainfrom
fran/1295-assistant-handoff-url-params
Open

fix: assistant handoff to stepper (incl. disabled launch in galaxy bug) (#1295)#1330
frano-m wants to merge 7 commits into
mainfrom
fran/1295-assistant-handoff-url-params

Conversation

@frano-m

@frano-m frano-m commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes #1295.

  • Root cause of disabled "Launch In Galaxy": the assistant captured the ENA accession in schema.data_source.value (free-text) but SchemaPanel.handleContinue wrote only dataSource to the handoff. The accession was dropped at the boundary, so the sequencing step arrived empty and the launch button was correctly disabled.
  • Fix: app-level reducer + provider at app/providers/workflowHandoff/. State is keyed by entity → path → inputs (e.g. state.assemblies["/data/assemblies/X/analyze/workflows/Y"] = {accessions, sequencingSource}). The assistant dispatches setHandoff for the target page; the destination stepper reads via useHandoffInputs(entity, currentPath) and useHandoffSync consumes (dispatches clearHandoff) after applying. Single-use semantics.
  • Path comparison: both the dispatch site and the read site route through normalizePagePath() (strips query, fragment, and trailing slashes) so they always compare canonical strings regardless of backend format quirks. Read site uses useCurrentPath() over router.asPath.
  • No more entry-button reset contract: removed the four onClearSource calls (Configure, Configure Inputs, Select a Workflow, Custom Analysis). The path-keyed lookup + consume-on-apply makes any wrong-target navigation a no-op — no apply, no leak.
  • Simplified useConfigureInputs: removed the hasMergedInitialRef discriminator (a strict-mode footgun where the effect double-fire wiped the merged initial state). The hook is now a one-line merge. The "wipe on assembly change" UX moved to the call site (SelectCell) via a new DEFAULT_CONFIGURED_INPUT constant that explicitly spreads the wipe into the dispatch.
  • SequencingStep toggle now derived from configuredInput (via getInitialView) rather than internal useState, so the toggle always reflects current state — important after an assembly re-pick wipes via DEFAULT_CONFIGURED_INPUT.
  • Lifted useHandoffEnaQuery out of SequencingStep: useHandoffSync is the sole caller. Loading status travels to deep consumers via HandoffStatusContext (at providers/workflowHandoff/contexts/HandoffStatus/). On pages that don't mount useHandoffSync, SequencingStep reads the default non-loading context — no spurious handoff fetches.

Test plan

  • npm test — 553/553 unit tests pass
  • npx tsc --noEmit clean
  • npm run build:local succeeds
  • Manual Sc.1 (ASSEMBLY + ENA, P. falciparum + ERR16655350): accession pre-fills paired sequencing, launch enabled
  • Manual Sc.2 (ASSEMBLY + Upload, P. falciparum): upload toggle pre-selects, launch enabled
  • Manual Sc.3 (ORGANISM + ENA, E. coli + Shovill + SRR12345678): accession pre-fills, launch enabled
  • Manual entry-button reset (Workflows → Card → Configure): fresh stepper, no leaked state from prior assistant session
  • Part 2 of Assistant: validate handoff to the stepper and Galaxy (incl. disabled "Launch In Galaxy" bug) #1295 (Galaxy landing payload validation) — not in scope of this PR

Notes

  • Minor cosmetic: upload mode shows both "Paired-End Sequencing Data" and "Single-End Sequencing Data" in the right-panel Configuration. The assistant doesn't know the workflow's stepKey, so clearSequencingData([]) seeds both array fields. The launch validator only checks the workflow's actual step, so this is harmless but worth a follow-up.
  • Multiple commits on this branch tracking the design evolution (URL params → reducer per source → reducer per entity+path with consume-on-apply, final design per discussion with @dannon and @NoopDog). Squash on merge.

🤖 Generated with Claude Code

image image image image image

frano-m and others added 2 commits June 8, 2026 17:17
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 8, 2026 13:24
@github-actions github-actions Bot added the fix label Jun 8, 2026
@frano-m frano-m moved this to Ready for Review in BRC development tasks Jun 8, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes the assistant → workflow stepper handoff so sequencing accessions and sequencing-source intent (ENA vs upload) survive the boundary and correctly enable “Launch In Galaxy”. It introduces a shared, app-level state container for handoff payloads and updates the stepper to consume and asynchronously resolve ENA accessions into configured sequencing inputs.

Changes:

  • Added WorkflowInputsView/state/ reducer + provider, used by the assistant to dispatch { accessions, sequencingSource } and by the stepper to read/clear per-source state.
  • Implemented async ENA handoff resolution (useHandoffSync + React Query) and sequencing-step utilities to translate array-shaped sequencing selections into file-scalar fields when needed.
  • Simplified useConfigureInputs to a pure merge and moved “wipe on assembly change” behavior to the Reference Assembly selection call site via DEFAULT_CONFIGURED_INPUT.

Reviewed changes

Copilot reviewed 46 out of 46 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/configureWorkflow/sequencingStep/translateForSequencingStep.test.ts Adds unit coverage for array→scalar translation behavior.
tests/configureWorkflow/sequencingStep/deriveInitialView.test.ts Adds unit coverage for sequencing-step initial toggle derivation.
tests/components/assistant/schemaPanel.utils.test.ts Tests accession extraction + sequencing-source normalization from assistant schema.
pages/_app.tsx Mounts the new WorkflowInputsStateProvider at the app root.
app/views/WorkflowsView/components/Workflows/components/Card/card.tsx Clears assistant-source state when entering the stepper via workflow cards.
app/views/WorkflowInputsView/workflowInputsView.tsx Consumes assistant handoff initial input + runs useHandoffSync; updates step targeting.
app/views/WorkflowInputsView/state/types.ts Defines source-keyed state and context value types.
app/views/WorkflowInputsView/state/reducer.ts Adds reducer wiring for ClearSource/SetSource actions.
app/views/WorkflowInputsView/state/provider.tsx Implements the context provider component.
app/views/WorkflowInputsView/state/hooks/UseWorkflowInputsReducer/hook.ts Wires useReducer to the new reducer/initial state.
app/views/WorkflowInputsView/state/hooks/UseSourceState/types.ts Adds hook return type alias.
app/views/WorkflowInputsView/state/hooks/UseSourceState/hook.ts Adds useSourceState selector with default fallback.
app/views/WorkflowInputsView/state/hooks/UseSourceDispatch/types.ts Defines typed dispatch API for set/clear source actions.
app/views/WorkflowInputsView/state/hooks/UseSourceDispatch/hook.ts Implements useSourceDispatch action helpers.
app/views/WorkflowInputsView/state/context.ts Creates the WorkflowInputsContext with a “dispatch outside provider” throw.
app/views/WorkflowInputsView/state/constants.ts Adds SEQUENCING_SOURCE, SOURCE_KEYS, and default/initial state constants.
app/views/WorkflowInputsView/state/actions/types.ts Defines action union + action-kind enum.
app/views/WorkflowInputsView/state/actions/setSource/types.ts Defines SetSource action/payload types.
app/views/WorkflowInputsView/state/actions/setSource/dispatch.ts Adds SetSource action creator.
app/views/WorkflowInputsView/state/actions/setSource/action.ts Implements reducer helper for SetSource.
app/views/WorkflowInputsView/state/actions/clearSource/types.ts Defines ClearSource action/payload types.
app/views/WorkflowInputsView/state/actions/clearSource/dispatch.ts Adds ClearSource action creator.
app/views/WorkflowInputsView/state/actions/clearSource/action.ts Implements reducer helper for ClearSource (resets to defaults).
app/views/WorkflowInputsView/sequencing/utils.ts Adds helper to find the workflow’s sequencing-step key.
app/views/WorkflowInputsView/sequencing/constants.ts Defines the set of sequencing step keys (array + file-scalar).
app/views/WorkflowInputsView/hooks/UseHandoffSync/utils.ts Adds ENA read-run → configured-input update builder.
app/views/WorkflowInputsView/hooks/UseHandoffSync/useHandoffSync.ts Applies resolved ENA handoff data into configured input (with step translation).
app/views/WorkflowInputsView/hooks/UseHandoffSync/query/ena/types.ts Types the React Query key for ENA handoff queries.
app/views/WorkflowInputsView/hooks/UseHandoffSync/query/ena/options/queryFn.ts Implements queryFn reusing existing ENA fetch utilities.
app/views/WorkflowInputsView/hooks/UseHandoffSync/query/ena/hook.ts Adds useHandoffEnaQuery driven by assistant-source accessions.
app/views/WorkflowInputsView/hooks/UseConfigureInputs/useConfigureInputs.ts Simplifies configure-input state updates to a pure merge; supports initial state.
app/views/WorkflowInputsView/hooks/UseConfigureInputs/constants.ts Adds DEFAULT_CONFIGURED_INPUT used to explicitly wipe config state.
app/views/WorkflowInputsView/hooks/UseAssistantHandoff/utils.ts Derives initial configured input + handoff flag from assistant-source state.
app/views/WorkflowInputsView/hooks/UseAssistantHandoff/useAssistantHandoff.ts Switches handoff source from localStorage to the new state context.
app/views/WorkflowInputsView/hooks/UseAssistantHandoff/types.ts Replaces localStorage handoff types with UseAssistantHandoff shape.
app/views/AnalyzeWorkflowsView/components/Main/components/Accordion/components/Workflow/workflow.tsx Clears assistant-source state before routing to stepper from workflow accordion.
app/views/AnalyzeView/components/Main/main.tsx Clears assistant-source state on “Configure Inputs” / “Custom Analysis” entry buttons.
app/components/Entity/components/ConfigureWorkflowInputs/components/Main/types.ts Removes initialDataSourceView prop plumb-through.
app/components/Entity/components/ConfigureWorkflowInputs/components/Main/main.tsx Removes initialDataSourceView usage.
app/components/Entity/components/ConfigureWorkflowInputs/components/Main/components/Stepper/types.ts Removes initialDataSourceView from stepper props.
app/components/Entity/components/ConfigureWorkflowInputs/components/Main/components/Stepper/components/Step/types.ts Removes initialDataSourceView from step props.
app/components/Entity/components/ConfigureWorkflowInputs/components/Main/components/Stepper/components/Step/SequencingStep/utils.ts Adds getInitialView + translateForSequencingStep.
app/components/Entity/components/ConfigureWorkflowInputs/components/Main/components/Stepper/components/Step/SequencingStep/sequencingStep.tsx Uses new utilities; shows loading while assistant ENA handoff resolves.
app/components/Entity/components/ConfigureWorkflowInputs/components/Main/components/Stepper/components/Step/ReferenceAssemblyStep/.../SelectCell/selectCell.tsx Moves “wipe configured inputs on assembly change” into call site via DEFAULT_CONFIGURED_INPUT.
app/components/Assistant/SchemaPanel/utils.ts Adds accession extraction + sequencing-source normalization from free-text schema.
app/components/Assistant/SchemaPanel/schemaPanel.tsx Dispatches handoff state and uses SPA navigation to preserve provider state.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread app/components/Assistant/SchemaPanel/utils.ts Outdated
Comment thread app/views/WorkflowInputsView/hooks/UseHandoffSync/query/ena/hook.ts
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@frano-m frano-m marked this pull request as ready for review June 8, 2026 13:40
Copilot AI review requested due to automatic review settings June 8, 2026 13:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 46 out of 46 changed files in this pull request and generated 2 comments.

@dannon

dannon commented Jun 8, 2026

Copy link
Copy Markdown
Member

Happy to defer to you all on the React side here -- so just to make sure I'm reading the scope right: the bug itself is just a dropped accessions: string[] at the boundary, but you took it as a chance to pull the handoff up into the app-level reducer/provider pattern (the same one findable-ui's exploreState uses)? That sounds good to me -- just want to make sure I'm understanding the intent.

One thing I'm genuinely not sure about on the new global-state approach -- might just be me misreading the flow: it looks like the assistant source never gets cleared on consume (clearing only happens on the entry buttons' onClick via onClearSource?). And useHandoffEnaQuery is enabled: accessions.length > 0 with no route awareness, with SequencingStep calling it unconditionally for its loading state, while useHandoffSync (the part that actually applies the result) is only mounted in WorkflowInputsView. So I'm wondering -- if you land on a SequencingStep in WorkflowView or OrganismWorkflowInputsView via browser back/forward (which wouldn't fire those onClicks), would the leftover accessions kick off the ENA query, flip the step inactive, and flash a spinner, with the fetched data dropped since useHandoffSync isn't there? It'd self-resolve so not a blocker either way, but it seems like a leak the old consume-once localStorage handoff couldn't produce. Would clearing the source once the handoff's applied make sense, or is there something already handling it that I'm missing?

…gle from state (#1295)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@frano-m frano-m marked this pull request as draft June 9, 2026 01:57
…1295)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 9, 2026 05:41
@frano-m

frano-m commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

Good call — addressed in 96251ad. The reducer now keys handoffs by entity → path → inputs, and useHandoffSync dispatches clearHandoff after applying, so state is single-use. Two implications:

  1. No more cross-workflow bleed. If state has a handoff for path A and the user lands on path B (back/forward, direct nav, whatever), useHandoffInputs(entity, B) returns the default — no apply, no spinner. The path-match check sits in the same data lookup, not in a separate gate.
  2. No more entry-button reset contract. Removed the four onClearSource calls on Configure/etc. buttons — consume-on-apply handles it, so adding a new entry point to a stepper page in the future doesn't need anyone to remember to wire a clear.

Trade-off you noted upthread: forward-after-back to the same workflow now shows a blank stepper (the handoff was consumed). We landed on accepting that — it matches the rest of the stepper's per-mount semantics ("leave the page, come back fresh") and the only way to get pre-fills back is a fresh assistant session, which is consistent with how localStorage behaved on refresh.

@frano-m frano-m marked this pull request as ready for review June 9, 2026 05:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 47 out of 47 changed files in this pull request and generated 4 comments.

Comment thread app/providers/workflowHandoff/actions/setHandoff/action.ts
Comment thread app/components/Assistant/SchemaPanel/utils.ts
frano-m and others added 2 commits June 9, 2026 16:24
…l import (#1295)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…l import (#1295)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Ready for Review

Development

Successfully merging this pull request may close these issues.

Assistant: validate handoff to the stepper and Galaxy (incl. disabled "Launch In Galaxy" bug)

5 participants