Skip to content

Commit 8d65665

Browse files
patrick91patrick91
committed
Address review feedback: deduplicate query, add error handling, exponential backoff
- Extract get_accepted_submissions() to remove duplicated query between admin.py and tasks.py - Add try/except in Celery task that caches error state so the frontend can show the actual failure instead of a timeout - Frontend handles {"status": "error"} responses from cached error state - Replace fixed 3s polling with exponential backoff (1s, 2s, 3s, 5s...)
1 parent 9b81155 commit 8d65665

3 files changed

Lines changed: 104 additions & 82 deletions

File tree

‎backend/reviews/admin.py‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,19 @@
1717
from users.admin_mixins import ConferencePermissionMixin
1818

1919

20+
def get_accepted_submissions(conference):
21+
return (
22+
Submission.objects.filter(conference=conference)
23+
.filter(
24+
Q(pending_status=Submission.STATUS.accepted)
25+
| Q(pending_status__isnull=True, status=Submission.STATUS.accepted)
26+
| Q(pending_status="", status=Submission.STATUS.accepted)
27+
)
28+
.select_related("speaker", "type", "audience_level")
29+
.prefetch_related("languages")
30+
)
31+
32+
2033
class AvailableScoreOptionInline(admin.TabularInline):
2134
model = AvailableScoreOption
2235

@@ -366,16 +379,7 @@ def review_shortlist_view(self, request, review_session_id):
366379
return TemplateResponse(request, adapter.shortlist_template, context)
367380

368381
def _get_accepted_submissions(self, conference):
369-
return (
370-
Submission.objects.filter(conference=conference)
371-
.filter(
372-
Q(pending_status=Submission.STATUS.accepted)
373-
| Q(pending_status__isnull=True, status=Submission.STATUS.accepted)
374-
| Q(pending_status="", status=Submission.STATUS.accepted)
375-
)
376-
.select_related("speaker", "type", "audience_level")
377-
.prefetch_related("languages")
378-
)
382+
return get_accepted_submissions(conference)
379383

380384
def review_recap_view(self, request, review_session_id):
381385
review_session = ReviewSession.objects.get(id=review_session_id)

‎backend/reviews/tasks.py‎

Lines changed: 53 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -8,65 +8,70 @@
88
@app.task
99
def compute_recap_analysis(conference_id, force_recompute=False):
1010
from django.core.cache import cache
11-
from django.db.models import Q
1211

12+
from reviews.admin import get_accepted_submissions
1313
from reviews.similar_talks import (
1414
_get_cache_key,
1515
compute_similar_talks,
1616
compute_topic_clusters,
1717
)
18-
from submissions.models import Submission
1918

20-
accepted_submissions = list(
21-
Submission.objects.filter(conference_id=conference_id)
22-
.filter(
23-
Q(pending_status=Submission.STATUS.accepted)
24-
| Q(pending_status__isnull=True, status=Submission.STATUS.accepted)
25-
| Q(pending_status="", status=Submission.STATUS.accepted)
26-
)
27-
.select_related("speaker", "type", "audience_level")
28-
.prefetch_related("languages")
29-
)
19+
from conferences.models import Conference
3020

31-
similar_talks = compute_similar_talks(
32-
accepted_submissions,
33-
top_n=5,
34-
conference_id=conference_id,
35-
force_recompute=force_recompute,
36-
)
21+
conference = Conference.objects.get(id=conference_id)
22+
accepted_submissions = list(get_accepted_submissions(conference))
3723

38-
topic_clusters = compute_topic_clusters(
39-
accepted_submissions,
40-
min_topic_size=3,
41-
conference_id=conference_id,
42-
force_recompute=force_recompute,
24+
combined_cache_key = _get_cache_key(
25+
"recap_analysis", conference_id, accepted_submissions
4326
)
4427

45-
submissions_list = sorted(
46-
[
47-
{
48-
"id": s.id,
49-
"title": str(s.title),
50-
"type": s.type.name,
51-
"speaker": s.speaker.display_name if s.speaker else "Unknown",
52-
"similar": similar_talks.get(s.id, []),
53-
}
54-
for s in accepted_submissions
55-
],
56-
key=lambda x: max(
57-
(item["similarity"] for item in x["similar"]), default=0
58-
),
59-
reverse=True,
60-
)
28+
try:
29+
similar_talks = compute_similar_talks(
30+
accepted_submissions,
31+
top_n=5,
32+
conference_id=conference_id,
33+
force_recompute=force_recompute,
34+
)
35+
36+
topic_clusters = compute_topic_clusters(
37+
accepted_submissions,
38+
min_topic_size=3,
39+
conference_id=conference_id,
40+
force_recompute=force_recompute,
41+
)
6142

62-
result = {
63-
"submissions_list": submissions_list,
64-
"topic_clusters": topic_clusters,
65-
}
43+
submissions_list = sorted(
44+
[
45+
{
46+
"id": s.id,
47+
"title": str(s.title),
48+
"type": s.type.name,
49+
"speaker": s.speaker.display_name if s.speaker else "Unknown",
50+
"similar": similar_talks.get(s.id, []),
51+
}
52+
for s in accepted_submissions
53+
],
54+
key=lambda x: max(
55+
(item["similarity"] for item in x["similar"]), default=0
56+
),
57+
reverse=True,
58+
)
6659

67-
combined_cache_key = _get_cache_key(
68-
"recap_analysis", conference_id, accepted_submissions
69-
)
70-
cache.set(combined_cache_key, result, 60 * 60 * 24)
60+
result = {
61+
"submissions_list": submissions_list,
62+
"topic_clusters": topic_clusters,
63+
}
64+
65+
cache.set(combined_cache_key, result, 60 * 60 * 24)
7166

72-
return result
67+
return result
68+
except Exception:
69+
logger.exception(
70+
"Failed to compute recap analysis for conference %s", conference_id
71+
)
72+
cache.set(
73+
combined_cache_key,
74+
{"status": "error", "message": "Analysis failed. Please try again."},
75+
60 * 5,
76+
)
77+
raise

‎backend/reviews/templates/reviews-recap.html‎

Lines changed: 37 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -572,18 +572,43 @@ <h2 class="recap-section-title">🔗 Similar Talks</h2>
572572

573573
var pollTimer = null;
574574
var pollStartTime = null;
575-
var POLL_INTERVAL = 3000;
575+
var pollAttempt = 0;
576576
var POLL_TIMEOUT = 120000;
577577

578+
function getNextPollInterval() {
579+
// Exponential backoff: 1s, 2s, 3s, 5s, 5s, 5s...
580+
var intervals = [1000, 2000, 3000, 5000];
581+
return intervals[Math.min(pollAttempt, intervals.length - 1)];
582+
}
583+
578584
function stopPolling() {
579585
if (pollTimer) {
580586
clearTimeout(pollTimer);
581587
pollTimer = null;
582588
}
583589
pollStartTime = null;
590+
pollAttempt = 0;
591+
}
592+
593+
function showError(message) {
594+
stopPolling();
595+
loading.style.display = 'none';
596+
btn.disabled = false;
597+
btn.textContent = 'Compute Topic Clusters & Similar Talks';
598+
recomputeBtn.disabled = false;
599+
recomputeBtn.textContent = 'Recompute (ignore cache)';
600+
errorDiv.textContent = message;
601+
errorDiv.style.display = '';
584602
}
585603

586-
function handleResult(data, recompute) {
604+
function handleResult(data) {
605+
if (data.status === 'error') {
606+
showError(data.message || 'Analysis failed. Please try again.');
607+
recomputeBtn.style.display = '';
608+
btn.style.display = 'none';
609+
return;
610+
}
611+
587612
loading.style.display = 'none';
588613
btn.style.display = 'none';
589614
recomputeBtn.style.display = '';
@@ -594,21 +619,13 @@ <h2 class="recap-section-title">🔗 Similar Talks</h2>
594619
renderSimilarTalks(data.submissions_list);
595620
}
596621

597-
function pollForResults(recompute) {
622+
function pollForResults() {
598623
if (pollStartTime && (Date.now() - pollStartTime) > POLL_TIMEOUT) {
599-
stopPolling();
600-
loading.style.display = 'none';
601-
var activeBtn = recompute ? recomputeBtn : btn;
602-
activeBtn.disabled = false;
603-
activeBtn.textContent = recompute ? 'Recompute (ignore cache)' : 'Compute Topic Clusters & Similar Talks';
604-
errorDiv.textContent = 'Analysis is taking longer than expected. Please try again later.';
605-
errorDiv.style.display = '';
624+
showError('Analysis is taking longer than expected. Please try again later.');
606625
return;
607626
}
608627

609-
var url = recompute ? computeUrl + '?recompute=1' : computeUrl;
610-
611-
fetch(url, {
628+
fetch(computeUrl, {
612629
headers: { 'X-Requested-With': 'XMLHttpRequest' }
613630
})
614631
.then(function(response) {
@@ -617,20 +634,15 @@ <h2 class="recap-section-title">🔗 Similar Talks</h2>
617634
})
618635
.then(function(data) {
619636
if (data.status === 'processing') {
620-
pollTimer = setTimeout(function() { pollForResults(false); }, POLL_INTERVAL);
637+
pollAttempt++;
638+
pollTimer = setTimeout(pollForResults, getNextPollInterval());
621639
return;
622640
}
623641
stopPolling();
624-
handleResult(data, recompute);
642+
handleResult(data);
625643
})
626644
.catch(function(err) {
627-
stopPolling();
628-
loading.style.display = 'none';
629-
var activeBtn = recompute ? recomputeBtn : btn;
630-
activeBtn.disabled = false;
631-
activeBtn.textContent = recompute ? 'Recompute (ignore cache)' : 'Compute Topic Clusters & Similar Talks';
632-
errorDiv.textContent = 'Failed to compute analysis: ' + err.message;
633-
errorDiv.style.display = '';
645+
showError('Failed to compute analysis: ' + err.message);
634646
});
635647
}
636648

@@ -653,10 +665,11 @@ <h2 class="recap-section-title">🔗 Similar Talks</h2>
653665
.then(function(data) {
654666
if (data.status === 'processing') {
655667
pollStartTime = Date.now();
656-
pollTimer = setTimeout(function() { pollForResults(false); }, POLL_INTERVAL);
668+
pollAttempt = 0;
669+
pollTimer = setTimeout(pollForResults, getNextPollInterval());
657670
return;
658671
}
659-
handleResult(data, recompute);
672+
handleResult(data);
660673
})
661674
.catch(function(err) {
662675
loading.style.display = 'none';

0 commit comments

Comments
 (0)