chore: Harden track state component allocation - #5801
Merged
kodiakhq[bot] merged 4 commits intoAug 3, 2026
Merged
Conversation
Three inconsistencies around `addTrackStateComponents`: - `MutableMultiTrajectoryBackend` did not require it, even though every fitter now calls `TrackStateProxy::addComponents()`. A third-party backend satisfied the concept and only failed at the first instantiation. - `VectorMultiTrajectory` decided whether a component was present from the stored `allocMask`, while the Podio backend uses the component index. `shareFrom_impl` assigns an index without touching `allocMask` and `unset_impl` clears an index without touching it either, so the two can disagree: sharing a component and then adding it replaced the shared index, and unsetting a component and then adding it allocated nothing. Decide from the index like Podio does, and keep `allocMask` for the storage this track state actually owns, which is what `statistics()` reports. - `AnyTrackStateProxy` had the handler entry but never surfaced a public `addComponents()`. Covered by a new shared backend test exercising both sequences.
github-actions
Bot
added
Component - Core
Contributor
andiwand
commented
Aug 3, 2026
Contributor
Public API surface diff+1 added, 0 breaking. ➕ Added public APINew call signatures (incl. defaulted-arg overloads) (1)
|
benjaminhuth
approved these changes
Aug 3, 2026
benjaminhuth
left a comment
benjaminhuth
left a comment
Member
There was a problem hiding this comment.
Looks good, I have only a few minor question out of curiosity
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three inconsistencies around
addTrackStateComponents:MutableMultiTrajectoryBackenddid not require it, even though every fitternow calls
TrackStateProxy::addComponents(). A third-party backend satisfiedthe concept and only failed at the first instantiation.
VectorMultiTrajectorydecided whether a component was present from thestored
allocMask, while the Podio backend uses the component index.shareFrom_implassigns an index without touchingallocMaskandunset_implclears an index without touching it either, so the two candisagree: sharing a component and then adding it replaced the shared index,
and unsetting a component and then adding it allocated nothing. Decide from
the index like Podio does, and keep
allocMaskfor the storage this trackstate actually owns, which is what
statistics()reports.AnyTrackStateProxyhad the handler entry but never surfaced a publicaddComponents().Covered by a new shared backend test exercising both sequences.
--- END COMMIT MESSAGE ---
Supersedes #5786, which was accidentally opened from a branch in this
repository instead of my fork. Unlike the previous version this is no longer
stacked on #5784 and #5785 — it touches a disjoint set of files and can be
reviewed on its own.
🤖 Generated with Claude Code