I have enough context. Cross-checks with the LXR index confirmed how parallelworld_volume is handled in DD4hep, how Geant4TVUserParticleHandler consumes it, and the established upstream pattern. The CI logs nail down what the two failing jobs are actually catching. Composing the review now.

Review: eic/epic#1081 — "Implement a tracking region volume to support Geant4TVUserParticleHandler"

Verdict: Request changes. Two check-overlap-tgeo jobs fail with 995 / 339 overlaps of tracking_volume_0 against real detector volumes (HcalEndcapPInsert, B0ECal, BeamPipeB0), introduced by shipping connected="true" on the new parallelworld volume. The failure is a direct consequence of the geometry design choice in this PR, not pre-existing. Merging in this state turns the tgeo overlap gate red on main. There's also a small-but-real geometry offset in the forward extrusion placement worth tightening before merge.

Scope

3 files, +54 / −11. Author: @veprbl. Base: main. Paired with eic/npsim#44 (switches ddsim default to Geant4TVUserParticleHandler). Addresses eic/npsim#43.

• compact/definitions.xml — removes the three legacy constants tracker_region_rmax, tracker_region_zmax, tracker_region_zmin and their block comment. (definitions.xml#L733-735)
• compact/tracking_region.xml — new file; re-defines the three legacy constants (still needed by Geant4TCUserParticleHandler via DDG4/python/DDSim/Helper/ParticleHandler.py), adds three new constants (tracker_region_forward_extrusion_{rmax,zmax,rot_y}), and declares a parallelworld_volume name="tracking_volume" whose shape is a Union of the legacy tracker cylinder and a narrow 38 m tube tilted by ionCrossingAngle to cover the ion-forward beamline.
• templates/epic.xml.jinja2 — pulls the new file in.

The volume name tracking_volume is meaningful, not cosmetic: Compact2Objects.cpp:1602-1603 special-cases exactly that string to call description.setTrackingVolume(vol), which is how Geant4TVUserParticleHandler::Geant4TVUserParticleHandler retrieves it (plugin cpp:98). So the naming is correct.

Findings

1. [correctness / ops — blocker] connected="true" makes this volume a real placement in the world, which is why overlap checks fail — and the handler does not require it

[cross-repo LXR lookup]

The tgeo overlap jobs are producing failures like

= Overlap ov00975: world_volume/tracking_volume_0 overlapping world_volume/HcalEndcapPInsert_28/HcalEndcapPInsert_layer5_L_65 ovlp=0.379911
... 995 overlaps found!

(craterlake job 72233745802) and

= Overlap ov00319: world_volume/tracking_volume_0 overlapping world_volume/BeamPipeB0_assembly_21/v_vacuum_1_11 ovlp=2.16238
... 339 overlaps found!

(ip6_extended job 72233745782).

The mechanism, traced in DD4hep's parallelworld converter:

// Compact2Objects.cpp:1587
Volume par = conn ? description.worldVolume() : description.parallelWorldVolume();
...
// line 1593
vol.setFlagBit(Volume::VETO_SIMU);

(Compact2Objects.cpp#1558-1607).

With connected="true" the volume is placed inside the real worldVolume, so the tgeo overlap checker legitimately reports it against every real detector that the tracking region is designed to enclose. VETO_SIMU stops Geant4 from transporting through it (which is why check-overlap-geant4 and npsim-* still pass), but it does nothing for tgeo.

And the handler doesn't need connected="true": Geant4TVUserParticleHandler::Geant4TVUserParticleHandler reads ctxt->kernel().detectorDescription().trackingVolume() (handler#L98) and uses m_trackingVolume.ptr()->Contains(point) (handler#L107-109). description.setTrackingVolume(vol) is called regardless of conn (Compact2Objects.cpp#1602-1603). So connected="false" is sufficient for the feature.

The file itself acknowledges this:

<!-- compact/tracking_region.xml (new) -->
Set `connected="true"` to make it visible in a geometry display

…then sets connected="true" unconditionally. That is inconsistent with the comment and with the CI contract. The DD4hep example you patterned after (DD4hep/examples/ClientTests/compact/TrackingRegion.xml line 62) does ship connected="true" — but it's a toy test without a real calorimeter, so the overlap wasn't a concern there; that pattern does not transfer to epic.

Fix: default to connected="false". If a geometry-display-friendly variant is desired, introduce a separate debug compact (or a jinja toggle) rather than making every user's overlap check fail. Alternatively, if there is a strong reason to keep it connected (none is stated in the PR description), the tgeo overlap action needs an exclusion for volume names matching tracking_volume* — but that's the worse path, because it silences a real class of bug.

2. [correctness — minor] Forward-extrusion axis is offset from the IP along z

[cross-repo LXR lookup: confirmed via parallelworld_volume parser and pattern in DD4hep's TrackingRegion.xml]

The BooleanShape is placed at

<position x="0" y="0" z="(tracker_region_zmax + tracker_region_zmin) / 2" />

so its local origin is at world z = (zmax + zmin)/2. With tracker_region_zmax = +EcalEndcapP_zmin and tracker_region_zmin = -EcalEndcapN_zmin, this midpoint is not at the IP — the EM endcaps are not at symmetric |z|.

The forward-extrusion tube is placed inside the BooleanShape at

<position x="sin(ionCrossingAngle)*tracker_region_forward_extrusion_zmax/2"
          y="0"
          z="cos(ionCrossingAngle)*tracker_region_forward_extrusion_zmax/2" />
<rotation x="0" y="ionCrossingAngle" z="0" />

with half-length 19 m and rmax = 15 cm. That places the near end of the extrusion axis at the local origin — i.e., at world (0, 0, (zmax+zmin)/2), not at the IP (0, 0, 0). Consequently the extrusion axis in world is the ion-beam direction translated by ~(zmax+zmin)/2 along z. At a 25 mrad crossing angle and a ~0.1–0.3 m midpoint offset (depending on endcap asymmetry), the extrusion axis misses the nominal ion beam line by a few mm at the IP. That's well inside the 15 cm tube, so it "works", but it's a silent physical inaccuracy that will bite later if someone tightens the radius or uses the volume in any geometry-based acceptance reasoning.

Fix: place the forward-extrusion tube so its axis passes through the IP regardless of endcap asymmetry. Either move the Union shape's outer <position> to z = 0 and give the inner legacy tube a z-offset of (zmax+zmin)/2 (the legacy one is the asymmetric one, not the beamline tube), or add a compensating -z offset inside the extrusion placement.

3. [style] Duplicated "These are used by ddsim …" comment, and one of them is now wrong

The top-of-file comment

<comment>
  These are used by ddsim, the region where we store all secondaries
  For now they are forced to be symmetric, and we cover up to the start of the backward ECAL.
  ...
</comment>

is lifted verbatim from the pre-existing block in definitions.xml. Inside <define> the same idea is repeated, attributed to Geant4TCUserParticleHandler, then another <comment> explains the extrusion, then another explains the volume for Geant4TVUserParticleHandler. The outer-most comment no longer describes what this file is for — the file's headline purpose is the new tracking_volume for TV, not the legacy TC constants. Reorder so the first comment states the file's purpose in the post-#44 world and keep the "symmetric, up to the backward ECAL" paragraph adjacent to the three legacy constants where it applies.

Also: the "forced to be symmetric" remark is inherited language that was already slightly misleading (the values are EcalEndcapP_zmin vs −EcalEndcapN_zmin, i.e., only symmetric if the two endcaps are at equal |z|). This is a good opportunity to drop that sentence.

Process

• Fetched PR metadata, diff, checks, inline review comments, and conversation via the gh CLI (no GitHub MCP tools are exposed in this environment).
• Pulled failing-job logs for the two check-overlap-tgeo jobs to identify the specific overlap source (tracking_volume_0 vs HcalEndcapPInsert, B0ECal, BeamPipeB0) and rule out a flake.
• Read the companion eic/npsim#44 ("feat: use Geant4TVUserParticleHandler by default") and eic/npsim#43 to confirm how this PR is consumed downstream.
• Used LXR (lxr_search, lxr_source, lxr_ident) to:
• confirm who reads the three legacy constants (DD4hep/DDG4/python/DDSim/Helper/ParticleHandler.py#L132-151) — still needed, so re-defining them in the new file is correct;
• locate the parallelworld_volume XML parser and decode the connected attribute's semantics (DD4hep/DDCore/src/plugins/Compact2Objects.cpp:1558-1607);
• read the handler plugin itself (DD4hep/DDG4/plugins/Geant4TVUserParticleHandler.cpp) to establish that description.trackingVolume() is all it needs — connected is not a functional dependency;
• compare against the upstream pattern (DD4hep/examples/ClientTests/compact/TrackingRegion.xml) and confirm the attribute set matches, but the context differs (no real calorimeter to overlap with in the example).
• No subagents used.