feat: allow multiple objects on a layer to get lookahead from a single piece SOFIE-464#1776
Merged
Julusian merged 1 commit intoJun 29, 2026
Conversation
Julusian
added
the
Contribution from BBC
Walkthrough
ChangesMulti-object-per-layer lookahead
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/job-worker/src/playout/lookahead/findObjects.ts (1)
164-169:⚠️ Potential issue | 🟠 Major | ⚡ Quick winTransition filtering is over-broad for multi-object pieces.
At Line 168, filtering uses
piece.piece.enable.start === 0, so once a transition object exists on this layer, all objects from that piece are dropped—even objects that start later. This can incorrectly suppress non-overlapping lookahead objects from a multi-object piece.Proposed fix (filter by object timing, not parent-piece timing)
if ( hasTransitionObj && - obj.pieceInstanceId && piece.piece.pieceType === IBlueprintPieceType.Normal && - piece.piece.enable.start === 0 + !Array.isArray(obj.enable) && + obj.enable.start === 0 ) { return }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/job-worker/src/playout/lookahead/findObjects.ts` around lines 164 - 169, The transition object filtering at line 168 in the condition block is overly broad because it checks piece.piece.enable.start === 0 (the parent piece's start time) rather than the individual object's start time. This causes all objects from that piece to be dropped if the piece enables at 0, even if individual objects start later and don't actually overlap with the transition. Replace the check of piece.piece.enable.start === 0 with a check of the individual object's enable.start property (such as obj.enable.start) so that only objects whose timing actually conflicts with the transition are filtered out.
🧹 Nitpick comments (1)
packages/job-worker/src/playout/lookahead/__tests__/findObjects.test.ts (1)
1073-1415: ⚡ Quick winAdd one regression case for temporary-piece ID matching.
The implementation now matches by
getBestPieceInstanceId(seefindObjects.tsLine 160), but this new suite only exercises non-temporary pieces. Please add a case withisTemporary: trueand nopartInstanceIdto verify objects still map to the correct piece and aren’t dropped.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/job-worker/src/playout/lookahead/__tests__/findObjects.test.ts` around lines 1073 - 1415, Add a new test case within the 'single piece with multiple objects on the same layer' describe block to verify that temporary pieces are correctly handled during ID matching. Create a test that sets `isTemporary: true` on a piece instance and omits or sets `partInstanceId` to undefined/null, then call `findLookaheadObjectsForPart` and verify that all objects from the temporary piece are correctly returned and mapped to the correct piece using `getBestPieceInstanceId` logic. This ensures the new ID matching implementation properly handles temporary pieces without dropping objects.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/job-worker/src/playout/lookahead/findObjects.ts`:
- Around line 164-169: The transition object filtering at line 168 in the
condition block is overly broad because it checks piece.piece.enable.start === 0
(the parent piece's start time) rather than the individual object's start time.
This causes all objects from that piece to be dropped if the piece enables at 0,
even if individual objects start later and don't actually overlap with the
transition. Replace the check of piece.piece.enable.start === 0 with a check of
the individual object's enable.start property (such as obj.enable.start) so that
only objects whose timing actually conflicts with the transition are filtered
out.
---
Nitpick comments:
In `@packages/job-worker/src/playout/lookahead/__tests__/findObjects.test.ts`:
- Around line 1073-1415: Add a new test case within the 'single piece with
multiple objects on the same layer' describe block to verify that temporary
pieces are correctly handled during ID matching. Create a test that sets
`isTemporary: true` on a piece instance and omits or sets `partInstanceId` to
undefined/null, then call `findLookaheadObjectsForPart` and verify that all
objects from the temporary piece are correctly returned and mapped to the
correct piece using `getBestPieceInstanceId` logic. This ensures the new ID
matching implementation properly handles temporary pieces without dropping
objects.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3001734b-823f-46e3-aa16-732709667c3e
📒 Files selected for processing (5)
packages/job-worker/src/playout/lookahead/__tests__/findObjects.test.tspackages/job-worker/src/playout/lookahead/__tests__/lookaheadOffset/constants.tspackages/job-worker/src/playout/lookahead/__tests__/lookaheadOffset/lookaheadOffset.test.tspackages/job-worker/src/playout/lookahead/findObjects.tspackages/job-worker/src/playout/lookahead/util.ts
nytamin
approved these changes
Jun 24, 2026
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.
About the Contributor
This pull request is posted on behalf of the BBC
Type of Contribution
This is a: Bug fix / Feature
Current Behavior
A piece is only able to produce one lookahead object on any layer. This is a problem if the piece needs to claim 2 sessions from an AB pool.
New Behavior
This removes the limit of producing one lookahead object per layer per piece. This was an artificial limitation, and I do not remember why it was added.
In our testing this has been fine, but I am not 100% confident that this wont cause negative side effects of producing too many objects in other circumstances. A potential side effect of this could be some pieces claiming too many AB sessions, or other timeline oddities. Others should keep an eye out for any side effects from this change.
Testing
Affected areas
Time Frame
Other Information
Status