Load only the first note when validating reply confidentiality

What does this MR do and why?

A reply to a thread must match the confidentiality of the thread's first note. That check loaded every note in the thread just to read the first one, and validation runs it twice per reply. It now loads only the first note, or reuses the thread if it is already loaded, so each reply loads the whole thread two times instead of four.

The change is behind the reply_confidentiality_check_loads_first_note feature flag, disabled by default. With the flag off, the code path is unchanged.

Follow-up to !262926 (merged), which removes the larger per-reply cost on the same endpoint.

Query plans

The new query uses the existing index_notes_on_discussion_id. On the production clone it takes about 0.2 ms on a typical thread and about 11 ms on the 4,529-note incident thread, with the same plan and buffers as the whole-thread load it replaces.

Plans for the new query: 5-note thread, 4,529-note incident thread. For comparison, the query master runs today on the incident thread (no LIMIT 1): plan.

SELECT "notes".* FROM "notes" WHERE "notes"."noteable_id" = <merge_request_id> AND "notes"."noteable_type" = 'MergeRequest' AND "notes"."discussion_id" = '<discussion_id>' ORDER BY "notes"."created_at" ASC, "notes"."id" ASC LIMIT 1

References

• https://gitlab.com/gitlab-org/gitlab/-/work_items/634293
• !262926 (merged)
• Rollout: #634437

Screenshots or screen recordings

No UI change. On GDK, with !262926 (merged) applied, the CPU cost of an API reply to a 3,000-note merge request thread drops from 0.62 s to 0.31 s.

How to set up and validate locally

1. In a Rails console, enable the flag, build a reply to an existing thread and log the queries:

   Feature.enable(:reply_confidentiality_check_loads_first_note)
   first = Note.where(type: 'DiscussionNote').last
   reply = DiscussionNote.new(noteable: first.noteable, project: first.project, author: first.author, note: 'reply', discussion_id: first.discussion_id)
   ActiveRecord::Base.logger = Logger.new($stdout)
   reply.valid?

2. The confidentiality check runs a single notes query with LIMIT 1. Run Feature.disable(:reply_confidentiality_check_loads_first_note) and reply.valid? again: it loads every note in the thread, as on master.

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Detailed context for AI agents

Change. Note#ensure_confidentiality_discussion_compliance (app/models/note.rb) used start_of_discussion? and discussion.first_note. Note#discussion builds a Discussion from every note of the thread (Note.find_discussion), and the validation's ensure clears that memo, so valid? in Notes::CreateService and the validation inside save each loaded the full thread. The new private Note#discussion_first_note:

• reuses the discussion when it is already memoized (strong_memoized?(:discussion)). This branch is required: Discussions::UpdateDiffPositionService saves every note of a loaded thread (inverse relations preset the memo), and without the branch that loop would issue one query per note. The /internal_note quick action also loads the discussion before validation.
• returns self when there is no noteable or the note is not part of a discussion, matching start_of_discussion? on master.
• otherwise runs noteable.notes.with_discussion_ids(discussion_id).order_created_at_id_asc.first, the same ordering Discussion#first_note uses, falling back to self for the note that starts a new thread.

The ensure clear_memoization(:discussion) is kept so note.discussion after save is fresh for the controller JSON and TodoService.

Query plans. Run on 2026-10-19 with postgres.ai Database Lab (Joe) on a gitlab-production-main thin clone. Each plan was run twice. The numbers are from the warm (second) run unless stated. The SQL is the statement shown under "Query plans" above, exactly as Rails runs it.

• Typical thread: 5 notes, on a gitlab-org/gitlab merge request. A BitmapAnd of index_notes_on_discussion_id and index_notes_on_noteable_id_noteable_type_and_id, then a top-N heapsort. 0.18 ms execution, 22 shared buffers.
• Worst case: the thread from the incident, 4,529 notes in one discussion, on a merge request with 5,021 notes. Same indexes as the typical case. The project is private, so the merge request and discussion IDs were picked with subqueries. They show as InitPlans 1-3 in the plan. Postgres counts an InitPlan's buffers inside the node that first uses its result, so the index scan buffer counts include them. The query's own cost is about 4,200 buffers (about 33 MiB) and about 11 ms warm (22.5 ms total minus 11.2 ms for InitPlan 3). The cold first run on the clone took 5.8 s, almost all disk reads (4,703 buffers read).
• Compared with master: master's whole-thread load is the same SQL without LIMIT 1. On the worst-case thread it gets the same plan, the same indexes and the same 9,317 buffers (23.7 ms execution). The only differences are the sort (a quicksort of all 4,529 rows, 4.9 MB, instead of a top-N heapsort) and the rows returned (4,529 instead of 1). So the database work is unchanged. The saving is on the Rails side, which no longer builds 4,529 note objects twice per reply.
• notes has an index_notes_on_created_at index. The planner did not use it for the ORDER BY ... LIMIT 1 in either case, because it estimates about 110 rows for one discussion_id.
• The query still reads every row of the thread from the heap before the top-N sort (4,145 heap blocks in the worst case). These are the same pages master already reads. An index on (discussion_id, created_at, id) would make it a single index probe, but it is not part of this MR. There is no regression against master, and a new index on notes is expensive.

Plan for the typical thread (5 notes):

 Limit  (cost=11.10..11.10 rows=1 width=2745) (actual time=0.132..0.133 rows=1 loops=1)
   Buffers: shared hit=22
   ->  Sort  (cost=11.10..11.10 rows=1 width=2745) (actual time=0.131..0.132 rows=1 loops=1)
         Sort Key: notes.created_at, notes.id
         Sort Method: top-N heapsort  Memory: 25kB
         Buffers: shared hit=22
         ->  Bitmap Heap Scan on public.notes  (cost=9.57..11.09 rows=1 width=2745) (actual time=0.076..0.084 rows=5 loops=1)
               Recheck Cond: (((notes.discussion_id)::text = '<discussion_id>'::text) AND (notes.noteable_id = <merge_request_id>) AND ((notes.noteable_type)::text = 'MergeRequest'::text))
               Heap Blocks: exact=5
               Buffers: shared hit=16
               ->  BitmapAnd  (cost=9.57..9.57 rows=1 width=0) (actual time=0.050..0.051 rows=0 loops=1)
                     Buffers: shared hit=11
                     ->  Bitmap Index Scan on index_notes_on_discussion_id  (cost=0.00..3.02 rows=109 width=0) (actual time=0.023..0.023 rows=5 loops=1)
                           Index Cond: ((notes.discussion_id)::text = '<discussion_id>'::text)
                           Buffers: shared hit=5
                     ->  Bitmap Index Scan on index_notes_on_noteable_id_noteable_type_and_id  (cost=0.00..6.29 rows=259 width=0) (actual time=0.025..0.025 rows=163 loops=1)
                           Index Cond: ((notes.noteable_id = <merge_request_id>) AND ((notes.noteable_type)::text = 'MergeRequest'::text))
                           Buffers: shared hit=6
Settings: random_page_cost = '1.5', seq_page_cost = '4', effective_cache_size = '472585MB', work_mem = '230MB', jit = 'off'

Plan for the worst-case thread (4,529 notes in one discussion):

 Limit  (cost=426.71..426.71 rows=1 width=2745) (actual time=22.541..22.547 rows=1 loops=1)
   Buffers: shared hit=9317
   InitPlan 1
     ->  Index Scan using index_merge_requests_on_target_project_id_and_iid on public.merge_requests  (cost=0.57..17.17 rows=11 width=8) (actual time=0.015..0.016 rows=1 loops=1)
           Index Cond: ((merge_requests.target_project_id = <project_id>) AND (merge_requests.iid = <iid>))
           Buffers: shared hit=5
   InitPlan 3
     ->  Limit  (cost=398.42..398.42 rows=1 width=49) (actual time=11.152..11.155 rows=1 loops=1)
           Buffers: shared hit=5090
           InitPlan 2
             ->  Index Scan using index_merge_requests_on_target_project_id_and_iid on public.merge_requests merge_requests_1  (cost=0.57..17.17 rows=11 width=8) (actual time=0.026..0.027 rows=1 loops=1)
                   Index Cond: ((merge_requests_1.target_project_id = <project_id>) AND (merge_requests_1.iid = <iid>))
                   Buffers: shared hit=8
           ->  Sort  (cost=381.25..381.90 rows=261 width=49) (actual time=11.151..11.152 rows=1 loops=1)
                 Sort Key: (count(*)) DESC
                 Sort Method: top-N heapsort  Memory: 25kB
                 Buffers: shared hit=5090
                 ->  HashAggregate  (cost=377.33..379.94 rows=261 width=49) (actual time=11.139..11.144 rows=22 loops=1)
                       Group Key: n.discussion_id
                       Batches: 1  Memory Usage: 37kB
                       Buffers: shared hit=5090
                       ->  Index Scan using index_notes_on_noteable_id_noteable_type_and_id on public.notes n  (cost=0.70..376.03 rows=261 width=41) (actual time=0.063..10.146 rows=5021 loops=1)
                             Index Cond: ((n.noteable_id = (InitPlan 2).col1) AND ((n.noteable_type)::text = 'MergeRequest'::text))
                             Buffers: shared hit=5090
   ->  Sort  (cost=11.12..11.13 rows=1 width=2745) (actual time=22.539..22.541 rows=1 loops=1)
         Sort Key: notes.created_at, notes.id
         Sort Method: top-N heapsort  Memory: 27kB
         Buffers: shared hit=9317
         ->  Bitmap Heap Scan on public.notes  (cost=9.60..11.11 rows=1 width=2745) (actual time=13.522..19.500 rows=4529 loops=1)
               Recheck Cond: (((notes.discussion_id)::text = ((InitPlan 3).col1)::text) AND (notes.noteable_id = (InitPlan 1).col1) AND ((notes.noteable_type)::text = 'MergeRequest'::text))
               Heap Blocks: exact=4145
               Buffers: shared hit=9311
               ->  BitmapAnd  (cost=9.60..9.60 rows=1 width=0) (actual time=12.950..12.951 rows=0 loops=1)
                     Buffers: shared hit=5166
                     ->  Bitmap Index Scan on index_notes_on_discussion_id  (cost=0.00..3.03 rows=110 width=0) (actual time=11.963..11.963 rows=4529 loops=1)
                           Index Cond: ((notes.discussion_id)::text = ((InitPlan 3).col1)::text)
                           Buffers: shared hit=5100
                     ->  Bitmap Index Scan on index_notes_on_noteable_id_noteable_type_and_id  (cost=0.00..6.31 rows=261 width=0) (actual time=0.835..0.836 rows=5021 loops=1)
                           Index Cond: ((notes.noteable_id = (InitPlan 1).col1) AND ((notes.noteable_type)::text = 'MergeRequest'::text))
                           Buffers: shared hit=66
Settings: random_page_cost = '1.5', seq_page_cost = '4', effective_cache_size = '472585MB', work_mem = '230MB', jit = 'off'

Still loading the whole thread per reply (out of scope): Notes::BuildService#find_discussion, which needs resolved? across all notes, and Notes::CreateService#update_discussions after save. The todo part is tracked in https://gitlab.com/gitlab-org/gitlab/-/work_items/634352. The missing notes_create rate limit on these endpoints is https://gitlab.com/gitlab-org/gitlab/-/work_items/634354.

Feature flag. reply_confidentiality_check_loads_first_note (gitlab_com_derisk, default off), rollout in #634437. The actor is Note#resource_parent, which is noteable.try(:resource_parent) || project. For project notes it is the noteable's project, the same project as before. For epics and group-level work items it is the group. Personal snippet notes have neither, so they evaluate the flag with no actor and stay on the old path until the flag is fully enabled. Group-level threads therefore follow the actor and percentage rollout, instead of only switching on at true. Project notes add no query: the validation that checks the note's project matches the noteable's project loads noteable.project in the same valid? call anyway. With the flag off the method runs start_of_discussion? and discussion.first_note, the same calls as master. No changelog entry, as the flag is off by default.

Verification.

• Specs in spec/models/note_spec.rb cover both flag states: the reply confidentiality results run as a shared example with the flag on and off. With the flag on, validation does not call Note.find_discussion and runs no first-note query when the discussion is already loaded. With the flag off, it does call Note.find_discussion. Forcing the gate to always on, or always off, fails a spec.

• Throwaway oracle matrix, 44 examples, run once with the flag on and once with it off. Each compares the validation errors from master's method and the new one on the same inputs. With the flag off it also requires the SQL inside the validation to match master's exactly, for #valid? on fresh records, for the validation call alone, and through every service caller and the API endpoint. Scenarios:

  • new individual, thread-starting and reply notes, public and confidential in all combinations
  • first-note ordering: created_at order different from id order, ties, the first note deleted, a system note first
  • persisted saves, including a legacy thread with mixed confidentiality
  • memoized and stale discussions
  • Issue, MergeRequest, DiffNote, Commit, project snippet and Epic noteables, and a note with no noteable
  • callers: Notes::CreateService (including internal: true, /internal_note, converting individual issue and MR notes, and resolved diff threads), Notes::UpdateService, Note#resolve!, Notes::CopyService, and the API reply endpoint

  Two mutants were caught: ordering by id only, and dropping the memo branch.

• Existing specs pass, apart from the EE Elasticsearch indexing examples, which need a running Elasticsearch. Covered: the CE and EE note models, the discussion model, the notes create, build and update services, UpdateDiffPositionService, the API discussions and notes endpoints, the notes controller, CopyService and quick actions.

• Measurement on GDK (in-process API reply into a 3,000-note MR thread, Bullet off): master 3.7 s, !262926 (merged) alone 0.62 s, with this MR 0.31 s.

• Query plans checked on the production clone (see "Query plans" above).