Add the three OPTIONAL per-note feedpak 1.5.0 teaching marks — fg (fret-hand
finger), ch (strum-group key), sd (scale degree) — to the Note model and wire
format, mirroring the bend-shape work (#531). These are DISPLAY/TEACHING ONLY:
nothing in the scoring / note-verification path reads them.
- lib/song.py: Note.fret_finger / strum_group / scale_degree, default-omitted
on the wire (fg/ch/sd) and decoded via _wire_int_optional; _parse_note reads
the GP-written fretFinger XML attr. Pure helpers key_to_tonic_pc (§7.7 key
name -> tonic pitch class) + scale_degree_for_pitch, plus base_open_string_midis
/ pitch_from_base / note_pitch_midi (tuning offsets + capo + fret -> MIDI,
mirroring app.js _TUNING_BASE_MIDI).
- lib/gp2rs.py: GP5 note.effect.leftHandFinger -> fg (RsNote field + fretFinger
XML attr), reusing the chord Fingering value convention.
- lib/gp2rs_gpx.py: GP8/GPIF per-note <LeftFingering> (p-i-m-a-c letter codes,
verified against real GP8 exports) -> fg.
- server.py highway_ws: derive sd for notes + chord notes from the active
keys.json key + sounding pitch when the author didn't author one (author value
wins); base hoisted out of the per-note loop.
Tests: round-trip + omit-when-default + malformed-tolerance for fg/ch/sd;
key_to_tonic_pc + scale_degree_for_pitch + note_pitch_midi (standard/drop-D/
capo/bass) units; GP5 leftHandFinger and GP8 <LeftFingering> import.
Part of got-feedback/feedback#334
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Post-merge Codex review of the bend-curve PRs (#531/#532) surfaced edge cases:
- GP8 (#531 P2): bnv timing used rn.sustain, which is zeroed for notes <= 0.2s,
so short GP8 bends kept the scalar bn but lost bt/bnv. Use the beat duration
`dur` (matching the GP5 path) so the curve survives.
- 2D highway (#532 P2): bnvNormalizedPoints mapped x over the curve's own t-range
[first,last] instead of the note span, so curves not starting at 0 / ending at
sus were time-distorted. Now maps over [0, sus] (clamped), with a curve-span
fallback when sus<=0 (existing no-sus callers unaffected).
- 3D highway (#532 P3): the sustain ribbon + bend chevron were gated on bn>0, so a
note carrying an authoritative bnv with bn==0 drew no ribbon/marker. Both now
also fire on bnv presence; chevron steps derived from max(bn, bnv peak).
Codex-reviewed: clean (no findings). +1 JS test (sus-relative mapping + fallback).
JS 8/8, 250 core GP/song tests pass.
NB: GP8's short-bend path still lacks a dedicated synthetic-GPIF fixture (same gap
as the GP8 offset-prop-names P3) — _gpx_bend_shape units cover the function; the
fix is the one-line caller change.
Part of got-feedback/feedback#334.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Implements feedpak spec §6.2.1 (feedpak 1.4.0) per-note bend shape on the
core side:
- `bn` stays the bend's peak magnitude in semitones (unchanged).
- `bt` — bend intent (0 up, 1 release, 2 pre-bend, 3 pre-bend-release,
4 round-trip), default 0, default-omitted on the wire.
- `bnv` — time-stamped bend curve [{t: seconds-from-onset, v: semitones}],
authoritative when present; default-omitted. Older readers ignore both.
Wire (lib/song.py): Note.bend_intent/bend_values; note_to_wire emits bt/bnv
only when set; note_from_wire reads them via _sanitize_bend_curve (drops
malformed entries, empty -> None never []). _parse_note reads them from the
GP-import XML (bendIntent attr + bendValues JSON) so GP curves survive
import -> XML -> wire -> highway.
GP5 (lib/gp2rs.py): _gp_bend_shape maps pyguitarpro BendPoints to a bnv
curve — semitones = value/2.0 (consistent with the existing scalar bn),
t = position/12 * duration — and _bend_intent_from_values derives bt from
the shape. Emitted for <note> and <chordNote> via the shared _build_xml.
GP8 (lib/gp2rs_gpx.py): _gpx_bend_shape builds a 3-point curve from the
GPIF origin/middle/destination value+offset Properties (value/divisor
semitones, offset/100 * sustain seconds), reusing the shared _build_xml.
GPIF offset Property names should be confirmed against a real GP8 export.
Tests cover wire round-trip + default-omit + sanitization, GP5 unit/time
mapping + intent classification end-to-end through the XML, and the GP8
curve builder.
Part of got-feedback/feedback#334
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Core never read or emitted the manifest `feedpak_version` field. Adopt it:
- sloppak.py: `FEEDPAK_VERSION = "1.2.0"` constant (the format version this build
targets); `LoadedSloppak.feedpak_version` read from the manifest on load
(string, else None for legacy/absent).
- Stamp the version on the two core manifest-rewrite paths, without downgrading
an existing (possibly higher) declared version:
- gp2notation: `setdefault` before its notation-add rewrite.
- songmeta: opportunistically when a metadata field is supplied (gated on the
existing `dirty` flag, so never a standalone rewrite).
Core has no create-from-scratch path (RS-free repo) — the editor plugin's
create-mode save stamping FEEDPAK_VERSION is a follow-up in that repo. Internal
"sloppak" naming is intentionally left as-is (a rename is out of scope / risky).
Codex-reviewed: no P1/P2. +6 tests (read present/absent/non-string; metadata-write
stamp-when-absent / preserve-existing / no-op-no-stamp) + updated the gp2notation
key-order test for the appended version. 197 sloppak/songmeta/gp2notation tests pass.
Closes#527. Part of got-feedback/feedback#334.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
feedpak 1.2.0 added song-level `tempos` + `time_signatures` to song_timeline.json
and a per-chart `tempos` override on arrangements (§6.10). Core stored the raw
song_timeline dict but never consumed the maps, and didn't read per-chart tempos.
- song.py: shared `sanitize_tempos([{time,bpm}])` (finite non-bool time, finite
bpm>0, sorted); `Arrangement.tempos` field wired through arrangement_to_wire
(omitted when None/empty per §6.10) / arrangement_from_wire.
- sloppak.py: `_sanitize_time_signatures([{time,ts:[num,den]}])`;
LoadedSloppak.tempos / .time_signatures, loaded from song_timeline.json
INDEPENDENTLY of beats/sections (all are optional in 1.2.0).
- server.py: stream `tempos` + `time_signatures` highway-WS messages; the active
arrangement's per-chart `tempos` overrides the song-level map for that chart.
Renderer/UI surfacing is a thin follow-up; this lands the data plumbing.
Codex-reviewed: clean (no findings). +9 tests (sanitizers, per-chart wire
round-trip + omit-when-absent, song-level load/sanitize/absent + maps-without-
beats). 90 song/sloppak tests pass. (Pre-existing unrelated failure:
test_diagnostics_redact, fails on clean main too.)
Closes#526. Part of got-feedback/feedback#334.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The feedpak spec defines keys.json (instrument-independent key/scale-change
track, §7.7) but core never loaded it. Add it, mirroring the song_timeline /
drum_tab side-file pattern:
- lib/sloppak.py: LoadedSloppak gains a `keys` field; a permissive loader reads
the manifest `keys:` key, path-safety-checks it, and stores a SANITIZED
{version, events:[{t, key, scale?}]} — finite non-bool t (bad-t events dropped,
not rewritten to 0), non-empty string key, optional string scale, sorted.
Missing / unreadable / malformed -> None, never fatal. int-only version
(a float/NaN version can't abort the load).
- server.py: stream a `keys` highway-WS message when present + a `has_keys`
song_info flag so a consumer can light up a key/scale display.
Renderer/HUD surfacing is a thin follow-up; this lands the data plumbing so
the highway, plugins, and the upcoming scale-degree (`sd`) annotation can read
the active key/mode from the WS.
Codex-reviewed (2 rounds: version-int-coercion + bad-t-drop hardening); clean.
+7 loader tests (happy path, absent/permissive variants, sanitize/sort,
non-int-version no-abort). 150 sloppak/load tests pass.
Closes#525. Part of got-feedback/feedback#334.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Post-merge Codex review of E3 (PR #522) flagged two GP5 edge cases:
- P2: enrichment applied the diagram's name/fingers to the played template
without checking the diagram described the voicing actually played. A
mismatched chord label/diagram could mis-name/finger the played template, and
the back-fill spread it to other strums of the same played pattern. Now gated
on an exact, full-span fret-pattern match (new _chord_diagram_frets), mirroring
the GP8 guard — and comparing over max(played width, num_strings) so a
7/8-string diagram can't falsely match a narrower played voicing.
- P3: name and fingers back-fill were coupled (a name-only first annotation
blocked a later beat's fingers). Now independent.
Codex re-reviewed twice (the first match-gate trimmed extended strings; fixed by
the full-span compare); final pass clean. +4 tests (mismatch-not-applied,
name-then-fingers decoupled, higher-position absolute match, 7-string extended
string regression). 163 GP tests pass.
Follow-up to #522. Part of got-feedback/feedback#334.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
GP imports previously landed chord templates with blank fingerings (GP5 +
GP8) and blank names (GP8), so the editor/highway had nothing to show even
though E0 preserves and E1 authors that data. E3 extracts the real chord
diagrams so imports arrive rich, keyed on the same fret-pattern join key the
editor (E0) and GP5 already use.
GP8 (lib/gp2rs_gpx.py): parse the per-track GPIF DiagramCollection
(Item @name + Diagram Fret/Fingering) into a fret-pattern -> {name, fingers}
map and enrich matching played voicings at the template build site. Diagram
string indices share the note String index space, so they go through the same
pitch-rank transform; <Fret fret> is treated as absolute.
GP5 (lib/gp2rs.py): pyguitarpro exposes the voicing on beat.effect.chord
(.strings indexed 0=highest string, .fingerings the parallel Fingering enum,
already RS finger ints). New _chord_fingers maps them to RS string order; the
template enrich now back-fills any still-blank template so the annotated chord
attaches even when an earlier unannotated strum of the same voicing created it.
Finger encoding (none/open=-1, thumb=0, index=1, middle=2, ring=3, pinky=4)
matches the editor E1 + RS serializer. Only enriches on exact fret-pattern
match; diagram-less charts import identically (blank). Verified end-to-end
against real files (GP8_Test.gp, joplin-janis-piece_of_my_heart.gp4).
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* sloppak: read cover without unpacking + serialize/cap zip unpacks
Album art for a zip-form sloppak was served by resolve_source_dir(), which
unpacks the ENTIRE archive (stems included, ~30 MB) to disk just to read
cover.jpg. On the library grid that meant a full extraction per card on scroll.
- read_cover_bytes(): opens only the cover member from the zip (or reads the
file for dir-form), with zip-slip guarding. ~4 ms vs a full unpack.
- resolve_source_dir(): per-file lock + bounded global semaphore so concurrent
callers don't rmtree + re-extract the same dest at once (a race), and a burst
can't saturate disk/CPU. 8 concurrent calls now dedupe to 1 unpack.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* server: serve sloppak art via read_cover_bytes + cache album-art responses
- get_song_art() sloppak branch now reads the cover directly (no full unpack),
off-thread via asyncio.to_thread.
- All art responses carry Cache-Control: public, max-age=86400. URLs are already
cache-busted with ?v=<mtime>, so the browser stops re-fetching every cover on
scroll-back; day bound self-heals any URL missing ?v.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* v3 library: lazy-load + async-decode card cover images
The grid (24 cards/page) and artist-row thumbnails emitted plain <img> with no
loading hint, so a whole page of covers fetched + decoded at once on each
scroll batch. Add loading="lazy" decoding="async" to defer off-screen fetches
and keep image decode off the main thread.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* address Codex review: zip cover normalization + correct art revalidation
Findings from the preflight Codex passes:
- sloppak.read_cover_bytes (zip form) read the raw manifest cover string via
zf.read(), so a non-canonical name like './cover.jpg' or 'art/../cover.jpg'
404'd. Normalize via safe_join → relative member; reject escape and the
degenerate root-collapse case ('.', 'subdir/..') like _unpack_zip does.
- Album-art caching is correctness-first: Cache-Control: no-cache plus a strong
validator, with real conditional handling (Starlette FileResponse emits an
ETag but doesn't evaluate If-None-Match). All three art paths route through
_art_conditional/_file_art_response → bodyless 304 on a matching validator.
A long immutable max-age was rejected because the frontend ?v=<mtime> buster
is only second-resolution and would pin a same-second rewrite.
- The sloppak cover is validated by CONTENT (sha1 of the bytes), not a stat:
a dir-form sloppak edited in place changes the cover file's mtime but not the
directory's, so a dir-stat ETag could emit a stale 304. Content hashing is
correct for both dir- and zip-form. get_song_art gained an optional request
(internal get_art caller passes none — safe).
Adds tests/test_sloppak_cover_art.py pinning read_cover_bytes (canonical,
non-canonical, degenerate/escape, dir/zip, webp) and the endpoint's 304 contract
incl. the dir-form in-place-edit no-stale-304 regression.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- diagnostics_redact: keep '.psarc' in the song-filename scrub regex.
The purge swapped it for '.archive' (not a real extension), which
would leak real .psarc filenames still on users' disks into
diagnostic bundles. This is a redaction allow-list, not brand text.
- sloppak: the legacy lyrics-source alias was a no-op
({"notechart": "notechart"}), so old manifests with
lyrics_source=="sng" fell back to "xml" instead of migrating.
Map {"sng": "notechart"} so the rename stays back-compatible.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Reword comments/docstrings/strings and rename identifiers that referenced
the external game and its file formats:
- format-id "psarc" -> "archive"; local vars psarc_path -> song_path,
psarc_base -> tone_base
- lyrics provenance value "sng" -> "notechart" (legacy "sng" still accepted)
- highway_3d fret-ghost scope value "rocksmith" -> "chords" (invalid/legacy
values fall back to the default, preserving behaviour)
- neutralise references in prose, test names/data, .gitattributes and docs
No functional change beyond the renamed identifiers; all Python compiles.
