fix(hr): backport the one-row-per-person class_people fix to release-2026.07.1 - #2509
Merged
aleksdotbar merged 2 commits intoAug 13, 2026
Merged
Conversation
`silver.class_people` overstated headcount by ~5% on virtuozzo (1440 rows for
1420 employees; 414 "active" rows for 394 active people). Two defects
compounded — neither is sufficient alone:
1. The `__to_class_people` staging views read append-only bronze WITHOUT
`FINAL`. Airbyte appends a full snapshot per sync and bronze is
RMT(_airbyte_extracted_at), which only collapses on background merge, so a
bare read emits every unmerged snapshot row.
2. Those views appended a version axis (`lastChanged` / `createdDateTime` /
`whenCreated` / `Last_Functionally_Updated`) to `unique_key`, per ADR-0004.
That turned each transient bronze duplicate into a permanently distinct
silver key, so the versionless RMT could never collapse it — and `FINAL`
could not either, because the keys genuinely differ. `valid_to` was
hardcoded NULL, so nothing marked which row was current.
The surplus was regenerated on every run and drifted with bronze merge timing,
which is why it surfaced as two separate audit findings ("duplicate active
emails" and "active-count drift").
class_people is a current-state snapshot, not an SCD2 history table: it is
`materialized='table'` and rebuilt in full each run, so it cannot accumulate
history by construction. HR attribute history already lives in the per-source
`*_snapshot` / `*_fields_history` chain, which is incremental+append and tracks
strictly more fields.
Changes:
* All four `__to_class_people` views (active-directory, bamboohr, ms-entra,
workday): entity-level `unique_key`, `valid_to` dropped, bronze read with
`FINAL`. Projections stay aligned 1:1 for the positional UNION.
* `ELSE 'active'` catch-alls replaced with `'unknown'`. A BambooHR record with
`status=''` / `employmentHistoryStatus='Third party'` was being counted as
active — this is the residual +1 between bronze (393) and silver (394).
`accepted_values` updated accordingly.
* Same missing `FINAL` fixed in `bamboohr__working_hours` /
`workday__working_hours`: there the `status='Active'` filter ran before
dedup, so a leaver could still qualify via a stale snapshot row.
* New `assert_class_people_one_row_per_person` data-quality check, plus
`unique`/`not_null` on `class_people.unique_key`.
* Reconcile the contract, which contradicted itself: ADR-0004 mandated the
version axis while DESIGN.md and ADR-0001 called class_people a pure
snapshot, and ADR-0001 exempted it from the read-dedup cleanup on the
now-false premise that it "already collapses to one row per key".
Verified against the live virtuozzo cluster: the fixed transform yields
1420 rows / 1420 distinct keys, active 394 -> 393, unknown 1; the new check
reports exactly the 20 current violations and goes green after a rebuild.
No migration needed — `materialized='table'` fully replaces the table.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Roman Mitasov <Roman.Mitasov@constructor.tech>
(cherry picked from commit 3062d4f)
The committed DDL snapshot still declared `valid_to Nullable(DateTime)` on `silver.class_people`. The `connectors-ddl snapshot + field parity` job rebuilds the database from the dbt models and re-dumps the DDL, so removing the column from the four `__to_class_people` producers made the snapshot drift and fail the gate. This is exactly the one-line delta the CI drift check computed; no other snapshot references `valid_to`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Roman Mitasov <Roman.Mitasov@constructor.tech> (cherry picked from commit 83db3ab)
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ktursunov
approved these changes
Aug 13, 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
2 participants
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.
Backport of #2125 (
3062d4f2+83db3abf, merged to main 2026-08-03; the release branch predates it).Why
The release branch still has both defects #2125 fixed, and together they create duplicate current rows in
silver.class_people:__to_class_peoplestaging views read append-only bronze withoutFINAL, so unmerged Airbyte snapshot rows leak through as extra rows.unique_keyembeds a per-version axis (lastChangedfor BambooHR), turning each transient duplicate into a permanently distinct silver key that neither RMT norFINALcan collapse.On an environment running this branch,
class_people FINALcan hold more rows than there are accounts, which overstates headcount and duplicates people downstream of the person registry.What
Clean cherry-pick of the two PR #2125 commits, covering all four HR connectors (bamboohr, active-directory, ms-entra, workday): entity-level
unique_key, deterministicFINALreads, theassert_class_people_one_row_per_persondbt test, and the matching DDL/docs updates. One conflict in the BambooHRschema.ymlmodel descriptions resolved in favor of this branch's wording (it already carries the full-field-sync descriptions).dbt parseclean.Validation
After deploy,
SELECT count() FROM silver.class_people FINALgrouped by source should return exactly one row per account; the new dbt test enforces this in CI.