Add construct_block / construct_item tags on dim_question - #54
Merged
Conversation
Two optional text attributes for marking a question as part of a reusable scale (PHQ-9, GAD-7, eNPS, ...) — authored as custom JSON attributes on the survey definition, flowing through int_survey_questions to two new columns on dim_question. Provenance only: cross-survey/cross-version pooling stays the parent_question_id opt-in (invariant 5). - Publish-gate validation lints shape + the block/item co-occurrence and rejects construct_item on a matrix or paneldynamic container (an item belongs to a leaf question, not the container that groups them). - dbt staging inherits construct_block from a matrix/panel container to its leaves; construct_item is leaf-only. - New singular test construct_pair_integrity backstops the warehouse-side invariant against backdoor inserts. - Design doc §3.5 gains a "Construct membership" subsection; §4.10 records the rejected alternative of treating construct_block as a pooling key. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds optional construct provenance tags (construct_block, construct_item) to flow from authored SurveyJS JSON through dbt staging into the dim_question mart, with publish-time validation and a warehouse-side integrity test. This supports identifying questions that belong to reusable scales (e.g., PHQ-9, GAD-7) without changing the explicit cross-survey/version pooling mechanism (parent_question_id).
Changes:
- Add publish-gate validation + unit tests to enforce construct tag shape, container/leaf placement rules, and block/item co-occurrence.
- Extend dbt intermediate + mart models to propagate construct tags into
dim_question, and document the new columns. - Add a dbt singular test to enforce
construct_item⇒construct_blockintegrity.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| survey-engine-design-doc.md | Documents new dim_question columns and clarifies “provenance only” (not a pooling key). |
| dbt/tests/singular/construct_pair_integrity.sql | Adds singular test asserting construct_item is never present without construct_block. |
| dbt/models/marts/dim_question.sql | Adds construct_block/construct_item columns to the stable question dimension. |
| dbt/models/marts/_marts.yml | Documents the two new dim_question columns for analysts. |
| dbt/models/intermediate/int_survey_questions.sql | Extracts construct tags from definition JSON, including inheritance for matrix/paneldynamic leaves. |
| api/tests/test_publish_gate.py | Adds publish-gate tests covering construct tag acceptance/rejection cases. |
| api/survey_engine/validation.py | Implements publish-time validation for construct tag type, placement, and inheritance rules. |
Comment on lines
42
to
+44
| min(panel_name) as panel_name, | ||
| min(construct_block) as construct_block, | ||
| min(construct_item) as construct_item, |
Comment on lines
+609
to
+615
| if qtype in MATRIX_TYPES: | ||
| if isinstance(owner_item, str): | ||
| raise InvalidDefinition( | ||
| f"question {name!r}: construct_item belongs to a matrix row, " | ||
| "not the matrix itself — move it onto the row" | ||
| ) | ||
| for row in element.get("rows", []) or []: |
…ility test Two silent-failure paths Copilot flagged on #54: 1. construct_block / construct_item on a matrixdropdown column were accepted by validation but silently dropped by int_survey_questions (which only reads construct_* from rows). Reject loudly at publish — a cell's construct identity is its row's, and per-column tagging is a future extension if we ever need different items per row dimension. 2. dim_question collapses the (survey_id, stable_name) grain with min(), which silently picks one tag when versions disagree (v1 'phq9_q1', v2 'gad7_q1'). Methodologically a tag change is the same event as a rename and should go through parent_question_id — so surface the divergence rather than hide it. New construct_tag_stability singular test fails when a question has more than one distinct non-null construct_block / construct_item across versions (untagged → tagged backfill is still permitted). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
construct_block,construct_item) on the survey JSON flow throughint_survey_questionsto new columns ondim_question, marking questions that belong to a reusable scale (PHQ-9, GAD-7, eNPS, …).parent_question_idopt-in (invariant 5); §4.10 records that decision.construct_itemon a matrix / paneldynamic container (an item identifies a leaf, not the container that groups leaves).construct_pair_integritybackstops the warehouse-side invariant against backdoor inserts.Authoring shape
Test plan
uv run ruff check . && uv run ruff format .uv run mypy api/python3 scripts/check_invariants.py— all 4 invariant checks cleanuv run pytest— 435/435 passing (incl. 13 new construct-tag tests)dbt build— 59/59 models + tests passing (incl. the newconstruct_pair_integritysingular test)dim_question.construct_block/construct_itempopulate as expected end-to-end🤖 Generated with Claude Code