Skip highlight cache when selecting AI reviewable diff files

What does this MR do and why?

Duo Code Review's add_comments endpoint builds the merge request diffs twice and decodes the Redis highlight cache both times. Nothing on the review path reads highlighted lines, so this is wasted work and causes large allocation spikes.

• The reviewable file list now uses raw_diff_files, which skips highlight-cache decoration.
• The context-exclusion check now gets its paths from the same expanded collection, so only one collection is built.
• The legacy Duo Enterprise path (ReviewMergeRequest) gets the same two changes.
• Selected files are unchanged. On MRs near or over the diff limit, the "I do not have access" note no longer lists excluded files past what Duo reviews. There is no feature flag.

Detailed context for AI agents

Problem

POST /api/:version/ai/duo_workflows/code_review/add_comments (Duo Code Review, Duo Agent Platform path) has a long allocation tail in production: p99 is 94 MB and p99.9 is 397 MB. The earlier MR removed blob loading from Gitaly behind the duo_code_review_skip_blob_loading flag, which was enabled on GitLab.com on 2026-10-02. The tail shrank, but spikes of hundreds of MB remain.

Root cause

Each request built two full diff collections of the merge request:

• MergeRequest#ai_reviewable_diff_files (ee/app/models/ee/merge_request.rb) called diffs(expanded: true).diff_files.
• FileExclusion#excluded_files (ee/app/services/ai/duo_workflows/code_review/file_exclusion.rb) called merge_request.diffs.diff_files.map(&:file_path). This is a second, non-expanded collection, built only to get file paths for the context-exclusion check.

The two option hashes differ, so the raw diffs memoised on MergeRequestDiff#raw_diffs were not shared.

Both collections went through Gitlab::Diff::FileCollection::MergeRequestDiffBase#diff_files, which runs highlight_cache.decorate on every file. For each file that means one Redis HMGET, then gzip-decompress, JSON-parse, and building a Gitlab::Diff::Line (with rich_text HTML) for every cached highlighted line. Measured cost is about 4.4 KB and 19 objects per cached line.

Nothing on the review path reads highlighted lines:

• The publish worker (ProcessCommentsService) reads diff_lines.
• The legacy completion reads raw_diff and diff_lines.

The merge request diffs UI writes exactly these two cache keys. The default view writes the key the non-expanded collection reads. "Expand all files" writes the key the expanded collection reads. So on a large MR that someone has viewed, the request decoded the whole highlight cache twice for nothing. At GitLab.com limits (100,000 diff lines), the expanded key alone extrapolates to roughly 440 MB of allocations, which matches the p99.9 band.

The change

1. MergeRequest#ai_reviewable_diff_files uses diffs(expanded: true).raw_diff_files instead of diff_files. These are the same Gitlab::Diff::File objects, without highlight-cache decoration.
2. FileExclusion#excluded_files takes paths from merge_request.diffs(expanded: true).diff_file_paths. The options match the expanded collection, so MergeRequestDiff#raw_diffs memoisation is reused and no second collection is built. diff_file_paths returns new_path.presence || old_path, which is exactly Gitlab::Diff::File#file_path. Renamed and deleted files are therefore excluded by the same string as before.
3. FileExclusion#ai_reviewable_diff_files converts excluded_files to a Set before filtering. It was Array#include? per file.
4. The same two changes are applied to the legacy Duo Enterprise path: Gitlab::Llm::AiGateway::Completions::ReviewMergeRequest#collect_excluded_files and #ai_reviewable_diff_files (ee/lib/gitlab/llm/ai_gateway/completions/review_merge_request.rb). It had the same two-collection pattern, running in Sidekiq.

Why no feature flag

Both changes return the same Diff::File objects and the same path strings. Only a cache read and a redundant second load are removed. Verified identical selected files and excluded paths on four local MRs, including renames and deletes.

Exclusion list on MRs at the diff limit

The exclusion paths now come from the expanded collection, not the non-expanded one. On an MR whose stored diff is near or over diff_max_lines or max_bytes, the expanded collection holds fewer files. Gitaly does not count @@ hunk headers when it stores the diff, but Rails counts them when it reloads it, so the expanded collection overflows on load. The non-expanded collection collapses files past the safe limits to 0 lines, so it never overflows on lines or bytes.

The dropped files were already outside ai_reviewable_diff_files, which was expanded before this MR, so they were never reviewed and never got comments. The only visible change is that the "I do not have access to the following files due to an active context exclusion policy" note no longer lists excluded files from that tail.

Verified on Gitaly-built MRs with diff_max_lines: 700, running the merge-base and MR-head code on both the DAP and legacy paths. A 60-file MR (overflowed at creation, 43 stored, 38 reviewed) and a 40-file MR (stored as collected, 38 reviewed) gave the same result: the old note listed f001 and f039, the new note lists f001. DAP draft notes and the legacy reviewable list were identical on both refs, and no excluded or tail file got a comment.

Call order

Order of calls does not matter. Gitlab::Git::DiffCollection#decorate! replaces elements of the already-populated array, so a paths-first call followed by raw_diff_files yields the same objects. Verified empirically.

Alternatives rejected

• MergeRequest#changed_paths (FindChangedPaths) for the exclusion paths, to keep listing tail files. It has no file cap, lists renames as two paths (no find_renames), and adds a Gitaly call to every review, for files Duo does not review anyway.
• merge_request_diff.merge_request_diff_files.pluck(:new_path, :old_path) or MergeRequest#modified_paths for the paths (0.5 MB). Both return [] for without_files diffs, where fetching_repository_diffs falls back to Gitaly. That would silently disable context exclusion there. modified_paths also returns both old and new paths, not file_path semantics.
• Early exit (any? instead of select) for the presence check in valid?. No gain, because load_diffs materialises every row before any limit applies (measured equal).
• Moving the "nothing to review" decision into the worker. This changes the API contract (400 to 201) and needs a Duo Workflow Service change, so it is out of scope.

Side effect worth knowing

The DAP path never posts the "nothing to review" message today. The Duo Workflow executor treats the 400 as a tool error and retries up to three times. The MR then sits until the 1-hour TimeoutWorker posts a generic error. This is not addressed here, and a separate issue is warranted.

Specs changed

• ee/spec/models/merge_request_spec.rb: the double now exposes raw_diff_files. A new example asserts Gitlab::Diff::HighlightCache is never instantiated.
• ee/spec/services/ai/duo_workflows/code_review/process_review_output_service_spec.rb: stubs moved to diff_file_paths. A new example with real diffs asserts that Gitlab::Git::DiffCollection.new is called once, that every file excluded by path leads to nothing_to_review, and that the message lists * CHANGELOG.
• ee/spec/lib/gitlab/llm/ai_gateway/completions/review_merge_request_spec.rb: stubs moved to diff_file_paths. A new example with real diffs asserts that Gitlab::Git::DiffCollection.new is called once and that the note lists every excluded file.

All pass locally. process_comments_service_spec and the request spec ee/spec/requests/api/ai/duo_workflows/code_review_spec.rb also pass.

Benchmark method

ProcessReviewOutputService#valid? with the flag on, measured with memory_profiler. Second run, fresh MergeRequest each time. The highlight cache was warm for both keys where noted in the table.

References

• #630829 (work item)
• !258482 (merged) (previous MR, whose follow-ups this implements)
• #631315 (closed) (rollout issue of the previous MR's flag)

Screenshots or screen recordings

This is a local benchmark, not a UI change.

MR	Before	After
1000 files, 1.2 MB patch, both highlight caches warm	111 MB	20.5 MB
1000 files, 1.2 MB patch, default cache warm	45.1 MB, 0.91 s	21.4 MB, 0.38 s
312 files, 3.7 MB patch	41.8 MB, 1.58 s	7.3 MB, 0.79 s
400 files, 108 KB patch	12.3 MB	8.2 MB
5 files with renames and deletes	1.7 MB	1.6 MB (identical path lists)

How to set up and validate locally

1. Open an MR with a few hundred changed files. View its Changes tab in the browser (default view, then "Expand all files") to warm both highlight-cache keys.
2. In a Rails console, run Feature.enable(:duo_code_review_skip_blob_loading, project). Then measure Ai::DuoWorkflows::CodeReview::ProcessReviewOutputService.new(user:, project:, merge_request:, review_output: 'x').valid? under Gitlab::Memory::Instrumentation.with_memory_allocations { ... } (or memory_profiler if that is unavailable). Do this on master and on this branch. Allocations drop and the result is identical.
3. Add a Duo context exclusion rule matching one changed file in project settings. Confirm it is still listed in the "I do not have access to the following files" message when all files are excluded, and still omitted from the published comments otherwise.

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.