osprey: Built the counts-only projection from parquet footers instead of scanning every row

[brendanx67]

Summary

• Built every counts-only FdrProjectionSet from Parquet footer NumRows instead of scanning every row, on all three producers
• Removed counts-only mode from FdrProjectionSet.Builder, so the scan cannot be reintroduced at a call site
• Widened the cohort row totals to long - they wrap past ~505 files, and a wrap silently skipped first-pass protein FDR
• Tightened the resume schema probe to the columns that make a declared row count trustworthy
• Deleted the Stage-5 survivor-buffer bench, which could only ever skip

Follows #4646. Raised by the developer from a 446-run CHS job.

What the scan was doing

PerFileScoringTask streamed every row of every .scores.parquet through
FdrProjectionSet.Builder(countsOnly: true), whose AddRow discards all five fields and
increments a counter:

if (_countsOnly)
{
    _curFileCount++;
    return;            // every value thrown away
}

read	1,342,686,095 rows across 446 files - entry_id, charge, is_decoy, coelution_sum and a STRING modified_sequence, decoded row group by row group
produced	446 file names + 446 row counts
cost	618 s and ~5.7 GB, measured on the resume path

The phase immediately after it re-reads the same columns from the same files to do the actual
work. The counts were always free: Parquet's footer declares NumRows, and the load loop was
already opening that footer for its schema check.

Why it surfaced now, and why it is not a regression

c4921f3d6c (#4633) made FdrProjections a deferred factory. That was right - it made the cost
CONDITIONAL, and a --task PerFileRescoring worker skips FirstPassFdrTask.Run and so never
pays it, which is the Loading scored entries 9m46s -> 10s win recorded in
TODO-20260901_osprey_stage5_reload_materialization.md. It just deferred work that should not
exist. 9m46s and 618 s are the same scan seen from the two sides of that condition.

Three runs on the same 446-run CHS cohort and shape:

run	build	gaps >=30s	max gap
2026-09-02	pre-#4633	1	47 s
2026-09-03	pre-#4633	0	24 s
2026-09-09	post-#4633	1	618 s

Verified at 446 files

stage5-start-live and projection counts-only are the two lines that bracket the scan. They
were 618 s apart; on this build, on the full cohort:

13:31:47  [MEM stage5-start-live] ... (post-GC, entering first-pass FDR, files=446)
13:31:47  [MEM projection counts-only: 1342686095 rows across 446 files (no resident rows)]

Same second, and the same row total - so the footer counts reproduce the scan's answer on real
data, not just in a unit test. That matches the pre-#4633 behaviour, so this is a restoration.

The overflow, which is the part that could have been wrong output

FdrProjectionSet.TotalRows summed the per-file counts into an unchecked int.
FirstPassFdrTask gates first-pass protein FDR on TotalRows > 0, so a wrapped negative total
skips it: every row keeps its placeholder experiment_protein_qvalue of 1.0, no patch failure is
counted, and the run exits 0 reporting success. PerFileScoringTask already recorded the wrap at
~505 files for the 4.2 M-per-file shape, and TEIREX (936 runs) is in
ai/docs/osprey-large-datasets.md. Run's totalScored carried the same exposure while both
resume paths were already long.

Deliberately not in scope

• FdrProjectionSet.Builder is now dead production code - its only remaining reference is
  FdrTest.cs, and its parity test certifies a class nothing ships calls. Worth deleting, but
  that is a refactor rather than a defect and does not belong in this change.
• The bench-test deletion is here at the developer's explicit request, not because this PR
  required it: it could only ever Assert.Inconclusive without an opt-in multi-GB run directory,
  so it was a permanent skip reported as green. The coverage it nominally represented is covered
  by the planned synthetic test set.

Test plan

• Build-Osprey.ps1 -RunTests -RunInspection - 593 tests, 593 passed, 0 skipped, zero-warning
• regression.ps1 -Dataset Stellar - PASSED, mode1 (vs golden) green, Tokens REQUIRED: 0
• Re-run after the schema-probe change, which alters what a gating predicate accepts
• New TestFooterRowCountMatchesScan - footer NumRows == scan count, across multiple row groups
• 446-file check: stage5-start-live -> projection counts-only in the same second, row total identical
• TeamCity Perf/Regression on pull/<N>

See TODO-20260909_osprey_projection_scan_progress.md in pwiz-ai/todos

Co-Authored-By: Claude noreply@anthropic.com

[Copilot]

🟡 Changes recommended

A couple of updated validation paths now produce misleading user-facing error text and stale commentary relative to the tightened schema probe, which should be corrected before merge.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR removes the expensive “counts-only projection” Parquet row scan by deriving per-file row counts directly from Parquet footer metadata (NumRows), and hardens the pipeline against silent integer overflow in large cohorts by widening projection totals to long. It also tightens the schema probe used to trust footer row counts, adds a unit test that pins footer counts to scan counts across multiple row groups, and deletes an always-skipped Stage 5 bench test.

Changes:

• Build counts-only FdrProjectionSet instances from Parquet footer row counts (no per-row scan) and remove counts-only mode from FdrProjectionSet.Builder.
• Widen cohort-level row totals (TotalRows, totalScored, etc.) to long to prevent overflow-induced logic skips on large datasets.
• Tighten ProbeResumeSchemaAndRows to validate additional required columns and add TestFooterRowCountMatchesScan; remove the Stage 5 survivor-buffer bench test.

File summaries

File	Description
pwiz_tools/Osprey/Osprey.Test/Stage5SurvivorBufferBenchTest.cs	Deleted an opt-in bench test that could only ever skip.
pwiz_tools/Osprey/Osprey.Test/IOTest.cs	Added a test asserting footer NumRows matches a full scalar scan across multiple row groups.
pwiz_tools/Osprey/Osprey.Tasks/PerFileScoringTask.cs	Replaced counts-only scans with footer counts; refactored lean projection building and widened totals.
pwiz_tools/Osprey/Osprey.Tasks/FirstPassFdrTask.cs	Widened pre-compaction cohort totals to long and updated related signatures/comments.
pwiz_tools/Osprey/Osprey.IO/ParquetScoreCache.cs	Tightened resume probe schema validation to make footer row counts trustworthy.
pwiz_tools/Osprey/Osprey.FDR/PercolatorEngine.cs	Updated usage of TotalRows to long.
pwiz_tools/Osprey/Osprey.FDR/FdrProjection.cs	Widened TotalRows to long and removed counts-only mode from FdrProjectionSet.Builder.

Review details

Suppressed comments (1)

pwiz_tools/Osprey/Osprey.Tasks/PerFileScoringTask.cs:1551

• This exception message still mentions "--input-scores" (which is documented elsewhere in the repo as retired) and also only references missing PIN feature columns, even though the probe now fails when entry_id or is_decoy are missing too. Please make the message reflect the actual validation and avoid referring to a removed CLI flag.

                    if (!probe.HasPinFeatures)
                        throw new InvalidDataException(string.Format(
                            @"--input-scores: parquet {0} is missing the PIN feature columns -- it is not a valid Osprey scores parquet. Delete it and re-run so it is regenerated.",
                            parquetPath));

• Files reviewed: 7/7 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.