Skip to content

Fix trimmed ranges of deeply nested children - #2048

Merged
jminor merged 1 commit into
AcademySoftwareFoundation:mainfrom
jhuanxx44:codex/fix-2043-nested-trimmed-range
Sep 18, 2026
Merged

jminor merged 1 commit into
AcademySoftwareFoundation:mainfrom
jhuanxx44:codex/fix-2043-nested-trimmed-range

Conversation

@jhuanxx44

Copy link
Copy Markdown
Contributor

Link the Issue(s) this Pull Request is related to.

Fixes #2043

Summarize your change.

Composition::trimmed_range_of_child fails with NOT_A_CHILD_OF (exposed as NotAChildError in Python) when it traverses three or more parent compositions. The first iteration advances current, but subsequent iterations leave it pointing to an earlier descendant, so the next parent cannot find it among its direct children.

Advance current after accumulating each parent range, matching the traversal in range_of_child.

Reference associated tests.

  • C++ regression: a Stack → Track → Stack → Clip hierarchy with a nonzero track offset; checks both the error status and the returned range.
  • Python regression: one through four parent compositions, accumulated track offsets, and an outer source range that either clips the result or excludes the clip entirely.
  • Both regressions fail before the production change and pass after it.

Validated locally on macOS arm64 with AppleClang 21 and Python 3.14.6:

  • C++ static Debug build: all 10 CTest targets pass.
  • make test: 482 tests run, 2 existing skips.
  • make lint, make manifest, clang-format --dry-run --Werror on the changed C++ files, and git diff --check pass.

Assisted-by: Codex / GPT-6

@linux-foundation-easycla

linux-foundation-easycla Bot commented Sep 13, 2026

Copy link
Copy Markdown

CLA Signed
The committers listed above are authorized under a signed CLA.

  • ✅ login: jhuanxx44 / name: jinghuan (0c65eec)

@jhuanxx44
jhuanxx44 force-pushed the codex/fix-2043-nested-trimmed-range branch from d5ded16 to 0c65eec Compare September 13, 2026 17:35
@codecov-commenter

codecov-commenter commented Sep 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.52%. Comparing base (a639246) to head (65cc72d).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2048      +/-   ##
==========================================
+ Coverage   83.48%   83.52%   +0.04%     
==========================================
  Files         182      182              
  Lines       13517    13533      +16     
  Branches     1254     1255       +1     
==========================================
+ Hits        11284    11303      +19     
+ Misses       2060     2057       -3     
  Partials      173      173              
Flag Coverage Δ
py-unittests 83.52% <100.00%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/opentimelineio/composition.cpp 80.32% <100.00%> (+1.05%) ⬆️
tests/test_composition.py 99.72% <100.00%> (+<0.01%) ⬆️

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a639246...65cc72d. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Advance the current child at every parent traversal step, matching
range_of_child. Previously the third parent lookup used a stale
descendant and returned NOT_A_CHILD_OF.

Add C++ and Python regressions for nested compositions, accumulated
offsets, and outer source-range clipping.

Fixes AcademySoftwareFoundation#2043

Assisted-by: Codex / GPT-6
Signed-off-by: jinghuan <74035015+jhuanxx44@users.noreply.github.com>
@jhuanxx44
jhuanxx44 force-pushed the codex/fix-2043-nested-trimmed-range branch from 0c65eec to 65cc72d Compare September 14, 2026 03:59
@jminor
jminor requested a review from ssteinbach September 14, 2026 14:45
@jminor

jminor commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

Hi @jhuanxx44 - thanks, this PR is looking pretty good. We'll arrange for an OTIO committer to review it. Out of curiosity, how did you come across this issue, and what lead you to contribute to OTIO?

@jhuanxx44

Copy link
Copy Markdown
Contributor Author

Hi @jminor, thanks for taking a look!

I’m exploring a video editing project of my own and will need a timeline model, which is how I came across OTIO. Around the same time, I saw #2043, where you were looking for someone to verify a bug identified by an LLM-based review, and it seemed like something I could help with.

To be upfront, I used Codex to help investigate and implement the change, hence the Assisted-by: trailer in the PR. I reviewed the diff myself and ran the full local test and lint suite; the new tests failed before the fix and passed afterward.

I’m happy to answer any questions from the committer and follow whatever conventions OTIO prefers for AI-assisted contributions.

@jminor
jminor removed the request for review from ssteinbach September 16, 2026 20:08
@jminor

jminor commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

We're actually right in the midst of establishing our AI policy, and this PR matches very closely to what we're aiming for. Your timing is perfect. See details here: #2046

@jminor
jminor merged commit 696150b into AcademySoftwareFoundation:main Sep 18, 2026
51 checks passed
@jhuanxx44
jhuanxx44 deleted the codex/fix-2043-nested-trimmed-range branch September 18, 2026 07:19
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.

trimmed_range_of_child broken for ≥3-level nesting

3 participants