Skip to content

Skip inactive animation key encoding#182

Closed
Alek99 wants to merge 1 commit into
mainfrom
agent/skip-inactive-animation-keys
Closed

Skip inactive animation key encoding#182
Alek99 wants to merge 1 commit into
mainfrom
agent/skip-inactive-animation-keys

Conversation

@Alek99

@Alek99 Alek99 commented Jul 22, 2026

Copy link
Copy Markdown
Member

Root cause

Declarative key= values were always resolved, validated, canonicalized, hashed, retained, and serialized, even when no effective animation policy could perform keyed updates. A normal 100k scatter with animation=False therefore spent O(N) Python-object work building two identity columns and shipped an extra 8 bytes per row that the browser never consumed. Keys above the browser's 200k matching bound also paid the full digest cost before the payload layer discarded them.

Changes

  • resolve and encode keys only when the merged chart/mark policy:
    • is present
    • is not explicitly disabled
    • uses match="key"
    • has update != "none"
  • retain enabled="auto" as an effective policy because reduced-motion preference is browser-resolved
  • leave inactive keys inert, including no-policy, index/append match, disabled, and update-disabled cases
  • skip per-row validation/digest work above MAX_ANIMATION_MATCH_ROWS and carry an internal explicit fallback to the payload
  • preserve snap:key-limit for direct traces and snap:aggregate for aggregate tiers
  • keep bounded active keyed payloads and their binary identity columns unchanged
  • add CodSpeed coverage for inactive construction and payload assembly
  • document the effective-policy and bounded-fallback contract

No JavaScript or wire-protocol shape changes are required.

Measured impact

100k-row scatter with string keys and animation explicitly disabled; 9 warmed construction repetitions and 15 payload repetitions:

metric main this branch impact
chart construction median 75.976 ms 0.250 ms ~304× faster
payload median 0.224 ms 0.129 ms 42.4% lower
payload bytes 1,600,000 800,000 50% lower
u32 identity columns 2 0 removed

A real 200,001-row direct chart additionally produced snap:key-limit, zero u32 identity columns, and no key digest.

Active keyed scatter, stable-sorted line, and role-expanded errorbar specs/blobs were SHA-256 byte-identical to current main.

Verification

  • full suite: 2237 passed, 66 skipped, 1 deselected
  • pytest -q tests/test_animation.py benchmarks/test_codspeed_animation.py: 71 passed
  • override matrix covers no policy, inherited/overridden key/index match, disabled/auto/enabled, and update="none"
  • all supported keyed mark families cover direct/aggregate over-limit fallbacks and no-digest behavior
  • bounded key validation, sort order, errorbar role expansion, and payload parity remain covered
  • Ruff full-tree check and format check: passed
  • changed Python modules: ty passed
  • changed-file pre-commit: passed
  • git diff --check: passed

The one deselection is the known current-main minified-bundle marker mismatch in tests/reflex_adapter/test_assets.py; it reproduces unchanged at b0f8780 and is already fixed by #177.

Closes #172

@greptile-apps

greptile-apps Bot commented Jul 22, 2026

Copy link
Copy Markdown

Greptile Summary

This PR skips animation-key encoding when keyed updates cannot run. The main changes are:

  • Gates key resolution and hashing on the merged animation policy.
  • Adds an explicit fallback for keyed traces above the browser matching limit.
  • Preserves aggregate-tier fallback behavior without shipping identity columns.
  • Adds policy, payload, fallback, benchmark, and documentation coverage.

Confidence Score: 5/5

This looks safe to merge.

No blocking issues were found in the changed code. The policy gate matches the documented effective-animation rules, and fallback-only state reaches direct and aggregate payload paths without adding identity columns.

None.

T-Rex T-Rex Logs

What T-Rex did

  • Ran a focused test suite and repository-backed probes, and both exited with status 0.
  • Examined the over-limit direct trace, which remained direct and showed a transition_key_fallback value of snap:key-limit without identity payload columns.
  • Verified the inactive trace exposed neither fallback nor transition-key state, demonstrating policy gating.

View all artifacts

T-Rex Ran code and verified through T-Rex

Important Files Changed

Filename Overview
python/xy/components.py Gates key processing on effective keyed updates and records a fallback when the row limit is exceeded.
python/xy/_payload.py Serializes fallback-only transition metadata while retaining aggregate-tier fallback precedence.
python/xy/_trace.py Adds optional trace state for an over-limit key fallback.
tests/test_animation.py Covers policy overrides, inactive keys, row-limit fallbacks, supported marks, and binary payload behavior.
benchmarks/test_codspeed_animation.py Measures inactive-key construction and payload assembly alongside active keyed payloads.

Reviews (1): Last reviewed commit: "Skip inactive animation key encoding" | Re-trigger Greptile

@codspeed-hq

codspeed-hq Bot commented Jul 22, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 97 untouched benchmarks
🆕 2 new benchmarks
⏩ 1 skipped benchmark1

Performance Changes

Benchmark BASE HEAD Efficiency
🆕 test_animation_inactive_key_build_100k N/A 4.2 ms N/A
🆕 test_animation_inactive_key_payload_100k N/A 3.3 ms N/A

Comparing agent/skip-inactive-animation-keys (b77b8e3) with main (b0f8780)

Open in CodSpeed

Footnotes

  1. 1 benchmark was skipped, so the baseline result was used instead. If it was deleted from the codebase, click here and archive it to remove it from the performance reports.

@Alek99 Alek99 closed this Jul 24, 2026
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.

Animation identity keys ship 8 B/row even when animation is disabled

1 participant