[codex] Add merged PR issue bookkeeping#6
Merged
Conversation
|
Warning Review limit reached
More reviews will be available in 53 minutes and 43 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughAdds a ChangesMerged PR Issue Bookkeeping
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 42ffda6530
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In @.github/workflows/merged-pr-bookkeeping.yml:
- Around line 216-220: The multiline string assigned to the `body` property in
the GitHub Actions workflow is breaking YAML syntax because of unescaped
backticks and template literal syntax within the `script: |` block. Fix this by
properly escaping the backticks in the template literal string (the ones around
main, ${milestoneTitle}, and ${pendingReleaseLabel.name}), or wrap the entire
body string assignment in quotes to ensure the YAML parser treats it as a valid
literal string while preserving the JavaScript template literal functionality.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: efac137f-4db3-4f83-8efb-e82b205dca35
📒 Files selected for processing (3)
.github/pull_request_template.md.github/workflows/merged-pr-bookkeeping.ymlCONTRIBUTING.md
EtienneLescot
added a commit
that referenced
this pull request
Jul 1, 2026
Bugs from the spine test pass: #4 - clips display as 60s instead of the real video length. Root cause: insertClipAt falls back to PLACEHOLDER_DURATION_SEC (60s) when asset.durationSec is undefined, and the loadedmetadata auto-correct (handleLoadedMetadata) didn't fire reliably (in the browser-shim the preview path is unreliable; in the real Electron window the preview <video> remounts only when the activeSource swaps, so a freshly-added asset's metadata never reaches the auto-correct). Fix: probe the file directly via a throwaway <video> (src/lib/ai-edition/timeline/duration.ts, 5 unit tests) and insert the clip at the real duration synchronously. Persist the probed duration back onto the asset so subsequent inserts don't re-probe. Falls back to PLACEHOLDER on probe failure. #17 - dragging a skip's left/right chevron appeared to move the whole skip instead of just one edge. Root cause: clipSegments() was called once with the source skipRanges, then the cut segment was patched with dragPreview.startSec/endSec - but the surrounding keep segments weren't regenerated, so their flex widths didn't shrink to make room and the cut's visual position drifted. Fix: build a virtual skipRanges with the dragPreview'd bounds and re-run clipSegments() end-to-end so keep+cut stay consistent. #18 - clicking the trash button on a skip didn't remove it. Root cause: startClipReorder calls setPointerCapture on the clip block, so the subsequent pointerup is captured on the block (not the trash button) and the click event fires on the block (not the button). Fix: stop pointerdown propagation on the trash button so the clip block never captures the pointer. #5/#19 - clips were overlapping by 1px (the join-border overlap from T09). User: this is NOT wanted for clips - only for skip strips inside a clip. Fix: remove the JS left/width shift on hasJoinedPrev. CSS .joinedPrev/.joinedNext still zero the adjacent border-radius + border-width so corners blend. #6 - the reorder marker wasn't visible. Root cause: z-index 11, but the moving clip has z-index 20, and the marker's soft box-shadow was only 8px so it disappeared against dark backgrounds. Fix: bump width to 4px, z-index to 30 (above the moving clip), and stronger box-shadow. Verified: tsc clean, biome clean, 412 tests pass.
EtienneLescot
added a commit
that referenced
this pull request
Jul 1, 2026
…he correct boundary Two follow-up bugs from the spine test pass: #5/#19 — clips were STILL merging visually when adjacent. I had already removed the JS left/width shift on hasJoinedPrev in f9d62e1, but the .joinedPrev / .joinedNext CSS classes were still in the rendered classList, and the CSS for them was still zeroing the border-radius + border-width on the joined side. User feedback (round 2): there is NO reason to merge clips visually, ever — clips keep their own border + border-radius on every side, even when adjacent (and even more so when there are gaps between them, which the user pointed out can happen). Dropped the hasJoinedPrev / hasJoinedNext calculations, the .joinedPrev / .joinedNext classList additions, the CLIP_JOIN_THRESHOLD_PX constant, and the CSS rules. Only the keep-vs-cut segment rendering (which IS the right place to overlap freely) keeps its proportional layout. #6 — the reorder marker was at the wrong boundary. My port kept axcut's exact formula: boundarySec = remainingClips[insertIndex]. timelineStartSec. In axcut that's correct because clips can have gaps, so the moving clip's new START = the next non-moving clip's original START. In our model clips are resequenced to be contiguous, so the moving clip's new START = the END of the preceding clip (not the START of the next). With clips [A(0-10), B(15-25), C(30-40)] and B moving to slot 1, axcut puts the marker at 30 (C.start); ours should put it at 10 (A.end == new B.start). Fix: use remainingClips[insertIndex-1].timelineEndSec as the boundary. Also simplifies — insertIndex >= remaining.length no longer needs a special-case since remaining.length-1's timelineEndSec equals virtualDurationSec. Verified: tsc clean, biome clean, 412 tests pass.
EtienneLescot
added a commit
that referenced
this pull request
Jul 2, 2026
Bugs from the spine test pass: #4 - clips display as 60s instead of the real video length. Root cause: insertClipAt falls back to PLACEHOLDER_DURATION_SEC (60s) when asset.durationSec is undefined, and the loadedmetadata auto-correct (handleLoadedMetadata) didn't fire reliably (in the browser-shim the preview path is unreliable; in the real Electron window the preview <video> remounts only when the activeSource swaps, so a freshly-added asset's metadata never reaches the auto-correct). Fix: probe the file directly via a throwaway <video> (src/lib/ai-edition/timeline/duration.ts, 5 unit tests) and insert the clip at the real duration synchronously. Persist the probed duration back onto the asset so subsequent inserts don't re-probe. Falls back to PLACEHOLDER on probe failure. #17 - dragging a skip's left/right chevron appeared to move the whole skip instead of just one edge. Root cause: clipSegments() was called once with the source skipRanges, then the cut segment was patched with dragPreview.startSec/endSec - but the surrounding keep segments weren't regenerated, so their flex widths didn't shrink to make room and the cut's visual position drifted. Fix: build a virtual skipRanges with the dragPreview'd bounds and re-run clipSegments() end-to-end so keep+cut stay consistent. #18 - clicking the trash button on a skip didn't remove it. Root cause: startClipReorder calls setPointerCapture on the clip block, so the subsequent pointerup is captured on the block (not the trash button) and the click event fires on the block (not the button). Fix: stop pointerdown propagation on the trash button so the clip block never captures the pointer. #5/#19 - clips were overlapping by 1px (the join-border overlap from T09). User: this is NOT wanted for clips - only for skip strips inside a clip. Fix: remove the JS left/width shift on hasJoinedPrev. CSS .joinedPrev/.joinedNext still zero the adjacent border-radius + border-width so corners blend. #6 - the reorder marker wasn't visible. Root cause: z-index 11, but the moving clip has z-index 20, and the marker's soft box-shadow was only 8px so it disappeared against dark backgrounds. Fix: bump width to 4px, z-index to 30 (above the moving clip), and stronger box-shadow. Verified: tsc clean, biome clean, 412 tests pass.
EtienneLescot
added a commit
that referenced
this pull request
Jul 2, 2026
…he correct boundary Two follow-up bugs from the spine test pass: #5/#19 — clips were STILL merging visually when adjacent. I had already removed the JS left/width shift on hasJoinedPrev in f9d62e1, but the .joinedPrev / .joinedNext CSS classes were still in the rendered classList, and the CSS for them was still zeroing the border-radius + border-width on the joined side. User feedback (round 2): there is NO reason to merge clips visually, ever — clips keep their own border + border-radius on every side, even when adjacent (and even more so when there are gaps between them, which the user pointed out can happen). Dropped the hasJoinedPrev / hasJoinedNext calculations, the .joinedPrev / .joinedNext classList additions, the CLIP_JOIN_THRESHOLD_PX constant, and the CSS rules. Only the keep-vs-cut segment rendering (which IS the right place to overlap freely) keeps its proportional layout. #6 — the reorder marker was at the wrong boundary. My port kept axcut's exact formula: boundarySec = remainingClips[insertIndex]. timelineStartSec. In axcut that's correct because clips can have gaps, so the moving clip's new START = the next non-moving clip's original START. In our model clips are resequenced to be contiguous, so the moving clip's new START = the END of the preceding clip (not the START of the next). With clips [A(0-10), B(15-25), C(30-40)] and B moving to slot 1, axcut puts the marker at 30 (C.start); ours should put it at 10 (A.end == new B.start). Fix: use remainingClips[insertIndex-1].timelineEndSec as the boundary. Also simplifies — insertIndex >= remaining.length no longer needs a special-case since remaining.length-1's timelineEndSec equals virtualDurationSec. Verified: tsc clean, biome clean, 412 tests pass.
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
1 participant
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.
Summary
Adds a GitHub Actions workflow that updates linked issues after a PR is merged into
main.What changed
closingIssuesReferences.status: fixed in mainandstatus: pending release.Next Release.Why
Closed issues can otherwise look released even when the desktop installer has not shipped yet. This keeps the issue lifecycle accurate for desktop users while still allowing issues to close when the fix lands in
main.Validation
git diff --cached --check.actions/github-scriptJavaScript.No issue linked; this is repository workflow policy work.
Summary by CodeRabbit
Release Notes
Documentation
Chores