Skip to content

fix: keep hierarchy paste-complete boundaries exact so grouping doesn't fail - #570

Open
FelipeDefensor wants to merge 3 commits into
devfrom
fix/hierarchy-paste-boundary-drift
Open

fix: keep hierarchy paste-complete boundaries exact so grouping doesn't fail#570
FelipeDefensor wants to merge 3 commits into
devfrom
fix/hierarchy-paste-boundary-drift

Conversation

@FelipeDefensor

@FelipeDefensor FelipeDefensor commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

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:

  1. Copy the top-level hierarchy of one timeline.
  2. Paste it into a single hierarchy of another with Paste complete.
  3. Create a child in one of the pasted components.
  4. Decrease that child's level.
  5. Split the child in two.
  6. Try to group the two halves.

Expected the halves to be grouped; got the overlap error instead.

Cause

_create_child_from_paste_data 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.

In the reported file the pasted Modulation component starts at
134.18723783376336 while its left-hand neighbours end at 134.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 affine
map 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 — compare
boundaries 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 its
    child'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_parent check inside grouping itself.
  • get_boundary_conflicts — reported the very grouping unit the validators had
    just 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 * factor uniformly to starts and ends, so shared boundaries stay
identical. Paste-complete was the only offender.

fix: report paste-complete child creation failures to the user — 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. It now goes through the same
COMPONENTS_PASTE_ERROR dialog as the other paste-complete failures. 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.

Also folded into the paste commit: the unused children_of_element accumulator
is gone, and a failed create_component no longer flows into
get_component_ui(None).

Tests

Each new test was confirmed to fail without the corresponding fix and pass with it.

  • Three tests asserting exact float equality of shared boundaries after
    paste-complete — for siblings, for grandchildren (where the old error
    compounded), and for an identity rescale. Two of them fail on dev with the
    visible drift 3.8999999999999995 vs 3.9000000000000004.
  • An end-to-end test walking the whole reported flow through commands
    (paste-complete → create_child → decrease_level → split → group), which fails
    on dev with the exact reported error message.
  • Two backend tests for the grouping tolerance: one where a neighbour's end sits
    a few ulps inside the group span (must succeed), one where it sits well inside
    (must still be rejected).
  • Three more for genealogy and conflict detection: a parent whose start drifted
    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.
  • Guard tests against the tolerance being too coarse: a genuinely too-narrow
    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.
  • Two tests for the paste-complete failure dialog: it fires with the creation
    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.

Comment thread tests/ui/timelines/hierarchy/test_hierarchy_timeline_ui.py Outdated
Comment thread tests/ui/timelines/hierarchy/test_hierarchy_timeline_ui.py Outdated
Comment thread tilia/ui/timelines/hierarchy/timeline.py Outdated
Comment thread tilia/timelines/hierarchy/timeline.py Outdated
@FelipeDefensor FelipeDefensor linked an issue Aug 21, 2026 that may be closed by this pull request
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
FelipeDefensor force-pushed the fix/hierarchy-paste-boundary-drift branch from 95cb1c9 to 7504d08 Compare August 21, 2026 18:27
@FelipeDefensor
FelipeDefensor requested a review from azfoo August 21, 2026 18:32
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)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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.

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.

Valid hierarchy unit grouping fails

2 participants