Skip charset detection where it cannot change the diff outcome

Every diff built from a stored merge_request_diff_files row ran CharlockHolmes charset detection twice per patch. On a 231-file MR those two scans were over half of the diffs_stats request wall time. Both scans are now skipped where they cannot change the result, for diffs loaded through the stored merge request diff paths. This is behind the feature flag diff_skip_redundant_charset_detection (type gitlab_com_derisk, default off, rollout issue #629894), rolled out per project, and behaviour is unchanged with the flag off.

Detailed context for AI agents

How it was found: while moving the rapid diffs diffs_stats and diff_files_metadata endpoints onto stored diff files (!256363), the stored path was slower than the live Gitaly CommitDiff path on mid-size MRs. Stackprof wall mode on raw_diff_files for a 231-file MR showed 55% of samples in CharlockHolmes::EncodingDetector#detect and another 28% in GC.

Call site one - Gitlab::Git::DiffCollection#expand_diff?: old logic computed allow_expansion = !@enforce_limits || @expanded, scanned every patch with detect_binary?, then returned allow_expansion for binary patches and @iterator.size == 1 || allow_expansion for the rest. Those two values only differ when allow_expansion is false and the collection holds exactly one file. The new code returns allow_expansion early in every other case and only scans in the single-file, not-expandable case. The truth table is identical.

Call site two - Gitlab::Git::Diff#diff_should_be_converted?: it decided whether to run encode_utf8_with_replacement_character. Gitlab::EncodingHelper#encode_utf8 starts with force_encode_utf8, which returns the same object when it is UTF-8 tagged and valid, then returns on valid_encoding?. So for a valid UTF-8 patch the conversion is a no-op and the scan decides nothing. The new guard skips the scan when the patch is UTF-8 tagged and valid. Patches from Postgres text columns are UTF-8 tagged. Base64-decoded rows come back as ASCII-8BIT and keep the old path. Gitaly patches also go through encode! first and are usually valid UTF-8, but the Gitaly callers do not pass skip_charset_detection yet, so they still run the old path. A nil patch now returns false without the flag; before, encode_utf8(nil) raised ArgumentError internally, rescued to nil, so the result is the same. The UTF-8 tagged and valid check is Gitlab::EncodingHelper#valid_utf8?, shared with force_encode_utf8, so the fast path and the early return it relies on cannot drift apart.

Flag placement: Only the two stored merge request diff callers pass skip_charset_detection now: MergeRequestDiff#load_diffs and Gitlab::Diff::FileCollection::PaginatedDiffs#diffs. Both call a new public MergeRequestDiff#skip_charset_detection?, which checks Feature.enabled?(:diff_skip_redundant_charset_detection, Project.actor_from_id(project_id)). Gitlab::Git::DiffCollection reads options.fetch(:skip_charset_detection, false) and passes it into each Gitlab::Git::Diff.new. It has no Feature reference and no helper method of its own. The actor comes from merge_request_diffs.project_id rather than MergeRequestDiff#project (which is merge_request.target_project), because calling project added a merge_requests query and a projects query to a freshly loaded diff, for example the Orbit internal endpoint that loads a diff by id. It also crashed the unsaved merge request fallback: MergeRequest#merge_request_diff can return an unsaved MergeRequestDiff.new(merge_request_id: id) with no compare, and calling target_project on that raised NoMethodError, even with the flag off. project_id is set directly from target_project_id, so the actor needs no extra query. On the unsaved fallback diff it is nil, and Project.actor_from_id(nil) still returns an actor (flipper id Project:), so a per-project enable never matches it but a global enable does turn it on there - harmless since that diff has no files. A regression spec covers this: it fails with NoMethodError on the commit before this fix and passes now. The Gitaly callers (Gitlab::Git::Repository#diff, Gitlab::Git::Commit#diffs) are not wired up, so they keep the old behaviour. That also removes the earlier container to project mapping (Feature::Gitaly.project_actor), and the wiki and snippet cases that could never be enabled per project anyway, since they have no project actor and only a global true turned them on.

Measured on GDK, median of 8, diffs_resource, whitespace shown, flag on vs off:

files	endpoint	before	after
43	diffs_stats	40 ms	35 ms
231	diffs_stats	178 ms	41 ms
268	diffs_stats	132 ms	50 ms
1000	diffs_stats	211 ms	128 ms
231	diff_files_metadata	218 ms	87 ms
268	diff_files_metadata	224 ms	176 ms
1000	diff_files_metadata	366 ms	287 ms

Charset scans per request went from 2 per file to 0 in every case.

A second measurement on GDK2 on 2026-09-25, in-process, median of 15 interleaved runs, on a 231-file MR built from real Ruby sources (1.3 MB of patch text), whitespace shown: diffs_stats work went from 702 ms to 348 ms, diff_files_metadata from 713 ms to 357 ms. Time inside CharlockHolmes went from about 337 ms (462 scans) to 0. On a 302-file corpus with tiny patches the scans only cost about 3.5 ms, so the saving scales with patch size.

Scope: Only stored merge request diffs are covered now, through MergeRequestDiff#load_diffs and PaginatedDiffs#diffs. The Gitaly callers, commits, compare, the new MR page, and MR diffs with whitespace hidden, keep the old path until the flag is removed or a follow-up wires them up. Whitespace-hidden diffs_stats and diff_files_metadata are covered once !256363 moves that path onto stored diff files.

Verification: specs cover the expansion truth table for expanded/limits/file count/text, binary and binary-notice patches with skip_charset_detection on and off, the scan counts per case, valid UTF-8 variants (ascii, multibyte, BOM, NUL byte, empty) with skip_charset_detection set and unset, the collection passing the option through to each diff with skip_charset_detection on and off, the nil and binary patch cases, a UTF-8 tagged but invalid patch with skip_charset_detection set (the scan runs and the patch is converted; CharlockHolmes reads the 0xAE byte as windows-1252, so the spec asserts valid UTF-8 rather than a replacement character), and a guard spec on encode_utf8_with_replacement_character asserting it returns valid UTF-8 unchanged, since the fast path depends on that. Ran spec/lib/gitlab/git/diff_collection_spec.rb, spec/lib/gitlab/git/diff_spec.rb and spec/lib/gitlab/encoding_helper_spec.rb, all green. Equivalence-tested on GDK2 on 2026-09-21 across a 12-MR corpus (302-file text, single small/large/too-large, binary, latin1, cp1252, NUL, UTF-16, BOM, whitespace-only, rename/delete/chmod/empty): master, flag off and flag on were identical across 384 in-process diff code paths and 456 anonymous HTTP responses. A further check on GDK2 with the flag enabled for one project only confirmed skip_charset_detection? returned true on that project and false on another, and output was byte-identical to flag off across 49 paths in the 12-MR corpus. All 91 diff HTTP endpoints returned 200 in both states. A freshly loaded MergeRequestDiff#raw_diffs runs 1 query (merge_request_diff_files only). A regression spec covers the unsaved merge request fallback described above, which crashed on the earlier commit and passes now.

Related, all independent of this MR: !256363 (stored files for the whitespace-hidden stats and metadata path), !256402 (SQL line and byte counts, stacked on !256363), gitaly!9274 (DiffStats whitespace option). Wider findings for the epic live on &23646.

Related to #629870 Epic: &23646