fix(data-warehouse): pin mark_primed streak-reset test to its run generation - #69839
Merged
Merged
Conversation
…eration 69101 made mark_primed CAS on backfill_run_uuid, but the call site added by 68785 was never updated — the two merged in sequence and left master red. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
EDsCODE
marked this pull request as ready for review
July 9, 2026 22:18
adamleithp
approved these changes
Jul 9, 2026
Contributor
|
Reviews (1): Last reviewed commit: "fix(data-warehouse): pin mark_primed str..." | Re-trigger Greptile |
EDsCODE
enabled auto-merge (squash)
July 9, 2026 22:28
…t fix 69025 landed the same one-line fix with a different argument layout. Making the two byte-identical means whichever merges first, the other auto-merges instead of conflicting in this hunk. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
EDsCODE
disabled auto-merge
July 9, 2026 23:14
fercgomes
pushed a commit
that referenced
this pull request
Jul 10, 2026
… streak-reset test" This fix is unrelated to the wizard PR-ready email feature and is already landed properly upstream on master (#69839). Reverting so this branch doesn't carry a duplicate/out-of-scope change; the next merge from master picks up the real fix. Generated-By: PostHog Code Task-Id: 2e8e65a4-30b1-41ff-b086-1fd0ac10d64a
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.
Problem
Master is red.
test_backfill.py::TestFailureStreak::test_mark_primed_resets_streakfails with:Two PRs landed back to back and collided semantically, which no merge conflict caught:
d4891c22) added the call sitemark_primed(str(candidate.schema_id)).5e47e579) then mademark_primedCAS on the run generation, giving it a required keyword-onlyrun_uuid.#69101's CI ran against a master that predated #68785, so nothing ever exercised the two together. Both are green in isolation and broken in combination.
Every open PR branched off master inherits the failure, so this blocks the rest of the duckgres sink stack.
Changes
Pin the test to a run generation, matching how
mark_primedis now called in production. The state row gets abackfill_run_uuid, and the call passes the matchingrun_uuidso the CAS actually matches.Test-only. No production code changes.
Note
The test's intent is unchanged: promotion to
PRIMEDresets the failure streak. It now just supplies the generation thatmark_primedrequires, rather than relying on the old unpinned signature.How did you test this code?
I (Claude) ran these automated tests. I did not do any manual testing.
origin/masterworktree with no local changes, confirming this is a master breakage and not a branch artifact.test_backfill.pyon this branch: 29 passed (was 1 failed on master).pipeline_v3/duckgres/suite: 115 passed. On master the same run gave 114 passed, 1 failed, 1 error. The error was a teardown cascade from the failure, not a second defect. It passes in isolation on master.**_STREAK_RESET_UPDATESfrommark_primedmakes the test fail on the assertion, as it should.No new test is added here. This restores an existing test that catches a real regression: a promotion that silently leaves
consecutive_failuresandfirst_failed_atset, which would keep a healed schema classified as failing.Automatic notifications
Docs update
Not applicable. Test-only change with no user-facing or documented behavior.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Claude (Claude Code, Opus 4.8) found this while bringing a stack of duckgres sink PRs up to date with master and driving their CI green. It surfaced when merging master into #68808: the merged tree failed a test that neither branch had touched. Running the same test against a clean
origin/mastercheckout isolated it to master itself.The fix was scoped deliberately.
mark_primed's newrun_uuidrequirement is correct and intentional (#69101 added it to stop a retired run's late promotion from flipping a newer run's row), so the production signature is left alone and the stale call site is updated instead. I considered givingrun_uuida default to unbreak the call site, and rejected it: a defaultable generation pin defeats the CAS that #69101 exists to enforce.The commit was first made on the #68808 branch, then split out and cherry-picked onto master so it can land on its own rather than waiting behind that PR. #68808 was independently force-pushed mid-work, so nothing from that branch is carried here.
Skills invoked:
/django-migrations(while resolving an unrelatedmax_migration.txtconflict on #68808; no migration is touched in this PR).Worth a reviewer's eye: master's product-test shards were still in progress when this was opened, so the breakage may not have surfaced on the master run yet. It reproduces locally on
origin/masteratc0067304c.