osprey: Collapsed the protein q-values into one field written per pass

[brendanx67]

Summary

• Collapsed the two per-entry protein q-value fields on FdrEntry into one experiment-wide
  ExperimentProteinQvalue, and renamed the sidecar column run_protein_qvalue ->
  experiment_protein_qvalue. The field was per-run in name only - measured on
  straight-through 1st-pass sidecars, 483,820 of 483,820 multi-file precursors carry ONE
  value across every file they appear in, and 0 carry different values.

• The 2nd-pass FDR sidecar now carries the pass-2 protein q, patched in after the
  second-pass protein FDR. It previously carried a pass-1 value unconditionally - the only
  column in that file that did. Protein q now follows the rule precursor and peptide q
  already follow: one field, with the pass recorded by which sidecar it lives in rather
  than by which field.

• Removed a latent C#/Rust divergence in the process. C#'s second-pass
  PropagateProteinQvalues passed (setRun: true, setExperiment: true); Rust's passed
  set_run: false. After Stage 7 the two implementations therefore held different
  values in that field - C# the pass-2 value, Rust the pass-1 one - and every parity gate
  was green through it, because effective_run_qvalue(Protein) has no caller on either
  side. The collapse makes the disagreement unrepresentable rather than merely fixed.

• Added a mode1c leg to regression.ps1 (Test-Pass2ProteinQvalue in
  Regression/FdrSidecars.ps1) asserting that the 2nd-pass sidecar's protein column really
  is a pass-2 value. Mode 3 cannot see this class of defect - it compares the two routes
  against each other, and both routes copied the same pass-1 value.

• The gate found the defect was much larger than Gap-fill entries get a run protein q on the batch-hydrate arm but not straight-through #4559 reported. Gap-fill entries get a run protein q on the batch-hydrate arm but not straight-through #4559 described a
  390-record sliver where the two routes disagreed; on the arm CI actually runs, the column
  was never a pass-2 value for any record - all 994,509 shared records on Stellar were
  identical to the 1st-pass column. That is what mode1c was red on before the fix.

• This dissolves Gap-fill entries get a run protein q on the batch-hydrate arm but not straight-through #4559's original question rather than answering it. "Should a gap-fill entry
  carry 0.0 or 1.0?" only arises because a pass-1 quantity was stored in a pass-2 file and
  then asked what it says about rows that did not exist in pass 1. A gap-fill row now gets
  whatever the second-pass protein FDR computes for it, from a pool that includes it.

• Hardened the patch itself after a review pass over the branch. The Stage-7
  OSPREY_STAGE7_PROTEIN_FDR_ONLY early exit calls Environment.Exit(0) inside
  RunProteinFdr, which is before the patch call that sat in the caller - so on that
  diagnostic path the 2nd-pass sidecar was left with its protein column at the ResetScores
  default, and the comment above the sidecar write says that exit deliberately leaves the file
  on disk for downstream rehydration. Both implementations now order it
  propagate -> patch -> dump; the dump reads only the parsimony / FDR result and never the
  stubs, so its content is unchanged. This also removes a cross-impl ordering difference (Rust
  dumped before propagation, C# after). No production path and no reported output is affected.

• Also from that pass: the patch-failure warning claimed unpatched files "keep a FIRST-pass
  protein q-value", which stopped being true when the seed was removed; the remaining
  runProtein* identifiers are gone, so the "run" framing is now unrepresentable rather than
  merely unused; and PatchProteinQvalues reports how many records it actually rewrote
  (out int, asserted in three tests) instead of the caller assuming the map size, which is
  what the Rust side already counted.

Sidecar version: decided, NOT bumping (Brendan, 2026-08-13). The question was whether the
2nd-pass protein column changing meaning warrants v4 -> v5. It does not, for now:

• Nothing structural changes. No offset, width or type moves; experiment_protein_qvalue
  stays at [52..60]. A v4 and a v5 reader would parse identical bytes identically. The
  1st-pass sidecar is unchanged in meaning as well as layout.
• The scenario a bump would guard is already guarded, harder and earlier.
  ParquetScoreCache hard-fails on any osprey version mismatch ("different daily build" /
  "incompatible release identity"), and .scores.parquet is read before any sidecar - so a
  cross-build resume never reaches this file. What remains is same-version-stamp-different-code,
  i.e. same-day dev builds or a run pinned under OSPREY_VERSION_OVERRIDE.
• The version byte gates the whole record, but only 1 of 8 columns changed meaning.
  Rejecting a v4 sidecar discards Score, Pep, run/experiment precursor and peptide q and
  ExperimentAggregateScore - all still correct - to protect the one column that
  PropagateProteinQvalues overwrites unconditionally in Stage 7 before anything reads it.
  (This point was originally stronger and is now partly overtaken: it also said the read path
  warns and proceeds rather than recomputing. osprey: Surfaced single-peak multiple-ID co-assignment in --model-diagnostics #4558's FdrScoresSidecar.IsCurrentFormat has
  since replaced the bare File.Exists gates at Pass2FdrSidecar.cs:188/:511 and
  PerFileRescoreTask.cs:268, so a stale-version sidecar now fails the "already done" test and
  is regenerated. A bump would do the right thing rather than the awkward thing. The
  decision below is unchanged, but it rests on the first and last bullets, not this one.)
• Osprey is pre-first-public-release, so the only consumers of these artifacts are
  development sessions, which will take fresh runs over -LinkFrom while this much is moving.
  The cost of a stale artifact is a re-run, not a wrong answer shipped to a user.

Revisit when either of these lands: the first public release, or #4561 (--fdr-level protein in C#), which is what gives the 2nd-pass protein column a real consumer - at which
point a stale pass-1 value becomes a wrong reported q rather than one that gets overwritten.
The bump should then be paired with making the sidecar read hard-fail instead of
warn-and-proceed, since a version guard that warns and continues is not really a guard.

Stacked on #4558 - review that one first. Retargeting to master and rebasing once it merges.

Fixes #4559

Test plan

• regression.ps1 -Dataset All - 52 legs, 52 PASS, 0 FAIL, run against osprey: Surfaced single-peak multiple-ID co-assignment in --model-diagnostics #4558's
  rebaselined goldens
• mode1 (vs golden) PASS on all four datasets - the change is sidecar-only and moves
  no reported output, which the code reading predicted (Stage 7 reports
  ProteinFdrResult.GroupQvalues, per GROUP, not either per-entry field) and this
  measures
• mode1c (new) red -> green on Stellar with the same command both ways: FAIL (all
  994,509 shared records identical to the 1st-pass column) -> PASS. Movement on every
  dataset: Stellar 25,006/994,509, StellarLibDecoy 6,486/923,246, StellarGenDecoyEntrap
  89,116/259,678, Astral 19,008/3,449,774
• mode1b FDR sanity bounds PASS on all three datasets that carry it - those are the
  entrapment-measured true-FDP ceilings -CreateGolden deliberately does not
  regenerate, so calibration is unmoved on evidence independent of the golden
• mode3 (sidecars == straight-through) PASS everywhere, up to 9,685,318 records on
  Astral - both routes still agree after the change
• Osprey.Test 579 tests pass; zero-warning ReSharper inspection
• The hardening commit on top was gated separately: build + 579 tests + inspection, and
  regression.ps1 -Dataset Stellar re-run to confirm the golden still does not move.
  It changes ordering only on the OSPREY_STAGE7_PROTEIN_FDR_ONLY diagnostic path plus
  comments, a log message and identifier names, so it cannot move production output -
  but the full -Dataset All figures above were measured on the commit below it, and
  the TeamCity Perf/Regression run on this PR covers the final tree.

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

[brendanx67]

TeamCity Perf/Regression is green on this PR: build 4131578, pull/4569 at 826666204f — Osprey regression PASSED, mode1 (vs golden) PASS on all four datasets.

That closes the one caveat in the test plan above. The local -Dataset All figures were measured on the commit below the tip, so the hardening commit only had -Dataset Stellar behind it locally; this run covers the tip on all four datasets.

It also reproduces the local mode1c counts exactly, on a different machine:

dataset	records moved	gap-fill absent from pass 1
Stellar	25,006 / 994,509	390
StellarLibDecoy	6,486 / 923,246	666
StellarGenDecoyEntrap	89,116 / 259,678	741
Astral	19,008 / 3,449,774	8,800

One note for anyone re-triggering this config: it only works on MacCoss TeamCity Agent 1. Its last two runs before this one (both on master, 2026-07-10) died in ~10 seconds with exit code 9009 on AWS agents — a tool missing from the PATH, not an Osprey failure — so an unpinned trigger can return a red that looks real but isn't.

[brendanx67]

Validated at 82-file scale

An 82-file SEA-AD Astral-DIA run finished on this branch tip (826666204f) — 8 h 38 m, exit 0, zero exceptions. Log ai/.tmp/seaad-82f-4559-20260812_135028.log; output D:\test\Pilot-MTG-Tissue-May2026\Astral-DIA\runs\seaad-82files-libdecoy-r1.0-protein-compact.

Test-Pass2ProteinQvalue (the mode1c gate this PR adds) run against that output:

matched records	88,554,423
moved to a pass-2 value	1,777,489 (2.01%)
gap-fill, absent from pass 1	513,952
verdict	PASS

That is ~26× the largest regression dataset (Astral, 3.45M records), so the second-pass protein FDR demonstrably reaches the sidecar on production-scale data and not just on the 3-file sets.

It also puts the original issue in proportion. #4559 was filed over a 390-record divergence on Stellar; the same population here is 513,952 records, and before this change every one of those 88.5M records carried a pass-1 value in a pass-2 file.

FDR calibration is unmoved, which is what the "sidecar-only" claim predicts. Entrapment FDP at reported q ≤ 0.01 is 1.575% combined / 0.800% lower-bound (r = 0.9695, 749,259 reported precursors) — the already-documented pass-2 recalibration figure of ~1.57% for this configuration, reproduced to three significant figures. Reproducibility looks normal too: peptides identified in ≥41 of 82 runs sit at 0.18% FDP, and the 5,422 seen in all 82 at 0.00%.

[Copilot]

Pull request overview

This PR updates Osprey’s protein q-value handling so the per-entry protein q is represented as a single experiment-wide field (ExperimentProteinQvalue) and ensures the 2nd-pass .2nd-pass.fdr_scores.bin sidecar actually contains the pass-2 protein q-value (patched in after second-pass protein FDR). It also adds a new regression leg (“mode1c”) to catch the previously undetectable shared-defect scenario where both routes copied an incorrect pass-1 protein q into the pass-2 sidecar.

Changes:

• Collapses RunProteinQvalue / ExperimentProteinQvalue into one ExperimentProteinQvalue, and renames the sidecar column to experiment_protein_qvalue.
• Patches the pass-2 protein q-values into the 2nd-pass sidecar after second-pass protein FDR runs, aligning sidecar semantics with other q-value columns.
• Adds a new regression check (mode1c) asserting the 2nd-pass sidecar’s protein q column is not a verbatim copy of the 1st-pass column.

Reviewed changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
pwiz_tools/Osprey/Regression/FdrSidecars.ps1	Adds a liveness check for pass-2 protein q-values and updates field naming/layout references.
pwiz_tools/Osprey/regression.ps1	Adds mode1c leg to run the pass-2 protein q-value liveness check on straight-through output.
pwiz_tools/Osprey/Osprey/OspreyFileDiagnostics.cs	Updates diagnostic dumps/comments to use ExperimentProteinQvalue.
pwiz_tools/Osprey/Osprey.Test/ReconciliationTest.cs	Updates tests to set/read ExperimentProteinQvalue.
pwiz_tools/Osprey/Osprey.Test/Pass2FdrSidecarTest.cs	Updates pass-2 sidecar tests to reflect renamed protein q field.
pwiz_tools/Osprey/Osprey.Test/OspreyConfigTest.cs	Updates comments/assertions referencing protein q field name.
pwiz_tools/Osprey/Osprey.Test/IOTest.cs	Updates sidecar roundtrip/patch tests and adds explicit pass-2 protein-q patch test.
pwiz_tools/Osprey/Osprey.Test/FdrTest.cs	Updates projection roundtrip to use ExperimentProteinQvalue.
pwiz_tools/Osprey/Osprey.Tasks/SecondPassFdrTask.cs	Ensures pass-2 protein q is patched into the pass-2 sidecar after protein FDR runs.
pwiz_tools/Osprey/Osprey.Tasks/RescoreHydration.cs	Updates hydration docs/behavior references to ExperimentProteinQvalue.
pwiz_tools/Osprey/Osprey.Tasks/PerFileRescoreTask.cs	Updates reconciliation/rehydration commentary and field naming for protein q.
pwiz_tools/Osprey/Osprey.Tasks/Pass2FdrSidecar.cs	Stops seeding protein q from pass-1 for pass-2 sidecars; adds pass-2 patch routine and updated messaging.
pwiz_tools/Osprey/Osprey.Tasks/ModelDiagnostics/PeakCoAssignmentSource.cs	Updates sidecar column naming in comments.
pwiz_tools/Osprey/Osprey.Tasks/FirstPassFdrTask.cs	Updates compaction predicate to use ExperimentProteinQvalue and renames patch method usage.
pwiz_tools/Osprey/Osprey.Tasks/FdrProjectionSinks.cs	Updates streaming sinks to write/lookup ExperimentProteinQvalue.
pwiz_tools/Osprey/Osprey.IO/FdrScoresSidecar.cs	Renames protein q column, updates read/write, and replaces patch API with PatchProteinQvalues(..., out recordsPatched).
pwiz_tools/Osprey/Osprey.IO/FdrScoreRecord.cs	Renames record field to ExperimentProteinQvalue.
pwiz_tools/Osprey/Osprey.FDR/Reconciliation/ConsensusRts.cs	Uses ExperimentProteinQvalue in protein rescue eligibility.
pwiz_tools/Osprey/Osprey.FDR/ProteinFdrEngine.cs	Updates propagation semantics to a single protein q field and simplifies propagation call.
pwiz_tools/Osprey/Osprey.FDR/ProteinFdr.cs	Collapses propagation to always set ExperimentProteinQvalue and updates first/second pass semantics/docs.
pwiz_tools/Osprey/Osprey.FDR/FdrProjectionOutput.cs	Updates documentation references for where protein q-values are produced.
pwiz_tools/Osprey/Osprey.Core/FdrEntry.cs	Removes RunProteinQvalue and documents single-field, pass-by-sidecar semantics for protein q.
pwiz_tools/Osprey/docs/14-intermediate-files.md	Updates intermediate file documentation to experiment_protein_qvalue and clarifies pass-dependent meaning.
pwiz_tools/Osprey/docs/08-protein-parsimony.md	Updates protein-parsimony documentation to reflect single protein-q field and pass-by-sidecar semantics.

Suppressed comments (2)

pwiz_tools/Osprey/Osprey.IO/FdrScoresSidecar.cs:456

• Comment in PatchProteinQvalues still says it streams a "60-byte" record, but the method streams RecordLength bytes per record (68 in v4).

                        // Stream one 60-byte record at a time: overwrite ONLY the

pwiz_tools/Osprey/Osprey.IO/FdrScoresSidecar.cs:763

• ReadRecords XML doc still says it streams a "60-byte" record. The implementation streams RecordLength bytes (68 in v4), and keeping the doc in sync helps avoid future offset/stride mistakes.

        /// discard) on any mismatch or IO failure. Streams one 60-byte record at a time from
        /// the source (one record resident, not an O(file-size) whole-file buffer), matching
        /// <see cref="PatchProteinQvalues"/>. Records are delivered in stored (file) order.

---

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