got-feedBack/feedBack
1
0
Fork 0
mirror of https://github.com/got-feedBack/feedBack.git synced 2026-07-23 05:11:34 +00:00
Code Issues Actions 5 Packages Projects Releases Wiki Activity
442 Commits 176 Branches 5 Tags 1,017 MiB
fix/plugin-esc-global
Commit Graph

3 Commits

Author SHA1 Message Date
Byron Gamatos
97a941c45d
fix(tests): join background scan/enrich workers before closing the DB (#735) 
Root cause of the flaky pytest segfault (exit 139): the background scan and enrichment daemon threads (_scan_runner/_enrich_runner) use the shared MetadataDB connection, but test fixtures closed that connection in teardown without stopping them. A daemon thread mid-query on a freed SQLite conn is a native use-after-free → SIGSEGV. The app's startup kicks a scan, so almost any app-booting fixture was vulnerable. It only surfaced now because got-feedback/feedBack#728 added a push trigger, so ci/test runs on every push to main.

Fix: server.py retains the scan/enrich thread handles and adds _join_background_db_threads(); every test fixture now joins the workers before conn.close(). Verified: the full suite runs to completion (no segfault) where it previously crashed at ~25%.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
 2026-07-03 12:15:43 +02:00
ChrisBeWithYou
55060c4f67
v3 library: artist sort orders titles within an artist (tree-view feel) (#720) 
* v3 library: artist sort orders titles within an artist (tree-view feel)

Tester report: "the list is set up by artist, but the cards are
alphabetical(-ish random)". Real: the tree orders artist -> album -> title,
while the grid's artist sort ordered within an artist by RAW FILENAME —
community-pack filename noise, so an artist's cards looked shuffled.

- artist / artist-desc gain a title secondary (direction baked per entry so
  the legacy `dir=desc` append can't land on the title term; titles stay
  A->Z under Z->A artists).
- The two-term (value, filename) keyset cursor can't seek a three-term
  order, so artist sorts leave _KEYSET_SORTS and page by OFFSET — measured
  trivial at real library sizes; title/recent keep their keyset. Restore
  via a composite sort-key column if 50k-song libraries ever hurt.
- The tree view says "List view groups by artist — the selected sort
  applies to the card grid" when a non-artist sort is active, instead of
  silently ignoring the picker.
- Keyset proof-tests repinned to the title sort (same property, a sort
  that still keysets); 2 new tests pin the title-within-artist order and
  the OFFSET pagination's no-skip/no-dupe across pages.

Full-suite failure set identical to the same-main baseline.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Nm7tHs1Yvjjtnnu4nzJgdN

* v3 library: honor legacy sort=artist&dir=desc (fold dir into effective sort)

Codex/review follow-up to the title-within-artist change: the new artist
ORDER BY bakes in `ASC` (for the title secondary), so the global `dir=desc`
append is suppressed and `sort=artist&dir=desc` silently returned A->Z
instead of Z->A — a regression on the legacy /api/library dir contract.

Fold `dir=desc` into the canonical sort key BEFORE the sort_map lookup via
the existing _effective_keyset_sort helper (same fold the cursor side already
does), so the ORDER BY is built from the effective sort. Only artist/title
fold (they have `-desc` twins); title/recent/tuning/year/mastery are
unaffected — verified by the keyset/filter suites.

New test pins that legacy `sort=artist&dir=desc` matches the explicit
`artist-desc` ordering (Z->A artists, A->Z titles within each).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: byrongamatos <xasiklas@gmail.com>
 2026-07-02 20:51:33 +02:00
ChrisBeWithYou
e9d95ad190
v3 library: multi-chart work grouping, complete (engine + API + card + drawer + toggle/split/filter-law) — P5a–P5e (#702) 
* v3 library: multi-chart work grouping engine + work-charts API — P5a/P5b

Charts of the same song (same normalized artist+title) now GROUP under a
computed work_key, with a materialized representative filter so the grid
can collapse them without breaking keyset paging:

- work_key = normalize(artist+title) (diacritics/punct/case folded,
  leading "The" folded on artist); resolves the effective artist via the
  artist_alias table when present (feature-detected, no hard dep).
- Sparse, never-purged-on-rescan tables: chart_group_pref(work_key,
  preferred_filename) + chart_group_split(filename, split_key); purged
  only by the explicit per-song delete.
- Materialized work_display(filename, work_key, effective_work_key,
  is_group_representative, group_size) read-model: lazy rebuild via a
  dirty flag set on put/delete; set_chart_preferred does an incremental
  re-flip (no full rebuild). Auto-pick representative = most
  arrangements -> most plays -> newest -> filename; a user pref wins and
  degrades to auto-pick if its file disappears.
- group=1 on query_page/query_stats = one extra representative
  predicate applied identically to page + total + sort_letters, so the
  keyset cursor (sort_value, filename) stays a valid total order and
  counts works, not charts. Grouped rows carry chart_count + work_key.
- Charts API: GET /api/work/{work_key}/charts (members + which is the
  keeper, your pick vs auto), PUT/DELETE .../preferred, and
  POST /api/chart/{filename}/split + /unsplit (under /api/chart so the
  DELETE /api/song catch-all can't shadow them).

Tests: 15 grouping-engine + 7 charts-API tests, including grouped
keyset pagination (no skip/dupe across pages).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Nm7tHs1Yvjjtnnu4nzJgdN

* v3 library: grouped grid card + persistent "N charts" chip — P5c

Flip the v3 grid to the grouped library (group=1 on /api/library and the
rail's /api/library/stats fetch): one card per song, showing the
representative (preferred/auto-pick) chart. group rides page, total and
sort_letters identically so the A-Z rail's cumulative-seek math and the
virtualized sizer stay consistent, counting works not charts; the keyset
cursor chains with group on every page.

- New groupingActive() helper, default ON per the design; the persisted
  per-view toggle (P5e) lands there. Only the local provider implements
  group=; smart collections and remote providers ignore it and stay
  flat, so it is safe to send unconditionally.
- chartsChipHtml(): a "flag N charts" chip rendered ONLY when
  chart_count >= 2 - single-chart cards emit byte-identical markup.
  First in the fixed-height chip row + shrink-0 so it never clips and
  card height is unchanged.
- Chip click = feature-detected window.__fbOpenChartsDrawer (the Charts
  drawer arrives in P5d); until then a no-op. Plain-click / play / the
  arrangement chips are untouched and play the representative.
- The library-home repertoire meter's stats fetch deliberately stays
  ungrouped: its mastered numerator counts chart filenames, so a works
  denominator could exceed 100% - reconciling that is P5e's
  mastery-anchor work. The tree view stays flat (query_artists has no
  grouping; its opener is wired in P5d).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Nm7tHs1Yvjjtnnu4nzJgdN

* v3 library: Charts drawer + openers — P5d

The single deep-management surface for a work's charts (design UX-2/3):
a body-appended slide-in drawer (filter-drawer idiom) listing every
chart of the work as a radiogroup — the checked row is the keeper the
grid card plays.

- Rows show format / tuning / arrangements / year / your accuracy (or
  "not played") plus the pack filename, usually the only human-readable
  distinguisher between duplicate charts. Keeper is labeled
  "Preferred (auto)" vs "Preferred - your pick".
- Row click (or Enter/Space) = one-tap Set-preferred; "Reset to auto
  pick" appears when the keeper is an explicit pick. Writes go through
  the work-charts API and the drawer re-renders from the response; the
  grid re-fetches in place since the representative may have flipped.
- Per-row Play (plays that exact chart) and Add-to-playlist (the picker
  is z-[200], layering over the z-50 drawer).
- a11y: Tab focus-trap, Escape closes, ArrowUp/Down move focus between
  rows (focus only - arrow-select would fire a preferred write per
  keystroke), focus restored to the opener on close.
- Openers: the "N charts" chip opens the drawer directly; the card's
  overflow menu gains "Charts (N)..." and "Play version >" (expands
  inline; picking one plays it as a one-off - the keeper/headline does
  not move). Tree rows ride the ungrouped artists endpoint, so the menu
  resolves their work lazily via the new GET /api/chart/{fn}/work
  ({work_key, chart_count}) and slots a "Charts (N)..." entry in when
  versions exist. A window.__fbOpenChartsDrawer global lets other views
  open the drawer. Right-click is deferred: the open native card
  context-menu PR should host that entry once both merge.
- tailwind.min.css rebuilt: carries the new utility classes from this
  and the previous commit (the grouped-card chip tint was missing).

Split keys contain '#', so clients MUST URL-encode work_key in paths
(the v3 client does; a test documents the round-trip). 4 new endpoint
tests; 26/26 grouping+charts tests green.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Nm7tHs1Yvjjtnnu4nzJgdN

* v3 library: group toggle, split UI, the filter law + mastery-anchor rules — P5e

Completes the multi-chart grouping slice (design 7.1):

- Filter law under group=1: work-identity (artist/album/search) and
  practice-state (favorites/mastery/tags/difficulty) predicates stay on
  the representative row, while CHART-INTRINSIC predicates (format/
  arrangements/stems/lyrics/tuning) now match if ANY member of the work
  does - a song you own in Drop D is no longer hidden because your
  preferred chart is E Standard. Intrinsic clauses moved to an
  alias-aware builder and re-applied as a member EXISTS; identical in
  query_page and query_stats so counts and the A-Z rail stay in
  lockstep. A pure predicate - keyset paging is untouched (tested).
- Display-chart switch: when the representative itself doesn't match,
  the row carries a display_chart override (the matching member). The
  row stays the representative's - swapping rows wholesale would break
  the (sort_value, filename) cursor - and the card renders/plays the
  member while the accuracy badge and heart stay anchored on the
  preferred chart.
- Mastery sort aggregates MAX across the group ("a song surfaces on any
  chart you've touched"); OFFSET-paged, so cursor-safe. The
  Recently-Added aggregate is deliberately deferred: mtime IS a keyset
  sort, so its aggregate would need materializing into work_display.
- History-sticky auto-pick: most-played -> most-complete -> newest.
  A newer/"more complete" import can't silently take the pick from the
  chart your reps accrued on, and a one-off try of an alternate can't
  out-rank a practiced incumbent; all-unplayed groups still pick by
  completeness.
- Persisted "One card per song" toggle in the filter drawer (default
  ON; OFF = one card per chart). A view mode: never counted in the
  filter badge, never saved into collection rules, local provider only.
- Split escape hatch: "Split out" per drawer row gives a chart its own
  card; the split card's overflow menu offers "Rejoin other versions"
  (rows and the chart-work lookup now carry is_split).
- Mastery-anchor heads-up: after set-preferred the drawer shows a
  one-line ambient note that practice history stays with each chart
  (no toast - hearing-safe).

10 new filter-law tests; 38/38 grouping tests green. tailwind.min.css
rebuilt for the new utility classes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Nm7tHs1Yvjjtnnu4nzJgdN

* fix(v3): work-grouping — escape Charts-drawer meta (XSS), keep non-Latin titles distinct, guard mid-rebuild reads (PR #702 review)

- XSS: esc() the composed `meta` string in _chartRowHtml (arrangement/tuning
  names come from untrusted feedpak metadata) before innerHTML; acc stays HTML.
- Non-Latin titles: _norm_token falls back to raw lowercased whitespace-collapsed
  text when the NFKD+strip fold yields "" (CJK/Cyrillic/Greek/Arabic), so distinct
  non-Latin titles keep distinct _work_key values instead of collapsing into one
  bogus work. Latin names still hit the folded branch — behavior unchanged.
- Mid-rebuild reads: wrap the grouped representative SELECT in query_page and
  query_stats under self._lock (nullcontext when ungrouped, so lazy reads stay
  lock-free) so a reader can't observe work_display between rebuild_work_display's
  DELETE and INSERT/commit. _ensure_work_display stays OUTSIDE the lock — it
  self-locks the rebuild and self._lock is non-reentrant — so only the SELECT is
  guarded (rebuild fully completes before the guarded SELECT runs).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: byrongamatos <xasiklas@gmail.com>
 2026-07-02 13:37:15 +02:00