refactor(ui): panel-feather flourish review follow-ups#188
Conversation
Review follow-ups for the merged #182/#184 feather work: - Remove the dead close-burst path. Feathers emit on panel *open* only; the Close transition just clears them. The `transition` parameter threaded through emit -> seed_particles -> seed_particle and the Close `rotation_bias` branch were unreachable from the app (only a test still seeded with Close). Drop them. - panel_burst_emitter now falls back to Center on a zero-length edge (a panel collapsed to zero height/width) instead of seeding a degenerate edge line that stacks every particle on one point. Pinned with a test. - De-brittle the motion tests: assert the dust-vs-feather ordering invariants instead of absolute pixel margins coupled to current tuning, and assert lateral spread across the feather set rather than hunting for two particles with byte-identical seeds (which panicked on any seeding refactor). - Split the now over-budget side_panel_flourish.rs (>400) into emitter-geometry and motion suites sharing a flourish_support module. - Small cleanup: compute wobble phase advance once per tick; reconcile the design spec's Non-Goals with the shipped open-only behavior. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 3 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe ChangesFeather Flourish Transition Removal
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Panels as panels.rs
participant Flourish as PanelFlourish
participant Emitter as panel_burst_emitter
Panels->>Flourish: emit(panel, window_size, now)
Flourish->>Emitter: compute BurstEmitter (check span validity)
alt non-degenerate edge span
Emitter-->>Flourish: edge BurstEmitter
else degenerate/floating panel
Emitter-->>Flourish: center BurstEmitter
end
Flourish->>Flourish: seed_particles(emitter)
Flourish->>Flourish: seed_particle (rotation from scatter, no transition bias)
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/superpowers/specs/2026-06-26-panel-feather-edge-emitter-design.md (1)
83-95: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDesign doc still stale on the emit signature and rotation bias outside the Non-Goals bullet.
Non-Goals was updated for the open-only behavior, but:
- Data Flow (Lines 101-107) still shows
PanelFlourish::emit(panel, window, transition, now); the shipped signature dropstransition(seesrc/ui/side_panel/flourish.rsLines 120-122).- Particle Seeding (Lines 92-95) still says velocity keeps "close-transition rotation bias," but
seed_particleno longer applies any transition-based bias.- Testing (Lines 133-136) only lists
cargo test --test side_panel_flourish; the newside_panel_flourish_emittertest binary isn't mentioned.📝 Suggested doc touch-ups
-PanelFlourish::emit(panel, window, transition, now) +PanelFlourish::emit(panel, window, now)-jitter prevents the particles from looking mechanically spaced. Velocity keeps -the current outward direction, scatter, speed variation, close-transition -rotation bias, gravity, and cursor interaction. +jitter prevents the particles from looking mechanically spaced. Velocity keeps +the current outward direction, scatter, speed variation, gravity, and cursor +interaction.cargo test --test side_panel_flourish +cargo test --test side_panel_flourish_emitter cargo test app::panels::testsAlso applies to: 97-107, 131-136
🤖 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 `@docs/superpowers/specs/2026-06-26-panel-feather-edge-emitter-design.md` around lines 83 - 95, Update the panel feather edge emitter spec to match the shipped API and behavior: in the Data Flow section, change the `PanelFlourish::emit` signature reference to the current `PanelFlourish::emit(panel, window, now)` shape from `flourish.rs`; in Particle Seeding, remove the mention of “close-transition rotation bias” since `seed_particle` no longer uses transition-based bias; and in Testing, add the `side_panel_flourish_emitter` test binary alongside `side_panel_flourish` so the doc reflects both test targets.
🤖 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 `@docs/superpowers/specs/2026-06-26-panel-feather-edge-emitter-design.md`:
- Around line 83-95: Update the panel feather edge emitter spec to match the
shipped API and behavior: in the Data Flow section, change the
`PanelFlourish::emit` signature reference to the current
`PanelFlourish::emit(panel, window, now)` shape from `flourish.rs`; in Particle
Seeding, remove the mention of “close-transition rotation bias” since
`seed_particle` no longer uses transition-based bias; and in Testing, add the
`side_panel_flourish_emitter` test binary alongside `side_panel_flourish` so the
doc reflects both test targets.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ad11dbed-c9fc-4c01-9130-82b556425a81
📒 Files selected for processing (6)
docs/superpowers/specs/2026-06-26-panel-feather-edge-emitter-design.mdsrc/app/panels.rssrc/ui/side_panel/flourish.rstests/flourish_support/mod.rstests/side_panel_flourish.rstests/side_panel_flourish_emitter.rs
Fixes the CI lint (cargo fmt --check) failure on this branch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The spec still showed the removed transition parameter on PanelFlourish::emit, mentioned the dropped close-transition rotation bias, and omitted the side_panel_flourish_emitter test binary. Addresses CodeRabbit nitpick on PR #188. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
CodeRabbit body nitpick addressed: the feather emitter design doc was stale on the shipped API. Fixed in c6ea92d — The CI lint failure was a pure |
Both advisories are DoS-via-untrusted-XML in quick-xml < 0.41. The sole consumer is wayland-scanner, a build-time proc-macro parsing vendored Wayland protocol XML; no untrusted XML is parsed at runtime, so neither issue is reachable in the shipped binary. wayland-scanner 0.31.10 (latest) still pins quick-xml ^0.39 — drop these ignores when it moves to 0.41. Closes #192. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 participant
Why
Follow-ups from a local review of the recently-merged, CodeRabbit-unreviewed feather work (#182 "spread feathers across full edge", #184 "smooth feather motion"). These address the dead code, a degenerate-input bug, and test brittleness that review surfaced.
What
transitionparameter threaded throughemit → seed_particles → seed_particleand the Closerotation_biasbranch were unreachable from the app (only a test still seeded withClose).panel_burst_emitternow falls back toCenteron a zero-length edge (a panel collapsed to zero height/width) instead of seeding a zero-length edge line that stacks every particle on one point.panic!d on any seeding refactor).side_panel_flourish.rs(>400 lines) into emitter-geometry and motion suites sharing aflourish_supportmodule.Testing
cargo test)cargo clippy --all-targets --all-features -- -D warnings)🤖 Co-authored by Claude Opus 4.8. Follow-up to #182 / #184.
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation