feat(v3 library): batch→popup handoff + English-base romaji (metadata-curation capstone)#782
Merged
Merged
Conversation
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds per-song catalog field overrides with optional locks, backed by a new SQLite table and GET/PUT APIs. Query/list responses overlay overrides on pack values with romaji fallback; genre filtering respects overrides. Enrichment and gap-fill honor locks. Frontend updates fix-metadata value adoption and adds a clickable unmatched badge. ChangesBackend overrides, storage, and enrichment/gap-fill locking
Estimated code review effort: 3 (Moderate) | ~30 minutes Fix-metadata editor and unmatched badge UI
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
static/v3/songs.js (1)
3845-3856: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
toggleUnmatchedFilterdoesn't refresh the Filters count badge.
render()callsupdateFilterBadge()after building the toolbar (Line 3640), implying filter-count display is meant to trackstate.filters. This new toggle mutatesstate.filters.matchdirectly but skips that call, so the "Filters" button's count chip (v3-songs-filter-count) goes stale after using this shortcut until the next full render.♻️ Suggested fix
function toggleUnmatchedFilter() { const m = state.filters.match || (state.filters.match = []); const i = m.indexOf('unmatched'); const on = i < 0; if (on) m.push('unmatched'); else m.splice(i, 1); const btn = document.getElementById('v3-songs-unmatched'); if (btn) { btn.classList.toggle('bg-fb-primary', on); btn.classList.toggle('text-white', on); } + updateFilterBadge(); reload(); }🤖 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 `@static/v3/songs.js` around lines 3845 - 3856, The toggleUnmatchedFilter shortcut updates state.filters.match but does not refresh the Filters count badge, so the v3-songs-filter-count chip can become stale. After mutating the match filter and before or after calling reload(), invoke the same badge refresh path used by render(), specifically updateFilterBadge(), so the toolbar count stays in sync with state.filters whenever this shortcut is used.
🤖 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 501-533: The click handler for the [data-meta-fix] badge is
getting lost when enrichBadge is re-rendered by _patchCardEnrich() via
outerHTML, so the live-updated badge no longer opens Fix metadata. Update the
click wiring in wireCards() to use delegated handling from a stable parent
element, or ensure the handler is reattached after _patchCardEnrich() replaces
the node, so clicks on the nomatch badge continue to trigger the metadata fix
flow instead of the card play handler.
---
Nitpick comments:
In `@static/v3/songs.js`:
- Around line 3845-3856: The toggleUnmatchedFilter shortcut updates
state.filters.match but does not refresh the Filters count badge, so the
v3-songs-filter-count chip can become stale. After mutating the match filter and
before or after calling reload(), invoke the same badge refresh path used by
render(), specifically updateFilterBadge(), so the toolbar count stays in sync
with state.filters whenever this shortcut is used.
🪄 Autofix (Beta)
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: 6c0f0575-2840-46f5-ba07-e1f2af2a4c14
📒 Files selected for processing (8)
server.pystatic/tailwind.min.cssstatic/v3/match-review.jsstatic/v3/songs.jstests/test_field_overrides.pytests/test_gap_fill.pytests/test_library_filters.pytests/test_mb_enrichment.py
Comment on lines
+501
to
+533
| // Cards whose enrichment landed 'failed' (from the grid payload) — tracked so | ||
| // the PERSISTENT "no match" badge survives a batch tile clearing (a | ||
| // _patchCardEnrich with no flag falls back to this instead of wiping it). | ||
| // Populated as cards render (enrichBadge is called per card with the flag). | ||
| const _unmatched = new Set(); | ||
| function enrichBadge(fn, unmatched) { | ||
| if (unmatched !== undefined) { if (unmatched) _unmatched.add(fn); else _unmatched.delete(fn); } | ||
| // A live batch tile wins over the resting no-match marker (they never | ||
| // coexist — the batch clears its tiles when it finishes). | ||
| const st = _metaTile[fn] || (_unmatched.has(fn) ? 'nomatch' : null); | ||
| if (!st) return ''; | ||
| const M = { | ||
| queued: ['bg-black/60 text-fb-textDim', '• Queued'], | ||
| working: ['bg-fb-primary text-white', '⟳ Matching…'], | ||
| done: ['bg-fb-good/90 text-black', '✓ Updated'], | ||
| nochange: ['bg-black/60 text-fb-textDim', '— No match'], | ||
| queued: ['bg-black/60 text-fb-textDim', '• Queued', ''], | ||
| working: ['bg-fb-primary text-white', '⟳ Matching…', ''], | ||
| done: ['bg-fb-good/90 text-black', '✓ Updated', ''], | ||
| nochange: ['bg-black/60 text-fb-textDim', '— No match', ''], | ||
| // Resting indicator: subtle, so a mostly-unmatched library isn't a | ||
| // wall of loud badges. Clickable — a one-click handoff into the | ||
| // Fix-metadata popup for this song (see the [data-meta-fix] wiring). | ||
| nomatch: ['bg-black/60 text-fb-textDim', 'No match', 'Click to fix the metadata by hand'], | ||
| }; | ||
| const conf = M[st] || M.queued; | ||
| const fixable = st === 'nomatch'; // resting badge → opens Fix-metadata | ||
| // top-10 clears the tuning chip (top-2) in both normal and select mode; | ||
| // z-20 sits it above the art. Non-interactive so it never eats a click. | ||
| return '<span class="v3-meta-tile absolute top-10 left-2 z-20 ' + conf[0] + | ||
| ' text-[0.5625rem] font-bold px-1.5 py-0.5 rounded-sm leading-tight pointer-events-none">' + | ||
| conf[1] + '</span>'; | ||
| // z-20 sits it above the art. Batch states are non-interactive; the | ||
| // resting "no match" badge is the handoff into the popup. | ||
| const cls = 'v3-meta-tile absolute top-10 left-2 z-20 ' + conf[0] + | ||
| ' text-[0.5625rem] font-bold px-1.5 py-0.5 rounded-sm leading-tight ' + | ||
| (fixable ? 'pointer-events-auto cursor-pointer hover:bg-fb-primary hover:text-white transition-colors' : 'pointer-events-none'); | ||
| return '<span class="' + cls + '"' + | ||
| (fixable ? ' data-meta-fix="1"' : '') + | ||
| (conf[2] ? ' title="' + conf[2] + '"' : '') + | ||
| '>' + conf[1] + '</span>'; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant structure first
ast-grep outline static/v3/songs.js --view expanded
# Read the badge wiring and patching regions with line numbers
sed -n '460,560p' static/v3/songs.js | cat -n
sed -n '3850,3895p' static/v3/songs.js | cat -n
# Find every caller of _patchCardEnrich and every data-meta-fix wiring
rg -n "_patchCardEnrich\\(|data-meta-fix|wireCards\\(" static/v3/songs.jsRepository: got-feedBack/feedBack
Length of output: 10641
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1468,1492p' static/v3/songs.js | cat -n
sed -n '3868,3880p' static/v3/songs.js | cat -n
rg -n "meta-fix|addEventListener\\('click'|closest\\('\\[data-meta-fix\\]'" static/v3/songs.jsRepository: got-feedBack/feedBack
Length of output: 12994
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1447,1565p' static/v3/songs.js | cat -nRepository: got-feedBack/feedBack
Length of output: 8117
Badge clicks need delegated handling after live enrichment updates. _patchCardEnrich() replaces the badge node with outerHTML, so the [data-meta-fix] listener attached in wireCards() is lost on the live-update path. The recreated badge still looks clickable, but clicking it now falls through to the card play handler instead of opening Fix metadata. Delegate this click from a stable parent or rebind after patching.
🤖 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 `@static/v3/songs.js` around lines 501 - 533, The click handler for the
[data-meta-fix] badge is getting lost when enrichBadge is re-rendered by
_patchCardEnrich() via outerHTML, so the live-updated badge no longer opens Fix
metadata. Update the click wiring in wireCards() to use delegated handling from
a stable parent element, or ensure the handler is reattached after
_patchCardEnrich() replaces the node, so clicks on the nomatch badge continue to
trigger the metadata fix flow instead of the card play handler.
This was referenced Jul 5, 2026
Contributor
Author
📋 Merge order — metadata-curation epic (7 PRs)
#778–#780 are stacked (each based on the branch above), so GitHub re-points each to
|
This was referenced Jul 5, 2026
ChrisBeWithYou
marked this pull request as draft
July 5, 2026 05:25
ChrisBeWithYou
marked this pull request as ready for review
July 5, 2026 21:32
…handoff Connects the two halves: the "No match" badge (the unmatched pile) now opens the Fix-metadata popup for that song in one click, instead of right-click → menu. The resting badge becomes interactive (pointer-events-auto + hover), carrying a data-meta-fix hook; wireCards opens window.__fbFixMatch(playTarget) on click and stops propagation so it doesn't also play the card. Batch tile states stay non-interactive. Loop becomes: Unmatched filter → see the pile → click one → fix it. tailwind.min.css regenerated for the badge's hover classes.
…English base)
Two changes so an English-speaking base never sees a blank name or native script:
- Filename romaji fallback: a blank-artist CDLC pack ("Artist_Title_v1_p") shows
nothing useful (artist blank; title = the raw filename), and a match fills it
with kanji/kana. query_page + pack_fields now surface the author's own romaji
parsed from the filename ("Junko Yagami — BAY CITY") when the pack has no
artist of its own — display-only, keyset-safe (raw title stashed for the
cursor), a real pack artist or a user override still wins.
- Smart adopt: "Use these values" now KEEPS the readable romaji name + title the
card already shows and takes only album/year/genre (+ art via the pin) from the
match, so identifying a Japanese song gives "Junko Yagami — BAY CITY — FULL MOON"
with the right cover, never native script.
Tests: romaji fallback fires for a blank-artist CDLC pack (grid + pack_fields
agree) and is left alone when the pack has a real artist.
byrongamatos
force-pushed
the
feat/v3-batch-popup-handoff
branch
from
July 5, 2026 21:35
c56997a to
d025002
Compare
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
2 participants
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.
The capstone of the metadata-curation epic — it composes and sits on top of #777 → #778 → #779 → #780 (the Fix-metadata popup stack) + #781 (no-match badge/filter), then adds the two pieces that make the whole loop click. Best merged after those land; it then rebases to just the last two commits below.
New in this PR
1. Batch → popup handoff (
1823e84)The persistent "No match" badge is now clickable → opens the Fix-metadata popup for that song in one tap (
data-meta-fix+wireCards→window.__fbFixMatch, stopPropagation so it doesn't also play). The loop becomes: Unmatched filter → see the pile → click a miss → fix it → next.2. English-base romaji (
c56997a)Native script (kanji/kana) is treated as identity only, never player-facing:
Artist_Title_v1_p) has no readable name (artist blank; title = the raw filename), and a match would fill it with native script.query_page+pack_fieldsnow surface the author's own romaji parsed from the filename (Junko Yagami — BAY CITY). Display-only, keyset-safe (raw title stashed for the cursor); a real pack artist or a user override still wins.八神純子 / 黄昏のBAY CITY.Verified live
The whole ~640-song city-pop pile now reads in clean romaji (was blank artist + filename-junk titles); the Unmatched badge opens the popup; smart-adopt keeps romaji + fills album/art. Tests green (romaji fallback fires for blank-artist packs and is left alone for real ones; handoff + unmatched suites pass).
Scoping note: the romaji fallback covers the grid + popup; the artist tree/albums still bucket blank-artist packs under "Unknown Artist" (follow-up). Genuinely-blank-filename gap → MB romaji alias (aliases already fetched in #772) is the deeper fallback, not wired here.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes