feat: add variety to loading messages with shimmer animation#208
Open
nhicks00 wants to merge 12 commits intompfaffenberger:mainfrom
Open
feat: add variety to loading messages with shimmer animation#208nhicks00 wants to merge 12 commits intompfaffenberger:mainfrom
nhicks00 wants to merge 12 commits intompfaffenberger:mainfrom
Conversation
added 7 commits
February 26, 2026 12:14
- Add loading_messages.py with 160+ fun/silly messages across 5 categories (puppy, dev, fun, action, standalone) - Add plugin registry (register_messages/unregister_messages) so plugins can inject their own message categories - Add shimmer.py: travelling highlight animation for Rich Text - Modify spinner_base.py: rotate through shuffled message deck per cycle - Modify console_spinner.py: use shimmer effect on thinking message - Update status_display.py: use shared message pool - Add comprehensive tests for loading_messages and shimmer modules Resolves genaica/code-puppy#248 (Walmart internal tracking)
- Add 'register_loading_messages' phase to callbacks.py so plugins
can inject custom loading messages via the standard callback system
- Fire the callback lazily in loading_messages.py on first access
- Plugins call register_messages(category, msgs) inside their callback
Example plugin usage:
register_callback('register_loading_messages', lambda: register_messages(
'my_org', ['doing org things...', 'synergizing...']))
- Add threading.Lock for TOCTOU race on _plugins_initialized (double-checked locking)
- Fix first message skip: init _message_index = -1 so _advance_message lands on 0
- Add empty deck fallback ('thinking...') to prevent IndexError
- Add shimmer param validation (speed > 0, width > 0, padding >= 0)
- Add defensive fallback in status_display for empty message list
- Fix get_messages_by_category to merge plugin 'standalone' instead of overwriting
- Make test assertions less brittle (no hardcoded strings, reset _plugins_initialized)
- _all_spinner_messages() now skips the 'standalone' category - get_all_messages() includes plugin standalone messages for status display - Updated register_messages docstring to document the standalone convention - Addresses remaining CodeRabbit comment on loading_messages.py:276
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a plugin-extensible loading message system, a shimmer text animation, and integrates rotating, shimmery messages into the spinner and status display; also exposes a new Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant Callbacks as code_puppy.callbacks
participant LoadMsg as code_puppy.messaging.loading_messages
participant Plugin as Plugin/Fork
App->>Callbacks: on_register_loading_messages()
activate Callbacks
Callbacks->>LoadMsg: trigger (lazy) _ensure_plugins_loaded()
activate LoadMsg
LoadMsg->>Plugin: invoke registered callbacks
Plugin->>LoadMsg: register_messages('category', [...])
LoadMsg->>LoadMsg: store plugin categories
deactivate LoadMsg
deactivate Callbacks
App->>LoadMsg: get_spinner_messages()
activate LoadMsg
LoadMsg->>LoadMsg: _all_spinner_messages() (built-in + plugins)
LoadMsg-->>App: shuffled spinner list
deactivate LoadMsg
sequenceDiagram
participant Spinner as SpinnerBase
participant Console as ConsoleSpinner
participant Shimmer as shimmer
participant Rich as RichText
Spinner->>Spinner: start() -> _advance_message()
Spinner->>Console: render frame
Console->>Spinner: current_thinking_message
Spinner-->>Console: "puppy_name" + deck[index]
Console->>Shimmer: shimmer_text(message)
activate Shimmer
Shimmer->>Shimmer: compute centre (time.monotonic)
Shimmer->>Rich: apply peak/mid/base styles per char
Shimmer-->>Console: styled RichText
deactivate Shimmer
Console->>Console: render panel with shimmer effect
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
tests/test_loading_messages.py (1)
65-70: Prefer API-based cleanup over mutating private globals.Directly editing
_plugin_categories/_plugins_initializedmakes tests more brittle and bypasses normal synchronization/initialization paths.♻️ Suggested teardown cleanup
def teardown_method(self): """Clean up any test categories.""" - _plugin_categories.pop("test_cat", None) - _plugin_categories.pop("test_cat2", None) - _lm._plugins_initialized = False + unregister_messages("test_cat") + unregister_messages("test_cat2")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_loading_messages.py` around lines 65 - 70, The teardown currently mutates private state directly (_plugin_categories.pop and setting _lm._plugins_initialized = False); replace this with the public API for unregistering/clearing plugin categories and resetting plugin state instead of touching private globals — e.g., call the plugin manager's unregister/remove methods for "test_cat" and "test_cat2" and use the library's public reset/initialize method on the _lm instance (referencing teardown_method, _plugin_categories and _lm._plugins_initialized in the diff) so tests clean up via supported APIs and keep initialization/synchronization logic intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code_puppy/messaging/loading_messages.py`:
- Around line 242-266: The register_messages function currently allows plugins
to register categories that collide with core names; update register_messages to
reject reserved core category names (e.g. "dev", "fun", "action", "puppy") by
checking category against a small reserved set (declare RESERVED_CATEGORIES =
{...}) inside the same _plugin_categories_lock block and raise a ValueError
(with a clear message) when a plugin attempts to register one of these reserved
names; leave the existing append/assignment behavior for non-reserved categories
intact and keep using _plugin_categories and _plugin_categories_lock as before.
- Around line 301-306: get_all_messages currently reads plugin data twice (once
for "standalone" and once indirectly via _all_spinner_messages), which can yield
inconsistent results under concurrent register/unregister; grab a single
consistent snapshot of _plugin_categories while holding _plugin_categories_lock,
e.g. inside the lock copy both _plugin_categories.get("standalone", []) and
whatever category data _all_spinner_messages needs (or modify
_all_spinner_messages to accept a snapshot), then release the lock and use those
copies together with _STANDALONE_MESSAGES to build the return value. Ensure you
reference get_all_messages, _plugin_categories_lock, _plugin_categories,
_all_spinner_messages, and _STANDALONE_MESSAGES when applying the change.
In `@code_puppy/messaging/spinner/spinner_base.py`:
- Around line 52-60: Make start() idempotent: if self._is_spinning is already
True, do nothing and return immediately so repeated calls won't advance the
deck; otherwise set self._is_spinning = True, reset self._frame_index and then
call self._advance_message(). Update the start() method to check
self._is_spinning first and only call _advance_message() when beginning a new
spin cycle to preserve per-cycle message locking.
In `@tests/test_loading_messages.py`:
- Around line 23-31: The test test_returns_shuffled_copy is flaky because it
probabilistically asserts two independent calls to get_spinner_messages()
differ; make it deterministic by controlling randomness: patch or seed the RNG
used by get_spinner_messages. For example, in test_returns_shuffled_copy use
unittest.mock.patch to replace random.shuffle (or the module's shuffle function
that get_spinner_messages imports) with a deterministic function or call
random.seed(...) before each get_spinner_messages invocation so the order is
predictable; update the test to assert the exact expected order for a given seed
or mocked shuffle while still asserting contents match via sorted(a) ==
sorted(b).
In `@tests/test_shimmer.py`:
- Line 44: The test currently asserts on the private attribute r1._spans which
is fragile; change the assertion to use Rich's public API by comparing r1.spans
and r2.spans instead (replace uses of _spans with spans for r1 and r2 in
tests/test_shimmer.py) so the test relies on the public property rather than
private internals.
---
Nitpick comments:
In `@tests/test_loading_messages.py`:
- Around line 65-70: The teardown currently mutates private state directly
(_plugin_categories.pop and setting _lm._plugins_initialized = False); replace
this with the public API for unregistering/clearing plugin categories and
resetting plugin state instead of touching private globals — e.g., call the
plugin manager's unregister/remove methods for "test_cat" and "test_cat2" and
use the library's public reset/initialize method on the _lm instance
(referencing teardown_method, _plugin_categories and _lm._plugins_initialized in
the diff) so tests clean up via supported APIs and keep
initialization/synchronization logic intact.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
code_puppy/callbacks.pycode_puppy/messaging/loading_messages.pycode_puppy/messaging/spinner/console_spinner.pycode_puppy/messaging/spinner/shimmer.pycode_puppy/messaging/spinner/spinner_base.pycode_puppy/status_display.pytests/test_console_spinner_coverage.pytests/test_loading_messages.pytests/test_shimmer.pytests/test_status_display.py
- Enforce reserved core category names in register_messages() - Make get_all_messages() use single plugin snapshot for consistency - Make SpinnerBase.start() idempotent (no-op if already spinning) - Replace flaky shuffle test with deterministic mock assertion - Use public Rich Text .spans instead of private ._spans - Use unregister_messages() API in test teardown - Add tests for reserved category and empty category validation
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code_puppy/messaging/loading_messages.py`:
- Around line 242-275: The doc says plugins may register "standalone" messages
but _RESERVED_CATEGORIES includes "standalone" causing register_messages to
reject it; fix by removing "standalone" from the reserved set (or add a targeted
exception in register_messages to allow category == "standalone") so
register_messages will accept plugin-provided "standalone" entries; update
references to _RESERVED_CATEGORIES and ensure register_messages still rejects
core names "puppy", "dev", "fun", "action" while allowing "standalone" to be
appended as documented.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
code_puppy/messaging/loading_messages.pycode_puppy/messaging/spinner/spinner_base.pytests/test_loading_messages.pytests/test_shimmer.py
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/test_shimmer.py
- tests/test_loading_messages.py
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
code_puppy/messaging/loading_messages.py (1)
232-239:⚠️ Potential issue | 🟠 MajorPlugin init can get stuck “initialized” after a callback failure.
_plugins_initializedis set toTruebefore callbacks run. Ifon_register_loading_messages()throws, future calls won’t retry plugin registration.🛠️ Proposed fix
def _ensure_plugins_loaded() -> None: @@ with _plugins_init_lock: if _plugins_initialized: return - _plugins_initialized = True - from code_puppy.callbacks import on_register_loading_messages - - on_register_loading_messages() + _plugins_initialized = True + try: + on_register_loading_messages() + except Exception: + _plugins_initialized = False + raise🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code_puppy/messaging/loading_messages.py` around lines 232 - 239, The code sets _plugins_initialized = True before running callbacks, which can leave plugins marked initialized if on_register_loading_messages() raises; change the logic in the block guarded by _plugins_init_lock so that _plugins_initialized is only set to True after on_register_loading_messages() completes successfully (or, alternatively, use a try/except around the callback and reset _plugins_initialized to False on exception); ensure you still hold _plugins_init_lock while running the callback and reference the existing symbols _plugins_init_lock, _plugins_initialized, and on_register_loading_messages to locate and update the flow.
🧹 Nitpick comments (1)
code_puppy/messaging/loading_messages.py (1)
269-272: Normalize category names before reserved-category checks.Whitespace variants like
" dev "currently bypass reserved-name validation and create awkward category keys.♻️ Proposed fix
if not isinstance(category, str) or not category.strip(): raise ValueError("category must be a non-empty string") + category = category.strip() if category in _RESERVED_CATEGORIES: raise ValueError(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code_puppy/messaging/loading_messages.py` around lines 269 - 272, Normalize the incoming category string before checking against reserved names: after verifying it's a non-empty string, immediately trim whitespace (and optionally normalize case to match how _RESERVED_CATEGORIES is defined) e.g. assign a normalized value back to the local variable (category = category.strip() and category = category.lower() if reserved names are lowercase) and then perform the membership check against _RESERVED_CATEGORIES so values like " dev " can't bypass reserved-name validation; use the normalized category for any subsequent logic that creates or stores category keys.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code_puppy/messaging/loading_messages.py`:
- Around line 257-259: The register_messages docstring incorrectly lists
"standalone" as a reserved core category while the implementation in
register_messages allows it; update the docstring in loading_messages.py for the
register_messages function to remove "standalone" from the reserved/core
category list and clarify that messages for existing categories are appended
(matching the code behavior), so the documentation aligns with the
implementation.
In `@tests/test_loading_messages.py`:
- Around line 69-74: teardown_method currently only resets
_lm._plugins_initialized but leaves plugin-registered categories in place;
update teardown_method to also clear/unregister all plugin message categories so
callbacks don't accumulate between tests — e.g., call unregister_messages for
the test categories or clear the module's plugin/category registry (the same
structure that register_messages uses) before setting _lm._plugins_initialized =
False, ensuring teardown_method fully resets plugin-related state (references:
teardown_method, unregister_messages, _lm._plugins_initialized, and the module's
plugin/category registry used by register_messages).
---
Duplicate comments:
In `@code_puppy/messaging/loading_messages.py`:
- Around line 232-239: The code sets _plugins_initialized = True before running
callbacks, which can leave plugins marked initialized if
on_register_loading_messages() raises; change the logic in the block guarded by
_plugins_init_lock so that _plugins_initialized is only set to True after
on_register_loading_messages() completes successfully (or, alternatively, use a
try/except around the callback and reset _plugins_initialized to False on
exception); ensure you still hold _plugins_init_lock while running the callback
and reference the existing symbols _plugins_init_lock, _plugins_initialized, and
on_register_loading_messages to locate and update the flow.
---
Nitpick comments:
In `@code_puppy/messaging/loading_messages.py`:
- Around line 269-272: Normalize the incoming category string before checking
against reserved names: after verifying it's a non-empty string, immediately
trim whitespace (and optionally normalize case to match how _RESERVED_CATEGORIES
is defined) e.g. assign a normalized value back to the local variable (category
= category.strip() and category = category.lower() if reserved names are
lowercase) and then perform the membership check against _RESERVED_CATEGORIES so
values like " dev " can't bypass reserved-name validation; use the normalized
category for any subsequent logic that creates or stores category keys.
- Fix docstring: standalone is not reserved (plugins can register it) - Add try/except around plugin init callback (reset flag on failure) - Clear all plugin categories in test teardown for clean slate
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
code_puppy/messaging/loading_messages.py (1)
304-315: Type hint style inconsistent with imports.Line 305 uses Python 3.10+ union syntax (
Dict[str, List[str]] | None) but the module importsDict, Listfromtyping(typically for Python 3.9 compatibility). Either useOptional[Dict[str, List[str]]]for consistency with older Python, or remove the typing imports if targeting 3.10+ only.♻️ Option A: Use Optional for pre-3.10 compatibility
-from typing import Dict, List +from typing import Dict, List, Optional ... def _all_spinner_messages( - snapshot: Dict[str, List[str]] | None = None, + snapshot: Optional[Dict[str, List[str]]] = None, ) -> List[str]:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code_puppy/messaging/loading_messages.py` around lines 304 - 315, The function _all_spinner_messages uses the Python 3.10 union syntax (Dict[str, List[str]] | None) while the module imports typing symbols Dict and List; change the annotation to Optional[Dict[str, List[str]]] and add Optional to the typing imports (or, if you prefer 3.10+ style everywhere, remove typing imports and standardize unions across the module) so the type hint style matches the existing imports and compatibility; update the function signature _all_spinner_messages(snapshot: Optional[Dict[str, List[str]]] = None) accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code_puppy/messaging/loading_messages.py`:
- Around line 235-243: The import of on_register_loading_messages should be
moved inside the try so import errors are caught and _plugins_initialized is
only set true after successful callback execution; change the flow in the block
referencing _plugins_initialized and on_register_loading_messages so you try:
import on_register_loading_messages; call on_register_loading_messages(); set
_plugins_initialized = True on success, and in except: ensure
_plugins_initialized remains False and re-raise the exception.
---
Nitpick comments:
In `@code_puppy/messaging/loading_messages.py`:
- Around line 304-315: The function _all_spinner_messages uses the Python 3.10
union syntax (Dict[str, List[str]] | None) while the module imports typing
symbols Dict and List; change the annotation to Optional[Dict[str, List[str]]]
and add Optional to the typing imports (or, if you prefer 3.10+ style
everywhere, remove typing imports and standardize unions across the module) so
the type hint style matches the existing imports and compatibility; update the
function signature _all_spinner_messages(snapshot: Optional[Dict[str,
List[str]]] = None) accordingly.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
code_puppy/messaging/loading_messages.pytests/test_loading_messages.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/test_loading_messages.py
- Move import inside try block in _ensure_plugins_loaded() so import failures properly reset _plugins_initialized flag - Normalize category names with strip() before reserved-name check to prevent whitespace bypass (e.g. " dev ") - Fix type hint inconsistency: use Optional[] to match existing typing imports instead of Python 3.10+ union syntax Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replaces the static "thinking..." spinner text with rotating fun/silly messages and a shimmer highlight animation. Adds a plugin hook so forks can inject their own org-specific messages without modifying core code.
Replaces #203 with a clean branch (no unrelated formatting churn).
What Changed
code_puppy/messaging/loading_messages.py): Plugin-extensible message registry with ~120 built-in messages across 5 categories (puppy, dev, fun, action, standalone). Thread-safe with_plugin_categories_lock.code_puppy/messaging/spinner/shimmer.py): Rich Text shimmer highlight that sweeps across the message text with configurable speed/width/padding.spinner_base.py): Shuffled deck rotation — no message repeats until the entire deck is exhausted. Defensive fallbacks for empty decks.callbacks.py): Newregister_loading_messagesphase so plugins can inject custom messages viaregister_messages(category, messages).status_display.py): Swapped hardcoded 15-message list for dynamicget_all_messages().Files Changed (10 files, +642/-25)
code_puppy/messaging/loading_messages.pycode_puppy/messaging/spinner/shimmer.pycode_puppy/messaging/spinner/spinner_base.pycode_puppy/messaging/spinner/console_spinner.pycode_puppy/callbacks.pyregister_loading_messagesphasecode_puppy/status_display.pytests/test_loading_messages.pytests/test_shimmer.pytests/test_status_display.pytests/test_console_spinner_coverage.pyResolves #202
Summary by CodeRabbit
New Features
Tests