Skip to content

Resolve template merge conflicts in nf-core pipelines sync - #4519

Open
paulinefx wants to merge 4 commits into
devfrom
sync-resolve-template-conflicts
Open

paulinefx wants to merge 4 commits into
devfrom
sync-resolve-template-conflicts

Conversation

@paulinefx

Copy link
Copy Markdown

Summary

nf-core pipelines sync now tries to resolve the merge conflicts of template update PRs before opening them, using the hunk logic from nf-migrate by @Skitionek (contributed under the MIT licence with the author's agreement).

How it works

After pushing TEMPLATE, the new resolve_merge_conflicts() step merges TEMPLATE into the PR base branch in a temporary git worktree, so the user's checkout is never touched:

  1. git merges everything it can, as usual.
  2. For each file git leaves conflicted, the template's own change (merge base → new TEMPLATE) is split into small hunks with the minimal context that makes them unique (diff_hunks), and each hunk is applied to the pipeline's version of the file (apply_hunk): replaced if its old text occurs exactly once, skipped if its new text is already there, otherwise the file is left unresolved.
  3. If every conflicted file is resolved, the merge is committed and the PR is opened from that merge commit (parents: base branch and TEMPLATE), so it merges without conflicts. The PR body lists the auto-resolved files to review.
  4. Otherwise the merge is aborted and the PR is opened from TEMPLATE as before, with the conflicting files listed in the PR body.

Any git error in this step is logged as a warning and falls back to the previous behaviour.

Differences from nf-migrate

  • No pre-generated migrations or {{placeholders}}: the template diff comes from the pipeline's own TEMPLATE branch at sync time, so nothing needs maintaining per release, and multi-version jumps and skip_features work.
  • git merges first; hunks only touch the files git could not merge.
  • Hunks may have zero context lines when the changed lines are unique on their own. Since git only conflicts where the pipeline edited lines next to a template change, a mandatory context line would fail exactly where it is needed.

Things for reviewers

  • When all conflicts are resolved, the PR head is a merge commit authored by nf-core-bot instead of the TEMPLATE tip. It contains textual edits to pipeline-owned files, listed in the PR body. These PRs must still be merged with "Create a merge commit".
  • Partial resolutions are discarded: if one file cannot be resolved, the PR is opened as before.
  • The template sync docs will need a short section on auto-resolved PRs (separate PR on the website repo).

Testing

  • New tests/pipelines/test_sync_merge.py: hunk splitting, apply_hunk, and merges in a real repo (edits on neighbouring lines resolved, already-applied change resolved, real conflict left alone, clean merge needs no commit).
  • New test in tests/pipelines/test_sync.py: merge branch starts from the resolved merge commit, and the PR body lists the files.
  • All 37 sync tests pass locally. Note: on current dev, the template's .github/workflows/branch.yml has HEAD_REPO: ${{ (double space), which prettier reformats after the initial commit and leaves fixture pipelines dirty; locally that makes most test_sync.py tests fail on dev as well, independent of this PR.

🤖 Generated with Claude Code

paulinefx and others added 3 commits October 15, 2026 15:24
Before opening the template update PR, merge TEMPLATE into the base branch
in a temporary worktree. For each file git cannot merge, split the
template's change into small hunks with minimal unique context and apply
them to the pipeline's version of the file. If every conflict is resolved,
the PR is opened from the merge commit so it can be merged without
conflicts; otherwise it is opened from TEMPLATE as before. The PR body
lists the resolved or conflicting files.

The hunk logic is adapted from nf-migrate (https://github.com/Skitionek/nf-migrate).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@codecov

codecov Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 74.88152% with 106 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.28%. Comparing base (3f64b31) to head (0e66f6d).
⚠️ Report is 1 commits behind head on dev.

Files with missing lines Patch % Lines
nf_core/pipelines/sync_merge.py 77.48% 86 Missing ⚠️
nf_core/pipelines/sync.py 50.00% 20 Missing ⚠️

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@codspeed

codspeed Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Merging this PR will improve performance by 22.44%

⚡ 1 improved benchmark
✅ 8 untouched benchmarks

Performance Changes

Benchmark BASE HEAD Efficiency
⚡ test_modules_install_startup 6.7 s 5.5 s +22.44%

Tip

Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.


Comparing sync-resolve-template-conflicts (0e66f6d) with dev (77ba349)1

Open in CodSpeed

Footnotes

  1. No successful run was found on dev (cfba18b) during the generation of this report, so 77ba349 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

@mashehu

mashehu commented Oct 15, 2026

Copy link
Copy Markdown
Member

@codspeedbot explain why performance improved

@codspeed

codspeed Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

@codspeedbot explain why performance improved

Why test_modules_install_startup got faster (6.7 s → 5.6 s)

This PR's code did not cause the speedup. It comes from upstream changes in nf-core/modules between the two CI runs. The new sync code in sync.py and sync_merge.py is never imported by nf-core modules install.

What the flame graphs show

  • Python time is unchanged. Each nf-core process takes about 335 ms in both runs, and the prettier/node step takes about 1 s in both.
  • All the saved time is in the git child processes: about 2.2 s in the base run and about 0.96 s in the head run. Most of the drop is zlib work (inflate, inflate_fast, inflate_table), which fell from about 626 ms to about 158 ms. That means git unpacked far fewer objects.

Why git did less work

In every round, SyncedRepo.install_component checks out the latest commit that touched fastp in the nf-core/modules cache, copies the files, then runs checkout_branch() to go back to master. Each round therefore rewrites every file that differs between those two commits, twice.

The cache is cloned fresh in each CI run, so this depends on the state of nf-core/modules that day:

Run modules master Latest fastp commit Gap
Base (dev 77ba349, Oct 12) dfdf72c 826a9b2 (Jul 31) 411 commits, 2,983 files
Head (this PR, Oct 15) f265e4a cec0863 (today, nf-core/modules#13169) 10 commits, 197 files

Because fastp was updated upstream this morning, the checkout round trip now touches about 15× fewer files, which accounts for the ~1.1 s gain.

Other things to know

Comment @codspeedbot pin the modules cache in the install benchmark and I'll make that change.

Summary
  • Warming up the performance wizard...
  • Compare base and head runs and flame graphs — Python time unchanged; the git child processes dropped from ~2.2 s to ~0.96 s, mostly less zlib decompression
  • Trace the git work in nf-core modules install — The change comes from the state of the nf-core/modules cache, not from the PR code
  • Reply with explanation — Reply with explanation
  • Handling comment — Comment handling finished successfully

Learnings updated. See what CodSpeed remembers about this repository.

Was this response helpful? React with 👍 or 👎 to help us improve CodSpeedBot.

- Fixed rules for generated files (RO-Crate, container configs), logos,
  CHANGELOG.md, .nf-core.yml and nf-core components (modules.json and the
  template modules/subworkflows keep the pipeline's version)
- After the merge commit, update template components to the template
  version (never downgrading) and regenerate container configs and the
  RO-Crate, one commit each; failures are listed in the PR body as to-dos
- Discard the resolution if hunk-resolved Nextflow files gain new
  `nextflow lint` errors
- Use the TEMPLATE commit named by .nf-core.yml as merge base when the git
  history is older (squash-merged template updates)
- Raise instead of treating a failed merge without conflicts as clean
- PR body lists conflicts, rule resolutions, follow-up commits and warnings

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants