docs: capture merge-conflict learning (extraction vs semantic change)#1374
Conversation
Document the refactor-vs-semantics merge resolution pattern (port the upstream semantic change into the extracted helper, never pick a side) and the parallel-bootstrap CONCEPTS.md add/add union, with the FN-5902 / runFeatureValidation merge as the worked example. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 2 minutes and 4 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds a new best-practice documentation page guiding developers through resolving merge conflicts that arise when one branch extracts code into a helper while another branch modifies the semantics of that extracted code. The guide provides conflict resolution criteria, concrete examples showing incorrect and correct resolutions, and references to related documentation. ChangesMerge Conflict Extraction-vs-Semantics Best-Practice Guide
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Greptile SummaryThis docs-only PR adds a best-practices guide for a tricky class of merge conflict that arises when one branch structurally refactors code (extraction to a helper) while another branch simultaneously changes that same code's semantics — a case where accepting either hunk verbatim silently reverts one of the changes. It also covers the parallel-bootstrap add/add collision pattern for accretive docs like
Confidence Score: 5/5Docs-only change with no runtime impact; safe to merge. The change adds a single markdown document with no code modifications. All cross-referenced files exist, the frontmatter follows the conventions used by sibling docs, and no changeset is required under repo policy for internal docs. The only annotations previously flagged as noise ((session history)) have already been raised in prior review threads and are not new findings. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Merge conflict encountered] --> B{What shape is the conflict?}
B -->|One side: call/one-liner\nOther side: multi-line block| C[Extraction-vs-semantic conflict]
B -->|Both sides added content\nto the same file| D[Add/add collision on accretive doc]
B -->|Other| E[Standard conflict resolution]
C --> C1[Keep the call site\npreserve the refactor]
C1 --> C2[Port semantic change\ninto helper body]
C2 --> C3[Sweep moved code's\ndoc comments for lies]
C3 --> C4[Run union test suite\nboth branches' tests]
C4 --> C5[Name merge commit\nafter adopted change]
D --> D1[Merge as union\nkeep one preamble]
D1 --> D2[Take diff-minimizing wording\nfor near-identical pointer lines]
D2 --> D3[Sweep merged prose for\nstatements incoming commits falsified]
C5 --> F[Resolution complete]
D3 --> F
E --> F
Reviews (2): Last reviewed commit: "Update docs/solutions/best-practices/mer..." | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@docs/solutions/best-practices/merge-conflict-extraction-vs-semantics-and-parallel-bootstrap.md`:
- Line 65: Remove the authoring artifact "(session history)" from the sentence
in the paragraph that begins "The collision class is not hypothetical or rare
here: four parallel branches bootstrapped or substantially extended
`CONCEPTS.md` on the same day..." so the doc reads as a clean statement; simply
delete the parenthetical, ensure spacing/punctuation remains correct, and commit
the updated sentence (no code changes required).
- Line 70: Remove the editorial artifact "(session history)" from the sentence
that reads "One branch moved/extracted/renamed code that the other branch
changed the behavior of. A prior incident in the merger had the same shape in
reverse: a refactor that merged two distinct error cases into one throw site
silently changed error semantics downstream. (session history)"; simply delete
the parenthetical note so the paragraph ends with "downstream." and no authoring
annotation remains.
- Line 42: Remove the authoring annotation artifact "(auto memory [claude])"
from the example merge-commit line so the documentation only shows the intended
commit-name example (e.g. "Merge main: adopt FN-5902 lazy assertion linkage in
shared runFeatureValidation"); update the sentence containing that example to
delete the trailing annotation and ensure punctuation/spaces remain correct.
- Line 31: Update the branch name typo from "gsxdsm/missonsdebug" to
"gsxdsm/missionsdebug" wherever it appears in this doc sentence describing the
merge (the observed merging line referencing merge `60c307320`); ensure the
corrected branch name is used in the description that mentions the extracted
helper `runFeatureValidation` and the conflicting FN-5902 changes so the text
consistently reads "gsxdsm/missionsdebug".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ff12630a-d095-4efc-85a5-5e52473e3682
📒 Files selected for processing (1)
docs/solutions/best-practices/merge-conflict-extraction-vs-semantics-and-parallel-bootstrap.md
…antics-and-parallel-bootstrap.md Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
…antics-and-parallel-bootstrap.md Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
What
Adds the merge-conflict-resolution learning doc that was committed to the #1345 branch two minutes after that PR merged — the push silently re-created the deleted branch, so the doc never reached main.
docs/solutions/best-practices/merge-conflict-extraction-vs-semantics-and-parallel-bootstrap.mddocuments two patterns from resolving #1345's conflicts with main:Docs-only; no changeset per repo convention.
🤖 Generated with Claude Code
Summary by CodeRabbit