Skip to content

Comments

wip#29

Open
epavanello wants to merge 2 commits intomainfrom
claude/complete-ai-presets-LOtwH
Open

wip#29
epavanello wants to merge 2 commits intomainfrom
claude/complete-ai-presets-LOtwH

Conversation

@epavanello
Copy link
Owner

@epavanello epavanello commented Feb 22, 2026

Summary by CodeRabbit

  • New Features

    • Added enter/exit transition support for layers with preset selection and customizable duration
    • Introduced interactive animation preset selector with live preview on hover
    • Timeline now displays visual indicators for layer enter and exit transitions
  • UI/UX Improvements

    • Renamed "Time Range" panel to "Time & Transitions" for clearer organization
    • Enhanced animation system with three approaches: automatic transitions, keyframe presets, and custom keyframes

epavanello and others added 2 commits February 22, 2026 13:01
…iew, and AI integration

- Centralized presets config (presets.ts): Added descriptions, categories (enter/exit/emphasis),
  and AI summary generation. Single source of truth for UI, rendering, and AI.

- Preset preview (animation-preset-select.svelte): Rewrote component to use CSS-based
  interpolation preview on hover - no createLayer dependency, uses interpolateValue directly.
  Shows animated preview box with description tooltip.

- Fixed time-range-group.svelte: Removed invalid $derived mutation pattern. Transition
  state is now read-only derived from layer, updates go through projectStore.updateLayer.

- Fixed layer-rendering.ts: Rewrote transition system to use additive offsets for position/rotation
  and multiplicative factors for scale/opacity. Transitions now correctly compose with the layer's
  base transform instead of replacing it.

- Timeline transition markers: Added visual enter/exit transition zones on timeline bars
  with "In"/"Out" labels.

- AI schema updates: Added enterTransition/exitTransition fields to both create_layer and
  edit_layer tool schemas, with descriptions explaining offset/factor semantics.

- AI system prompt: Added complete presets documentation with available presets list,
  usage guidance (transitions vs keyframe presets vs custom keyframes), and updated
  default creative baseline to prefer transitions.

- AI mutations: Both mutateCreateLayer and mutateEditLayer now handle enterTransition
  and exitTransition, validating preset IDs before applying.

https://claude.ai/code/session_01V2YcPwVv25drxzAw7R17Wy
@coderabbitai
Copy link

coderabbitai bot commented Feb 22, 2026

📝 Walkthrough

Walkthrough

This PR introduces enter and exit transitions for layers through preset-based animations. Changes include schema extensions for transition fields, AI mutation logic to apply transitions, rendering engine updates to compute transition effects, updated UI components for transition selection, and a refactored type-safe preset system with helper utilities.

Changes

Cohort / File(s) Summary
AI Mutations & Schemas
src/lib/ai/mutations.ts, src/lib/ai/schemas.ts
Added enterTransition and exitTransition fields to layer creation/edit inputs; mutations resolve preset IDs and apply/clear transitions based on input presence and truthiness; schema introduces LayerTransitionFieldSchema with presetId and duration.
System Prompt & AI Guidance
src/lib/ai/system-prompt.ts
Expanded system prompt with comprehensive Animation System documentation covering transitions, keyframe presets, and custom keyframes; integrated dynamic presets summary; updated tool definitions and default creative baseline to reference enter/exit transitions.
Engine & Rendering
src/lib/engine/layer-rendering.ts, src/lib/stores/project.svelte.ts
Extended getLayerTransform and getLayerStyle to accept projectDuration parameter; introduced computeTransitionValues to derive additive/multiplicative offsets and style adjustments from enter/exit presets; updated all calls to pass duration.
Preset System
src/lib/engine/presets.ts
Refactored to type-safe system with new types (TransformProperty, StyleProperty, PresetKeyframe, TypedAnimationPreset, PresetCategory); added category-based query helpers (getEnterPresets, getExitPresets, getEmphasisPresets) and getPresetsSummaryForAI for AI integration.
Layer Schema
src/lib/schemas/animation.ts
Extended LayerSchema with optional enterTransition and exitTransition fields; introduced LayerTransitionSchema and LayerTransition type.
UI: Timeline & Properties Panels
src/lib/components/editor/timeline/timeline-layer.svelte, src/lib/components/editor/panels/properties-panel.svelte, src/lib/components/editor/panels/properties/groups/time-range-group.svelte
Updated timeline layer display to show enter/exit transition zones; renamed "Time Range" to "Time & Transitions" in properties panel; enhanced time-range-group with preset selection and duration controls for transitions.
UI: Animation Preset Components
src/lib/components/editor/panels/properties/animation-preset-select.svelte, src/lib/components/editor/panels/properties/groups/animation-presets-group.svelte
Introduced new AnimationPresetSelect component with popover dropdown, live preview loop using requestAnimationFrame, and interpolation of preset keyframes; updated AnimationPresetsGroup to use new select component.
UI: Canvas Renderer & Model Selector
src/lib/components/editor/canvas/layers-renderer.svelte, src/lib/components/ai/model-selector.svelte
Updated canvas renderer calls to getLayerTransform/getLayerStyle to include duration parameter; removed unused Lock icon import from model selector.

Sequence Diagram

sequenceDiagram
    participant User
    participant UI as UI (Time Range Group)
    participant Mutations as AI Mutations
    participant Store as Project Store
    participant Engine as Render Engine
    participant Canvas as Canvas Renderer

    User->>UI: Select enter transition preset
    UI->>Mutations: mutateEditLayer with enterTransition
    Mutations->>Mutations: Resolve preset ID to valid preset
    Mutations->>Store: Update layer.enterTransition
    
    User->>Canvas: Trigger render at time T
    Canvas->>Engine: getLayerTransform(layer, time, duration)
    Engine->>Engine: computeTransitionValues(layer, time, duration)
    Engine->>Engine: Interpolate preset keyframes at time T
    Engine->>Engine: Combine base transform + transition offsets
    Engine-->>Canvas: Return transform with transition applied
    
    Canvas->>Engine: getLayerStyle(layer, time, duration)
    Engine->>Engine: Apply transition style effects (opacity, blur, etc.)
    Engine-->>Canvas: Return style with transition applied
    
    Canvas->>Canvas: Render layer with transition effects
    Canvas-->>User: Display animated transition
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

  • PR #26: Modifies mutateCreateLayer and mutateEditLayer to alter how layers are constructed from input payloads, directly intersecting with this PR's transition mutation logic.
  • PR #16: Adds timing fields (enterTime/exitTime/contentDuration) to layer mutations and schemas, complementing this PR's transition-based approach to layer lifecycle management.
  • PR #19: Updates src/lib/ai/system-prompt.ts to modify animation-related AI guidance, similar in scope to this PR's prompt expansion for transition documentation.

Poem

🐰 Hops of joy! With transitions so sweet,
Layers now dance at entrance and retreat,
Presets applied with presets so fine,
Durations computed along the timeline,
Animation preset dreams made true! 🎬✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'wip' is vague and generic, providing no meaningful information about the changeset's primary purpose or scope. Replace 'wip' with a descriptive title that summarizes the main change, such as 'Add enter/exit transition support for layers' or 'Implement animation presets with transition system'.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 88.24% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch claude/complete-ai-presets-LOtwH

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 9

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/lib/components/ai/model-selector.svelte (1)

16-17: 🛠️ Refactor suggestion | 🟠 Major

Remove dead user and isPro derived values.

After dropping the Lock icon import, neither user nor isPro is referenced anywhere in the template. They should be removed rather than left as unused $derived computations. As per coding guidelines, unused variables must at minimum be prefixed with _, but outright removal is the right call here.

♻️ Proposed fix
-  const user = $derived(getUser());
-  const isPro = $derived(user.current?.plan === 'pro');

Also remove the now-unused getUser import:

-  import { getUser } from '$lib/functions/auth.remote';
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/components/ai/model-selector.svelte` around lines 16 - 17, Remove the
unused derived values and import: delete the const declarations for user and
isPro (the $derived(getUser()) and $derived(user.current?.plan === 'pro') lines)
and also remove the now-unused getUser import from the top of the module; this
cleans up dead code left after removing the Lock icon and avoids leaving unused
$derived computations.
🧹 Nitpick comments (7)
src/lib/engine/presets.ts (1)

502-519: getPresetsSummaryForAI could be a module-level constant

The function takes no arguments and operates only on the static animationPresets array, so it always produces the same string. Pre-computing it once at module load would be slightly more efficient.

♻️ Optional refactor
-export function getPresetsSummaryForAI(): string {
-  const sections = [
-    ...
-  ];
-  return sections.map(...).join('\n\n');
-}
+export const presetsSummaryForAI: string = (() => {
+  const sections = [
+    {
+      title: 'Enter presets (use for layer entrances or enterTransition)',
+      presets: getEnterPresets()
+    },
+    { title: 'Exit presets (use for layer exits or exitTransition)', presets: getExitPresets() },
+    {
+      title: 'Emphasis presets (use as keyframe animations at any time)',
+      presets: getEmphasisPresets()
+    }
+  ];
+  return sections
+    .map(
+      (s) => `### ${s.title}\n${s.presets.map((p) => `- **${p.id}**: ${p.description}`).join('\n')}`
+    )
+    .join('\n\n');
+})();
+
+/** `@deprecated` Use `presetsSummaryForAI` constant instead */
+export function getPresetsSummaryForAI(): string {
+  return presetsSummaryForAI;
+}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/engine/presets.ts` around lines 502 - 519, Replace the runtime
function getPresetsSummaryForAI with a module-level precomputed constant that
builds the same summary string once at module load: call getEnterPresets(),
getExitPresets(), and getEmphasisPresets() when initializing the constant,
format the sections exactly as the current implementation does, and export the
constant under the same exported name (e.g., export const getPresetsSummaryForAI
= PRECOMPUTED_STRING) so call sites remain unchanged.
src/lib/components/editor/panels/properties/animation-preset-select.svelte (2)

23-23: animationFrameId stored as $state triggers unnecessary reactivity on every animation frame.

animationFrameId is only used for bookkeeping (cancelling the rAF). Updating it via $state on every frame (line 131) causes Svelte to track and propagate a reactive change that nothing in the template consumes. Use a plain let instead:

Suggested fix
-  let animationFrameId = $state<number | null>(null);
+  let animationFrameId: number | null = null;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/components/editor/panels/properties/animation-preset-select.svelte`
at line 23, The variable animationFrameId is currently created via $state which
causes Svelte reactivity on every RAF tick; change it to a plain local variable
(let animationFrameId = null) instead of $state<number | null>, then update all
uses (where you assign the RAF id and where you call cancelAnimationFrame) to
read/write that plain variable; ensure you remove any reactive references
($animationFrameId or stores) and keep only the local animationFrameId for
bookkeeping in the functions that schedule/cancel the animation frame.

32-114: Duplicated keyframe interpolation logic — same concern as noted in layer-rendering.ts.

interpolatePresetAt reimplements the same grouping, surrounding-keyframe search, clamping, and interpolation as interpolatePresetKeyframes in layer-rendering.ts. A shared utility would prevent the two from diverging.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/components/editor/panels/properties/animation-preset-select.svelte`
around lines 32 - 114, interpolatePresetAt duplicates the
grouping/neighbor-finding/clamping/interpolation logic already present in
interpolatePresetKeyframes (layer-rendering.ts); refactor by extracting the
shared logic into a single helper (e.g., computeInterpolatedProperties or reuse
interpolatePresetKeyframes) in a shared module and call that helper from both
interpolatePresetAt and the function in layer-rendering.ts, keeping
interpolatePresetAt only responsible for applying the scaled position, mapping
properties to tx/ty/sx/sy/rx/ry/rz/opacity and building the transform string.
src/lib/components/editor/timeline/timeline-layer.svelte (1)

140-148: _isGroupLayer is declared but never used — consider removing it.

Even though the underscore prefix satisfies the linting convention, this is dead code with no consumer in the template or script. If it was removed as part of the refactor (replacing the old conditional rendering), it can be cleaned up.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/components/editor/timeline/timeline-layer.svelte` around lines 140 -
148, Remove the unused derived store _isGroupLayer from the
timeline-layer.svelte script: locate the constant declaration named
_isGroupLayer and delete it (and any related import or $derived invocation if it
becomes unused) so there is no dead code; ensure no template or other script
references to _isGroupLayer remain, and run the linter to confirm no
unused-symbol warnings persist.
src/lib/engine/layer-rendering.ts (2)

178-266: Duplicated interpolation logic between interpolatePresetKeyframes and animation-preset-select.svelte.

The keyframe grouping, surrounding-keyframe search, progress clamping, and interpolateValue call pattern in this function (lines 186–221) is nearly identical to interpolatePresetAt in animation-preset-select.svelte (lines 32–104). Consider extracting a shared utility (e.g., evaluatePresetAtTime(keyframes, normalizedTime)) to avoid the two implementations drifting apart.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/engine/layer-rendering.ts` around lines 178 - 266,
interpolatePresetKeyframes duplicates the keyframe evaluation logic found in
interpolatePresetAt (grouping by property, finding surrounding keyframes,
clamping progress and calling interpolateValue); extract that common behavior
into a shared utility (e.g., evaluatePresetAtTime or
evaluateKeyframesForProperty) that accepts a list of PresetKeyframe(s) and a
normalized time (or currentTime + duration) and returns the interpolated
value(s)/TransitionValues for each property; update interpolatePresetKeyframes
to call this new utility (using scaledKeyframes/time conversion as needed) and
refactor interpolatePresetAt in animation-preset-select.svelte to reuse the same
utility so both use one authoritative implementation.

74-78: Transition filter values (blur, brightness, etc.) replace base/animated values instead of composing.

When a transition preset animates blur or brightness, the transition value completely overrides the keyframed or base style value — even at the transition's resting state. For example, if a preset defines brightness keyframes (ending at 1.0), the layer's custom brightness keyframes are ignored for the entire transition duration.

This is fine if transitions are not expected to have filter keyframes commonly, but it's worth noting the asymmetry: opacity is multiplicative, position is additive, but filters are replacement.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/engine/layer-rendering.ts` around lines 74 - 78, The filter fields in
layer-rendering.ts currently replace base/animated values (lines setting blur,
brightness, contrast, saturate) instead of composing with transition values;
change them to compose: for brightness/contrast/saturate compute the base value
first (animatedStyle.<field> ?? layer.style.<field> ?? 1) and multiply it by
(transition.<field> ?? 1), and for blur compute the base blur
(animatedStyle.blur ?? layer.style.blur ?? 0) and add (transition.blur ?? 0);
keep opacity as-is (baseOpacity * (transition.opacityFactor ?? 1)). Update the
lines referencing transition, animatedStyle and layer.style for blur,
brightness, contrast and saturate accordingly.
src/lib/components/editor/panels/properties/groups/animation-presets-group.svelte (1)

21-22: Unnecessary alias and explicit type annotation.

allPresets is just a direct re-assignment of animationPresets with an explicit type that TypeScript can already infer. You can use animationPresets directly as the options prop on line 45, or at minimum drop the type annotation. As per coding guidelines, "Prefer type inference over explicit type annotations in TypeScript."

Suggested simplification
-  // All presets available for keyframe application (all categories)
-  const allPresets: TypedAnimationPreset[] = animationPresets;

Then on line 45:

-      options={allPresets}
+      options={animationPresets}

And remove the unused import of TypedAnimationPreset on line 6.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/lib/components/editor/panels/properties/groups/animation-presets-group.svelte`
around lines 21 - 22, The local alias allPresets is redundant and explicitly
typed as TypedAnimationPreset[]; remove the alias and either use
animationPresets directly for the options prop in the component or, if you
prefer to keep a local name, drop the explicit TypedAnimationPreset annotation
so TypeScript can infer the type; also remove the now-unused
TypedAnimationPreset import. Locate the declaration allPresets and the component
options usage and replace the reference with animationPresets (or keep the
variable without the type annotation) and delete the unused import.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/lib/ai/mutations.ts`:
- Around line 134-152: When getPresetById(...) returns undefined for
enterTransition or exitTransition inside mutateCreateLayer and mutateEditLayer,
don't silently drop it — collect a warning (e.g., "invalid presetId for
enterTransition: <id>") and append or include these warnings in the mutation's
returned message/response so the AI sees the rejection; modify the blocks that
set layer.enterTransition / layer.exitTransition (where getPresetById is called)
to push warnings into a local warnings array and ensure mutateCreateLayer and
mutateEditLayer include that warnings text in their final return payload/message
(and include references to getPresetById, layer.enterTransition,
layer.exitTransition, mutateCreateLayer, mutateEditLayer).
- Around line 276-301: The conditional else branches that set
layer.enterTransition/exitTransition = undefined are dead because
input.enterTransition and input.exitTransition cannot be null/false per the
current LayerTransitionFieldSchema; either make the schema nullable so callers
can send null to clear transitions (update LayerTransitionFieldSchema to
.nullable().optional() for enterTransition/exitTransition) or remove the
unreachable else blocks and only handle the presence check (if
(input.enterTransition !== undefined) / if (input.exitTransition !==
undefined)), keeping the preset validation via getPresetById and assignment to
layer.enterTransition/layer.exitTransition as currently written.

In `@src/lib/ai/schemas.ts`:
- Around line 226-231: The schema fields enterTransition and exitTransition on
EditLayerInputSchema currently use LayerTransitionFieldSchema.optional(), but
the description and the mutations handler expect null to explicitly remove a
value; change both enterTransition and exitTransition to
LayerTransitionFieldSchema.nullish() so the schema accepts undefined (omit =
keep) and null (explicit remove) and aligns with the mutation logic that checks
for !== undefined and treats falsy/null as removal.

In `@src/lib/components/editor/panels/properties/animation-preset-select.svelte`:
- Around line 121-156: The effect watching open can leak requestAnimationFrame
if the component is destroyed while open; update the $effect that references
open to return a cleanup function that calls stopPreview() (or add an onDestroy
teardown that calls stopPreview()) so any pending animationFrameId is cancelled
when the effect is torn down or the component unmounts; locate the $effect block
and ensure it either returns () => stopPreview() or add an onDestroy(() =>
stopPreview()) to use the existing stopPreview/animationFrameId logic.

In `@src/lib/components/editor/panels/properties/groups/time-range-group.svelte`:
- Around line 78-83: The label "for" targets (enter-preset and exit-preset)
don't work because AnimationPresetSelect doesn't accept or forward an id; update
AnimationPresetSelect to accept an id prop and ensure it is set on the
interactive element it renders (e.g., the underlying button or select) so label
clicks focus the control, and then pass id="enter-preset" and id="exit-preset"
where AnimationPresetSelect is used in time-range-group.svelte (the instances
around enterPresetId and exitPresetId); ensure any internal event handlers still
use the existing onchange/updateTransition logic.
- Around line 72-93: InputsWrapper's fields list must be built dynamically so
the "Duration (s)" label is only emitted when the corresponding ScrubInput
exists; update the enter block to construct the fields array using enterPresetId
(e.g., include the {for: 'enter-duration', labels: 'Duration (s)'} entry only
when enterPresetId is truthy) and do the same for the exit block using
exitPresetId, ensuring the field `for` values ('enter-duration' /
'exit-duration') match the ScrubInput id attributes and the
AnimationPresetSelect entries remain first.

In `@src/lib/components/editor/timeline/timeline-layer.svelte`:
- Around line 219-244: The enter/exit transition zones can overlap because
enterTransitionWidth and exitTransitionWidth are each clamped only to barWidth;
update the logic in TimelineLayer to compute clampedEnterWidth and
clampedExitWidth so that clampedEnterWidth + clampedExitWidth <= barWidth (e.g.,
if enter+exit <= barWidth use min(each, barWidth), otherwise scale them
proportionally: clampedEnter = enter * barWidth / (enter+exit) and clampedExit =
exit * barWidth / (enter+exit), or choose to hide the smaller zone when overlap
would occur), then replace uses of style:width="{Math.min(...)}px" with
style:width="{clampedEnterWidth}px" and style:width="{clampedExitWidth}px" and
adjust the conditional text labels to use those clamped widths.

In `@src/lib/engine/layer-rendering.ts`:
- Around line 268-282: mergeTransitionValues currently uses last-wins (b ?? a)
for positionOffset and rotationOffset and multiplicative for opacityFactor,
causing snaps when enter and exit overlap; change it so positionOffset and
rotationOffset are additive (if both a.positionOffset and b.positionOffset
exist, return their sum, otherwise use the defined one), make scaleFactor
multiplicative like opacityFactor (if both defined return a.scaleFactor *
b.scaleFactor, else pick the defined one), keep opacityFactor multiplicative
as-is, and leave blur/brightness/contrast/saturate with the existing
null-coalescing behavior; update the logic in mergeTransitionValues to handle
undefineds for these fields accordingly.

In `@src/lib/schemas/animation.ts`:
- Around line 233-236: LayerTransitionSchema's duration property is currently
unbounded (z.number().positive()), so update the duration validator in
LayerTransitionSchema to enforce the UI limit by adding a maximum constraint
(e.g., .max(5)) so it becomes z.number().positive().max(5) and update the
duration .describe text to mention the 5s maximum; this ensures programmatic
inputs match the UI constraint and prevents oversized transition durations.

---

Outside diff comments:
In `@src/lib/components/ai/model-selector.svelte`:
- Around line 16-17: Remove the unused derived values and import: delete the
const declarations for user and isPro (the $derived(getUser()) and
$derived(user.current?.plan === 'pro') lines) and also remove the now-unused
getUser import from the top of the module; this cleans up dead code left after
removing the Lock icon and avoids leaving unused $derived computations.

---

Nitpick comments:
In `@src/lib/components/editor/panels/properties/animation-preset-select.svelte`:
- Line 23: The variable animationFrameId is currently created via $state which
causes Svelte reactivity on every RAF tick; change it to a plain local variable
(let animationFrameId = null) instead of $state<number | null>, then update all
uses (where you assign the RAF id and where you call cancelAnimationFrame) to
read/write that plain variable; ensure you remove any reactive references
($animationFrameId or stores) and keep only the local animationFrameId for
bookkeeping in the functions that schedule/cancel the animation frame.
- Around line 32-114: interpolatePresetAt duplicates the
grouping/neighbor-finding/clamping/interpolation logic already present in
interpolatePresetKeyframes (layer-rendering.ts); refactor by extracting the
shared logic into a single helper (e.g., computeInterpolatedProperties or reuse
interpolatePresetKeyframes) in a shared module and call that helper from both
interpolatePresetAt and the function in layer-rendering.ts, keeping
interpolatePresetAt only responsible for applying the scaled position, mapping
properties to tx/ty/sx/sy/rx/ry/rz/opacity and building the transform string.

In
`@src/lib/components/editor/panels/properties/groups/animation-presets-group.svelte`:
- Around line 21-22: The local alias allPresets is redundant and explicitly
typed as TypedAnimationPreset[]; remove the alias and either use
animationPresets directly for the options prop in the component or, if you
prefer to keep a local name, drop the explicit TypedAnimationPreset annotation
so TypeScript can infer the type; also remove the now-unused
TypedAnimationPreset import. Locate the declaration allPresets and the component
options usage and replace the reference with animationPresets (or keep the
variable without the type annotation) and delete the unused import.

In `@src/lib/components/editor/timeline/timeline-layer.svelte`:
- Around line 140-148: Remove the unused derived store _isGroupLayer from the
timeline-layer.svelte script: locate the constant declaration named
_isGroupLayer and delete it (and any related import or $derived invocation if it
becomes unused) so there is no dead code; ensure no template or other script
references to _isGroupLayer remain, and run the linter to confirm no
unused-symbol warnings persist.

In `@src/lib/engine/layer-rendering.ts`:
- Around line 178-266: interpolatePresetKeyframes duplicates the keyframe
evaluation logic found in interpolatePresetAt (grouping by property, finding
surrounding keyframes, clamping progress and calling interpolateValue); extract
that common behavior into a shared utility (e.g., evaluatePresetAtTime or
evaluateKeyframesForProperty) that accepts a list of PresetKeyframe(s) and a
normalized time (or currentTime + duration) and returns the interpolated
value(s)/TransitionValues for each property; update interpolatePresetKeyframes
to call this new utility (using scaledKeyframes/time conversion as needed) and
refactor interpolatePresetAt in animation-preset-select.svelte to reuse the same
utility so both use one authoritative implementation.
- Around line 74-78: The filter fields in layer-rendering.ts currently replace
base/animated values (lines setting blur, brightness, contrast, saturate)
instead of composing with transition values; change them to compose: for
brightness/contrast/saturate compute the base value first (animatedStyle.<field>
?? layer.style.<field> ?? 1) and multiply it by (transition.<field> ?? 1), and
for blur compute the base blur (animatedStyle.blur ?? layer.style.blur ?? 0) and
add (transition.blur ?? 0); keep opacity as-is (baseOpacity *
(transition.opacityFactor ?? 1)). Update the lines referencing transition,
animatedStyle and layer.style for blur, brightness, contrast and saturate
accordingly.

In `@src/lib/engine/presets.ts`:
- Around line 502-519: Replace the runtime function getPresetsSummaryForAI with
a module-level precomputed constant that builds the same summary string once at
module load: call getEnterPresets(), getExitPresets(), and getEmphasisPresets()
when initializing the constant, format the sections exactly as the current
implementation does, and export the constant under the same exported name (e.g.,
export const getPresetsSummaryForAI = PRECOMPUTED_STRING) so call sites remain
unchanged.

Comment on lines +134 to +152
// Set enter/exit transitions if provided
if (input.enterTransition) {
const preset = getPresetById(input.enterTransition.presetId);
if (preset) {
layer.enterTransition = {
presetId: input.enterTransition.presetId,
duration: input.enterTransition.duration
};
}
}
if (input.exitTransition) {
const preset = getPresetById(input.exitTransition.presetId);
if (preset) {
layer.exitTransition = {
presetId: input.exitTransition.presetId,
duration: input.exitTransition.duration
};
}
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Silent failure for invalid presetId — AI receives success: true even if transition not applied

When getPresetById(input.enterTransition.presetId) returns undefined (invalid ID), the transition is silently dropped and mutateCreateLayer still returns success: true. The AI has no signal that the transition was rejected, which could cause it to assume the transition is in effect or repeatedly submit the same invalid preset ID.

The same pattern repeats for exitTransition (lines 144–152) and in mutateEditLayer (lines 278–284, 291–297).

🛡️ Proposed fix — surface invalid preset ID in the output message
     if (input.enterTransition) {
       const preset = getPresetById(input.enterTransition.presetId);
       if (preset) {
         layer.enterTransition = {
           presetId: input.enterTransition.presetId,
           duration: input.enterTransition.duration
         };
+      } else {
+        console.warn(`[mutateCreateLayer] Unknown enterTransition presetId: "${input.enterTransition.presetId}"`);
       }
     }

For mutateEditLayer, include the warning in the returned message so the AI can self-correct.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/ai/mutations.ts` around lines 134 - 152, When getPresetById(...)
returns undefined for enterTransition or exitTransition inside mutateCreateLayer
and mutateEditLayer, don't silently drop it — collect a warning (e.g., "invalid
presetId for enterTransition: <id>") and append or include these warnings in the
mutation's returned message/response so the AI sees the rejection; modify the
blocks that set layer.enterTransition / layer.exitTransition (where
getPresetById is called) to push warnings into a local warnings array and ensure
mutateCreateLayer and mutateEditLayer include that warnings text in their final
return payload/message (and include references to getPresetById,
layer.enterTransition, layer.exitTransition, mutateCreateLayer,
mutateEditLayer).

Comment on lines +276 to +301
if (input.enterTransition !== undefined) {
if (input.enterTransition) {
const preset = getPresetById(input.enterTransition.presetId);
if (preset) {
layer.enterTransition = {
presetId: input.enterTransition.presetId,
duration: input.enterTransition.duration
};
}
} else {
layer.enterTransition = undefined;
}
}
if (input.exitTransition !== undefined) {
if (input.exitTransition) {
const preset = getPresetById(input.exitTransition.presetId);
if (preset) {
layer.exitTransition = {
presetId: input.exitTransition.presetId,
duration: input.exitTransition.duration
};
}
} else {
layer.exitTransition = undefined;
}
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

rg -n 'EditLayerInputSchema|LayerTransitionSchema' --type=ts -B 2 -A 5

Repository: epavanello/devmotion

Length of output: 3265


🏁 Script executed:

sed -n '198,242p' src/lib/ai/schemas.ts

Repository: epavanello/devmotion

Length of output: 1486


🏁 Script executed:

rg -n 'LayerTransitionFieldSchema' --type=ts -B 2 -A 5

Repository: epavanello/devmotion

Length of output: 2155


The else branches (lines 285-287, 298-300) are dead code — the schema does not allow null values

The description comment states "set to null to remove", but LayerTransitionFieldSchema.optional() only allows undefined or the object type, not null. The else branch can never execute since input.enterTransition cannot be falsy while also passing validation.

To support clearing transitions via null, add .nullable() to the schema:

enterTransition: LayerTransitionFieldSchema.nullable().optional().describe(...)

Alternatively, remove the dead else branch and rely on the if (input.enterTransition !== undefined) check to determine intent.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/ai/mutations.ts` around lines 276 - 301, The conditional else
branches that set layer.enterTransition/exitTransition = undefined are dead
because input.enterTransition and input.exitTransition cannot be null/false per
the current LayerTransitionFieldSchema; either make the schema nullable so
callers can send null to clear transitions (update LayerTransitionFieldSchema to
.nullable().optional() for enterTransition/exitTransition) or remove the
unreachable else blocks and only handle the presence check (if
(input.enterTransition !== undefined) / if (input.exitTransition !==
undefined)), keeping the preset validation via getPresetById and assignment to
layer.enterTransition/layer.exitTransition as currently written.

Comment on lines +226 to +231
enterTransition: LayerTransitionFieldSchema.optional().describe(
'Set enter transition preset. Omit to keep existing, set to null to remove.'
),
exitTransition: LayerTransitionFieldSchema.optional().describe(
'Set exit transition preset. Omit to keep existing, set to null to remove.'
)
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, check the schema definitions in schemas.ts
cat -n src/lib/ai/schemas.ts | sed -n '50,70p'

Repository: epavanello/devmotion

Length of output: 987


🏁 Script executed:

# Check the CreateLayerInputSchema definition
cat -n src/lib/ai/schemas.ts | sed -n '138,155p'

Repository: epavanello/devmotion

Length of output: 1031


🏁 Script executed:

# Check the EditLayerInputSchema definition around lines 220-235
cat -n src/lib/ai/schemas.ts | sed -n '220,240p'

Repository: epavanello/devmotion

Length of output: 976


🏁 Script executed:

# Search for how enterTransition/exitTransition are handled in mutations
rg -n 'enterTransition|exitTransition' --type=ts src/lib -B3 -A3

Repository: epavanello/devmotion

Length of output: 12983


Change .optional() to .nullish() to match description and mutations.ts implementation.

The EditLayerInputSchema description tells the AI to "set to null to remove," but .optional() only accepts undefined, not null. Meanwhile, the mutations.ts code explicitly handles falsy values after checking !== undefined (lines 276-301), expecting to receive null to clear the field.

Use .nullish() to accept both undefined (field omitted = don't change) and null (explicit = remove):

Fix: Use .nullish()
-    enterTransition: LayerTransitionFieldSchema.optional().describe(
+    enterTransition: LayerTransitionFieldSchema.nullish().describe(
       'Set enter transition preset. Omit to keep existing, set to null to remove.'
     ),
-    exitTransition: LayerTransitionFieldSchema.optional().describe(
+    exitTransition: LayerTransitionFieldSchema.nullish().describe(
       'Set exit transition preset. Omit to keep existing, set to null to remove.'
     )
📝 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.

Suggested change
enterTransition: LayerTransitionFieldSchema.optional().describe(
'Set enter transition preset. Omit to keep existing, set to null to remove.'
),
exitTransition: LayerTransitionFieldSchema.optional().describe(
'Set exit transition preset. Omit to keep existing, set to null to remove.'
)
enterTransition: LayerTransitionFieldSchema.nullish().describe(
'Set enter transition preset. Omit to keep existing, set to null to remove.'
),
exitTransition: LayerTransitionFieldSchema.nullish().describe(
'Set exit transition preset. Omit to keep existing, set to null to remove.'
)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/ai/schemas.ts` around lines 226 - 231, The schema fields
enterTransition and exitTransition on EditLayerInputSchema currently use
LayerTransitionFieldSchema.optional(), but the description and the mutations
handler expect null to explicitly remove a value; change both enterTransition
and exitTransition to LayerTransitionFieldSchema.nullish() so the schema accepts
undefined (omit = keep) and null (explicit remove) and aligns with the mutation
logic that checks for !== undefined and treats falsy/null as removal.

Comment on lines +121 to +156
function startPreview(presetId: string) {
hoveredPresetId = presetId;
previewTime = 0;

const startTime = performance.now();
const duration = 1200;

function animate(currentTime: number) {
const elapsed = currentTime - startTime;
previewTime = (elapsed % duration) / duration;
animationFrameId = requestAnimationFrame(animate);
}

animationFrameId = requestAnimationFrame(animate);
}

function stopPreview() {
hoveredPresetId = null;
previewTime = 0;
if (animationFrameId !== null) {
cancelAnimationFrame(animationFrameId);
animationFrameId = null;
}
}

function selectPreset(presetId: string) {
onchange(presetId);
open = false;
stopPreview();
}

$effect(() => {
if (!open) {
stopPreview();
}
});
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Potential requestAnimationFrame leak if the component is destroyed while the popover is open.

The $effect on line 152 calls stopPreview() only when open becomes falsy. If the component is unmounted while the popover is still open, the effect is torn down without calling stopPreview(), leaving the rAF loop running and setting state on a destroyed component.

Return a cleanup function from the effect (or add a dedicated teardown effect):

Suggested fix
   $effect(() => {
-    if (!open) {
-      stopPreview();
-    }
+    if (!open) {
+      stopPreview();
+    }
+    return () => {
+      stopPreview();
+    };
   });
📝 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.

Suggested change
function startPreview(presetId: string) {
hoveredPresetId = presetId;
previewTime = 0;
const startTime = performance.now();
const duration = 1200;
function animate(currentTime: number) {
const elapsed = currentTime - startTime;
previewTime = (elapsed % duration) / duration;
animationFrameId = requestAnimationFrame(animate);
}
animationFrameId = requestAnimationFrame(animate);
}
function stopPreview() {
hoveredPresetId = null;
previewTime = 0;
if (animationFrameId !== null) {
cancelAnimationFrame(animationFrameId);
animationFrameId = null;
}
}
function selectPreset(presetId: string) {
onchange(presetId);
open = false;
stopPreview();
}
$effect(() => {
if (!open) {
stopPreview();
}
});
$effect(() => {
if (!open) {
stopPreview();
}
return () => {
stopPreview();
};
});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/components/editor/panels/properties/animation-preset-select.svelte`
around lines 121 - 156, The effect watching open can leak requestAnimationFrame
if the component is destroyed while open; update the $effect that references
open to return a cleanup function that calls stopPreview() (or add an onDestroy
teardown that calls stopPreview()) so any pending animationFrameId is cancelled
when the effect is torn down or the component unmounts; locate the $effect block
and ensure it either returns () => stopPreview() or add an onDestroy(() =>
stopPreview()) to use the existing stopPreview/animationFrameId logic.

Comment on lines +72 to +93
<InputsWrapper
fields={[
{ for: 'enter-preset', labels: 'Animation' },
{ for: 'enter-duration', labels: 'Duration (s)' }
]}
>
<AnimationPresetSelect
value={enterPresetId}
options={enterPresets}
placeholder="None"
onchange={(v) => updateTransition('enter', v, enterDuration)}
/>
{#if enterPresetId}
<ScrubInput
id="enter-duration"
value={enterDuration}
min={0.1}
max={5}
step={0.1}
onchange={(v) => updateTransition('enter', enterPresetId, v)}
/>
{/if}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

fd -type f \( -name '*.svelte' -o -name '*.ts' -o -name '*.tsx' \) | xargs rg -l 'InputsWrapper' | head -20

Repository: epavanello/devmotion

Length of output: 622


🏁 Script executed:

cat src/lib/components/editor/panels/properties/groups/time-range-group.svelte | head -100

Repository: epavanello/devmotion

Length of output: 3507


🏁 Script executed:

fd -type f -name '*inputs-wrapper*'

Repository: epavanello/devmotion

Length of output: 234


🏁 Script executed:

cat src/lib/components/editor/panels/inputs-wrapper.svelte

Repository: epavanello/devmotion

Length of output: 1916


🏁 Script executed:

sed -n '95,125p' src/lib/components/editor/panels/properties/groups/time-range-group.svelte

Repository: epavanello/devmotion

Length of output: 873


Make fields dynamic in both transition sections to prevent orphaned labels

InputsWrapper creates label containers by iterating over the fields array, then renders children separately below. When fields declares 2 items but only 1 child is rendered (e.g., ScrubInput is conditional on enterPresetId), the "Duration (s)" label remains orphaned with no corresponding input. This creates layout misalignment and an accessibility issue—the label's for attribute points to a non-existent input.

Apply this fix to both the enter transition (lines 72–93) and exit transition (lines 99–121):

Fix
      <InputsWrapper
        fields={[
          { for: 'enter-preset', labels: 'Animation' },
+         ...(enterPresetId ? [{ for: 'enter-duration', labels: 'Duration (s)' }] : [])
-         { for: 'enter-duration', labels: 'Duration (s)' }
        ]}
      >

Repeat for the exit transition block, using exitPresetId instead of enterPresetId.

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

Suggested change
<InputsWrapper
fields={[
{ for: 'enter-preset', labels: 'Animation' },
{ for: 'enter-duration', labels: 'Duration (s)' }
]}
>
<AnimationPresetSelect
value={enterPresetId}
options={enterPresets}
placeholder="None"
onchange={(v) => updateTransition('enter', v, enterDuration)}
/>
{#if enterPresetId}
<ScrubInput
id="enter-duration"
value={enterDuration}
min={0.1}
max={5}
step={0.1}
onchange={(v) => updateTransition('enter', enterPresetId, v)}
/>
{/if}
<InputsWrapper
fields={[
{ for: 'enter-preset', labels: 'Animation' },
...(enterPresetId ? [{ for: 'enter-duration', labels: 'Duration (s)' }] : [])
]}
>
<AnimationPresetSelect
value={enterPresetId}
options={enterPresets}
placeholder="None"
onchange={(v) => updateTransition('enter', v, enterDuration)}
/>
{`#if` enterPresetId}
<ScrubInput
id="enter-duration"
value={enterDuration}
min={0.1}
max={5}
step={0.1}
onchange={(v) => updateTransition('enter', enterPresetId, v)}
/>
{/if}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/components/editor/panels/properties/groups/time-range-group.svelte`
around lines 72 - 93, InputsWrapper's fields list must be built dynamically so
the "Duration (s)" label is only emitted when the corresponding ScrubInput
exists; update the enter block to construct the fields array using enterPresetId
(e.g., include the {for: 'enter-duration', labels: 'Duration (s)'} entry only
when enterPresetId is truthy) and do the same for the exit block using
exitPresetId, ensuring the field `for` values ('enter-duration' /
'exit-duration') match the ScrubInput id attributes and the
AnimationPresetSelect entries remain first.

Comment on lines +78 to +83
<AnimationPresetSelect
value={enterPresetId}
options={enterPresets}
placeholder="None"
onchange={(v) => updateTransition('enter', v, enterDuration)}
/>
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

fd -e svelte 'animation-preset-select' --exec cat {}

Repository: epavanello/devmotion

Length of output: 6315


🏁 Script executed:

sed -n '70,90p' src/lib/components/editor/panels/properties/groups/time-range-group.svelte

Repository: epavanello/devmotion

Length of output: 688


🏁 Script executed:

fd -type f -name '*input*wrapper*' -o -name '*inputs-wrapper*' | head -20

Repository: epavanello/devmotion

Length of output: 234


🏁 Script executed:

fd 'inputs-wrapper' -e svelte

Repository: epavanello/devmotion

Length of output: 118


🏁 Script executed:

cat src/lib/components/editor/panels/inputs-wrapper.svelte

Repository: epavanello/devmotion

Length of output: 1916


🏁 Script executed:

sed -n '100,115p' src/lib/components/editor/panels/properties/groups/time-range-group.svelte

Repository: epavanello/devmotion

Length of output: 521


AnimationPresetSelect is missing an id prop — breaks label-for accessibility binding

InputsWrapper creates labels with for="enter-preset" and for="exit-preset", but AnimationPresetSelect doesn't expose or accept an id prop to match these associations. The label will be rendered but clicking it won't focus the button, breaking keyboard navigation and screen reader functionality.

Apply the same fix at both the enter preset (line 78) and exit preset (line 105) locations by adding id support to AnimationPresetSelect.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/components/editor/panels/properties/groups/time-range-group.svelte`
around lines 78 - 83, The label "for" targets (enter-preset and exit-preset)
don't work because AnimationPresetSelect doesn't accept or forward an id; update
AnimationPresetSelect to accept an id prop and ensure it is set on the
interactive element it renders (e.g., the underlying button or select) so label
clicks focus the control, and then pass id="enter-preset" and id="exit-preset"
where AnimationPresetSelect is used in time-range-group.svelte (the instances
around enterPresetId and exitPresetId); ensure any internal event handlers still
use the existing onchange/updateTransition logic.

Comment on lines +219 to +244
{#if enterTransitionWidth > 0}
<div
class="absolute top-0 bottom-0 left-0 w-1.5 cursor-col-resize rounded-l-sm bg-white/30 hover:bg-white/50"
onmousedown={startDragEnter}
></div>
class="pointer-events-none absolute top-0 bottom-0 left-0 rounded-l-sm bg-sky-400/25"
style:width="{Math.min(enterTransitionWidth, barWidth)}px"
>
{#if enterTransitionWidth > 20}
<span class="absolute top-1/2 left-1 -translate-y-1/2 text-[8px] text-sky-300/70">
In
</span>
{/if}
</div>
{/if}

<!-- Exit handle -->
<!-- svelte-ignore a11y_no_static_element_interactions -->
<!-- Exit transition zone -->
{#if exitTransitionWidth > 0}
<div
class="absolute top-0 right-0 bottom-0 w-1.5 cursor-col-resize rounded-r-sm bg-white/30 hover:bg-white/50"
onmousedown={startDragExit}
></div>

<!-- Label -->
{#if barLabel && barWidth > 40}
<span
class="pointer-events-none absolute top-1/2 left-2 -translate-y-1/2 text-[9px] font-medium text-white/60"
>
{barLabel}
</span>
{/if}
</div>
{/if}
class="pointer-events-none absolute top-0 right-0 bottom-0 rounded-r-sm bg-orange-400/25"
style:width="{Math.min(exitTransitionWidth, barWidth)}px"
>
{#if exitTransitionWidth > 20}
<span class="absolute top-1/2 right-1 -translate-y-1/2 text-[8px] text-orange-300/70">
Out
</span>
{/if}
</div>
{/if}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Enter and exit transition zones can visually overlap when their combined width exceeds the bar width.

Both zones are absolutely positioned from their respective edges (left-0 for enter, right-0 for exit) and individually clamped to barWidth, but there's no guard preventing them from overlapping in the middle when enterTransitionWidth + exitTransitionWidth > barWidth. This could cause confusing visuals on short layers with long transitions.

Consider clamping proportionally or hiding one when they'd overlap:

Example proportional clamping
+  const totalTransitionWidth = $derived(enterTransitionWidth + exitTransitionWidth);
+  const clampedEnterWidth = $derived(
+    totalTransitionWidth > barWidth && totalTransitionWidth > 0
+      ? (enterTransitionWidth / totalTransitionWidth) * barWidth
+      : Math.min(enterTransitionWidth, barWidth)
+  );
+  const clampedExitWidth = $derived(
+    totalTransitionWidth > barWidth && totalTransitionWidth > 0
+      ? (exitTransitionWidth / totalTransitionWidth) * barWidth
+      : Math.min(exitTransitionWidth, barWidth)
+  );

Then use clampedEnterWidth / clampedExitWidth in the style:width attributes.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/components/editor/timeline/timeline-layer.svelte` around lines 219 -
244, The enter/exit transition zones can overlap because enterTransitionWidth
and exitTransitionWidth are each clamped only to barWidth; update the logic in
TimelineLayer to compute clampedEnterWidth and clampedExitWidth so that
clampedEnterWidth + clampedExitWidth <= barWidth (e.g., if enter+exit <=
barWidth use min(each, barWidth), otherwise scale them proportionally:
clampedEnter = enter * barWidth / (enter+exit) and clampedExit = exit * barWidth
/ (enter+exit), or choose to hide the smaller zone when overlap would occur),
then replace uses of style:width="{Math.min(...)}px" with
style:width="{clampedEnterWidth}px" and style:width="{clampedExitWidth}px" and
adjust the conditional text labels to use those clamped widths.

Comment on lines +268 to +282
function mergeTransitionValues(a: TransitionValues, b: TransitionValues): TransitionValues {
return {
positionOffset: b.positionOffset ?? a.positionOffset,
rotationOffset: b.rotationOffset ?? a.rotationOffset,
scaleFactor: b.scaleFactor ?? a.scaleFactor,
opacityFactor:
a.opacityFactor !== undefined && b.opacityFactor !== undefined
? a.opacityFactor * b.opacityFactor
: (b.opacityFactor ?? a.opacityFactor),
blur: b.blur ?? a.blur,
brightness: b.brightness ?? a.brightness,
contrast: b.contrast ?? a.contrast,
saturate: b.saturate ?? a.saturate
};
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

mergeTransitionValues uses last-wins for position/rotation/scale but multiply for opacity — semantics diverge when enter and exit overlap.

When a layer is short enough that both enter and exit transitions are active simultaneously, position/rotation offsets from the exit transition silently replace the enter transition's offsets (via b ?? a), while opacity correctly multiplies. This produces an abrupt visual snap for position/scale at the moment the exit transition starts overlapping the enter transition.

If overlapping transitions are a supported scenario, consider making position/rotation additive and scale multiplicative (consistent with opacity):

Example fix for additive/multiplicative merge
 function mergeTransitionValues(a: TransitionValues, b: TransitionValues): TransitionValues {
   return {
-    positionOffset: b.positionOffset ?? a.positionOffset,
-    rotationOffset: b.rotationOffset ?? a.rotationOffset,
-    scaleFactor: b.scaleFactor ?? a.scaleFactor,
+    positionOffset:
+      a.positionOffset && b.positionOffset
+        ? {
+            x: a.positionOffset.x + b.positionOffset.x,
+            y: a.positionOffset.y + b.positionOffset.y,
+            z: a.positionOffset.z + b.positionOffset.z
+          }
+        : (b.positionOffset ?? a.positionOffset),
+    rotationOffset:
+      a.rotationOffset && b.rotationOffset
+        ? {
+            x: a.rotationOffset.x + b.rotationOffset.x,
+            y: a.rotationOffset.y + b.rotationOffset.y,
+            z: a.rotationOffset.z + b.rotationOffset.z
+          }
+        : (b.rotationOffset ?? a.rotationOffset),
+    scaleFactor:
+      a.scaleFactor && b.scaleFactor
+        ? { x: a.scaleFactor.x * b.scaleFactor.x, y: a.scaleFactor.y * b.scaleFactor.y }
+        : (b.scaleFactor ?? a.scaleFactor),
     opacityFactor:
       a.opacityFactor !== undefined && b.opacityFactor !== undefined
         ? a.opacityFactor * b.opacityFactor
         : (b.opacityFactor ?? a.opacityFactor),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/engine/layer-rendering.ts` around lines 268 - 282,
mergeTransitionValues currently uses last-wins (b ?? a) for positionOffset and
rotationOffset and multiplicative for opacityFactor, causing snaps when enter
and exit overlap; change it so positionOffset and rotationOffset are additive
(if both a.positionOffset and b.positionOffset exist, return their sum,
otherwise use the defined one), make scaleFactor multiplicative like
opacityFactor (if both defined return a.scaleFactor * b.scaleFactor, else pick
the defined one), keep opacityFactor multiplicative as-is, and leave
blur/brightness/contrast/saturate with the existing null-coalescing behavior;
update the logic in mergeTransitionValues to handle undefineds for these fields
accordingly.

Comment on lines +233 to +236
export const LayerTransitionSchema = z.object({
presetId: z.string().describe('ID of the animation preset to apply'),
duration: z.number().positive().describe('Duration of the transition in seconds')
});
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

duration has no upper bound — consider adding a .max() constraint

z.number().positive() allows arbitrarily large durations. While the UI enforces max={5}, AI-driven mutations and direct JSON manipulation bypass this. An unbounded transition duration longer than the layer's visible window is technically valid at the schema level but would produce unexpected visuals.

🛡️ Proposed fix
-  duration: z.number().positive().describe('Duration of the transition in seconds')
+  duration: z.number().positive().max(30).describe('Duration of the transition in seconds')
📝 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.

Suggested change
export const LayerTransitionSchema = z.object({
presetId: z.string().describe('ID of the animation preset to apply'),
duration: z.number().positive().describe('Duration of the transition in seconds')
});
export const LayerTransitionSchema = z.object({
presetId: z.string().describe('ID of the animation preset to apply'),
duration: z.number().positive().max(30).describe('Duration of the transition in seconds')
});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/schemas/animation.ts` around lines 233 - 236, LayerTransitionSchema's
duration property is currently unbounded (z.number().positive()), so update the
duration validator in LayerTransitionSchema to enforce the UI limit by adding a
maximum constraint (e.g., .max(5)) so it becomes z.number().positive().max(5)
and update the duration .describe text to mention the 5s maximum; this ensures
programmatic inputs match the UI constraint and prevents oversized transition
durations.

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.

2 participants