TrkHitReco: fix defects found by a static sweep of the package

[oksuzian]

Four defects found by a static review of TrkHitReco at 0ef02a66d. I have not built this locally — relying on mu2e/buildtest.

---

1. 🔴 CombineStrawHits: ComboHit parent and stored indices reference two different collections

src/CombineStrawHits_module.cc:131 binds the output to the original input handle:

chcolNew->setParent(chcH);

but the Unsorted:true branch (:133-151) permutes the hits into a local chcolsort and then calls combine(ewm, chcolsort, *chcolNew). Inside, combohit.init(hit1,ich) and combohit.addIndex(jch) store ich/jch — positions in the permutation — while every consumer resolves them against chcOrig. Per-hit provenance is silently wrong, and wrong precisely when the permutation is non-trivial, which is the only reason the branch exists.

Live via Production/Processing/vstdata_epilog.fcl:5 (physics.producers.makePH.Unsorted : true). The prolog default is false (prolog.fcl:116, "sim data are sorted, VST currently not"), so MC reconstruction is unaffected — this is VST data processing only.

Fixed by threading the original-index mapping into combine() and storing original-collection indices in both branches. Which hits get combined is unchanged; only the recorded provenance index changes.

2. 🟡 FilterHits abandons the event instead of dropping the hit

The four cuts at :209,217,223,229 are if(_filter)break; inside the per-event for (size_t ich...) loop, so one failed cut discards every later panel's hits rather than the hit that failed. continue is what the surrounding code means. Dormant — no committed fcl sets makePH.FilterHits:true.

3. 🟡 StrawHitReco: flagCrossTalk runs per digi, and null-dereferences when WriteStrawHitCollection is false

Two problems at the same call site (:217-219):

• It sits inside the per-digi loop, but flagCrossTalk rescans the entire collection (StrawHitRecoUtils.cc:44-62), so it runs N times per event instead of once.
• It dereferences shCol, which :190-193 only allocates when _writesh is true. FlagCrossTalk:true with WriteStrawHitCollection:false null-dereferences a unique_ptr.

Hoisted the call out of the loop, and rejected the impossible configuration in the constructor rather than silently skipping the flagging. Both are dormant: every committed config sets FlagCrossTalk : false.

4. 🟡 StereoLineTest: integer distribution over [-M_PI, M_PI]

src/StereoLineTest_main.cc:80 — std::uniform_int_distribution<int> phirange(-M_PI,M_PI). M_PI truncates to 3, so the stereo stress test only ever sampled 7 discrete orientations instead of a continuous angle. Changed to uniform_real_distribution<double>.

---

Validation

• Build: not run locally. Everything above is static. Please let buildtest be the arbiter.
• No art job was run, so finding 1 is a predicted failure, not an observed one. The cheapest confirmation is one VST job through vstdata_epilog.fcl, comparing a ComboHit's resolved parent hit against the straw it actually came from.
• Deliberately not included, and offered as follow-ups:
  • The ComboHit quality chi-square at :296, chisq = wacc2 - wacc/wwtsum, looks dimensionally inconsistent — _qual is frequently negative so TMath::Prob returns 0 for every multi-hit ComboHit. I did not touch it because the intended formula needs a physics sign-off, and the confirmed consumers are diagnostic.
  • Six defects in FlagBkgHits, TNTClusterer, Chi2Clusterer and DBSClusterer (unclamped writes into a fixed 256-bucket array; an unsigned underflow defeating an imin=0 clamp; a tbin_==0.0 divide-by-zero path; per-event bounds held in never-reset members; a dead cperr2_; std::prev on an empty vector). No committed configuration reaches any of it — PrepareHits ends in flagPH, which Production/JobConfig/reco/prolog.fcl:34 and mu2e-trig-config/core/producers/trigTrkProducers.fcl:70 both bind to CalPatRec DeltaFinder, with the FlagBkgHits alternative commented out; the trigger CI reference log confirms TrigFlagPH:DeltaFinder is what runs. Whether ~1300 lines of unreached code should be fixed, deleted or revived is a maintainer call, not one for this PR.
  • Four defects in the PeakFit family, which StrawHitRecoUtils.cc:29-30 makes unreachable (it throws unless FitType ∈ {1,2,5}; every committed config sets 1 or 5, never combopeakfit(3) or peakfit(4)).
• MakeStereoHits and ProtonBunchTimeFromStrawDigis produced no findings that survived adversarial verification. Read that as "nothing found", not "verified clean".

[FNALbuild]

Hi @oksuzian,
You have proposed changes to files in these packages:

• TrkHitReco

which require these tests: build.

@Mu2e/fnalbuild-users, @Mu2e/write have access to CI actions on main.

⌛ The following tests have been triggered for dcb0886: build (Build queue - API unavailable)

About FNALbuild. Code review on Mu2e/Offline.

[FNALbuild]

☀️ The build tests passed at dcb0886.

Test	Result	Details
test with	✅	Command did not list any other PRs to include
merge	✅	Merged dcb0886 at 0ef02a6
build (prof)	✅	Log file. Build time: 04 min 23 sec
ceSimReco	✅	Log file.
g4test_03MT	✅	Log file.
transportOnly	✅	Log file.
POT	✅	Log file.
g4study	✅	Log file.
cosmicSimReco	✅	Log file.
cosmicOffSpill	✅	Log file.
ceSteps	✅	Log file.
ceDigi	✅	Log file.
muDauSteps	✅	Log file.
ceMix	✅	Log file.
rootOverlaps	✅	Log file.
g4surfaceCheck	✅	Log file.
trigger	❓	Log file. Return Code 1.
check_cmake	✅	Log file.
FIXME, TODO	✅	TODO (0) FIXME (0) in 3 files
clang-tidy	➡️	4 errors 71 warnings
whitespace check	✅	no whitespace errors found

N.B. These results were obtained from a build of this Pull Request at dcb0886 after being merged into the base branch at 0ef02a6.

For more information, please check the job page here.
Build artifacts are deleted after 5 days. If this is not desired, select Keep this build forever on the job page.