Add missing descriptions for templates - #44
Open
Zhimin-arya wants to merge 2 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Several new user-facing descriptions incorrectly describe their nodes’ actual behavior.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds user-facing descriptions to flow-template nodes and introduces configurable database/schema selectors.
Changes:
- Replaces placeholder node descriptions across templates.
- Adds database and schema configuration to several flows.
- Clarifies ARTEMIS documentation.
File summaries
| File | Description |
|---|---|
flows/pyomop.json |
Adds configurable database/schema references. |
flows/nlpflow.json |
Documents NLP nodes and configures storage targets. |
flows/nlp_pyner.json |
Moves database/schema values into selectors. |
flows/Load_DICOM_Vocab.json |
Documents relationship transformation. |
flows/fhir_anonymizer.json |
Adds anonymization workflow descriptions. |
flows/etlQuestionnaireResponseEQ-5D-5L.json |
Documents conversion and mapping nodes. |
flows/etlPatientToOMOPPerson.json |
Documents patient ETL stages. |
flows/etlfhirtoomop.json |
Documents FHIR-to-OMOP processing stages. |
flows/clinicaltrialsomop.json |
Documents ETL nodes and adds target selectors. |
flows/artemis.json |
Clarifies the README node description. |
Review details
Suppressed comments (3)
flows/fhir_anonymizer.json:147
- This node only downloads and extracts the R4 command-line package and returns its executable path; it does not anonymize any resources. The description should identify this setup step.
"description": "Anonymize FHIR R4 resources using the Windows-compatible processing path.",
flows/fhir_anonymizer.json:171
- This node downloads and extracts the STU3 package and returns its DLL path; the actual anonymization occurs in the downstream
anonymizernode. The current description assigns processing behavior to the wrong node.
"description": "Anonymize FHIR STU3 resources using the macOS-compatible processing path.",
flows/fhir_anonymizer.json:195
- This node only downloads and extracts the STU3 package and returns its executable path; it does not anonymize FHIR resources. Describe the setup operation instead.
"description": "Anonymize FHIR STU3 resources using the Windows-compatible processing path.",
- Files reviewed: 9/10 changed files
- Comments generated: 5
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| @@ -517,7 +517,7 @@ | |||
| "data": { | |||
| "name": "python_node", | |||
| "error": false, | |||
| "description": "Describe the task of node python_node_0", | |||
| "description": "Initialize the FHIR-to-OMOP transformation flow and shared runtime values.", | |||
| "data": { | ||
| "name": "anonymizer", | ||
| "description": "Describe the task of node python_node_2", | ||
| "description": "Load the FHIR anonymization library and expose its processing functions to downstream nodes.", |
| "data": { | ||
| "name": "R4 Fhir Anonymizer - Mac", | ||
| "description": "Describe the task of node python_node_2", | ||
| "description": "Anonymize FHIR R4 resources using the macOS-compatible processing path.", |
| "data": { | ||
| "name": "data_file_processor", | ||
| "description": "Describe the task of node python_node_6", | ||
| "description": "Read the supplied NDJSON file and prepare its FHIR resources for anonymization.", |
| "data": { | ||
| "name": "config_file_processor", | ||
| "description": "Describe the task of node python_node_7", | ||
| "description": "Parse and validate the supplied FHIR anonymization configuration.", |
csafreen
reviewed
Sep 10, 2026
| { "code": "demo_database", "name": "database_code" } | ||
| ], | ||
| "schemas": [ | ||
| { "name": "schema_name", "schema": "cdmdefault" } |
Contributor
There was a problem hiding this comment.
for demo_database, shouldn't demo_cdm be the schema
csafreen
reviewed
Sep 10, 2026
| "schemas": [ | ||
| { | ||
| "name": "schema_name", | ||
| "schema": "cdmdefault" |
Contributor
There was a problem hiding this comment.
for demo_database, shouldn't demo_cdm be the schema
csafreen
requested changes
Sep 10, 2026
csafreen
approved these changes
Sep 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Merge Checklist
Please review this list and check any items that require additions or modifications beyond your core changes. Reviewers can also use it to help confirm that nothing was missed.
Test if the flow runs successfully as an imported json file
Test if the flow runs successfully as template
DATAFLOW_TEMPLATE_BRANCHenvironment variable in docker-compose.yaml to the branch being reviewed and successfully run the flowInspect the nodes in the flow (either as a template or imported json file):
[Currently not visible in UI]