Fix where the brief and the exercise data disagree - #6
Merged
Merged
Conversation
A dry run through the exercise surfaced five claims in the README and sources.yml that the repo doesn't deliver, and one thing candidates are never told: how long they have. The data edits are three cells, not new rows. The dataset is small enough to read in full during the session, and it should stay that way. - Session length is now stated: 60 minutes. - A Meridian order carries a real grouping separator, so "thousands/decimal separators" is true of the data and not just the brief. - Email now lines up across the two systems in more than one shape, so cross-source identity is a decision rather than a single exact match. - customer_id is documented as living only on pages; the money keys on account_id. - The mid-session ingest step is announced the way the add-partner step is. - Vendor names dropped from source descriptions; the exercise shouldn't imply a stack.
There was a problem hiding this comment.
Pull request overview
Aligns the exercise brief with its fixtures and runtime behavior.
Changes:
- Documents the 60-minute limit and mid-session ingestion.
- Clarifies identity, currency, and source semantics.
- Adds representative identity and currency edge cases.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
README.md |
Updates exercise guidance and data examples. |
dbt/models/sources.yml |
Clarifies source semantics and currencies. |
data/initial/meridian_orders.csv |
Adds European grouping separators. |
data/initial/meridian_customers.csv |
Adds cross-system identity edge cases. |
.gitignore |
Ignores generated dbt artifacts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| description: Stable; correlates to accounts. | ||
| - name: customer_id | ||
| description: Unstable; may be merged at any time (e.g. Segment Unify). Resolve via identity_merges when building marts. | ||
| description: Unstable behavioural id, emitted by the tracker and merged at any time. Only pages carries it — orders and transactions key on account_id. Resolve via identity_merges when building marts. |
santiaro90
marked this pull request as ready for review
August 6, 2026 02:12
pvcarrao
approved these changes
Aug 10, 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.
A dry run through the exercise — fresh clone, Docker up,
init.sh, then actually solving both challenges — turned up five places where the brief promises something the repo doesn't contain, and one thing candidates are never told: that they have 60 minutes.Nothing here makes the exercise easier. Two of the changes make it harder.
What was off, and what changed
mo_18is now"1.050,00 kr". A parser that strips commas produces1.05000and fails loudly instead of silently returning the wrong numbercustomer_iddescribed as TTG's identity, but onlypagescarries it and the money keys onaccount_idsources.ymlbin/ingestmid-session was unannounced, unlikebin/add-partnersources.ymllisted USD as a Meridian currency; no USD order existsTwo smaller ones: vendor product names are gone from the source descriptions, since the exercise shouldn't imply anyone's stack; and
dbt depsno longer leaves an untrackeddbt_packages/in the candidate's tree mid-session.What I deliberately didn't do
Explain the trap in
bin/ingest. The incremental batches turn the merge log into a chain and re-send a row that already exists. Single-hop resolution silently resolves people to the wrong id, and a duplicate breaks uniqueness. That is the sharpest thing in Challenge 1 and it stays unexplained — the README says the step is coming andsources.ymlsays the log is "appended to over time, like any other source", which is what a real engineer would get. Working out what that implies is the exercise.Same reasoning on identity:
sources.ymlnow says an address in both systems is "a hint, not a key", and stops there. The candidate finds the shared address themselves or they don't.Add rows or tables. The dataset is small enough to read end to end inside the session, which is most of why the exercise works. Three cells changed; nothing grew.
Verified
Loaded the edited data and built a full working solution on top of it — staging, currency conversion off the seed, facts at both grains, identity dimension, tests. 42 nodes green.
1.050,00 krparses to 1050.00; the naive strip-commas version errors rather than returning 105000.bin/add-partnermid-session plus a rebuild on an untouched tree still picks up the new brand, so Challenge 2's acceptance criterion is unaffected.