fix: handle dirty state before first build

[ciolansteen]

Summary

Fix the Song Library blank screen on its first activation when a library change arrives before the Songs screen has built its DOM shell.

screen:changed remains the primary navigation contract. The fix also keeps a one-time active-state check for late script registration and coalesces duplicate entry delivery while retaining one genuine rapid re-entry.

Root cause

On cold startup, library:changed can mark the library dirty while state.built is still false and #v3-songs-grid does not exist. The first Song Library entry then took this path:

_libraryDirty -> reload() -> return

reload() assumes the Songs DOM shell already exists, so the first entry stayed blank. The second entry reached the full render path and created the shell.

The dirty path now reloads and returns only when the screen is already built; otherwise it falls through to the existing full render path.

Why remove the MutationObserver

Event-order instrumentation showed that songs.js evaluated, the event bus was available, listeners registered before navigation, and screen:changed was emitted and received. The class-only MutationObserver recovery did not repair the dirty-before-first-build path and was unnecessary once the actual mechanism was fixed, so it was removed rather than retained as a second lifecycle source.

TDD evidence

RED before the root fix:

• dirty, unbuilt first entry: expected reload=0, render=1; observed reload=1, render=0;
• rapid leave/re-entry while the first entry was in flight: expected two entries; observed one;
• observer-removal assertion failed while the defensive observer remained.

GREEN after the root fix:

• focused lifecycle tests: 8/8;
• all v3 Songs tests: 30/30;
• full JavaScript suite: 1216/1216;
• ESLint: 0 errors, 11 pre-existing warnings;
• syntax and diff checks: passed.

Reproduction and live verification

Ten cold-start trials per implementation:

Implementation	First click rendered	Second click rendered
Upstream-equivalent build	0/10	10/10
Previous defensive patch	0/10	10/10
Root fix	10/10	10/10

Root-fix stability checks:

• first-click entry count was exactly one in every trial;
• first-click rejections: 0/10;
• leave/re-entry produced exactly one entry per return;
• three repeated returns produced exactly three entries;
• delayed-fetch rapid leave/re-entry produced exactly two entries in every trial;
• final active Song Library contained its grid in 10/10 trials;
• candidate console errors: 0/10.

First-render latency samples in milliseconds:

169.5, 166.2, 156.4, 175.5, 154.8, 169.5, 190.8, 212.3, 174.8, 164.2

Median: 169.5 ms. No pass/fail latency threshold was applied.

Packaged-artifact check

A fresh signed downstream Arch package build completed successfully. Its packaged songs.js was byte-identical to the tested root-fix candidate:

SHA-256 330016a5ae6acd37dd7a540046af532851048686b1198920c2830ac9579fab27

Summary by CodeRabbit

• Bug Fixes

  • Fixed a blank first library entry when the library changes before the Songs screen initially renders.
  • Improved screen-entry handling to prevent unnecessary reloads and ensure updates appear correctly during navigation.
  • Preserved pending refreshes and retried screen initialization when an earlier attempt fails.

• Documentation

  • Added a changelog entry describing the library rendering fix.

[coderabbitai]

📝 Walkthrough

Walkthrough

The Songs screen now coordinates navigation entry events, avoids reloading before its DOM exists, and preserves queued or retried entry work. Tests cover event timing, repeated navigation, coalescing, rejection recovery, and initial rendering.

Changes

Songs lifecycle

Layer / File(s)	Summary
Entry coordination and dirty-library handling static/v3/songs.js, tests/js/v3_songs_entry_lifecycle.test.js	Songs entry work is serialized and duplicate requests are coalesced. A request received during execution triggers one follow-up run. Dirty-library state reloads only after the screen DOM exists. Tests cover missed events, queued entry, retries, coalescing, and initial rendering.
Navigation and initial synchronization static/v3/songs.js, tests/js/v3_songs_entry_lifecycle.test.js, CHANGELOG.md	Navigation events and DOMContentLoaded active-screen checks use the lifecycle coordinator. Tests verify repeated navigation, single registration, and the absence of MutationObserver wiring. The changelog records the deferred reload fix.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Navigation
  participant SongsEntryLifecycle
  participant SongsScreen
  Navigation->>SongsEntryLifecycle: detect Songs activation
  SongsEntryLifecycle->>SongsScreen: run entry work
  SongsScreen->>SongsScreen: reload only when DOM is built
  SongsEntryLifecycle-->>Navigation: preserve queued follow-up entry

Loading

Possibly related PRs

• got-feedBack/feedBack#896: Related library:changed handling connects library updates with Songs reload behavior.
• got-feedBack/feedBack#923: Related v3 Songs navigation lifecycle changes address a separate issue.

Suggested reviewers: chrisbewithyou, topkoa, byrongamatos

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely identifies the dirty-state fix required before the first Songs screen build.
Description check	✅ Passed	The description explains the root cause, implementation, lifecycle behavior, testing, and verification results, but omits the template checklist sections.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.}

[coderabbitai]

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@static/v3/songs.js`:
- Around line 3942-3949: Update onScreenChanged() so runEntry() is called only
on a false-to-true transition of wasActive, ignoring duplicate v3-songs
activation events while already active. Preserve the existing leave-event
handling so a genuine leave followed by rapid re-entry can still queue one
rerun.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 07f27fb5-f12a-441d-a086-0481d32cfef2

📥 Commits

Reviewing files that changed from the base of the PR and between eef58c8 and 6e4a8da.

📒 Files selected for processing (3)

• CHANGELOG.md
• static/v3/songs.js
• tests/js/v3_songs_entry_lifecycle.test.js