Skip to content

[codex] Avoid lazy loading local character avatars#1167

Merged
cha1latte merged 2 commits into
refactorfrom
fix/refactor-pr-53-local-avatar-loading
May 25, 2026
Merged

[codex] Avoid lazy loading local character avatars#1167
cha1latte merged 2 commits into
refactorfrom
fix/refactor-pr-53-local-avatar-loading

Conversation

@munimunigamer

@munimunigamer munimunigamer commented May 24, 2026

Copy link
Copy Markdown
Collaborator

Migrated from Pasta-Devs/Marinara-Engine-Refactor#53: Pasta-Devs/Marinara-Engine-Refactor#53
Original author: @Promansis
Target base: Pasta-Devs/Marinara-Engine refactor

Linked issue

N/A - migrated refactor PR; no matching avatar-loading issue found in this repo.

Why this change

Importing multiple PNG character cards can immediately refresh character avatar thumbnails with local inline image data. Those thumbnails were marked for lazy loading, which can produce browser console warnings around load timing even though the image source is already local to the app.

What changed

  • Added a characters-owned avatar loading helper that keeps data: and blob: avatar sources eager while preserving lazy loading for normal URLs.
  • Applied the helper through the shared CharacterAvatarImage component so current character avatar surfaces inherit the behavior.

Refactor impact

Primary owner:

Catalog characters / React UI.

Impact areas reviewed:

  • Character avatar rendering
  • Character Library avatar cards
  • Character panel/group avatar usage

Boundary notes:

The change stays in feature/catalog character UI code. It does not touch engine, storage, shared API, Rust, prompt assembly, imports, or remote runtime boundaries.

Pressure points touched:

None.

Validation

  • pnpm check passes locally
  • pnpm typecheck passes locally
  • pnpm build passes locally
  • pnpm check:architecture passes locally
  • pnpm check:docs passes locally
  • cargo check --manifest-path src-tauri/Cargo.toml --workspace passes locally
  • Rust clippy/tests were run for Rust behavior changes
  • Browser or Tauri app manual verification completed
  • Playwright, screenshot, or recording evidence added for UI changes
  • Remote runtime smoke checked when relevant

Manual verification notes

Codex ran pnpm check locally after resolving the refactor conflict. GitHub CI passed Frontend/Architecture/Organization, Browser Smoke and Performance, and Rust Capability Layer on commit 07c30364. CodeRabbit completed with no actionable comments.

A human can optionally smoke-test the original console-warning path by importing multiple PNG character cards in the Tauri app with devtools open and confirming local avatar render does not emit the lazy-loading warning.

Docs and release impact

  • No docs changes needed
  • Updated README.md
  • Updated CONTRIBUTING.md
  • Updated docs/developer/
  • Updated repo skills or AGENTS.md
  • Confirmed this PR does not restore old staging/package-workspace/release claims

UI evidence

No screenshot attached; this is a browser loading-attribute behavior change for local character avatar rendering.

@coderabbitai

coderabbitai Bot commented May 24, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

Character avatar rendering now determines image loading behavior dynamically. A new getCharacterAvatarLoadingMode function classifies avatar sources as "eager" for data: and blob: URLs, falling back to "lazy" for network sources. The CharacterAvatarImage component imports and applies this classifier instead of hard-coding loading="lazy".

Changes

Character Avatar Loading Mode Control

Layer / File(s) Summary
Avatar loading mode type and classifier
src/features/catalog/characters/lib/character-avatar-loading.ts
CharacterAvatarLoadingMode type ("eager" | "lazy") and getCharacterAvatarLoadingMode function classify avatar src values as eager for data: and blob: protocols, lazy for all others including null/undefined.
Dynamic image loading in CharacterAvatarImage
src/features/catalog/characters/components/CharacterAvatarImage.tsx
Component imports the loading classifier and applies getCharacterAvatarLoadingMode(src) to the <img> element's loading prop instead of using a fixed "lazy" value.

Estimated Code Review Effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly Related PRs

  • Pasta-Devs/Marinara-Engine#1169: Both PRs touch CharacterAvatarImage.tsx; the retrieved PR introduces the component with loading="lazy", while this PR updates that prop to compute dynamically via getCharacterAvatarLoadingMode(src).

Suggested Labels

client

Poem

✨ Ah, magnifico... the image loading protocol reveals itself.
Where network paths drift lazily through time,
Yet data and blob—those swift fragments—leap eager to render.
Such elegant classification! The machinery of perception perfected.
Another small step toward the grand design. ✓

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR addresses issue #37 scope mismatch: changes involve character avatar loading optimization, but issue #37 explicitly requires JavaScript extension system support with specific APIs—unrelated requirements. Either link to the correct issue for avatar loading optimization or implement the JavaScript extension system requirements specified in issue #37 (apiFetch, addStyle, addElement, on, timers, observe, onCleanup with auto-cleanup).
Out of Scope Changes check ⚠️ Warning The avatar loading optimization changes are entirely out of scope for issue #37, which requires JavaScript extension system implementation with a specific sandboxed API for extensions. Remove this PR or reassign to the correct linked issue addressing character avatar loading behavior; implement issue #37 requirements separately.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title '[codex] Avoid lazy loading local character avatars' accurately describes the main change: implementing smart loading behavior for character avatars to avoid lazy loading for local data/blob URLs.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check ✅ Passed The PR description is comprehensive and well-structured, addressing all key template sections including linked issue, motivation, changes, validation, and docs impact.

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

✨ 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 fix/refactor-pr-53-local-avatar-loading

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.

@github-actions github-actions Bot added the bugfix Bug fix label May 24, 2026
@cha1latte cha1latte self-assigned this May 25, 2026
@cha1latte cha1latte marked this pull request as ready for review May 25, 2026 20:24
@cha1latte

Copy link
Copy Markdown
Contributor

@coderabbitai review

@coderabbitai

coderabbitai Bot commented May 25, 2026

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

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.

@coderabbitai coderabbitai Bot added the client label May 25, 2026
@github-actions github-actions Bot removed the client label May 25, 2026
@cha1latte cha1latte merged commit 2d5283b into refactor May 25, 2026
10 checks passed
@cha1latte cha1latte deleted the fix/refactor-pr-53-local-avatar-loading branch May 25, 2026 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants