Enrichment manager v2: working retry + bulk retry-all-failed
Fixes a correctness bug and adds bulk re-queuing. - Bug: per-row 'Retry' used clear-match, which sets an item to not_found with last_attempted=NULL. The worker only retries not_found items where last_attempted < (now - 30d), and 'NULL < cutoff' is false in SQLite, so those items were never re-queued. Fixed by resetting match_status to NULL (pending), which every worker's queue picks up on the next pass. - New POST /api/enrichment/<id>/retry with scope 'item' | 'failed' (failed = re-queue every not_found item of an entity type), backed by a pure whitelisted build_reset_query + MusicDatabase.reset_enrichment(). - UI: per-row Retry now hits /retry; a 'Retry all failed' bulk button appears when the current entity has not-found items (confirm + count toast); a hint line explains retry/match/auto-retry behaviour. - 11 new tests (38 enrichment tests total, all green).
This commit is contained in:
BoulderBadgeDad
parent
0b3c3f656d
commit
fc9a9f1c90
6 changed files with 200 additions and 5 deletions
|
|
@ -223,4 +223,31 @@ def create_blueprint() -> Blueprint:
|
|||
})
|
||||
return jsonify(result), 200
|
||||
|
||||
@bp.route('/api/enrichment/<service_id>/retry', methods=['POST'])
|
||||
def enrichment_retry(service_id: str):
|
||||
"""Re-queue item(s) so the worker re-attempts them.
|
||||
|
||||
Body: ``entity_type`` (artist|album|track), ``scope`` (item|failed),
|
||||
``entity_id`` (required when scope='item'). 'failed' re-queues every
|
||||
not_found item of that entity type.
|
||||
"""
|
||||
if service_id not in SERVICE_ENTITY_SUPPORT:
|
||||
return jsonify({'error': f'Unknown enrichment service: {service_id}'}), 404
|
||||
if _db_getter is None:
|
||||
return jsonify({'error': 'database unavailable'}), 503
|
||||
|
||||
data = request.get_json(silent=True) or {}
|
||||
entity_type = (data.get('entity_type') or 'artist').strip()
|
||||
scope = (data.get('scope') or 'item').strip()
|
||||
entity_id = data.get('entity_id')
|
||||
try:
|
||||
count = _db_getter().reset_enrichment(service_id, entity_type, scope, entity_id)
|
||||
except UnmatchedQueryError as e:
|
||||
return jsonify({'error': str(e)}), 400
|
||||
except Exception as e:
|
||||
logger.error("Error re-queuing %s %s (%s): %s", service_id, entity_type, scope, e)
|
||||
return jsonify({'error': str(e)}), 500
|
||||
return jsonify({'success': True, 'reset': count, 'service': service_id,
|
||||
'entity_type': entity_type, 'scope': scope}), 200
|
||||
|
||||
return bp
|
||||
|
|
|
|||
|
|
@ -173,6 +173,46 @@ def build_count_query(
|
|||
return sql, params
|
||||
|
||||
|
||||
# Reset scopes for re-queuing items so the worker re-attempts them.
|
||||
RESET_SCOPES = ('item', 'failed')
|
||||
|
||||
|
||||
def build_reset_query(
|
||||
service: str,
|
||||
entity_type: str,
|
||||
scope: str = 'item',
|
||||
entity_id=None,
|
||||
) -> Tuple[str, List]:
|
||||
"""Build the UPDATE that re-queues item(s) for enrichment.
|
||||
|
||||
Re-queuing means clearing ``<service>_match_status`` back to NULL (and
|
||||
``<service>_last_attempted`` to NULL): every worker's pending query selects
|
||||
``match_status IS NULL`` first, so the item is retried on the next pass.
|
||||
Nulling last_attempted alone is NOT enough — the not_found retry path uses
|
||||
``last_attempted < cutoff`` and ``NULL < cutoff`` is false, so the item
|
||||
would never be picked up.
|
||||
|
||||
* scope='item' -> a single row (requires entity_id)
|
||||
* scope='failed' -> every 'not_found' row for this entity type
|
||||
"""
|
||||
_validate(service, entity_type)
|
||||
if scope not in RESET_SCOPES:
|
||||
raise UnmatchedQueryError(f"Invalid reset scope: {scope!r}")
|
||||
|
||||
meta = _ENTITY_TABLE[entity_type]
|
||||
table = meta['table']
|
||||
ms = match_status_column(service)
|
||||
la = last_attempted_column(service)
|
||||
set_clause = f"SET {ms} = NULL, {la} = NULL"
|
||||
|
||||
if scope == 'item':
|
||||
if not entity_id:
|
||||
raise UnmatchedQueryError("entity_id is required for an item reset")
|
||||
return f"UPDATE {table} {set_clause} WHERE id = ?", [entity_id]
|
||||
# 'failed' — re-queue everything this source explicitly gave up on.
|
||||
return f"UPDATE {table} {set_clause} WHERE {ms} = 'not_found'", []
|
||||
|
||||
|
||||
def build_breakdown_query(service: str, entity_type: str) -> Tuple[str, List]:
|
||||
"""Build the matched / not_found / pending / total tally for one entity type."""
|
||||
_validate(service, entity_type)
|
||||
|
|
@ -211,4 +251,6 @@ __all__ = [
|
|||
'build_unmatched_query',
|
||||
'build_count_query',
|
||||
'build_breakdown_query',
|
||||
'build_reset_query',
|
||||
'RESET_SCOPES',
|
||||
]
|
||||
|
|
|
|||
|
|
@ -1079,6 +1079,24 @@ class MusicDatabase:
|
|||
finally:
|
||||
conn.close()
|
||||
|
||||
def reset_enrichment(self, service: str, entity_type: str, scope: str = 'item', entity_id=None) -> int:
|
||||
"""Re-queue item(s) for a source by clearing match_status back to NULL.
|
||||
|
||||
scope='item' resets one row (entity_id); scope='failed' resets every
|
||||
'not_found' row for that entity type. Returns the number of rows reset.
|
||||
Raises ``UnmatchedQueryError`` on bad input."""
|
||||
from core.enrichment.unmatched import build_reset_query
|
||||
|
||||
sql, params = build_reset_query(service, entity_type, scope, entity_id)
|
||||
conn = self._get_connection()
|
||||
try:
|
||||
cursor = conn.cursor()
|
||||
cursor.execute(sql, params)
|
||||
conn.commit()
|
||||
return cursor.rowcount or 0
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
def _add_mirrored_playlist_explored_column(self, cursor):
|
||||
"""Add explored_at column to mirrored_playlists to persist explore badge."""
|
||||
try:
|
||||
|
|
|
|||
|
|
@ -17,6 +17,7 @@ from core.enrichment.unmatched import (
|
|||
UnmatchedQueryError,
|
||||
build_breakdown_query,
|
||||
build_count_query,
|
||||
build_reset_query,
|
||||
build_unmatched_query,
|
||||
supported_entity_types,
|
||||
)
|
||||
|
|
@ -154,6 +155,43 @@ def test_db_raises_on_bad_input(db):
|
|||
db.get_enrichment_unmatched('spotify', 'artist', status='bogus')
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# Reset / retry (re-queue) — must clear match_status to NULL so the worker
|
||||
# re-attempts (nulling last_attempted alone leaves not_found in limbo).
|
||||
# --------------------------------------------------------------------------
|
||||
|
||||
def test_reset_builder_requires_id_for_item():
|
||||
with pytest.raises(UnmatchedQueryError):
|
||||
build_reset_query('spotify', 'artist', 'item') # no entity_id
|
||||
|
||||
|
||||
def test_reset_builder_bad_scope():
|
||||
with pytest.raises(UnmatchedQueryError):
|
||||
build_reset_query('spotify', 'artist', 'bogus', entity_id='x')
|
||||
|
||||
|
||||
def test_reset_builder_nulls_status_not_just_attempted():
|
||||
sql, _ = build_reset_query('spotify', 'artist', 'failed')
|
||||
assert 'spotify_match_status = NULL' in sql
|
||||
assert 'spotify_last_attempted = NULL' in sql
|
||||
assert "WHERE spotify_match_status = 'not_found'" in sql
|
||||
|
||||
|
||||
def test_reset_item_requeues_to_pending(db):
|
||||
n = db.reset_enrichment('spotify', 'artist', 'item', entity_id='a2') # was not_found
|
||||
assert n == 1
|
||||
# not_found dropped by 1, pending gained 1
|
||||
bd = db.get_enrichment_breakdown('spotify', 'artist')
|
||||
assert bd == {'matched': 1, 'not_found': 0, 'pending': 2, 'total': 3}
|
||||
|
||||
|
||||
def test_reset_failed_requeues_all(db):
|
||||
n = db.reset_enrichment('spotify', 'album', 'failed') # one not_found album
|
||||
assert n == 1
|
||||
bd = db.get_enrichment_breakdown('spotify', 'album')
|
||||
assert bd['not_found'] == 0 and bd['pending'] == 1
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# Flask routes
|
||||
# --------------------------------------------------------------------------
|
||||
|
|
@ -192,3 +230,24 @@ def test_route_breakdown(client):
|
|||
assert r.status_code == 200
|
||||
bd = r.get_json()['breakdown']
|
||||
assert bd['artist'] == {'matched': 1, 'not_found': 1, 'pending': 1, 'total': 3}
|
||||
|
||||
|
||||
def test_route_retry_item(client):
|
||||
r = client.post('/api/enrichment/spotify/retry',
|
||||
json={'entity_type': 'artist', 'scope': 'item', 'entity_id': 'a2'})
|
||||
assert r.status_code == 200
|
||||
body = r.get_json()
|
||||
assert body['success'] is True and body['reset'] == 1
|
||||
|
||||
|
||||
def test_route_retry_failed_bulk(client):
|
||||
r = client.post('/api/enrichment/spotify/retry',
|
||||
json={'entity_type': 'artist', 'scope': 'failed'})
|
||||
assert r.status_code == 200
|
||||
assert r.get_json()['reset'] == 1 # one not_found artist re-queued
|
||||
|
||||
|
||||
def test_route_retry_item_missing_id_400(client):
|
||||
r = client.post('/api/enrichment/spotify/retry',
|
||||
json={'entity_type': 'artist', 'scope': 'item'})
|
||||
assert r.status_code == 400
|
||||
|
|
|
|||
|
|
@ -511,15 +511,22 @@ function _emRenderUnmatchedControls() {
|
|||
const data = enrichmentManagerState.unmatched;
|
||||
const supported = (data && data.entity_types) || ['artist'];
|
||||
const total = data ? (data.total || 0) : null;
|
||||
const entity = enrichmentManagerState.entityTab;
|
||||
const failed = enrichmentManagerState.breakdown?.[entity]?.not_found || 0;
|
||||
const tabs = supported.map(e => `
|
||||
<button class="em-seg-tab ${e === enrichmentManagerState.entityTab ? 'active' : ''}"
|
||||
onclick="setEnrichmentEntityTab('${e}')">${_emEntityLabel(e, true)}</button>`).join('');
|
||||
const bulkBtn = failed
|
||||
? `<button class="em-btn em-btn--sm em-btn--ghost em-retry-all" title="Re-queue every not-found ${_emEntityLabel(entity, true).toLowerCase()}"
|
||||
onclick="retryAllFailedEnrichment(this)">↻ Retry all failed</button>`
|
||||
: '';
|
||||
|
||||
host.innerHTML = `
|
||||
<div class="em-unmatched-bar">
|
||||
<div class="em-section-label em-section-label--inline">
|
||||
Needs matching
|
||||
${total == null ? '' : `<span class="em-count">${total.toLocaleString()}</span>`}
|
||||
${bulkBtn}
|
||||
</div>
|
||||
<div class="em-filter-row">
|
||||
<div class="em-seg-tabs">${tabs}</div>
|
||||
|
|
@ -535,7 +542,8 @@ function _emRenderUnmatchedControls() {
|
|||
oninput="onEnrichmentSearchInput(this.value)">
|
||||
</div>
|
||||
</div>
|
||||
</div>`;
|
||||
</div>
|
||||
<div class="em-hint">Failed lookups auto-retry after 30 days · “Retry” re-queues immediately · “Match” assigns a result by hand.</div>`;
|
||||
}
|
||||
|
||||
function _emRenderUnmatchedList() {
|
||||
|
|
@ -671,13 +679,15 @@ async function toggleEnrichmentWorker(id) {
|
|||
async function retryEnrichmentItem(service, entityType, entityId, btn) {
|
||||
if (btn) { btn.disabled = true; btn.textContent = '…'; }
|
||||
try {
|
||||
const res = await fetch('/api/library/clear-match', {
|
||||
method: 'PUT',
|
||||
// Reset match_status to NULL (pending) so the worker re-attempts on its
|
||||
// next pass — see /retry (clearing to not_found would NOT re-queue).
|
||||
const res = await fetch(`/api/enrichment/${service}/retry`, {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ entity_type: entityType, entity_id: entityId, service }),
|
||||
body: JSON.stringify({ entity_type: entityType, scope: 'item', entity_id: entityId }),
|
||||
});
|
||||
const data = await res.json().catch(() => ({}));
|
||||
if (data.success) {
|
||||
if (res.ok && data.success) {
|
||||
showToast('Re-queued for enrichment', 'success');
|
||||
await Promise.all([_emLoadBreakdown(service), _emLoadUnmatched()]);
|
||||
_emRenderStats();
|
||||
|
|
@ -692,6 +702,39 @@ async function retryEnrichmentItem(service, entityType, entityId, btn) {
|
|||
}
|
||||
}
|
||||
|
||||
// Bulk: re-queue every not_found item of the current entity type.
|
||||
async function retryAllFailedEnrichment(btn) {
|
||||
const service = enrichmentManagerState.selected;
|
||||
const entity = enrichmentManagerState.entityTab;
|
||||
const bd = enrichmentManagerState.breakdown?.[entity];
|
||||
const failed = bd ? (bd.not_found || 0) : 0;
|
||||
if (!failed) { showToast('No failed items to retry', 'info'); return; }
|
||||
if (!confirm(`Re-queue all ${failed.toLocaleString()} not-found ${_emEntityLabel(entity, true).toLowerCase()} for ${_emWorkerById[service].name}? The worker will retry them on its next pass.`)) return;
|
||||
if (btn) { btn.disabled = true; btn.textContent = 'Re-queuing…'; }
|
||||
try {
|
||||
const res = await fetch(`/api/enrichment/${service}/retry`, {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ entity_type: entity, scope: 'failed' }),
|
||||
});
|
||||
const data = await res.json().catch(() => ({}));
|
||||
if (res.ok && data.success) {
|
||||
showToast(`Re-queued ${(data.reset || 0).toLocaleString()} item(s)`, 'success');
|
||||
enrichmentManagerState.page = 0;
|
||||
await Promise.all([_emLoadBreakdown(service), _emLoadUnmatched()]);
|
||||
_emRenderStats();
|
||||
_emRenderUnmatchedControls();
|
||||
_emRenderUnmatchedList();
|
||||
} else {
|
||||
showToast(data.error || 'Failed to re-queue', 'error');
|
||||
}
|
||||
} catch (_e) {
|
||||
showToast('Error re-queuing failed items', 'error');
|
||||
} finally {
|
||||
if (btn) { btn.disabled = false; btn.textContent = '↻ Retry all failed'; }
|
||||
}
|
||||
}
|
||||
|
||||
// ── Inline manual match (decoupled from the library artist-detail page) ───────
|
||||
|
||||
function openEnrichmentMatch(service, entityType, entityId, anchorBtn) {
|
||||
|
|
@ -813,4 +856,5 @@ window.onEnrichmentSearchInput = onEnrichmentSearchInput;
|
|||
window.changeEnrichmentPage = changeEnrichmentPage;
|
||||
window.toggleEnrichmentWorker = toggleEnrichmentWorker;
|
||||
window.retryEnrichmentItem = retryEnrichmentItem;
|
||||
window.retryAllFailedEnrichment = retryAllFailedEnrichment;
|
||||
window.openEnrichmentMatch = openEnrichmentMatch;
|
||||
|
|
|
|||
|
|
@ -65129,3 +65129,8 @@ body.em-scroll-lock { overflow: hidden; }
|
|||
.em-search-wrap .em-search { padding-left: 30px; }
|
||||
.em-search:focus, .em-select:focus { outline: none; border-color: rgba(var(--em-accent-rgb, 99,102,241), 0.6); }
|
||||
.em-row:hover { background: rgba(var(--em-accent-rgb, 255,255,255), 0.07); border-color: rgba(var(--em-accent-rgb, 255,255,255), 0.14); }
|
||||
|
||||
/* Bulk retry-all + helper hint */
|
||||
.em-retry-all { margin-left: 4px; }
|
||||
.em-hint { font-size: 11px; color: rgba(255,255,255,0.38); flex: 0 0 auto; margin-top: -2px; }
|
||||
.em-hint :is(b, strong) { color: rgba(255,255,255,0.6); }
|
||||
|
|
|
|||
Loading…
Reference in a new issue