Skip to content

fix(dataset-store): rewrite stored warnings that name refs into counted messages - #135

Open
jm-rivera wants to merge 1 commit into
devfrom
fix/rewrite-stored-warning-refs
Open

jm-rivera wants to merge 1 commit into
devfrom
fix/rewrite-stored-warning-refs

Conversation

@jm-rivera

@jm-rivera jm-rivera commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Dataset versions minted before #129 store warnings whose messages name dcids, which the frontend shows raw. #129 changed the writer to state counts and carry the refs in {count, refs} fields, but stored rows keep the old text, because record_version never rewrites a version it already holds. Migration 0039 rewrites them in place. On dev, python -m dataset_store.warning_formats (#133) counted 528 version rows to rewrite: 283 unserved-entity sentences, 239 range-window member sentences, 0 member sentences from a window that is not a range, 2 currency lists and 5 deflator lists. No stored warning carries member_count.

Each old format becomes the message and fields the current writer in materialise/finish.py produces, byte for byte:

  • A coverage_below_scope unserved-entity list becomes " No observations were served for N requested entities." with unserved_entities, keeping the stored order.
  • A member sentence from a window that is not a range becomes " K of M series report at a different frequency and are left out of the figures." with off_cadence_members and member_count. M is the length of the row's trace.resolved_stat_vars, one entry per member request. When the trace has no such list, "of M" and member_count are left out.
  • currency_unmapped_omitted becomes " It left out N entities." with omitted_entities, and deflator_missing_omitted becomes " It left out rows of N entities." with omitted_entity_years, counts included for "and N more" and "(truncated)".
  • A countries-fallback sentence after any of these stays as stored.

A member sentence from a range window ("... in the requested years.") counted two kinds together: members that report at another frequency, and members on the group's frequency with no point in the range. The current writer stores those as off_cadence_members and out_of_window_members. The row records only each member's ref, roles and observation count, so nothing in it can split the two. Filing all of them under one field would state a cause that is false for some, and a field the writer never produces would be a second stored shape. These rows therefore become " K of M series hold no observations at the group's observation period in the requested years and are left out of the figures." with no field. The sentence is true of both kinds, and the refs stay in immutable_body.

Only version_record.warnings changes. The patterns are anchored at both ends, so a message already in the new shape matches none of them and a rerun updates zero rows. A warning that is not an object, has no code or has a NULL message passes through unchanged. The rewrite runs in the migration's single transaction, so a failure leaves nothing applied.

Prod has served #129 since the production deploy of 1851e7e (#131), so once this migration reaches main, the image before it already writes the new shapes and no row is left in an old one. The counts module (#133) is on dev only. To read the prod counts before 0039 rewrites them, release dev to main with #133 and without this PR, run gcloud run jobs execute dfo-prod-datasets-migrate --region=europe-west1 --project=prod-fin-observatory --args=-m,dataset_store.warning_formats --wait, then merge this PR.

Testing: test_0039_rewrites_stored_warnings_that_name_refs_into_counted_fields in tests/dataset_store/test_live.py seeds one row of every old format from tests/dataset_store/_warning_refs.py. It checks that each rewritten warning equals the current writer's output and validates as a WarningModel, that immutable_body is unchanged, and that a rerun changes nothing. It includes range-window rows with and without resolved_stat_vars. test_0039_leaves_no_stored_warning_in_an_old_format checks that every old-format line of warning_formats reads 0 afterwards. Both fail with the range branch mapped to off_cadence_members. On a fresh postgres:18.4, the live, counts, migrate and immutability tests gave 120 passed. tests/datasets_api tests/dataset_store gave 3336 passed. ruff and ty are clean.

…ed messages

Migration 0039 rewrites each warning in `datasets.version_record.warnings` that #129's writer
no longer produces: a `coverage_below_scope` sentence listing unserved entities or group
members, and a `currency_unmapped_omitted` or `deflator_missing_omitted` list of entities.
Each becomes the counted message and `{count, refs}` field the current writer stores. A
range-window member sentence counted off-cadence and out-of-window members together, and the
row stores nothing that splits them, so it becomes a counted sentence true of both with no
field. Only the warnings column changes, and a rerun touches zero rows.

This branch has not been deployed

No deployments
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.

1 participant