Cascade mark-level animation over the chart policy instead of replacing it

[Alek99]

Fixes #329.

The bug

Animation.to_spec() emits every field, and both the Python merge in _attach_keys and the client's _resolvedAnimation are plain dict spreads. A mark-level xy.animation(duration=90) was therefore a complete spec spread over the chart's, resetting everything the caller did not mention:

xy.scatter_chart(
    xy.scatter(x=x, y=y, key=keys, animation=xy.animation(duration=90)),
    xy.animation(match="key", easing="linear"),
)
# before: match="index", easing="ease-out"
# after:  match="key",   easing="linear",  duration=90

Two consequences, both silent:

1. Chart-level match="key" stopped working the moment any mark carried its own animation=. No error, no animation_fallback — the chart just index-matched.
2. The match='key' requires key= guard stopped firing, because it reads the same clobbered dict.

The fix

animation() records the caller's exact argument names behind sentinel defaults, so "set to the default" is distinguishable from "not set" — a mark passing match="index" against a chart's match="key" still wins. to_override_spec() returns only those fields. Animation(...) is public and directly constructible too; it falls back to diffing against the defaults, which is right except for that same explicit-default case.

The cascade resolves once, in Python, over a defaults base:

{**_animation_defaults(), **(chart_spec or {}), **(mark_override or {})}

A trace carrying an override ships the complete resolved policy. Shipping the sparse override instead would break a mark-level animation on a chart with no chart-level one — the client would read undefined for duration and easing. Resolving here keeps the client's spread a no-op instead of a second merge needing its own copy of the defaults. A trace with no override still ships no animation dict, and a chart with neither ships none, which is what keeps an unanimated chart static.

No JS change was needed.

Verification

• 59-combination cascade matrix — 6 chart specs × 10 mark overrides (including True/False/None), each replaying the client's exact spread and checking all 8 fields plus whether identity planes ship. 0 wrong.
• Also pinned: chart-level spec stays complete on the wire; a trace with no override ships no dict; an override ships a complete policy; on_start/on_end never leak into the spec or count as overrides; all five construction-time validations still raise.
• pytest tests/ — 2578 passed, 4 skipped
• ruff@0.15.8 check / format --check clean
• ty at the 14-diagnostic baseline with an identical diagnostic set (an earlier draft added 9; Animation(**passed) and tuple() over a heterogeneous dict were the cause)
• scripts/check_public_api.py OK

Composes with #330

Identity-plane shipping is derived from the resolved match, so key matching that starts working again automatically gets its planes back. Confirmed in the matrix: should_ship is computed from the expected resolved config and matched against the payload for all 59 combinations.

Test changes

Three tests asserted the old behavior as a literal:

• test_mark_animation_spec_clobbers_chart_level_fields — added in Ship animation identity planes only when the client can key-match #330 to pin the bug, now inverted into test_mark_animation_cascades_over_chart_level_fields.
• test_mark_animation_overrides_chart_defaults and test_disabled_mark_does_not_require_chart_level_key_matching_identity asserted trace["animation"] == {"enabled": False}; they now assert the resolved policy.

New: explicit-default override, cascade-restores-the-guard, mark-animation-alone-is-complete, and to_override_spec unit coverage.

Summary by CodeRabbit

• Bug Fixes

  • Mark-level animation settings now cascade field by field over chart-level animation policies.
  • Unspecified settings, such as matching mode and easing, are preserved instead of being reset.
  • Explicitly setting a default value still correctly overrides the chart policy.
  • Key-based animation matching and validation now remain active when configured.
  • Disabling animation on one mark no longer affects other marks.

• Documentation

  • Clarified animation cascading, overrides, and disabling behavior.

[coderabbitai]

📝 Walkthrough

Walkthrough

Mark-level animation policies now cascade field-by-field over chart-level settings. Explicit fields, including default-valued ones, are tracked; Python resolves the complete effective policy while trace payloads retain sparse overrides. Tests and documentation cover inheritance, key matching, defaults, and disabling.

Changes

Animation cascade resolution

Layer / File(s)	Summary
Track explicit animation fields python/xy/components.py	Animation instances distinguish unset parameters from explicitly provided values and expose sparse override specifications.
Resolve and transmit mark policies python/xy/components.py	Mark transition metadata merges defaults, chart policy, and sparse mark overrides before serializing trace animation data.
Document and test cascade behavior tests/test_animation.py, docs/components/marks.md, docs/styling/animations.md, spec/design/animation.md, CHANGELOG.md	Tests and documentation cover inherited fields, explicit default-valued overrides, key-match validation, complete defaults, disabled marks, and the updated behavior.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ChartAnimation
  participant MarkAnimation
  participant ApplyMarkTransitionMetadata
  participant Trace
  ChartAnimation->>ApplyMarkTransitionMetadata: provide chart animation spec
  MarkAnimation->>ApplyMarkTransitionMetadata: provide sparse override spec
  ApplyMarkTransitionMetadata->>ApplyMarkTransitionMetadata: merge defaults, chart spec, and mark override
  ApplyMarkTransitionMetadata->>Trace: assign resolved policy and sparse trace animation

Loading

Possibly related PRs

• reflex-dev/xy#330: Both changes use resolved animation policy when handling trace-level transition behavior and identity key planes.

Suggested reviewers: carlosabadia

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: cascading mark-level animation over chart policy instead of replacing it.
Linked Issues check	✅ Passed	The changes implement sparse mark overrides, preserve chart-level fields, keep match='key' validation, and update tests/docs for issue #329.
Out of Scope Changes check	✅ Passed	The diffs are limited to the animation fix, related tests, and documentation updates; no unrelated scope appears present.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch alek/fix-animation-spec-clobbering

---

_{Comment @coderabbitai help to get the list of available commands.}

[codspeed-hq]

Merging this PR will not alter performance

✅ 103 untouched benchmarks
⏩ 2 skipped benchmarks¹

---

_{Comparing alek/fix-animation-spec-clobbering (2db0fd4) with main (a8a6639)}

Footnotes

1. 2 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

[coderabbitai]

🧹 Nitpick comments (1)

python/xy/components.py (1)

445-464: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value

_animation_defaults() caches a mutable dict by reference.

lru_cache(maxsize=1) means every caller gets the exact same dict object back. Current call sites only spread it ({**_animation_defaults(), ...}) or read individual keys, so it's safe today, but any future code that mutates the returned dict in place (e.g. defaults.pop(...)) would silently corrupt the single cached instance for the process lifetime, affecting every subsequent chart. Consider returning a copy (or wrapping in MappingProxyType) to make the cache immutable by construction.

🛡️ Defensive fix

 `@lru_cache`(maxsize=1)
-def _animation_defaults() -> dict[str, Any]:
+def _animation_defaults() -> Mapping[str, Any]:
     """The complete default policy, and the base every cascade starts from."""
-    return Animation().to_spec()
+    return MappingProxyType(Animation().to_spec())

🤖 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 `@python/xy/components.py` around lines 445 - 464, Update _animation_defaults
so callers cannot mutate the single dictionary stored by its lru_cache entry;
return a fresh copy or an immutable mapping while preserving the existing
default values and lookup behavior used by to_override_spec.

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

Nitpick comments:
In `@python/xy/components.py`:
- Around line 445-464: Update _animation_defaults so callers cannot mutate the
single dictionary stored by its lru_cache entry; return a fresh copy or an
immutable mapping while preserving the existing default values and lookup
behavior used by to_override_spec.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: dd2381eb-5c24-41f6-93fd-210648786cce

📥 Commits

Reviewing files that changed from the base of the PR and between a8a6639 and 2db0fd4.

📒 Files selected for processing (6)

• CHANGELOG.md
• docs/components/marks.md
• docs/styling/animations.md
• python/xy/components.py
• spec/design/animation.md
• tests/test_animation.py