fix: keep hierarchy paste-complete boundaries exact so grouping doesn't fail - #570
Open
FelipeDefensor wants to merge 3 commits into
Open
fix: keep hierarchy paste-complete boundaries exact so grouping doesn't fail#570FelipeDefensor wants to merge 3 commits into
FelipeDefensor wants to merge 3 commits into
Conversation
FelipeDefensor
commented
Aug 21, 2026
FelipeDefensor
commented
Aug 21, 2026
FelipeDefensor
commented
Aug 21, 2026
FelipeDefensor
commented
Aug 21, 2026
Paste-complete rescaled each copied component with arithmetic that differed between its start and its end: the start was taken relative to the source parent's start, the end relative to the source parent's end. A time appearing in two components -- as one's end and the next one's start -- therefore went through different operations and came out a few ulps apart. Nesting compounded this, since every level re-derived its scale factor from the already-rounded bounds of the level above. The resulting sub-ulp gaps are invisible but corrupt the stored geometry: get_parent, get_children and the grouping validators all compare boundaries exactly, so pasted siblings stop being recognised as adjacent. Grouping components derived from a pasted subtree then failed with 'Grouping component would overlap with existing component.' Build one affine map at the root of the paste and apply it to the absolute times of the whole subtree. The same time now maps to the same float everywhere it occurs, so coincident boundaries stay coincident. The map is mathematically identical to the old per-level nesting -- composing affine maps over nested intervals yields the root map -- but without the intermediate rounding. Endpoints are anchored exactly so the pasted subtree lines up flush with the target. Also drop the unused children_of_element accumulator and stop passing a failed creation into get_component_ui, which would raise on a None.
Hierarchy boundaries were compared with exact float operators throughout, so a neighbour whose end sat a few ulps inside a span counted as a real overlap. Grouping was refused with 'Grouping component would overlap with existing component.', get_parent silently failed to adopt a child whose parent started a few ulps later, and get_boundary_conflicts reported coincident boundaries as conflicting. Rescaling no longer produces such drift -- paste-complete now maps the whole subtree with a single affine map, and scaling for a media-duration change applies one factor uniformly -- but files saved before that fix already contain it. Compare with a 1 ns tolerance instead: far above the float noise (~1e-13) and far below any meaningful musical interval, so genuine overlaps are still detected. The tolerance is applied to every boundary comparison that runs over stored geometry -- the two grouping validators, get_parent, get_children and get_boundary_conflicts -- so the helpers cannot disagree about whether the same timeline is valid. Each comparison keeps its original strictness; only the equality test changes. Guard tests assert that a genuinely too-narrow parent is still rejected and that overlaps well past a boundary are still reported.
A child of the pasted subtree that could not be created was skipped with only a log line, so the user saw a silently incomplete paste. Surface it through the same COMPONENTS_PASTE_ERROR dialog the other paste-complete failures use. Pasting stays best-effort -- a failed child is skipped along with its own subtree and the remaining siblings are still pasted -- so the reasons are collected across the whole operation and reported once at the end rather than one dialog per failure.
FelipeDefensor
force-pushed
the
fix/hierarchy-paste-boundary-drift
branch
from
August 21, 2026 18:27
95cb1c9 to
7504d08
Compare
azfoo
requested changes
Aug 25, 2026
Comment on lines
+33
to
+45
| def _times_close(a: float, b: float) -> bool: | ||
| return isclose(a, b, abs_tol=_TIME_ABS_TOL) | ||
|
|
||
|
|
||
| def _le(a: float, b: float) -> bool: | ||
| """``a <= b``, treating near-coincident times as equal.""" | ||
| return a < b or _times_close(a, b) | ||
|
|
||
|
|
||
| def _lt(a: float, b: float) -> bool: | ||
| """``a < b``, treating near-coincident times as equal (i.e. not less).""" | ||
| return a < b and not _times_close(a, b) | ||
|
|
Collaborator
There was a problem hiding this comment.
is it worth converting each start/end into its own object with dedicated dunder methods? this seems to be a comparison that might be useful across all the different tls.
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
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.
Grouping components derived from a pasted hierarchy subtree failed with
Grouping failed: Grouping component would overlap with existing component.Closes #396.
Reproducing
Reported against a real file. In a hierarchy timeline:
Expected the halves to be grouped; got the overlap error instead.
Cause
_create_child_from_paste_datarescaled each copied component with arithmeticthat differed between its start and its end: the start was taken relative to the
source parent's start, the end relative to the source parent's end. A time
appearing in two components — as one's end and the next one's start — therefore
went through different operations and came out a few ulps apart. Nesting
compounded this, since every level re-derived its scale factor from the
already-rounded bounds of the level above.
In the reported file the pasted
Modulationcomponent starts at134.18723783376336while its left-hand neighbours end at134.1872378337634—4e-14 further right, so arithmetically inside the group span. Since the
grouping validators compare boundaries with exact float operators, that
sub-ulp gap read as a real overlap.
Fix
Three commits, one per concern:
fix: keep hierarchy paste-complete boundaries exact— build a single affinemap at the root of the paste and apply it to the absolute times of the whole
subtree, so the same time maps to the same float everywhere it occurs. This is
mathematically identical to the old per-level nesting (composing affine maps
over nested intervals yields the root map) but without the intermediate
rounding. Endpoints are anchored exactly so the pasted subtree lines up flush
with the target.
fix: tolerate float drift when comparing hierarchy boundaries— compareboundaries with a 1 ns tolerance instead of exact operators. Needed
independently of the above: files saved before this fix already contain the
drifted values, and exact comparison is fragile for any rescaling operation.
1 ns sits far above the float noise (~1e-13) and far below any meaningful
musical interval, so genuine overlaps are still rejected.
The tolerance is applied to every boundary comparison that runs over stored
geometry, not just the two grouping validators, so the helpers cannot disagree
about whether the same timeline is valid:
get_parent/get_children— a parent whose start sits a few ulps after itschild's start was not found, silently orphaning the child. Everything
depending on the parent link then misbehaves: delete cascade, level changes,
copy-with-children, and the
has_same_parentcheck inside grouping itself.get_boundary_conflicts— reported the very grouping unit the validators hadjust accepted as conflicting with its drifted neighbour.
Each comparison keeps its original strictness; only the equality test changes.
Checked
scale_segmentlike(media-duration changes) for the same class of bug —it applies
t * factoruniformly to starts and ends, so shared boundaries stayidentical. Paste-complete was the only offender.
fix: report paste-complete child creation failures to the user— a child ofthe pasted subtree that could not be created was skipped with only a log line,
so the user saw a silently incomplete paste. It now goes through the same
COMPONENTS_PASTE_ERRORdialog as the other paste-complete failures. Pastingstays best-effort — a failed child is skipped along with its own subtree and the
remaining siblings are still pasted — so the reasons are collected across the
whole operation and reported once at the end rather than one dialog per failure.
Also folded into the paste commit: the unused
children_of_elementaccumulatoris gone, and a failed
create_componentno longer flows intoget_component_ui(None).Tests
Each new test was confirmed to fail without the corresponding fix and pass with it.
paste-complete — for siblings, for grandchildren (where the old error
compounded), and for an identity rescale. Two of them fail on
devwith thevisible drift
3.8999999999999995vs3.9000000000000004.(paste-complete → create_child → decrease_level → split → group), which fails
on
devwith the exact reported error message.a few ulps inside the group span (must succeed), one where it sits well inside
(must still be rejected).
past its child's start, one whose end drifted before its child's end, and
coincident-but-drifted boundaries that must not be reported as conflicting.
parent is still not adopted, an overlap well past a boundary is still
reported, and a drifted duplicate on the same level is still a conflict.
failure's reason, and it stays silent when every child is created.
Every commit is green on its own. Full suite: 1844 passed, 13 skipped.
pre-commit(black + ruff) clean.