Repository navigation
feat(workflow): review coverage survives a migration renumber merge (PP-ncxx.4) - #2436
Closed
timothyfroehlich wants to merge 11 commits into
Closed
timothyfroehlich wants to merge 11 commits into
timothyfroehlich wants to merge 11 commits into
Conversation
…PP-ncxx.4) _is_pure_merge_from_main now also accepts a merge of main whose only resolution renumbers the branch's Drizzle migrations after main's: conflicts only under drizzle/, everything else equals the clean merge, main's drizzle/ untouched, and each renumbered migration keeps its name, gets the next idx and a later when, and has byte-identical SQL. Gate and handoff text name which rule carried the coverage. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
On a runner with no global git user, git refuses to start a merge, so the conflict fixtures produced an ordinary commit instead of a merge commit. The existing conflict-resolution test passed without ever exercising a merge; the new renumber test failed. Both now pass the identity env and assert MERGE_HEAD exists. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- the branch may change nothing under drizzle/ but its own migrations and the journal, so taking main's drizzle/ cannot drop a reviewed edit - journal top-level fields and the renumbered entries' other fields must match - merge-tree runs once per merge commit - tests for an extra snapshot, a changed journal field, an edited main entry, a dropped branch edit, and a conflict outside drizzle/ - skill: a hand-resolved conflict outside drizzle/ still needs a review Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- the branch journal must be the merge base's journal plus appended entries, so a reviewed edit to an existing entry or field is not dropped - jq fails with error() instead of a FAIL marker a tag name could contain - parse merge-tree's conflicted section with awk, stopping at the first blank line - tests for two branch migrations, a tag containing FAIL, a later clean merge keeping the renumber label, and a dropped journal edit Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… (PP-ncxx.4) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…mber (PP-ncxx.4) A conflict-free merge whose tree differs from the clean merge reached the renumber check, so a merge that edited the branch's own snapshot inherited coverage. Require a drizzle/ conflict and a higher idx for every entry. Qualify the AGENTS.md §8.12 reference. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…md and REVIEW.md (PP-ncxx.4) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Owner
Author
Claude Code review (medium)Reviewed head
—Claude |
timothyfroehlich
marked this pull request as ready for review
October 7, 2026 02:34
3 of 4 tasks
Owner
Author
|
Shelved: with CORE-ARCH-017 (#2449) moving migrations into their own early PRs, renumber collisions should be rare and cheap, so this tooling isn't worth finishing now. The branch is kept for revival. —Claude |
This branch was successfully deployed
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
1 participant
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
Implements the §8.12 change approved in #2434 (PP-ncxx.4). A merge of
mainwhose only resolution renumbers the branch's migrations now keeps the PR's review coverage, so a renumber no longer needs a new review round._is_pure_merge_from_mainstill accepts clean merges. A merge commit that doesn't match the clean merge now goes through_is_migration_renumber_merge, which requires all of the following:drizzle/, and every path outsidedrizzle/matches it.drizzle/files are unchanged; only the journal grows.idx(matching its file number), and awhenlater than everything before it..sqlis byte-identical to the reviewed one (same blob).drizzle/are those entries' SQL and snapshots.Snapshots aren't compared: they're regenerated and hold no SQL that runs. Inherited coverage now records
inherited_via, so the gate and handoff text print "migration renumber merge from main" instead of "pure merge from main".Also updates AGENTS.md §5 and the
pinpoint-pr-workflowskill.Merge order: #2434 (spec), then #2427 (adds
db-renumber-migration.sh, which the skill text names), then this. After #2434 lands I'll merge main here and delete its divergence row.Test plan
pnpm run check;pnpm run check:python(554 passed)test_pr_gates.pycases: a renumber merge inherits the record with the new message; five variants stay stale (renumbered SQL changed,whennot later, a change outsidedrizzle/, main's migration edited, the old SQL file left behind); a clean merge still reports "pure merge from main"🤖 Generated with Claude Code