Commit 4543b2f
feat(setbuilder): pool import resolves master tracks store + pre-build coverage gate (#554)
* feat(setbuilder): pool import resolves master tracks store + pre-build coverage gate
Cut the WrzDJSet pool import over to the global tracks store (#540/#541) so each
imported recording is enriched once and reused across sets and events. Every
import flow now runs hydrate_candidates_from_store BEFORE import_candidates:
- trusted+complete store row -> hydrate candidate gaps with ZERO provider calls
- store miss but candidate carries fields (Beatport/Tidal playlists) -> upsert
to POPULATE the store from the candidate (ZERO provider calls)
- genuine gaps + a connected DJ -> run the provider cascade once (enrich_track),
write back to the store, then hydrate
Commit discipline mirrors the request-side _safe_upsert_track: REST flows commit
the store write durably (never poisons the import); agent import tools pass
commit=False so the store write rides the single agent-turn transaction.
The cache short-circuit gates on bpm/key/genre/duration (the provider-fillable
fields); energy is excluded by design since it comes only from Soundcharts/
Lexicon (dark), so a provider-enriched row still serves as a cache hit instead
of re-hitting providers forever.
Adds a pure coverage check over the five required pool->builder fields
(bpm/key/genre/duration/energy) with per-field missing counts + a ready signal.
The deterministic build endpoint and the agent autobuild path attach it to their
response as a SOFT, overridable signal for the build-confirmation dialog (#538) —
the build is never hard-blocked on it. Extends analyze_pool_gaps to also report
energy + genre coverage, read from the resolved pool rows (not the dead pool
energy column). Regenerated openapi.json + dashboard api types.
Closes #542
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(setbuilder): persist pool source before hydration loop (#554 review)
Codex review P2: every REST import does get_or_create_source (flush, uncommitted)
-> hydrate_candidates_from_store(commit=True) -> import_candidates. If a candidate
raised before any _safe_upsert reached its commit-first db.commit() (e.g.
enrich_track throws on the first candidate), the per-candidate db.rollback() in the
commit=True recovery branch discarded the still-uncommitted source row, and
import_candidates then inserted pool tracks against a stale source.id.
Commit once before the candidate loop on the commit=True path so the flushed
source is durable; per-candidate rollbacks can then only discard in-flight
store-write state, never the source. The commit=False agent path owns its single
transaction and is unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(setbuilder): thread candidate ISRC into enrich write-back (#554 review)
Codex review P2: _write_candidate_to_store threaded the validated ISRC into the
store write, but _enrich_and_writeback hard-coded isrc=None. A Spotify/public-URL
candidate (carries an ISRC but no bpm/key/genre) takes the enrich path, so its
store row was written ISRC-less -> a later by-ISRC lookup missed and re-ran the
providers, defeating the dedupe win.
Thread the valid_isrc computed in _hydrate_one through to the enrich write-back
so the row is keyed by ISRC (consistent with #552 storing the submitted ISRC).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(setbuilder): per-field trusted hydration from the store (#554 review)
Codex review P2: a store row was ignored entirely unless bpm+key+genre+duration
were ALL present and authoritative, so a partially-cached row contributed nothing
— worst for a DJ with no connected providers, who could not enrich the gap and so
got none of the trusted fields the store already held.
Hydrate PER-FIELD instead: fill each missing candidate field whose row value is
present AND from an authoritative (50+ precedence) source, THEN recompute the
remaining gaps to decide populate-vs-enrich-vs-leave. Two guards preserve the
existing semantics:
- Values hydrated FROM the row are tracked and NOT written back to the store
(no churn, no legacy provenance downgrade of a beatport/tidal-sourced field).
- Energy may be hydrated from an authoritative row but stays excluded from the
provider-enrich gate (still only Soundcharts/Lexicon, dark per #543/#544).
Replaces the all-or-nothing _row_trusted_complete / _CACHE_GATE_FIELDS with
_hydrate_authoritative_fields + _has_provider_gap; the post-enrich tail fills the
candidate from its own freshly-resolved values via _fill_missing.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(setbuilder): carry source_service so manual provider picks store authoritative (#554 review)
Codex re-review P2: a manual Beatport/Tidal search pick (DJ search -> add, the most
common import path) was stored as legacy precedence, defeating the cache.
Root cause across the stack: the unified SearchResult schema exposes only
spotify_id, so ImportModal sends source_service="beatport"/"tidal" but
source_track_id=None. candidate_from_manual therefore left track_id=None, and
_candidate_source fell back to the (absent) track_id prefix -> "legacy"
(precedence 30). The provider-measured bpm/key/genre was written sub-authoritative,
so _hydrate_authoritative_fields (>=50 gate) never reused it and every later import
of that recording re-queried providers.
Carry the provider explicitly (backend; no FE change available):
- Add source_service to PoolCandidate (preserved across dataclasses.replace).
- candidate_from_manual sets it for beatport/tidal; spotify (not a bpm/key
authority) and hand-typed "manual" intentionally stay None -> legacy.
- _candidate_source prefers an explicit beatport/tidal source_service over the
track_id prefix; the playlist builders still match via their beatport:/tidal:
prefix; everything else stays legacy. Dropped the dead "manual" prefix branch
(it never produced a manual: track_id and would have wrongly claimed
precedence 100 for typed data).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(setbuilder): never trust client-asserted provenance for manual picks (#554 P1)
Codex final-pass P1 SECURITY: FIX 4 (598061d) trusted PoolImportManualIn's
client-supplied source_service as authoritative provider provenance. Any
authenticated DJ could POST fabricated bpm/key/genre with source_service="beatport"
and _candidate_source would write them as AUTHORITATIVE into the GLOBAL,
multi-tenant tracks store that other DJs hydrate from (and equal-precedence writes
could overwrite real provider fields) — cache poisoning. The backend cannot verify
client-asserted provenance (there is no server-side provider id; that is the same
SearchResult gap that motivated FIX 4), so manual picks MUST store as legacy.
Revert the trust, keep the unrelated cleanup:
- Remove the source_service field from PoolCandidate (only FIX 4 read it).
- candidate_from_manual: drop the store_source derivation + source_service= arg;
a manual pick with no server-trusted track_id stays track_id=None -> legacy. The
source_service PARAMETER stays (it still mints beatport:<id>/tidal:<id> track_ids
for picks that carry a real, server-side source_track_id).
- _candidate_source: drop the source_service preference; authority comes only from
a server-minted beatport:/tidal: track_id prefix. Kept FIX 4's removal of the dead
"manual" prefix branch (that cleanup was correct).
A legacy row self-heals: the next connected-DJ import runs enrich_track SERVER-SIDE
and upgrades it to real beatport/tidal precedence, so the precedence guard cleanly
overrides it. The one-time re-enrichment cost is consciously accepted — security
over the dedupe optimization.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(setbuilder): ISRC-conflict guard on signature-fallback hydration (#554 P2)
Codex final-pass P2: get_track is ISRC-first then signature-fallback, so a
candidate with a valid ISRC NOT in the store whose normalized artist/title
signature matches a DIFFERENT recording's row (different ISRC) gets that row back,
and _hydrate_one copied its bpm/key/genre/duration onto this candidate — the wrong
recording — suppressing enrichment because the gaps looked filled. upsert_track
already refuses the ISRC-mismatched WRITE (#552), but the read-side hydration had
already contaminated the candidate.
Guard hydration on ISRC compatibility (mirrors the request-side
sync/enrichment_pipeline check): hydrate from the row only when
isrc is None or row.isrc is None or row.isrc == isrc. On a genuine conflict, skip
_hydrate_authoritative_fields and let enrichment resolve the candidate's own
recording. The normal signature-hit case (row has no ISRC) is unaffected and still
hydrates.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(setbuilder): never mint a client-forgeable provider prefix for manual picks (#554 P1)
Codex final pass: the P1 revert (9a0f77e) was incomplete. candidate_from_manual
still minted track_id = f"{source_service}:{source_track_id}" from CLIENT-supplied
source_service + source_track_id (both from PoolImportManualIn). A crafted POST
{source_service:"beatport", source_track_id:"x", bpm:200, ...} produced
track_id="beatport:x" → _candidate_source returns "beatport" (authoritative) →
_write_candidate_to_store poisons the shared multi-tenant store. The docstring's
"server-trusted" claim was false — it was client input.
candidate_from_manual now mints ONLY a non-authoritative spotify:<id> reference
(Spotify resolves to legacy regardless, and is the one provider the FE sends an id
for); beatport/tidal/manual leave track_id=None → legacy. Authoritative
beatport:/tidal: prefixes come ONLY from the server-side playlist builders
(candidates_from_beatport/candidates_from_tidal) via the DJ's OAuth'd fetch.
_candidate_source is unchanged (no manual input can now produce a provider prefix).
Updated test_manual_import (it asserted the vulnerable tidal:555 minting) to assert
track_id is None for a client tidal pick, plus a spotify-reference-retained case.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(setbuilder): count duration_sec in the store-write gate (#554 P2)
Codex final pass: _candidate_has_writable_fields gated the store write on
bpm/key/genre only, but _write_candidate_to_store DOES persist duration_sec. So a
candidate whose only contributed field is duration_sec (Spotify/public-URL imports
carry duration but no bpm/key/genre), or one that adds duration after bpm/key/genre
were hydrated-from-row (all in exclude), returned False -> the store write was
skipped -> duration (a required pool->builder contract field) never cached, and
later imports couldn't hydrate it.
Add duration_sec to the gate under the same exclude rule
_write_candidate_to_store already applies; no other change needed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>1 parent 8bee480 commit 4543b2f
17 files changed
Lines changed: 1408 additions & 18 deletions
File tree
- dashboard/lib
- server
- app
- api
- schemas
- services/setbuilder
- tests
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2715 | 2715 | | |
2716 | 2716 | | |
2717 | 2717 | | |
| 2718 | + | |
| 2719 | + | |
| 2720 | + | |
| 2721 | + | |
2718 | 2722 | | |
2719 | 2723 | | |
2720 | 2724 | | |
| |||
4249 | 4253 | | |
4250 | 4254 | | |
4251 | 4255 | | |
| 4256 | + | |
4252 | 4257 | | |
4253 | 4258 | | |
4254 | 4259 | | |
| |||
5786 | 5791 | | |
5787 | 5792 | | |
5788 | 5793 | | |
| 5794 | + | |
| 5795 | + | |
| 5796 | + | |
| 5797 | + | |
| 5798 | + | |
| 5799 | + | |
| 5800 | + | |
| 5801 | + | |
| 5802 | + | |
| 5803 | + | |
| 5804 | + | |
| 5805 | + | |
| 5806 | + | |
| 5807 | + | |
| 5808 | + | |
| 5809 | + | |
| 5810 | + | |
| 5811 | + | |
| 5812 | + | |
| 5813 | + | |
| 5814 | + | |
5789 | 5815 | | |
5790 | 5816 | | |
5791 | 5817 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
54 | 54 | | |
55 | 55 | | |
56 | 56 | | |
| 57 | + | |
57 | 58 | | |
58 | 59 | | |
59 | 60 | | |
| |||
113 | 114 | | |
114 | 115 | | |
115 | 116 | | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
116 | 120 | | |
117 | 121 | | |
118 | 122 | | |
| |||
665 | 669 | | |
666 | 670 | | |
667 | 671 | | |
668 | | - | |
| 672 | + | |
| 673 | + | |
| 674 | + | |
| 675 | + | |
| 676 | + | |
669 | 677 | | |
670 | 678 | | |
671 | 679 | | |
| 680 | + | |
672 | 681 | | |
673 | 682 | | |
674 | 683 | | |
675 | 684 | | |
676 | 685 | | |
677 | 686 | | |
678 | 687 | | |
| 688 | + | |
679 | 689 | | |
680 | 690 | | |
681 | 691 | | |
| |||
1079 | 1089 | | |
1080 | 1090 | | |
1081 | 1091 | | |
| 1092 | + | |
1082 | 1093 | | |
1083 | 1094 | | |
1084 | 1095 | | |
| |||
1108 | 1119 | | |
1109 | 1120 | | |
1110 | 1121 | | |
| 1122 | + | |
1111 | 1123 | | |
1112 | 1124 | | |
1113 | 1125 | | |
| |||
1133 | 1145 | | |
1134 | 1146 | | |
1135 | 1147 | | |
| 1148 | + | |
1136 | 1149 | | |
1137 | 1150 | | |
1138 | 1151 | | |
| |||
1192 | 1205 | | |
1193 | 1206 | | |
1194 | 1207 | | |
| 1208 | + | |
1195 | 1209 | | |
1196 | 1210 | | |
1197 | 1211 | | |
| |||
1223 | 1237 | | |
1224 | 1238 | | |
1225 | 1239 | | |
1226 | | - | |
| 1240 | + | |
| 1241 | + | |
1227 | 1242 | | |
1228 | 1243 | | |
1229 | 1244 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
443 | 443 | | |
444 | 444 | | |
445 | 445 | | |
| 446 | + | |
| 447 | + | |
| 448 | + | |
| 449 | + | |
| 450 | + | |
| 451 | + | |
| 452 | + | |
| 453 | + | |
| 454 | + | |
| 455 | + | |
| 456 | + | |
| 457 | + | |
| 458 | + | |
| 459 | + | |
| 460 | + | |
446 | 461 | | |
447 | 462 | | |
448 | 463 | | |
449 | 464 | | |
450 | 465 | | |
451 | 466 | | |
452 | 467 | | |
| 468 | + | |
453 | 469 | | |
454 | 470 | | |
455 | 471 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
136 | 136 | | |
137 | 137 | | |
138 | 138 | | |
| 139 | + | |
| 140 | + | |
139 | 141 | | |
140 | 142 | | |
141 | 143 | | |
142 | | - | |
| 144 | + | |
| 145 | + | |
143 | 146 | | |
144 | 147 | | |
145 | 148 | | |
| |||
161 | 164 | | |
162 | 165 | | |
163 | 166 | | |
164 | | - | |
| 167 | + | |
165 | 168 | | |
166 | 169 | | |
167 | 170 | | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
168 | 177 | | |
169 | 178 | | |
170 | 179 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
186 | 186 | | |
187 | 187 | | |
188 | 188 | | |
189 | | - | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
190 | 193 | | |
191 | 194 | | |
192 | 195 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
104 | 104 | | |
105 | 105 | | |
106 | 106 | | |
| 107 | + | |
107 | 108 | | |
108 | 109 | | |
109 | 110 | | |
| |||
151 | 152 | | |
152 | 153 | | |
153 | 154 | | |
| 155 | + | |
154 | 156 | | |
155 | 157 | | |
156 | 158 | | |
| |||
219 | 221 | | |
220 | 222 | | |
221 | 223 | | |
| 224 | + | |
222 | 225 | | |
223 | 226 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
255 | 255 | | |
256 | 256 | | |
257 | 257 | | |
258 | | - | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
259 | 264 | | |
260 | | - | |
| 265 | + | |
| 266 | + | |
261 | 267 | | |
262 | 268 | | |
| 269 | + | |
| 270 | + | |
263 | 271 | | |
264 | 272 | | |
265 | 273 | | |
266 | 274 | | |
267 | | - | |
| 275 | + | |
268 | 276 | | |
269 | 277 | | |
270 | 278 | | |
271 | 279 | | |
| 280 | + | |
| 281 | + | |
272 | 282 | | |
273 | 283 | | |
274 | 284 | | |
275 | 285 | | |
276 | 286 | | |
277 | 287 | | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
278 | 292 | | |
279 | 293 | | |
280 | 294 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
16 | 16 | | |
17 | 17 | | |
18 | 18 | | |
| 19 | + | |
19 | 20 | | |
20 | 21 | | |
21 | 22 | | |
| |||
35 | 36 | | |
36 | 37 | | |
37 | 38 | | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
38 | 43 | | |
39 | 44 | | |
40 | 45 | | |
41 | | - | |
| 46 | + | |
| 47 | + | |
42 | 48 | | |
43 | 49 | | |
44 | 50 | | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
45 | 54 | | |
46 | | - | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
47 | 60 | | |
48 | 61 | | |
49 | 62 | | |
| |||
0 commit comments