Feat/problem labels - #298
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 57f784199f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| )> } | ||
| & { __typename: 'ConstraintViolations' } | ||
| ) | ||
| | { __typename: 'DatasetValidationViolations' } |
There was a problem hiding this comment.
When publication is blocked by dataset rules, the backend can now return DatasetValidationViolations, but handlePublish in src/app/root/[domain]/[lang]/model/page.tsx only handles ConstraintViolations and OperationInfo; this new variant therefore falls through to the success toast at line 398 even though nothing was published. Handle this typename explicitly and surface the violations or an error instead of reporting editor-model-published-ok.
Useful? React with 👍 / 👎.
| </Stack> | ||
| </Stack> | ||
| {validationViolations.length > 0 && ( | ||
| <Alert severity="warning" sx={{ mb: 2 }}> |
There was a problem hiding this comment.
When any returned violation has severity: ERROR, this hard-coded warning presentation downgrades it to the same yellow warning used for WARNING violations, even though the generated schema defines ProblemSeverity as controlling how a problem is presented. Derive the aggregate alert severity from the violations, or render entries separately, so blocking errors are visibly distinguished.
Useful? React with 👍 / 👎.
2 participants
Description
I created a feature for using category labels rather than ids in dataset violation messages.
Merge the backend first, see this PR.
kausaltech/kausal-paths#281
Backend (3 files, uncommitted)
DatasetDimensionCoordinate gains dimensionLabel and categoryLabel. Labels resolve at the GraphQL boundary, not at evaluation time — deliberately: a materialization is shared across users and languages, and RuleViolation.key baseline-diffs on the identifier coordinates at edit time, so baking labels into the stored payload would be wrong twice over.
build_coordinate_labels(violations) does one query pass over the datasets involved, so a violation list costs a fixed number of queries rather than one per coordinate. It reproduces the evaluator's own column rule — DatasetSchemaDimension.column_name when set, the scoped dimension identifier otherwise — so the lookup key matches what the evaluator recorded. Falls back to the identifier when unresolvable. Wired into all three construction sites: the per-dataset resolver, the instance-wide one, and the publish-mutation payload.
UI (5 files, on feat/dataset-validation)
Renders coordinates.map(c => c.categoryLabel).join(' + ') instead of the group id, falling back to the group id. Types regenerated. I also reworded both strings, since "Kategoriegruppe" no longer describes what the placeholder holds:
▎ Für Value fehlt ein erforderlicher Wert für „Industrie + Strom“ in den Jahren 2020, 2021, 2022, 2023. Eine ausdrücklich eingetragene Null gilt als Wert.
Verified against the real data — all 8 violations resolve: "Private Haushalte + Strom", "Industrie + Erdgas", "Private Haushalte + Fernwärme", etc. dimensionLabel is exposed too (giving "Sektoren: Industrie + Energieträger: Strom") but unused for now — available if you want it in a tooltip or a grid.
Two things you should know
English will show German labels, and it's a data problem, not a code one. MODELTRANS_FALLBACK/LANGUAGE_CODE make en the default language, so modeltrans reads the base column for English. These categories store German in the base column (label='Industrie') with i18n={'label_en': 'Industry'} duplicating English into the JSON — so English resolves to the base column and gets German, while German resolves by fallback and gets it right by accident. I verified the mechanism itself works: an in-memory category with label_de set resolves correctly under de and fi. So this affects anything rendering these labels, not just validation messages, and it's pre-existing. Fixing it means either flipping the storage for de-default instances or setting label_de explicitly.
The UI branch won't lint clean until the backend is deployed. eslint's GraphQL plugin resolves the schema via graphql.config.ts, which falls back to https://api.paths.kausal.dev when no local schema.graphql exists — and production doesn't have the new fields yet, so it reports Cannot query field "dimensionLabel". With a local schema export in place it's 0 errors, 9 pre-existing deprecation warnings. If you want it quiet locally: python manage.py export_schema paths.schema > ~/devel/kausal-paths-ui/schema.graphql (note that filename isn't gitignored, so I removed mine after use).
Backend: 1774 tests pass, ruff clean, Run Mypy...Passed. UI: tsc reports no errors in the files I touched (86 total, all pre-existing elsewhere), and the generated diff is purely the two added fields. One thing to flag — I used npm run graphql-codegen in what is a pnpm project, which rewrote packageManager in package.json; I reverted that, but pnpm isn't on PATH here, so you may want to re-run codegen with pnpm yourself if that matters for the lockfile.
✅ Pre-Merge Checklist
Type of Change
Testing
Manual testing instructions
If feature requires manual testing by reviewer, you can provide instructions here.Internationalization & Accessibility
Dependencies