Skip to content

fix(loaders): handle missing tracker hits and fallback to track state - #1029

Open
lavjeetrai wants to merge 1 commit into
HSF:mainfrom
lavjeetrai:fix/edm4hep-track-fallback
Open

lavjeetrai wants to merge 1 commit into
HSF:mainfrom
lavjeetrai:fix/edm4hep-track-fallback

Conversation

@lavjeetrai

Copy link
Copy Markdown
Contributor

Description

Fixes an issue where missing trackerHits collections or out-of-bound hit indices cause unhandled TypeError exceptions during EDM4hep JSON event loading. Additionally, enables graceful fallback to trackStates when tracker hit coordinates cannot be resolved.

Changes Made

  • Null Safety: Guarded trackerHits and trackStates array access and sub-properties (hit?.position, trackState?.referencePoint).
  • Dynamic Fallback: Decoupled trackerHits and trackStates branches so tracks with unresolvable hit references fall back to trackStates (if (pos.length === 0 && ...)).
  • Refactoring: Cached indexed hit lookup to avoid duplicate collection scans and redundant array indexing.
  • Unit Tests: Added full test suite in packages/phoenix-event-display/src/tests/loaders/edm4hep-json-loader.test.ts.

Test Plan

  • Tested with valid trackerHits references.
  • Tested fallback when collectionID is missing from rawEvent.
  • Tested fallback when hit index is out of bounds.
  • Tested track loading with only trackStates.
  • Tested empty track edge cases without throwing errors.

@github-actions

github-actions Bot commented Sep 13, 2026

Copy link
Copy Markdown

🚀 Preview deployed: http://phoenix-pr-1029.surge.sh

Built from a4a1c4f.

@lavjeetrai

Copy link
Copy Markdown
Contributor Author

@EdwardMoyse could u take a look to this.

@@ -210,27 +210,33 @@ export class Edm4hepJsonLoader extends PhoenixLoader {
trackCollection.forEach((rawTrack: edm4hep.Track) => {
const pos: number[][] = []; // An array of positions is needed to render the tracks as bars

// @todo trackerhits might always exist
if ('trackerHits' in rawTrack && rawTrack.trackerHits.length > 0) {
if (rawTrack.trackerHits?.length > 0) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this less protection than before? Do we know that trackerHits are guaranteed to exist in rawTrack? What is the harm in checking?

@lavjeetrai

Copy link
Copy Markdown
Contributor Author

@EdwardMoyse please take a look again

This branch was successfully deployed

1 active deployment
pull-request a4a1c4f7 Deployed Sep 16, 2026 by github-actions[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants