Bruker .baf nuget support and bug fix

[Alexander-Sol]

Bruker .baf: NuGet native-dependency support + search-correctness fixes

This branch makes the Bruker .baf reader usable from the packaged mzLib NuGet. These are files that were generated by a Bruker qQ-TOF. These files are organized as folders that end in '.d', and they contain at least three files: 'analysis.baf', 'analysis.baf_idx', 'analysis.baf_xtr'.

Initial tests in MetaMorpheus yielded a crash, after that was fixed, searches completed without crashing but yielded zero PSMs when searching qTOF files. Several changes were made to the reader to correct this. Now, MetaMorpheus can sucessfully search qq-TOF files, both top-down and bottom-up.

Packaging — ship the native deps

• Relocated the Bruker native DLLs (baf2sql_c, vcomp110, msvcp110,
  msvcr110) from Readers/Bruker/ to the assembly/lib root, and updated
  the DllImport path and Readers.csproj accordingly.
• mzLib.nuspec ships all four DLLs in both the net8.0 and
  net8.0-windows7.0 lib targets so consumers get them at runtime.
• Package version bumped to 9.2.103.

Fix: search returned no identifications

Two independent defects each broke precursor deconvolution (which runs on
the MS1 scan within each MS2's isolation window); either alone empties the
search.

1. Isolation width was never populated. Without it
   MsDataScan.IsolationRange is null, GetIsolatedMassesAndCharges
   returns an empty list, and every MS2 scan is dropped. The reader now
   reads the isolation width (Quadrupole_IsolationResolution_Act) — plus
   precursor charge (MSMS_PreCursorChargeState) and collision energy
   (Collision_Energy_Act) — per spectrum from the PerSpectrumVariables
   table, which it previously never touched.
2. MS1 scans were being trimmed. Trimming was applied to every scan
   whenever ApplyTrimmingToMsMs was set, so MS1 precursor scans were
   trimmed even though callers set ApplyTrimmingToMs1 = false, stripping
   the isotope envelopes deconvolution needs. Trimming is now gated on MS
   level, matching the mzML/Thermo/timsTOF readers.

Other fixes

• IndexOutOfRangeException on profile MS/MS trimming — profileMzs[^0]
  (one past the end) corrected to [^1], guarded by Length > 1.
• Retention time now converted from Bruker seconds to mzLib minutes.
• Dissociation type keyed on ScanMode instead of AcquisitionMode
  (the analyzer type), with broadband CID added and a safe Unknown
  fallback instead of throwing on an unmapped code.
• Steps↔spectrum association matches on spectrum Id rather than a
  loop index that assumed contiguous, RT-ordered ids.

Validation

Verified end-to-end against a real 2-hour QTOF yeast run
(20150306_yeast1_..._1663.d, ~72.5k MS2 scans) using its MaxQuant search
as ground truth:

• Precursor m/z, fragment m/z, and scan numbering match MaxQuant exactly;
  RT now reads 0.01–120.02 min (was 0.3–7201 s).
• All MS2 scans expose an isolation width/range; ~65k carry a charge.
• Feeding the scans MaxQuant identified through actual deconvolution
  reproduces the precursor call (correct charge + monoisotopic mass) for
  9/9 sampled scans — previously most produced zero envelopes.

Tests

New/strengthened cases in TestBruker: isolation width/range present and
bracketing the precursor across all three fixtures; MS1 untrimmed when
ApplyTrimmingToMs1 is false; RT-in-minutes; and the MS/MS-trimming test
now asserts trimmed-spectrum content (non-empty, sorted, m/z bounds within
the untrimmed range, peak count capped) rather than only "does not throw". This addressed the comment left by @trishorts

[codecov]

Codecov Report

❌ Patch coverage is 84.50704% with 22 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.45%. Comparing base (77fb7db) to head (c92434e).
⚠️ Report is 2 commits behind head on master.

⚠️ Current head c92434e differs from pull request most recent head 29d3910

Please upload reports for the commit 29d3910 to get more accurate results.

Files with missing lines	Patch %	Lines
mzLib/Readers/Bruker/BrukerFileReader.cs	84.50%	16 Missing and 6 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1096      +/-   ##
==========================================
- Coverage   91.46%   91.45%   -0.01%     
==========================================
  Files         427      427              
  Lines       52777    52892     +115     
  Branches     6325     6344      +19     
==========================================
+ Hits        48273    48373     +100     
- Misses       3268     3278      +10     
- Partials     1236     1241       +5     

Files with missing lines	Coverage Δ
mzLib/Readers/Bruker/BrukerFileReader.cs	81.44% <84.50%> (+1.93%)	⬆️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[trishorts]

Approving — the fix is correct. profileMzs[^0] was evaluating to profileMzs[Length] (one past the end), which is exactly the source of the IndexOutOfRangeException on profile-mode .baf MS/MS trimming; [^1] plus the Length > 1 guard is the right correction. The native-DLL relocation to the lib root is symmetric across both the net8.0 and net8.0-windows7.0 targets (all four of baf2sql_c, vcomp110, msvcp110, msvcr110 present in each), and the DllImport path change matches the new placement.

One non-blocking suggestion for TestLoadAllStaticDataWithMsMsTrimming: it currently asserts only DoesNotThrow + NumSpectra == 5. Since the whole point of the fix is a boundary index, a future regression that avoids the exception but silently drops the boundary peak (or returns an empty trimmed spectrum) would still pass. Consider adding one assertion on the trimmed profile MS/MS scan's content — e.g. that its MassSpectrum.XArray is non-empty and its first/last m/z fall within the expected window — so the corrected [0]/[^1] bounds are actually exercised, not just reached without throwing. A single-point-profile case would also nail down the new Length > 1 guard. Happy to merge as-is if you'd rather fold that into a follow-up.

[trishorts]

@Alexander-Sol The changes look good — the code side of this is done as far as I'm concerned. The only thing standing between this and green is codecov/patch, so I went and dug into exactly which lines it's unhappy about and wrote the tests to close them. These two tests take care of the coverage gap: I built and ran them against your head commit (c92434e) and they take patch coverage from 84.5% → 94.4%, clearing the threshold. All 22 tests in TestBruker pass.

Why it's red

There's no codecov.yml in the repo, so codecov's default patch: target: auto applies — the patch has to be at least as covered as the base project, which is 91.46%. You're at 84.5% (120 of 142 coverable new lines). BrukerFileReader.cs is the only file in the diff with coverable lines, so it's the whole story.

The 22 uncovered lines aren't scattered noise — they're two real gaps:

1. The "precursor-variable tables are optional" fallback has never executed (13 lines). All three catch (SQLiteException) blocks plus both early returns. Your comments promise graceful degradation on older/unusual acquisitions that lack SupportedVariables / PerSpectrumVariables — but all three committed fixtures carry both tables, so that promise has literally never run. A typo in a table name, or an exception escaping, would ship as a hard crash on exactly the files the fallback exists for.

2. The precursor charge-state assignment is unreachable (2 lines). I queried the fixtures' SQLite directly: all three record MSMS_PreCursorChargeState = 0 on every spectrum, so selectedIonChargeStateGuess = ... never runs — only the 0 → null sentinel path does. Given the PR body says ~65k scans in the real yeast run carry a charge, and a wrong precursor charge silently mis-assigns every precursor mass in a search, that's the most consequential untested behavior in the diff.

The thing that makes both testable

analysis.sqlite is a committed part of each .d fixture, and I confirmed empirically that baf2sql reuses that cache rather than regenerating it. So a test can copy a fixture to temp, edit the cache with plain SQL, and drive the reader against it — no new binary fixture needed. One helper unlocks both gaps.

The tests

Add using System; and using System.Data.SQLite; to TestBruker.cs, then:

// Copies a .d fixture to a fresh temp directory and runs the supplied SQL against the copy's
// analysis.sqlite cache. Lets a test stand up a Bruker file whose precursor-variable tables differ
// from the three committed fixtures (all of which record charge state 0 and carry both tables),
// without checking in another multi-megabyte .d folder.
private static string CopyFixtureAndEditSqlite(string fixtureName, string sql)
{
    string source = Path.Combine(TestContext.CurrentContext.TestDirectory, "DataFiles", fixtureName);
    string destination = Path.Combine(Path.GetTempPath(), "mzLibBrukerTest_" + Guid.NewGuid().ToString("N"), fixtureName);

    foreach (string directory in Directory.GetDirectories(source, "*", SearchOption.AllDirectories))
        Directory.CreateDirectory(directory.Replace(source, destination));
    Directory.CreateDirectory(destination);
    foreach (string file in Directory.GetFiles(source, "*", SearchOption.AllDirectories))
        File.Copy(file, file.Replace(source, destination), true);

    using var connection = new SQLiteConnection("DataSource=" + Path.Combine(destination, "analysis.sqlite"));
    connection.Open();
    using var command = new SQLiteCommand(sql, connection);
    command.ExecuteNonQuery();
    return destination;
}

// The reader treats the (Per)SpectrumVariables tables as optional: older/unusual acquisitions may not
// have them, and the reader is written to degrade gracefully (no isolation width) rather than throw.
// Nothing exercised that fallback -- all three fixtures carry both tables -- so a typo in a table name,
// or an exception escaping the SQLite handling, would have shipped as a hard crash on those files.
// Dropping each table in turn from a temp copy proves the file still loads, with the peak data and
// precursor m/z intact and IsolationWidth simply absent.
[Test]
[TestCase("SupportedVariables")]      // variable-id lookup fails -> no precursor variables at all
[TestCase("PerSpectrumVariables")]    // ids resolve, but the per-spectrum values are unavailable
public void TestPrecursorVariableTablesAreOptional(string tableToDrop)
{
    string path = CopyFixtureAndEditSqlite("centroid_1x_MS1_4x_autoMS2.d", "DROP TABLE " + tableToDrop);

    MsDataFile brukerData = null;
    Assert.DoesNotThrow(() => brukerData = MsDataFileReader.GetDataFile(path).LoadAllStaticData(),
        $"reader threw when {tableToDrop} was absent instead of falling back");

    // The file still reads: same scan count, same spectra, same precursor selection as the intact fixture.
    Assert.That(brukerData.NumSpectra, Is.EqualTo(5));
    Assert.That(brukerData.Scans[1].MsnOrder, Is.EqualTo(2));
    Assert.That(brukerData.Scans[1].SelectedIonMZ, Is.EqualTo(721.86865).Within(0.001));
    Assert.That(brukerData.Scans[1].MassSpectrum.XArray, Is.Not.Empty);
    // Only the precursor variables are lost, and they are lost as null rather than as a bogus zero.
    Assert.That(brukerData.Scans[1].IsolationWidth, Is.Null);
    Assert.That(brukerData.Scans[1].IsolationRange, Is.Null);
    Assert.That(brukerData.Scans[1].SelectedIonChargeStateGuess, Is.Null);

    // The dynamic path resolves the variables separately, so it needs its own fallback.
    var dynamicReader = MsDataFileReader.GetDataFile(path);
    dynamicReader.InitiateDynamicConnection();
    MsDataScan scan = null;
    Assert.DoesNotThrow(() => scan = dynamicReader.GetOneBasedScanFromDynamicConnection(2),
        $"dynamic connection threw when {tableToDrop} was absent");
    Assert.That(scan.SelectedIonMZ, Is.EqualTo(721.86865).Within(0.001));
    Assert.That(scan.IsolationWidth, Is.Null);
    dynamicReader.CloseDynamicConnection();
}

// A precursor charge read from the file is handed to search engines as the charge to deconvolute at,
// so getting it wrong silently mis-assigns every precursor mass in the run. Every committed fixture
// records MSMS_PreCursorChargeState = 0, so the branch that surfaces a real charge never ran in any
// test; only the "0 -> null" sentinel path did. Writing a real charge into a temp copy covers both:
// the charge is surfaced as-is on the scan it belongs to, and a 0 on another scan stays null rather
// than being reported as a charge of 0 (which would be an invalid charge state downstream).
[Test]
public void TestPrecursorChargeStateIsReadFromPerSpectrumVariables()
{
    // Scan 2 gets a real charge of 3; scans 3-5 keep Bruker's 0 = "could not determine".
    string path = CopyFixtureAndEditSqlite("centroid_1x_MS1_4x_autoMS2.d",
        @"UPDATE PerSpectrumVariables SET Value = 3
          WHERE Spectrum = 2
            AND Variable = (SELECT Variable FROM SupportedVariables
                            WHERE PermanentName = 'MSMS_PreCursorChargeState')");

    MsDataFile brukerData = MsDataFileReader.GetDataFile(path).LoadAllStaticData();

    Assert.That(brukerData.Scans[1].SelectedIonChargeStateGuess, Is.EqualTo(3));
    Assert.That(brukerData.Scans[2].SelectedIonChargeStateGuess, Is.Null,
        "a recorded charge of 0 means 'undetermined' and must not be surfaced as a charge of 0");

    // The dynamic path builds the scan through a different query and must agree.
    var dynamicReader = MsDataFileReader.GetDataFile(path);
    dynamicReader.InitiateDynamicConnection();
    Assert.That(dynamicReader.GetOneBasedScanFromDynamicConnection(2).SelectedIonChargeStateGuess,
        Is.EqualTo(3));
    dynamicReader.CloseDynamicConnection();
}

TestPrecursorVariableTablesAreOptional is the one that actually clears the gate — it covers 13 lines on its own and gets you to 93.7%. TestPrecursorChargeStateIsReadFromPerSpectrumVariables only adds one line (→94.4%), but it's ~15 lines on top of a helper the first test already needs, and it guards the feature most likely to silently corrupt a search. I'd take both.

What I'd deliberately leave uncovered

Eight lines stay uncovered and I don't think you should chase them: AssignVariable's type-coercion catch (only reachable via a blob value these three variables never hold), GetSpectraData's return -1, the MSn trim branch, and the dissociation-type Unknown fallback. Covering those means contriving fixtures for states that don't occur — coverage theater. The two tests above get you past the threshold on genuine behavior.