Fix draft notes not rendering on diff when base_sha != start_sha

Fixes #590869 - a draft note on an added line doesn't render on the merge request Changes page once the target branch has advanced past where the merge request started (base_sha != start_sha). A new service, DraftNotes::TraceMergeHeadPositionService, retraces each draft note's position onto the merge_head diff, the same mechanism already used for published notes, and the drafts controller exposes the retraced result in memory only on index, create and update. Nothing is persisted, so publishing is unaffected. The whole change sits behind the draft_note_merge_head_line_code feature flag (disabled by default) and touches no shared code path, so with the flag off the response is byte-for-byte what master returns.

Same draft notes on the Changes page after the target branch advanced, base_sha != start_sha:

	Flag off	Flag on
Rapid diffs
Legacy diffs

Detailed context for AI agents

Root cause

The Changes page renders the merge_head diff (the target branch with its new commits vs the merge-ref head). When the target gains lines above the commented line, that line's new-line number differs between the merge_head diff and the 3-way diff the note's stored position came from. Neither the note's stored line_code nor its stored position matches any rendered line, so the note is invisible.

Published notes are unaffected because Discussions::CaptureDiffNotePositionService traces their position onto the merge_head at create and publish time. Draft notes had no equivalent.

The fix

DraftNotes::TraceMergeHeadPositionService is called from the drafts controller on index, create and update. It retraces each draft note's position onto the merge_head diff using Gitlab::Diff::PositionTracer, the same mechanism used for published notes, and hands the note the retraced position in memory only.

Two outputs are needed because the two renderers place a draft differently:

• The rapid-diffs renderer matches on position plus diff_refs and never reads line_code (app/assets/javascripts/merge_request/stores/merge_request_draft_notes.js, via findApplicablePosition).
• The legacy renderer matches on file_hash plus line_code and never reads positions (app/assets/javascripts/batch_comments/store/getters.js).

So the retraced position is exposed as positions, the same field DiscussionEntity already serves the equivalent merge_head positions of published notes under, and the note's line_code is also overwritten with one the rendered diff contains. No frontend change is required.

Which stored position gets retraced depends on how the draft was created. Through the UI the frontend builds the position from the rendered diff file's refs, so on a mergeable merge request original_position carries merge-head refs while update_position normalises position onto merge_request.diff_refs. Through the API both carry the merge request's refs. The service takes whichever the tracer can consume, preferring original_position. A draft on a commit, or one whose line no longer exists on the current diff, matches neither ref set and is left untouched.

Feature flag scope was tightened

An earlier iteration extracted the merge_head diff_refs construction into a new shared factory, Gitlab::Diff::PositionTracer.for_merge_head, and pointed Discussions::CaptureDiffNotePositionService (the published-note path) at it. That ran unconditionally, outside the flag, so published notes took new code even with the flag off.

That extraction has been reverted. lib/gitlab/diff/position_tracer.rb, app/services/discussions/capture_diff_note_position_service.rb and spec/lib/gitlab/diff/position_tracer_spec.rb are back to their master state. The roughly 20 lines of diff_refs construction now live as a private tracer method inside the flag-gated service instead, duplicating a little logic with the published-note path deliberately, so no shared code path changes.

The flag is now checked at all three entry points, and in each case it is the first line of the method:

• DraftsController#trace_merge_head_positions checks it before the service is constructed at all, so the flag-off path does not enter new code on index, create or update.
• TraceMergeHeadPositionService#execute checks it as well, so the service is safe to call directly.
• DraftNoteEntity#expose_merge_head_position? checks it before looking at merge_head_position, so the serialized payload cannot change unless the flag is on.

All three use a Project actor for the same project, so there is no actor-type mixing.

Net effect: the merge request touches no shared file, and with the flag off the drafts response is byte-for-byte what master returns.

Feature flag: draft_note_merge_head_line_code, type gitlab_com_derisk, default disabled, milestone 19.4. Rollout issue: #604420.

The public REST API entity (API::Entities::DraftNote, serving /api/v4/.../draft_notes) is untouched, so the documented REST payload is unchanged either way. positions only appears on the internal serializer the Changes page consumes.

Performance

One PositionTracer per request, not per note, so its Gitaly comparisons run once and are reused across every draft note. Cost scales with the number of distinct commented files, not the note count. Measured on GDK: roughly 4 Gitaly RPCs and 100ms for 1 file, 22 RPCs and 750ms for 10 files, while going from 1 to 100 notes on a single file moved the wall time from 110ms to 270ms.

A hard cap of FILE_LIMIT = 20 distinct commented files applies. Above it, tracing is skipped for the whole collection rather than a subset, so every draft behaves the same way and it stays easy to debug. Only files that are actually traceable count toward the cap. Nothing is traced when the target has not advanced or the flag is off.

Alternatives rejected or deferred

• Rejected: an earlier version swapped the diff file (diff_file_from_merge_head) instead of tracing the position. That keeps the untranslated line number, so it does not handle a target edit that shifts the commented line's new-line number, which is the actual scenario in the issue. It only appeared to work where the new-line number happened to be preserved.
• Deferred: persisting a traced position per draft note, the way DiffNote does with diff_note_positions, with cleanup when a draft is published. That removes the render-time Gitaly cost entirely but needs a migration and publish-time cleanup. Worth doing if the rollout shows the render-time cost or the file cap actually biting; the flag plus the cap are there to find that out first.

Known limitations, deliberately out of scope

The unfold positions used to expand collapsed diff regions are still built from the stored 3-way positions (MergeRequest#note_positions_for_paths), so a draft note sitting in a collapsed region may still fail to render even with a correct traced position. This is pre-existing, was never reproduced, and fixing it properly means tracing the unfold positions in the diffs controller too. It is noted on the rollout issue as a known limitation. Draft notes on commits and viewing a superseded diff version are also not handled.

How to test

Reproduce a new-line shift: the source branch adds a function, the target branch prepends lines above it so the commented line's new-line number shifts in the merge result, giving base_sha != start_sha.

• Flag off: the draft note's stored position and line_code are both absent from the rendered merge_head diff, so it is invisible in either renderer.
• Flag on: the retraced position and line_code match the rendered line, so the note renders.

Worth testing from the Changes page as well as through the API, since a UI-created draft's position carries merge-head refs, which is a different code path from the API shape in the issue.

Verified locally on GDK with one API-created draft and one UI-created draft on the same line, in both rapid and legacy diffs (screenshots above). With the flag off neither draft renders; with the flag on both are retraced onto the shifted line, and advancing the target a second time retraces them again.

Test coverage

• spec/services/draft_notes/trace_merge_head_position_service_spec.rb - the new-line-shift regression, the UI-created shape after the target advances, a single-shared-tracer assertion, the file cap, the untraceable cases, and the flag-disabled case. 13 examples.
• spec/serializers/draft_note_entity_spec.rb - the positions exposure and the flag-disabled case. 15 examples.
• spec/controllers/projects/merge_requests/drafts_controller_spec.rb - GET #index delegates to the service before serializing, and does not construct it at all when the flag is off. 53 examples.
• spec/frontend/merge_request/stores/merge_request_draft_notes_spec.js - the renderer placing a draft by the retraced position.
• spec/services/discussions/capture_diff_note_position_service_spec.rb and spec/lib/gitlab/diff/position_tracer_spec.rb both still pass unchanged, confirming the published-note path is untouched.

All of the above were run locally and pass. Rubocop is clean on every changed file.

Changelog

No changelog entry, because the change sits behind a flag that is disabled by default.