Skip to content

Issue-2843/compatible wizard [help needed] - #2889

Open
seolhwa-kim-data4life wants to merge 9 commits into
OHDSI:developfrom
seolhwa-kim-data4life:issue-2843/compatible-wizard
Open

Issue-2843/compatible wizard [help needed]#2889
seolhwa-kim-data4life wants to merge 9 commits into
OHDSI:developfrom
seolhwa-kim-data4life:issue-2843/compatible-wizard

Conversation

@seolhwa-kim-data4life

@seolhwa-kim-data4life seolhwa-kim-data4life commented Jul 13, 2026

Copy link
Copy Markdown

Description #2843

When a condition is selected in the Wizards dashboards, the concept name (e.g. "Acute bronchitis") was being sent to the endpoint as CONCEPT_CODE, but the SQL joins on concept_code, so it never matched.
Per the ticket, non-HANA (duckdb/postgres) datasets should send the concept_id and HANA datasets should send the concept_code.

Here is following changes in wizard-dashboard (https://github.com/data2evidence/wizards-dashboard/pull/24)

Verified locally

  1. Incidence rate, prevalence rate, mortality rate, and demographics dashboards are visible for synpuf5pct dataset in the D2E develop portal
image

Help needed

I wish a careful review / helping me test on these — this isn't my main area (nor ts / vue / wizards test).
How can i also check Incidence rate, prevalence rate, mortality rate, and demographics dashboards are visible for HANA_CDMSYNPUF1K dataset?

seolhwa-kim-data4life and others added 4 commits July 10, 2026 15:38
…oards

Wizards dashboards sent the condition concept NAME to parquet-export as
CONCEPT_CODE, so the SQL join on concept_code never matched (and names with
spaces were rejected by the endpoint). Non-HANA (duckdb/postgres) datasets need
concept_id and HANA datasets need concept_code.

Frontend + config layer (Tasks 1-3):
- 03_Config seed: add condition attributes that return the identifier as `value`
  and the concept name as `text` (useRefValue + useRefText) -
  condition_occ_concept_id (@REF.CONCEPT_ID) for duckdb, conditionconceptcode
  (@REF.CONCEPT_CODE) for HANA and HANA-lean, in both CDW and PA configs.
- Add knex migration to patch these attributes into already-seeded configs
  (the seed is insert-ignore on existing environments).
- Point the wizards condition fields at the new attributes and set
  allowFreeText:false (identifiers are not free-typed); sync dev mocks/docs.
- Plumb the dataset dialect from the portal /dataset endpoint through
  useDashboardFlow -> ChartToolbar -> ShinyDashboardModal -> ShinyDashboardIframe
  into the postMessage AUTH_TOKEN context so the dashboard can pick the right key.
- Update shinyDashboardContext tests for the new dialect field.

Backend (parquet-export CONCEPT_ID support), SQL templates and Python dashboard
changes are follow-ups.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Non-HANA (duckdb/postgres) SQL templates now match conditions on concept_id, so
the endpoint must accept CONCEPT_ID1..5 in the request body alongside the
existing CONCEPT_CODE1..5 (HANA).

- Add CONCEPT_ID1..5 to RESERVED_PLACEHOLDERS and substitute them unquoted,
  validated as digits only (isValidConceptId), defaulting empty slots to 0 so
  the SQL stays valid (templates filter concept_id = 0).
- Add a dialect guard: reject with a clear 400 when a HANA dataset gets a
  concept_id template or a non-HANA dataset gets a concept_code template,
  instead of silently returning an empty result. Dialect is read from the
  portal /dataset response (added `dialect` to DatasetMetadata).
- Export substituteTemplateParams/isValidConceptId and guard app.listen behind
  import.meta.main so the module is unit-testable; add index.test.ts.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The wizard e2e spec embeds the canonical cross-sectional-demographics dashboard
(Python + SQL) that it pastes into Manage Dashboard for the duckdb Demo dataset.
Update it to match the new dialect-aware contract:

- Forward context.dialect from the AUTH_TOKEN postMessage into a d2e_dialect
  Shiny input and read it in get_context().
- Build the parquet-export payload with CONCEPT_ID{n} (int) for non-HANA and
  CONCEPT_CODE{n} (string) for HANA, keyed off the dialect.
- Rewrite the entry CTE to join {{VOCAB_SCHEMA}}.concept on concept_id
  ({{CONCEPT_IDn}}) and drop empty (0) slots, sourcing concept_code from the
  joined concept row.
- Show the condition displayName (not the raw id) in the dashboard description.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@seolhwa-kim-data4life
seolhwa-kim-data4life marked this pull request as ready for review July 13, 2026 14:41
Copilot AI review requested due to automatic review settings July 13, 2026 14:41
@seolhwa-kim-data4life seolhwa-kim-data4life changed the title Issue 2843/compatible wizard Issue-2843/compatible wizard [help needed] Jul 13, 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 updates the Patient Analytics “wizards” flow to correctly pass a condition identifier that matches the target database dialect (concept_id for non-HANA; concept_code for HANA), and propagates the dataset dialect into the Shiny dashboard context so parquet export SQL templates can be substituted safely.

Changes:

  • Update wizard condition fields/config to use concept identifiers appropriate for non-HANA vs HANA (and disable free-text for these fields).
  • Add dataset dialect propagation from UI → Shiny context and use it to shape condition payloads.
  • Extend parquet-export to support {{CONCEPT_IDn}} placeholders (validation + substitution) and add unit tests + a server-start guard for import-based tests.

Reviewed changes

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

Show a summary per file
File Description
tests/e2e/tests/09-patient-analytics/wizard.spec.ts Pass dialect into Shiny context and build condition filters as concept_code vs concept_id depending on dialect; update one SQL template to use {{CONCEPT_ID1}}.
plugins/ui/apps/wizards/wizards-config.json Switch standard wizard condition config paths to condition_occ_concept_id and disable free text.
plugins/ui/apps/wizards/wizards-config-hana-lean.json Switch HANA-lean wizard condition config paths to conditionconceptcode and disable free text.
plugins/ui/apps/wizards/src/config/wizardDefinitions.ts Update mock wizard field config paths to condition_occ_concept_id.
plugins/ui/apps/wizards/src/config/cdwConfig.ts Update mock CDW config + mock attribute values to use condition_occ_concept_id with numeric values.
plugins/ui/apps/wizards/CONFIGURATION.md Update documented attribute mapping to the new condition identifier fields.
plugins/ui/apps/vue-mri-ui-lib/src/utils/shinyDashboardContext.ts Add dialect to Shiny dashboard auth message context.
plugins/ui/apps/vue-mri-ui-lib/src/utils/tests/shinyDashboardContext.test.ts Extend tests to include dialect and verify defaulting to null.
plugins/ui/apps/vue-mri-ui-lib/src/composables/useDashboardFlow.ts Fetch dataset dialect from portal and include it in computed dashboard context.
plugins/ui/apps/vue-mri-ui-lib/src/components/ShinyViewer/ShinyDashboardModal.vue Thread dialect prop into the iframe component.
plugins/ui/apps/vue-mri-ui-lib/src/components/ShinyViewer/ShinyDashboardIframe.vue Include dialect in the postMessage auth context sent to Shiny.
plugins/ui/apps/vue-mri-ui-lib/src/components/ChartToolbar.vue Pass dialect from dashboardFlow context into the Shiny modal.
plugins/functions/parquet-export/index.ts Add CONCEPT_ID placeholder support, dialect/template mismatch checks, and guard app.listen behind import.meta.main.
plugins/functions/parquet-export/index.test.ts Add Deno unit tests for concept_id validation and template substitution behavior.
plugins/functions/mri-pg-config/src/db/seeds/03_Config.ts Seed new condition identifier attributes for HANA and DuckDB configs (and PA config entries).
plugins/functions/mri-pg-config/src/db/migrations/20260710120000_add_wizard_condition_identifier_attributes.ts Migration to patch existing stored config JSON with the new condition identifier attributes non-destructively.

Comment on lines 16 to 20
interface DatasetMetadata {
id: string;
databaseCode: string;
dialect?: string;
schemaName: string;
Comment on lines 752 to 753
} catch (error) {
dashboardSelectionError.value = (error as Error)?.message || 'Failed to load dashboard configuration'
@jerome-ng

jerome-ng commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Thanks for your PR @seolhwa-kim-data4life. I see changes have been merged in https://github.com/data2evidence/wizards-dashboard/pull/24 and this PR focuses on

  1. Providing the dialect to the shiny app
  2. Modifying the configs for the data to be available (concept id) in this case

No 2 is less trivial as the suggested modifications will also change how cohort builder behaves. We will discuss this during the analyze workshop to figure out what should be used and create appropriate issues for both these points.

@jerome-ng

Copy link
Copy Markdown
Collaborator

@seolhwa-kim-data4life Hey. Are you able to change the use of concept id to use concept codes instead? The issue is that in order to use concept id, it must be available in the filter cards as an attribute, but we do not want to show the concept id. We can use the SNOMED ones which should be available in the demo dataset. we possibly might not need the dialect anymore if we use the code

The condition typeahead previously returned concept_id for duckdb/postgres
datasets and concept_code for HANA, which required plumbing the dataset
dialect down to the Shinylive dashboards and exposing a raw concept id as a
cohort builder filter card attribute.

Return the SNOMED concept_code everywhere instead, so the dashboards send a
single CONCEPT_CODEn payload key and the dialect plumbing is unnecessary:

- parquet-export: drop the CONCEPT_IDn placeholders, isValidConceptId and the
  dialect/template mismatch guard
- mri-pg-config: the duckdb OMOP config gains a conditionconceptcode
  attribute (@REF.CONCEPT_CODE, restricted to standard SNOMED conditions);
  its PA entry stays hidden so no code column reaches the cohort builder
- vue-mri-ui-lib: remove the dataset dialect fetch and the dialect field of
  the postMessage dashboard context
- wizards configs and e2e wizard spec: point condition fields at
  conditionconceptcode and match on quoted {{CONCEPT_CODEn}} in SQL

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@seolhwa-kim-data4life

Copy link
Copy Markdown
Author

@seolhwa-kim-data4life Hey. Are you able to change the use of concept id to use concept codes instead? The issue is that in order to use concept id, it must be available in the filter cards as an attribute, but we do not want to show the concept id. We can use the SNOMED ones which should be available in the demo dataset. we possibly might not need the dialect anymore if we use the code

@jerome-ng now returns the SNOMED concept_code
One open question on point 2: the HANA configs already expose condition_concept_code with filtercard.visible: true, but I've kept the new duckdb entry hidden (visible: false, category: false). Should duckdb match HANA and show it, or stay hidden?

https://github.com/data2evidence/wizards-dashboard/pull/27

here it is now using concept_code back in the pr. idk what to do with vocabulary_id = 'SNOMED' tho. maybe will see with alicia.

seolhwa-kim-data4life and others added 2 commits July 31, 2026 15:11
The shipped dashboards send "abc123" for condition slots the user left empty;
the stub was sending "". Both fail the concept join, but the stub should
reproduce the real payload. With this the embedded SQL is byte-identical to
sql-scripts/duckdb/cross-sectional-demographics.sql.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants