feat(node-anim): sub-slice C3 — undo commands - #585
Conversation
Second sub-slice of #520. Adds QUndoCommand subclasses for the three NodeAnimationManager mutators so Ctrl+Z reverses authoring operations. Mirrors the morph A3 command shape: snapshot the prior state at construction, redo applies, undo restores from the snapshot. ## What ships ### `commands/NodeAnimCommands.{h,cpp}` - **`CreateNodeAnimClipCommand(name, length)`** — redo calls the manager's `createClip`, undo calls `deleteClip`. Trivial because the snapshot is just `(name, length)`. - **`DeleteNodeAnimClipCommand(name)`** — snapshots **every track + every keyframe** at construction (TRS values, associated node name, time). Redo drops the clip; undo rebuilds it through the manager so the per-clip handle allocator stays authoritative. - **`SetNodeKeyframeCommand(clip, node, time, T, R, S)`** — snapshots three cases at construction: 1. No track / no clip → undo deletes the track that redo will create (mTrackCreatedByRedo flag detects this). 2. Track exists, no key near `time` → undo removes the lone keyframe redo added (track stays — Ogre's `destroyNodeTrack` only fires when the track was created by this command's redo). 3. Key exists near `time` (within 1ms manager merge epsilon) → undo restores the prior TRS values in place. Uses the same `kKeyframeMergeEpsilon = 1e-3` as the manager so command and manager agree on which keyframe is "the same one." ## Tests (4 new) - `CreateClipCommandRoundTrips` — create / undo / redo cycle. - `DeleteClipCommandSnapshotsAndRestores` — two tracks + three keys preserved across undo. Verifies the deepest snapshot path. - `SetKeyframeCommandRoundTripsForFreshKey` — track-created-by- redo flag works (undo leaves clip in pre-redo state with no empty stub track). - `SetKeyframeCommandRestoresPriorOnOverwrite` — in-place value restoration when redo overwrote an existing key within epsilon. ## What's still deferred - C2 — Inspector "Node Animation" subgroup (the buttons that will push these commands). - C4 — dope-sheet integration. - C5 — glTF + FBX exporter round-trip. - C6 — CLI + MCP surface. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR introduces three Qt undo/redo command classes for node animation editing: ChangesNode Animation Undo/Redo Command System
Sequence DiagramsequenceDiagram
participant Client
participant SetNodeKeyframeCommand
participant NodeAnimationManager
participant Ogre
Client->>SetNodeKeyframeCommand: constructor(clipName, nodeName, time, TRS)
SetNodeKeyframeCommand->>Ogre: snapshot prior keyframe near time
SetNodeKeyframeCommand->>SetNodeKeyframeCommand: store mNewKeyframe and mPriorKeyframe
Client->>SetNodeKeyframeCommand: redo()
SetNodeKeyframeCommand->>NodeAnimationManager: setNodeKeyframe(clipName, nodeName, time, TRS)
NodeAnimationManager->>Ogre: add keyframe to node track
SetNodeKeyframeCommand->>Ogre: detect track creation by list size check
Client->>SetNodeKeyframeCommand: undo()
alt prior keyframe exists
SetNodeKeyframeCommand->>Ogre: restore translate/rotation/scale in-place
else prior keyframe missing
SetNodeKeyframeCommand->>Ogre: remove newly added keyframe by time scan
SetNodeKeyframeCommand->>Ogre: destroy node track if created and empty
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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: 08afbf3d91
ℹ️ 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".
| if (mTrackCreatedByRedo && track->getNumKeyFrames() == 0) { | ||
| anim->destroyNodeTrack(handle); |
There was a problem hiding this comment.
When undo removes a track with anim->destroyNodeTrack(handle), it bypasses NodeAnimationManager's m_trackHandles map, so the {clip,node} handle entry created during redo remains stale. In the next edits, another node can be assigned that now-free handle (allocator scans Ogre tracks), and a later addKeyframe for this node will reuse the stale handle and write into the other node’s track. This causes cross-node keyframe corruption after a redo/undo cycle followed by keying a different node in the same clip.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/NodeAnimationManager_test.cpp (1)
56-59:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFail fast on mesh-fixture prerequisites in SetUp.
Line 58 checks Ogre init, but the fixture should also assert mesh loadability so CI fails loudly when runtime prerequisites are broken.
Suggested fix
void SetUp() override { ASSERT_TRUE(tryInitOgre()); + ASSERT_TRUE(canLoadMeshFiles());Based on learnings: "In QtMeshEditor tests that depend on Ogre ... fixture SetUp must fail loudly in CI by using ASSERT_TRUE(tryInitOgre()) and ASSERT_TRUE(canLoadMeshFiles()) ...".
🤖 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 `@src/NodeAnimationManager_test.cpp` around lines 56 - 59, The SetUp() fixture currently only asserts that Ogre initialized via ASSERT_TRUE(tryInitOgre()) but must also fail fast when mesh files can't be loaded; add an assertion using ASSERT_TRUE(canLoadMeshFiles()) in SetUp() after tryInitOgre() so tests that depend on mesh assets (SetUp, tryInitOgre, canLoadMeshFiles) will abort early in CI when prerequisites are missing.
🤖 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 `@src/commands/NodeAnimCommands.cpp`:
- Around line 112-122: Add Sentry breadcrumbs to every undo/redo that performs
user-facing node animation mutations: call SentryReporter::addBreadcrumb(...)
inside CreateNodeAnimClipCommand::redo and ::undo (around the
createClip/deleteClip calls) and likewise inside the redo/undo implementations
in the other command methods referenced (the functions in the 141-150 and
211-281 ranges). Use category "ui.action", a short message like
"node_anim.create_clip"/"node_anim.delete_clip"/"node_anim.set_key"/etc., and
include useful data (mName, mLength or key index/time/value) as breadcrumb
metadata so each operation and its context are recorded. Ensure the breadcrumb
is added before/after the mutation call consistently for both redo and undo.
- Around line 112-122: The redo() currently ignores
NodeAnimationManager::createClip()'s return and undo() unconditionally calls
deleteClip(), so change CreateNodeAnimClipCommand::redo to capture the success
(or store a bool/created flag or the actual clip pointer/name) returned by
NodeAnimationManager::createClip(mName, mLength) and only perform delete in
CreateNodeAnimClipCommand::undo if that flag indicates this command actually
created the clip; additionally add SentryReporter::addBreadcrumb() calls in both
redo() and undo() with clear messages and relevant metadata (clip name, length,
success/failure) to instrument creation and deletion events.
---
Outside diff comments:
In `@src/NodeAnimationManager_test.cpp`:
- Around line 56-59: The SetUp() fixture currently only asserts that Ogre
initialized via ASSERT_TRUE(tryInitOgre()) but must also fail fast when mesh
files can't be loaded; add an assertion using ASSERT_TRUE(canLoadMeshFiles()) in
SetUp() after tryInitOgre() so tests that depend on mesh assets (SetUp,
tryInitOgre, canLoadMeshFiles) will abort early in CI when prerequisites are
missing.
🪄 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
Run ID: 64ed4e6f-4cef-4048-a74f-8be7c7777d56
📒 Files selected for processing (5)
src/CMakeLists.txtsrc/NodeAnimationManager_test.cppsrc/commands/NodeAnimCommands.cppsrc/commands/NodeAnimCommands.htests/CMakeLists.txt
| void CreateNodeAnimClipCommand::redo() | ||
| { | ||
| if (auto* m = NodeAnimationManager::instance()) | ||
| m->createClip(mName, mLength); | ||
| } | ||
|
|
||
| void CreateNodeAnimClipCommand::undo() | ||
| { | ||
| if (auto* m = NodeAnimationManager::instance()) | ||
| m->deleteClip(mName); | ||
| } |
There was a problem hiding this comment.
Add Sentry breadcrumbs for undo/redo mutations.
These methods perform significant user-facing state changes (create/delete clip, set keyframe) but do not emit SentryReporter::addBreadcrumb() calls.
As per coding guidelines: "All user-facing actions and significant operations must be tracked with SentryReporter::addBreadcrumb() using appropriate categories ('ui.action' for toolbar/menu clicks, 'ai.tool_call' for MCP tool invocations, 'file.import'/'file.export' for I/O operations)".
Also applies to: 141-150, 211-281
🤖 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 `@src/commands/NodeAnimCommands.cpp` around lines 112 - 122, Add Sentry
breadcrumbs to every undo/redo that performs user-facing node animation
mutations: call SentryReporter::addBreadcrumb(...) inside
CreateNodeAnimClipCommand::redo and ::undo (around the createClip/deleteClip
calls) and likewise inside the redo/undo implementations in the other command
methods referenced (the functions in the 141-150 and 211-281 ranges). Use
category "ui.action", a short message like
"node_anim.create_clip"/"node_anim.delete_clip"/"node_anim.set_key"/etc., and
include useful data (mName, mLength or key index/time/value) as breadcrumb
metadata so each operation and its context are recorded. Ensure the breadcrumb
is added before/after the mutation call consistently for both redo and undo.
🧩 Analysis chain
🏁 Script executed:
# First, explore the structure to understand the file locations
find . -type f -name "NodeAnimCommands.*" | head -20Repository: fernandotonon/QtMeshEditor
Length of output: 140
🏁 Script executed:
# Also check for NodeAnimationManager to understand the API
find . -type f -name "*NodeAnimationManager*" | head -20Repository: fernandotonon/QtMeshEditor
Length of output: 166
🏁 Script executed:
# Search for SentryReporter usage to understand breadcrumb patterns
rg "SentryReporter::addBreadcrumb" --type cpp | head -30Repository: fernandotonon/QtMeshEditor
Length of output: 2678
🏁 Script executed:
# Read NodeAnimCommands.cpp to see the current implementation
cat -n ./src/commands/NodeAnimCommands.cpp | head -150Repository: fernandotonon/QtMeshEditor
Length of output: 6198
🏁 Script executed:
# Read NodeAnimCommands.h to check the class definition
cat -n ./src/commands/NodeAnimCommands.hRepository: fernandotonon/QtMeshEditor
Length of output: 4479
🏁 Script executed:
# Check NodeAnimationManager.h for createClip/deleteClip signatures
cat -n ./src/NodeAnimationManager.hRepository: fernandotonon/QtMeshEditor
Length of output: 6559
🏁 Script executed:
# Search for other command classes to see if they use breadcrumbs
rg "class.*Command.*\{" --type cpp -A 20 src/commands/ | head -100Repository: fernandotonon/QtMeshEditor
Length of output: 7817
🏁 Script executed:
# Check the suggested diff to ensure it properly addresses the issue
cat -n ./src/commands/NodeAnimCommands.h | grep -A 5 "class CreateNodeAnimClipCommand"Repository: fernandotonon/QtMeshEditor
Length of output: 335
🏁 Script executed:
# Verify no other places where createClip/deleteClip are called without checking returns
rg "createClip|deleteClip" --type cpp -B 2 -A 2Repository: fernandotonon/QtMeshEditor
Length of output: 11631
🏁 Script executed:
# Check if there are any breadcrumb calls in SetNodeKeyframeCommand for comparison
cat -n ./src/commands/NodeAnimCommands.cpp | sed -n '200,281p'Repository: fernandotonon/QtMeshEditor
Length of output: 3631
🏁 Script executed:
# Look for pattern of how significant operations are typically instrumented
rg "createClip|deleteClip" -A 3 -B 3 src/NodeAnimationManager.cpp | head -40Repository: fernandotonon/QtMeshEditor
Length of output: 1295
Guard undo so it only deletes clips created by this command, and add breadcrumb instrumentation.
Lines 114–121 have two issues:
-
Undo guard: redo() ignores createClip()'s return value. If createClip() fails (duplicate name), no clip is created, but undo() will unconditionally delete by name—removing a pre-existing clip instead.
-
Missing breadcrumb: Creating and deleting clips are significant operations that must be tracked with SentryReporter::addBreadcrumb() per the repository guidelines.
Suggested fix
--- a/src/commands/NodeAnimCommands.h
+++ b/src/commands/NodeAnimCommands.h
@@ -54,6 +54,7 @@ class CreateNodeAnimClipCommand : public QUndoCommand
private:
QString mName;
double mLength = 0.0;
+ bool mCreatedByRedo = false;
--- a/src/commands/NodeAnimCommands.cpp
+++ b/src/commands/NodeAnimCommands.cpp
@@ -11,6 +11,7 @@
`#include` "../Manager.h"
`#include` "../NodeAnimationManager.h"
+#include "../SentryReporter.h"
`#include` <OgreAnimation.h>
`#include` <OgreAnimationState.h>
@@ -111,11 +112,16 @@ CreateNodeAnimClipCommand::CreateNodeAnimClipCommand(const QString& name,
void CreateNodeAnimClipCommand::redo()
{
- if (auto* m = NodeAnimationManager::instance())
- m->createClip(mName, mLength);
+ if (auto* m = NodeAnimationManager::instance()) {
+ mCreatedByRedo = m->createClip(mName, mLength);
+ if (mCreatedByRedo)
+ SentryReporter::addBreadcrumb("ui.action", QString("Create clip: %1").arg(mName));
+ } else {
+ mCreatedByRedo = false;
+ }
}
void CreateNodeAnimClipCommand::undo()
{
+ if (!mCreatedByRedo) return;
if (auto* m = NodeAnimationManager::instance())
- m->deleteClip(mName);
+ m->deleteClip(mName);
+ SentryReporter::addBreadcrumb("ui.action", QString("Delete clip: %1").arg(mName));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| void CreateNodeAnimClipCommand::redo() | |
| { | |
| if (auto* m = NodeAnimationManager::instance()) | |
| m->createClip(mName, mLength); | |
| } | |
| void CreateNodeAnimClipCommand::undo() | |
| { | |
| if (auto* m = NodeAnimationManager::instance()) | |
| m->deleteClip(mName); | |
| } | |
| void CreateNodeAnimClipCommand::redo() | |
| { | |
| if (auto* m = NodeAnimationManager::instance()) { | |
| mCreatedByRedo = m->createClip(mName, mLength); | |
| if (mCreatedByRedo) | |
| SentryReporter::addBreadcrumb("ui.action", QString("Create clip: %1").arg(mName)); | |
| } else { | |
| mCreatedByRedo = false; | |
| } | |
| } | |
| void CreateNodeAnimClipCommand::undo() | |
| { | |
| if (!mCreatedByRedo) return; | |
| if (auto* m = NodeAnimationManager::instance()) | |
| m->deleteClip(mName); | |
| SentryReporter::addBreadcrumb("ui.action", QString("Delete clip: %1").arg(mName)); | |
| } |
🤖 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 `@src/commands/NodeAnimCommands.cpp` around lines 112 - 122, The redo()
currently ignores NodeAnimationManager::createClip()'s return and undo()
unconditionally calls deleteClip(), so change CreateNodeAnimClipCommand::redo to
capture the success (or store a bool/created flag or the actual clip
pointer/name) returned by NodeAnimationManager::createClip(mName, mLength) and
only perform delete in CreateNodeAnimClipCommand::undo if that flag indicates
this command actually created the clip; additionally add
SentryReporter::addBreadcrumb() calls in both redo() and undo() with clear
messages and relevant metadata (clip name, length, success/failure) to
instrument creation and deletion events.
Two findings from PR #585 reviews: ## Codex P1 — stale handle cache after redo+undo `SetNodeKeyframeCommand::undo` was calling `anim->destroyNodeTrack(handle)` directly when the redo had created the track. That bypassed the manager's `m_trackHandles` map, leaving a stale `{clip, node} → handle` entry. A later `addKeyframe(differentNode, ...)` then: 1. Sees Ogre's now-empty track slot as free. 2. Allocates that handle for the new node. 3. The next `addKeyframe(originalNode, ...)` looks up the stale entry, finds the handle that now points to a *different* node's track, and writes into the wrong animation. This is the same class of bug as the original `qHash & 0xFFFF` collision Codex flagged on PR #584 — just in a different code path. ### Fix - New `NodeAnimationManager::forgetTrackHandle(clip, node)` — removes the entry from `m_trackHandles`, drops the per-clip inner map when empty. Public so commands can call it; main- thread-asserted like the other mutators. - `SetNodeKeyframeCommand::undo` calls it right after `destroyNodeTrack` so the manager's view stays in sync with Ogre's track table. ### Regression test - `SetKeyframeCommandUndoForgetsStaleHandle` reproduces the exact sequence: redo+undo on node A → keyframe node B → keyframe node A → assert both nodes still see only their own keyframe. Without the fix, A's second keyframe would land on B's track and the assertion `keysA.size() == 1` would fail (or keysB would show both keys). ## CodeRabbit major — missing Sentry breadcrumbs Commands are user-facing operations and the codebase convention is that they emit Sentry breadcrumbs. Added `scene.anim.node.cmd` breadcrumbs to redo and undo on all three commands (Create / Delete / SetKeyframe), with the clip + node + time in the message. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Third sub-slice of #520. Surfaces the C1 (manager) and C3 (commands) data layer through three new MCP tools so an AI agent can author node animations end-to-end via JSON-RPC. ## What ships ### Three new MCP tools - **`list_node_animations`** — no args. Returns `{ count, clips: [name…] }`. Light; pure read off `NodeAnimationManager::listClips()`. - **`add_node_animation_clip`** — required: `name`, `length` (seconds). Wraps `createClip`. Errors on missing args, non-positive length, name collision. - **`set_node_keyframe`** — required: `clip`, `node`, `time`. Optional: `translate`, `rotation`, `scale` (defaulting to ZERO, IDENTITY, ONE if omitted, so the agent can author "snapshot pose at T" in one call). Each TRS array is strictly parsed — malformed arrays return an error rather than silently falling back to defaults, so agent bugs surface immediately. All three operate on the **live scene** (not a transient import), matching `set_morph_weight`'s surface. Agents are expected to drive `load_mesh` / `save_scene` around them for persistence. ## 4 new tests - `AddNodeAnimationClip_MissingArgsRejected` — empty / name-only / zero-length all rejected with descriptive errors. - `AddNodeAnimationClipThenList` — round-trip: create one, list shows it. - `SetNodeKeyframe_RejectsMissingClip` — clip-not-found surfaces. - `SetNodeKeyframe_RejectsMalformedTransformArray` — wrong-arity translate array → error message names the bad field. ## #520 status | Sub-slice | Status | |-|-| | C1 — Manager data layer | shipped (#584) | | C3 — Undo commands | shipped (#585) | | C6 — MCP tools | **this PR** | | C2 — Inspector subgroup | follow-up | | C4 — Dope-sheet integration | follow-up | | C5 — glTF + FBX exporter round-trip | follow-up | Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Fourth sub-slice of #520. Adds a CLI subcommand to enumerate node-animation clips on a scene file from the shell. Mirrors the existing `qtmesh morph --list` precedent: one file in, names + count out (text or `--json`). Useful for CI scripts that audit Mixamo- style assets imported via the editor's GUI, and the round-trip target for the future C5 glTF/FBX exporter work. ## What ships - `cmdNodeAnim(argc, argv)` in `CLIPipeline.{h,cpp}` — parses `nodeanim <file> --list [--json]`, runs `MeshImporterExporter::importer`, reads `NodeAnimationManager::instance()->listClips()`, prints text or JSON. JSON shape mirrors `qtmesh morph --list --json`: `{ file, count, clips: [name…] }`. - Subcommand registration in `main.cpp` (CLI-mode detection by first non-flag arg) and `CLIPipeline::run` dispatcher. - Help text in `printUsage` next to the `morph` line. ## What's deferred - Authoring on the CLI side (`--add-clip`, `--add-keyframe`, `-o out.gltf`) needs the C5 exporter round-trip first — otherwise edits don't persist anywhere outside the in-process scene. ## #520 status | Sub-slice | Status | |-|-| | C1 — Manager data layer | shipped (#584) | | C3 — Undo commands | shipped (#585) | | C6 — MCP tools | shipped (#586, hardened #587 + #588) | | C-CLI — `qtmesh nodeanim --list` | **this PR** | | C2 — Inspector subgroup | follow-up | | C4 — Dope-sheet integration | follow-up | | C5 — glTF + FBX exporter round-trip | follow-up | Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 participant
Second sub-slice of #520 (node-transform animation). Adds
QUndoCommandsubclasses for the threeNodeAnimationManagermutators so Ctrl+Z reverses authoring operations. Mirrors the morph A3 command shape: snapshot prior state at construction, redo applies, undo restores from the snapshot.What ships
commands/NodeAnimCommands.{h,cpp}CreateNodeAnimClipCommand(name, length)— redo →createClip; undo →deleteClip. Trivial; snapshot is just(name, length).DeleteNodeAnimClipCommand(name)— snapshots every track + every keyframe at construction (TRS values, associated node name, time). Redo drops the clip; undo rebuilds it through the manager so the per-clip handle allocator (feat(node-anim): sub-slice C1 — NodeAnimationManager singleton #584 collision fix) stays authoritative.SetNodeKeyframeCommand(clip, node, time, T, R, S)— snapshots three cases at construction:mTrackCreatedByRedoflag detects this).time→ undo removes the lone keyframe redo added (track stays unless this command also created it).time(within 1ms manager merge epsilon) → undo restores prior TRS in place.Uses the same
kKeyframeMergeEpsilon = 1e-3as the manager so command and manager agree on which keyframe is "the same one."4 new tests
CreateClipCommandRoundTrips— create / undo / redo cycle.DeleteClipCommandSnapshotsAndRestores— two tracks + three keys preserved across undo. Verifies the deepest snapshot path.SetKeyframeCommandRoundTripsForFreshKey— track-created-by-redo flag works (undo leaves clip in pre-redo state with no empty stub track).SetKeyframeCommandRestoresPriorOnOverwrite— in-place value restoration when redo overwrote an existing key within epsilon.#520 status
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests