Static-review audit of CaloReco/ at a432e8d9b (main, 2026-08-25), companion to the Trigger/ audit in #1942. Method: multi-agent static sweep with an adversarial refutation pass, ranked by blast radius (who actually runs the code). No build was run and no art job was executed — all evidence is from reading the source and cross-repo grep of Offline, Production and mu2e-trig-config; the two headline findings were then re-verified by hand, including the numeric scenarios.
Blast-radius context, which is what makes this package different from #1942: all three modules are live in every production reco job (Production/JobConfig/reco/prolog.fcl pulls @table::CaloReco.producers and runs CaloReco.Reco), and CaloHitMakerFast additionally runs in the trigger (mu2e-trig-config/core/producers/trigCalProducers.fcl, label CaloHitFastMaker).
The two live bugs (production reco + trigger)
Both are in CaloHitMakerFast_module.cc::addPulse (lines 199–216), which merges in-time-window digis on the same crystal:
1. The "replace" branch leaves nSiPM_ stale, defeating the anti-noise gate.
if (fabs(ratio) < nSigmaNoise_*sigmaR) {pulse.add(time,eDep);}
else if (eDep>pulse.eDep_) {pulse.time_=time; pulse.eDep_=eDep;} // line 212HitInfo::add() is the only code that increments nSiPM_; the replace branch overwrites time and energy wholesale but keeps the old count. Concrete scenario with the production defaults (nSigmaNoise: 4, nPEperMeV: 30, noiseLevelMeV: 0.55, deltaTPulses: 11): two coincident ~5 MeV digis merge to {nSiPM_=2, eDep_≈5.05}; a third in-window 50 MeV digi gives ratio 0.82 against a threshold of 0.11, so the replace branch fires and the hit becomes {nSiPM_=2, eDep_=50} — backed by a single raw digi. Downstream, CaloHit::energyDepTot() returns eDep_*nSiPMs_ (now doubled from a single-digi energy), and CaloClusterFast_module.cc:97 cuts nSiPMs() >= minSiPMPerHit with minSiPMPerHit: 2 on exactly this module's output — so the gate built specifically to reject uncorroborated single-SiPM hits passes one, in both production reco and the trigger path.
2. A coincident digi that fails the ratio test and is not the larger one is silently discarded.
Once an in-window pulse is found, addNewHit is set false and the loop breaks unconditionally, but the if/else-if above handles only two of the three outcomes. The third — ratio outside the noise band and eDep <= pulse.eDep_ — has no branch: the digi is dropped entirely. Scenario with the same defaults: an existing 50 MeV pulse and a genuine 8 MeV pileup digi 0.5 ns later; the ratio (−0.72) fails the merge test and 8 > 50 is false, so the 8 MeV digi produces no CaloHit, no cluster-energy contribution, and no diagnostic at any diagLevel. If keeping only the dominant pulse per window is deliberate trigger design, the silent energy loss is at least undocumented — and offline reco inherits it, since production runs the same module.
Live modules, diagnostic-only effect surface
3. CaloHitMaker_module.cc:129 — if (hits.size()<2) continue; gates on the crystal's total digi count for the whole event, before time clustering. A crystal with exactly one real digi produces no CaloHit; the same digi accompanied by an unrelated time-distant digi on the same crystal (e.g. noise on the other SiPM) is split by the merge loop into two nSiPM=1 sub-clusters and both are emitted (the nSiPM<2 guard in fillBuffer is commented out, line 205). Identical physical information is kept or discarded depending on an uncorrelated coincidence. The live downstream consumer (CaloProtoClusterMaker) cuts only on energy, not nSiPMs(), so this is an energy-accounting inconsistency rather than a defeated gate.
4. CaloTemplateWFProcessor.hh:67 — isPileUp(i) returns i > 1, but the class contract (and its base) treats every peak after the first as pile-up, so the most common topology — exactly two peaks — writes the second peak with pileUp = false. TemplateFit is the configured strategy in every production reco job, so the wrong bit is written into every production CaloRecoDigi; today its only readers are a validation histogram and a debug printer, which is what holds this at S2. The author's own inline comment (// resAmp_.size() > 1 as alternative?) flags the doubt; the fix is i >= 1.
5. CaloRecoDigiMaker_module.cc:41 — the validated maxPlots parameter (set to 50 by the package prolog on every production run) is stored and never read; waveformProcessor_->plot(...) is never called. A dead, actively-configured diagnostic knob: anyone setting it gets zero plots, silently.
Compiled but unreached — fix before ever enabling
These sit behind config strings that no committed FHiCL in Offline, Production or mu2e-trig-config sets (processorStrategy: "RawExtract", refitLeadingEdge: true); verified by grep of all three repos.
6. CaloRawWFProcessor.cc:88 — resAmpErr_.push_back(0); fabricates a zero amplitude error for every peak. CaloRecoDigiMaker multiplies it into eDepErr, and the live cluster makers sum energyDepErr() in quadrature — so enabling RawExtract would silently shrink every cluster's energy uncertainty.
7. CaloTemplateWFUtil.cc:188 — while (xvec_[imax]<param_[2]) ++imax; in refitEdge() has no bound against xvec_.size(). The MINUIT time bound is the generic [0, 1e6], not the waveform window, and the code that guards against a fitted time past the last bin runs only after this loop. Flipping refitLeadingEdge: true exposes out-of-bounds vector reads.
8. CaloRawWFProcessor.hh:45 — isPileUp(i) returns nPeaks_ > 1, ignoring the index: any multi-peak waveform flags every peak, including well-separated non-overlapping ones, as pile-up.
What this audit did not check
No compilation, no executed job, no assessment of the template shapes or fit quality itself — only control flow, indexing and config plumbing. Consumers outside the three grepped repos (private analyzers, TrkAna-side readers) were not surveyed. Findings 6–8 keep their low severity only as long as nothing enables those config strings.
Happy to turn any subset of these into a PR.
Static-review audit of
CaloReco/ata432e8d9b(main, 2026-08-25), companion to theTrigger/audit in #1942. Method: multi-agent static sweep with an adversarial refutation pass, ranked by blast radius (who actually runs the code). No build was run and no art job was executed — all evidence is from reading the source and cross-repo grep of Offline, Production and mu2e-trig-config; the two headline findings were then re-verified by hand, including the numeric scenarios.Blast-radius context, which is what makes this package different from #1942: all three modules are live in every production reco job (
Production/JobConfig/reco/prolog.fclpulls@table::CaloReco.producersand runsCaloReco.Reco), andCaloHitMakerFastadditionally runs in the trigger (mu2e-trig-config/core/producers/trigCalProducers.fcl, labelCaloHitFastMaker).The two live bugs (production reco + trigger)
Both are in
CaloHitMakerFast_module.cc::addPulse(lines 199–216), which merges in-time-window digis on the same crystal:1. The "replace" branch leaves
nSiPM_stale, defeating the anti-noise gate.HitInfo::add()is the only code that incrementsnSiPM_; the replace branch overwrites time and energy wholesale but keeps the old count. Concrete scenario with the production defaults (nSigmaNoise: 4,nPEperMeV: 30,noiseLevelMeV: 0.55,deltaTPulses: 11): two coincident ~5 MeV digis merge to{nSiPM_=2, eDep_≈5.05}; a third in-window 50 MeV digi gives ratio 0.82 against a threshold of 0.11, so the replace branch fires and the hit becomes{nSiPM_=2, eDep_=50}— backed by a single raw digi. Downstream,CaloHit::energyDepTot()returnseDep_*nSiPMs_(now doubled from a single-digi energy), andCaloClusterFast_module.cc:97cutsnSiPMs() >= minSiPMPerHitwithminSiPMPerHit: 2on exactly this module's output — so the gate built specifically to reject uncorroborated single-SiPM hits passes one, in both production reco and the trigger path.2. A coincident digi that fails the ratio test and is not the larger one is silently discarded.
Once an in-window pulse is found,
addNewHitis set false and the loop breaks unconditionally, but the if/else-if above handles only two of the three outcomes. The third — ratio outside the noise band andeDep <= pulse.eDep_— has no branch: the digi is dropped entirely. Scenario with the same defaults: an existing 50 MeV pulse and a genuine 8 MeV pileup digi 0.5 ns later; the ratio (−0.72) fails the merge test and 8 > 50 is false, so the 8 MeV digi produces no CaloHit, no cluster-energy contribution, and no diagnostic at anydiagLevel. If keeping only the dominant pulse per window is deliberate trigger design, the silent energy loss is at least undocumented — and offline reco inherits it, since production runs the same module.Live modules, diagnostic-only effect surface
3.
CaloHitMaker_module.cc:129—if (hits.size()<2) continue;gates on the crystal's total digi count for the whole event, before time clustering. A crystal with exactly one real digi produces no CaloHit; the same digi accompanied by an unrelated time-distant digi on the same crystal (e.g. noise on the other SiPM) is split by the merge loop into two nSiPM=1 sub-clusters and both are emitted (thenSiPM<2guard infillBufferis commented out, line 205). Identical physical information is kept or discarded depending on an uncorrelated coincidence. The live downstream consumer (CaloProtoClusterMaker) cuts only on energy, notnSiPMs(), so this is an energy-accounting inconsistency rather than a defeated gate.4.
CaloTemplateWFProcessor.hh:67—isPileUp(i)returnsi > 1, but the class contract (and its base) treats every peak after the first as pile-up, so the most common topology — exactly two peaks — writes the second peak withpileUp = false. TemplateFit is the configured strategy in every production reco job, so the wrong bit is written into every productionCaloRecoDigi; today its only readers are a validation histogram and a debug printer, which is what holds this at S2. The author's own inline comment (// resAmp_.size() > 1 as alternative?) flags the doubt; the fix isi >= 1.5.
CaloRecoDigiMaker_module.cc:41— the validatedmaxPlotsparameter (set to 50 by the package prolog on every production run) is stored and never read;waveformProcessor_->plot(...)is never called. A dead, actively-configured diagnostic knob: anyone setting it gets zero plots, silently.Compiled but unreached — fix before ever enabling
These sit behind config strings that no committed FHiCL in Offline, Production or mu2e-trig-config sets (
processorStrategy: "RawExtract",refitLeadingEdge: true); verified by grep of all three repos.6.
CaloRawWFProcessor.cc:88—resAmpErr_.push_back(0);fabricates a zero amplitude error for every peak.CaloRecoDigiMakermultiplies it intoeDepErr, and the live cluster makers sumenergyDepErr()in quadrature — so enabling RawExtract would silently shrink every cluster's energy uncertainty.7.
CaloTemplateWFUtil.cc:188—while (xvec_[imax]<param_[2]) ++imax;inrefitEdge()has no bound againstxvec_.size(). The MINUIT time bound is the generic [0, 1e6], not the waveform window, and the code that guards against a fitted time past the last bin runs only after this loop. FlippingrefitLeadingEdge: trueexposes out-of-bounds vector reads.8.
CaloRawWFProcessor.hh:45—isPileUp(i)returnsnPeaks_ > 1, ignoring the index: any multi-peak waveform flags every peak, including well-separated non-overlapping ones, as pile-up.What this audit did not check
No compilation, no executed job, no assessment of the template shapes or fit quality itself — only control flow, indexing and config plumbing. Consumers outside the three grepped repos (private analyzers, TrkAna-side readers) were not surveyed. Findings 6–8 keep their low severity only as long as nothing enables those config strings.
Happy to turn any subset of these into a PR.