[nasagong]

Summary

Fixes #8729

Imported .codex-pet bundles play every sprite animation at a uniform 8 fps, roughly 9x faster than Codex intends, and the pet overlay lacks the Codex mascot's pointer interactions.

applyCodexPetDefaults mirrors Codex's animation rows (idle=0 … review=8) but flattens the pacing into a sheet-wide fps: 8. Codex paces frames unevenly: idle holds [1680, 660, 660, 840, 840, 1920] ms (a calm 6.6 s ambient cycle), and app-state rows run 120–150 ms/frame with a longer final hold (codex-rs/tui/src/pets/model.rs; the Codex App renderer plays the same values). At uniform 8 fps the idle poses cycle in 0.75 s, so pets frantically flip through poses regardless of state.

• fix(pet): SpriteAnimation gains optional frameDurationsMs; the Codex default tables (now with durations) move to src/shared/codex-pet-sprite-defaults.ts so main bakes them at import and the renderer reuses the same fingerprint. SpriteFrame renders uneven holds via one step-end keyframe stop per frame; uniform-fps sprites keep the existing steps() path. Pets imported before durations existed persist the exact legacy default fingerprint, so usePetUrl upgrades them in place — no re-import needed.
• feat(pet): pointer interactions align with the Codex App mascot. Hover plays jumping, grabbing snaps the pet to frame 0 of its live agent state and holds there, and dragging horizontally plays running-right/running-left with a 4px deadzone that keeps the last direction on vertical moves. Live agent states resume on release/leave.

Screenshots

Before (uniform 8 fps)	After (Codex per-frame pacing)

Testing

• pnpm lint
• pnpm typecheck
• pnpm test — no failures introduced by this branch (the 28 locally-failing tests fail identically on main in this environment; one flaky pty-connection test passes on rerun)
• pnpm build
• Added regression tests: duration-table invariants (lengths match frame counts, idle totals 6600 ms), legacy-fingerprint upgrade (hand-authored layouts/fps/durations pass through untouched), per-frame step-end keyframe emission, bundle-import length validation, and pointer interaction selection (hover, grab-and-hold, drag direction hysteresis)
• Live verification: launched the built app against a profile persisted by the previous build (legacy sprite without durations) and confirmed via CDP that the overlay renders 6.6s step-end infinite with measured ~1.7 s / ~1.9 s holds on the bookend frames, that hover switches to the jumping row, and that grab/drag play the held state and running rows

AI Review Report

• Ran a multi-angle review (line-by-line, removed-behavior, cross-file tracing, reuse/simplification/efficiency, conventions) with verification passes on the correctness candidates.
• Removed-behavior + cross-file: the CODEX_PET_* move keeps all consumers resolving via re-exports; frameDurationsMs is optional and round-trips the permissive persisted-UI/RPC schemas; blob-cache skip-detection stays consistent (the upgrade never changes dims/fps).
• A "hover state can latch and mask agent states" candidate was refuted against the Pointer Events spec: pointercancel/pointerup/capture-release all mandate pointerleave at the capture target, which is the same element carrying the leave handler.
• Confirmed-but-accepted edge: toFixed(4) stop quantization can collapse two keyframe stops for adversarial manifests (a sub-30 ms frame inside a ~30 s total, hand-crafted input only); the dropped frame is ≤1e-6 of the cycle, so impact is nil. Noted for the record.
• Cross-platform: explicitly checked for macOS/Linux/Windows — no shortcuts, labels, paths, or shell behavior touched; renderer CSS and standard Pointer Events are platform-neutral, and pet assets load from local userData over IPC, so SSH-hosted workspaces are unaffected.

Security Audit

The new manifest field is bounded by the existing zod schema (≤512 entries, each positive ≤60 s) plus an import-time length===frames check with a clear error; the manifest size cap and existing path-traversal/symlink/TOCTOU defenses in pet:importPetBundle are unchanged, and no new IPC channels or dependencies are added. Generated keyframe CSS interpolates only numbers that passed Number.isFinite && > 0 render guards (hand-edited persisted state is re-guarded independently, falling back to the uniform path), and the keyframes identifier keeps the pre-existing sanitizer.

Notes

• Codex plays app-state animations 3× and then settles into the slow idle even while the state persists; Orca keeps looping the current state's row (its overlay is a live status indicator). Happy to align in a follow-up if desired.
• v2 sheets (spriteVersionNumber: 2, 11 rows) carry 16 look-direction poses in rows 9–10 (pointer-gaze in the Codex App); intentionally unused here, as in the current defaults.
• The legacy upgrade is a render-time shim by design: persisted data is never rewritten, so the change is fully reversible on downgrade.

[coderabbitai]

📝 Walkthrough

Walkthrough

Pet sprite animation manifests now support per-frame durations with import validation. Shared Codex defaults upgrade eligible legacy sprites while preserving custom timing layouts. Renderer animation selection now handles drag direction and hover state through a unified pointer interaction hook. PetOverlay generates duration-based keyframes when available, restarts sprite animations across drag generations, and separates sprite motion from bobbing. Tests cover shared defaults, bundle validation, animation selection, pointer behavior, and rendered CSS.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 55.56% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The PR implements Codex per-frame timing, legacy upgrades, and the requested hover/grab/drag interactions for #8729.
Out of Scope Changes check	✅ Passed	The added tests, defaults, renderer updates, and shared types all support the stated pet timing and interaction goals.
Title check	✅ Passed	The title clearly summarizes the main change: Codex pet pacing and pointer interactions for imported pets.
Description check	✅ Passed	The description includes all required sections and adequately covers summary, screenshots, testing, review, security, and notes.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[nasagong]

@coderabbitai review

[coderabbitai]

The PR description image "issue" shows a dark-themed app titled "Clawd" with a small pixel-art pet character (an orange creature wearing a brown hat) displayed in the corner near a status bar showing icons and "2 hosts" with a green indicator.

The PR description image "issue2" shows the Orca app's welcome screen with the Orca logo and a pixel-art octopus/pet character wearing blue headphones, alongside Korean text inviting the user to add a project or create a workspace.

@nasagong, I'll review the changes now.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[brennanb2025]

Taking a look!

[nwparker]

Took this over and rebased it onto current main, then ran it through several rounds of adversarial review (GPT‑5.6‑Sol at max reasoning). The original approach is solid — mirroring Codex's exact model.rs duration tables (verified against upstream: idle [1680,660,660,840,840,1920], app‑state rows 120–150ms + longer final hold) and the step-end keyframe rendering are spot on. Layered these fixes on top (all with tests, typecheck, lint, and the full pet suite green):

Correctness

• Grab‑and‑hold now actually holds. spriteAnimate stayed true through a drag, so a stationary grab restarted to frame 0 then kept playing. It now freezes on frame 0 of the live state for a still/vertical grab and only animates once a horizontal drag crosses the deadzone.
• Each row starts at frame 0. The @keyframes name is keyed on the animation (and SpriteFrame is remounted per pet via key={url}), so hover/agent‑state/direction changes and pet switches no longer inherit the previous animation's currentTime and enter mid‑cycle (confirmed in Chromium).
• Drag direction tracks horizontal travel only, with the baseline advanced solely on an accepted direction — a slow diagonal drag now accumulates to the threshold instead of resetting every vertical component and never firing.
• Pointer hook scoped to its pointer (activePointerRef) so a second touch can't hijack the anchors and only the owning pointer's up/cancel — plus lost capture — ends the drag.
• Explicit fps honored for every value (incl. 8) by baking it as uniform per‑frame durations, which also keeps the sprite out of the legacy‑retiming path.
• Keyframe fallback guards: degrade to uniform steps() if two stops collapse or the final stop rounds to 100%, so no frame is silently dropped.
• Hardened against untrusted persisted data: null animation entries and non‑array frameDurationsMs fall back cleanly instead of throwing during render.

Structure

• Extracted the keyframe generation into a pure, unit‑testable buildSpriteAnimationCss, and the legacy detection into isLegacyCodexSprite (full‑geometry fingerprint so a hand‑authored sheet that merely reuses the row map isn't retimed).

Deliberate trade‑offs (documented in code): the legacy upgrade is a render‑time shim keyed on geometry, not a persisted provenance bit (a byte‑identical pre‑existing hand‑authored Codex‑layout sheet wanting uniform pacing would be retimed; opt‑out = declare any frameDurationsMs); and Orca keeps looping the current agent‑state row rather than Codex's 3×‑then‑idle, since the overlay is a live status indicator.

Credit to @nasagong for the original fix and the faithful Codex mirroring. Co‑authored on the follow‑up commits.