Fix: parse --workflow-dag-simple JSON before posting provenance - #158
Merged
Conversation
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.
Summary
--workflow-dag-simpleis never JSON-parsed, so the CLI posts it to the API as a string where the API requires an object. Every provenance write that passes this flag has failed with a 422 since app v0.3.0 was deployed on 2026-05-19. This parses the option in the three provenance commands, matching how--workflow-dagis already handled.The bug
In
cli_provenance.py, the two DAG options are treated inconsistently:The raw string is threaded into
get_provenance()(provenance.py:200) and dropped into the payload verbatim, so the API receives:The app declares
workflowDagSimpleas an object (packages/schemas/src/crud.ts:61, applied to input at:239), producing:c812fdf("Add simple workflow visualisation data", 2026-05-04) is the only commit that has ever touched this field, and it shipped without the parse. The path has therefore never worked.Why it wasn't caught
The database has 7 dataset / 5 geometries / 17 product runs where
workflow_dag_simpleis a valid object, which makes the feature look healthy. Those rows are not from the CLI. They're all dated 2026-04-29 and their images (0.0.0-485-g31b2df1and similar) predate the feature at commit 492, so the CLI could not have written them. Theirprovenance_json->'workflowDagSimple'is also absent while the column is populated — consistent with a direct backfill, likely to give the product-output sidebar UI something to render.Two things then hid the breakage:
dataset_run,geometries_run, orproduct_runcreated after 2026-05-19. The failure is total, not intermittent..jsonin S3 does not mean the write succeeded. The aus-states geometry run on 2026-05-25 wrote its provenance to S3 with a string and never landed in the database — it failed exactly this way and went unnoticed.Changes
cli_provenance.py— parse the option in_write_dataset_provenance,_write_geometry_provenance, and_write_product_provenance:and pass the parsed value through. Type hint on
_meta_provenancecorrectedstr | None→dict | None; docstring entry added.provenance.py— same type-hint correction onget_provenance.This also fixes the provenance JSON written to S3, which currently stores a string where the backfilled DB rows hold objects.
Verification
Ran the real
csdr provenance datasetcommand insidecsdr-cloud-spatial:latestwithrequests.postintercepted (no DB or S3 writes), using the seagrass template's actualworkflow-dag-simplevalue:workflowDagSimpleon the wirestr→ JSON stringinvalid_type— matches the reported error verbatimdict→ JSON objectAll 37 templates that pass
--workflow-dag-simplevalidate against the current schema once parsed, so no template changes are needed.ruff format --checkandruff checkpass.Not yet verified: a live 200 against
data.dev.sd17.org, since that writes a realdataset_runrow to dev. Suggest confirming via a seagrass-global dataset run after the image builds.Reviewer question
This assumes the intended contract is CLI parses, API receives an object, consistent with
--workflow-dagand with the objects already in the database. The alternative — loosening the app schema to accept a JSON string — would leave the existing object rows inconsistent, so I've gone the other way. Flagging in case the original design intended otherwise.Blast radius
Fixes provenance DB writes for all datasets, geometries, and products. Requires a new image build before the workflows will succeed. No migration needed; existing rows are already objects and stay valid.