-
-
Notifications
You must be signed in to change notification settings - Fork 100
fix: suppress duplicate pending detection SSE broadcasts #2433
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,6 +32,34 @@ type SSEPendingDetection struct { | |
| FirstDetected int64 `json:"firstDetected"` // Unix timestamp (seconds) | ||
| Source string `json:"source"` // Source display name | ||
| SourceID string `json:"sourceID"` // Raw source ID for client-side filtering | ||
| HitCount int `json:"hitCount"` // Number of inference hits accumulated | ||
| } | ||
|
|
||
| // sortPendingSnapshot sorts a pending detection snapshot by FirstDetected | ||
| // (oldest first), with species name and source ID as tie-breakers for determinism. | ||
| // This ordering is required by pendingSnapshotChanged which does index-based comparison. | ||
| func sortPendingSnapshot(s []SSEPendingDetection) { | ||
| slices.SortFunc(s, func(a, b SSEPendingDetection) int { | ||
| if a.FirstDetected != b.FirstDetected { | ||
| if a.FirstDetected < b.FirstDetected { | ||
| return -1 | ||
| } | ||
| return 1 | ||
| } | ||
| if a.Species != b.Species { | ||
| if a.Species < b.Species { | ||
| return -1 | ||
| } | ||
| return 1 | ||
| } | ||
| if a.SourceID < b.SourceID { | ||
| return -1 | ||
| } | ||
| if a.SourceID > b.SourceID { | ||
| return 1 | ||
| } | ||
| return 0 | ||
| }) | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| } | ||
|
|
||
| // CalculateVisibilityThreshold computes the minimum hit count for a pending | ||
|
|
@@ -68,27 +96,12 @@ func (p *Processor) SnapshotVisiblePending(minDetections int) []SSEPendingDetect | |
| FirstDetected: item.CreatedAt.Unix(), | ||
| Source: p.getDisplayNameForSource(item.Source), | ||
| SourceID: item.Source, | ||
| HitCount: item.Count, | ||
| }) | ||
| } | ||
| p.pendingMutex.RUnlock() | ||
|
|
||
| // Sort by FirstDetected (oldest first) for stable ordering across broadcasts. | ||
| slices.SortFunc(result, func(a, b SSEPendingDetection) int { | ||
| if a.FirstDetected != b.FirstDetected { | ||
| if a.FirstDetected < b.FirstDetected { | ||
| return -1 | ||
| } | ||
| return 1 | ||
| } | ||
| // Tie-break by species name for determinism. | ||
| if a.Species < b.Species { | ||
| return -1 | ||
| } | ||
| if a.Species > b.Species { | ||
| return 1 | ||
| } | ||
| return 0 | ||
| }) | ||
| sortPendingSnapshot(result) | ||
|
|
||
| return result | ||
| } | ||
|
|
@@ -107,7 +120,9 @@ func (p *Processor) getThumbnailURL(scientificName string) string { | |
| } | ||
|
|
||
| // broadcastPendingSnapshot broadcasts a pending detection snapshot via the | ||
| // PendingBroadcaster callback. If no broadcaster is set, this is a no-op. | ||
| // PendingBroadcaster callback only when the snapshot differs from the last | ||
| // broadcast (new species, removed species, or updated hit counts). | ||
| // If no broadcaster is set, this is a no-op. | ||
| func (p *Processor) broadcastPendingSnapshot(snapshot []SSEPendingDetection) { | ||
| p.pendingBroadcasterMu.RLock() | ||
| broadcaster := p.PendingBroadcaster | ||
|
Comment on lines
120
to
128
This comment was marked as outdated.
Sorry, something went wrong.
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed — added SourceID as tie-breaker in sortPendingSnapshot. |
||
|
|
@@ -117,6 +132,19 @@ func (p *Processor) broadcastPendingSnapshot(snapshot []SSEPendingDetection) { | |
| return | ||
| } | ||
|
|
||
| // Skip broadcast if snapshot is identical to the last one sent. | ||
| // This prevents spamming SSE clients with repeated messages when | ||
| // no new predictions arrived for any visible pending species. | ||
| p.lastBroadcastSnapshotMu.Lock() | ||
| if !pendingSnapshotChanged(p.lastBroadcastSnapshot, snapshot) { | ||
| p.lastBroadcastSnapshotMu.Unlock() | ||
| return | ||
| } | ||
| // Store a copy so subsequent comparisons are independent. | ||
| p.lastBroadcastSnapshot = make([]SSEPendingDetection, len(snapshot)) | ||
| copy(p.lastBroadcastSnapshot, snapshot) | ||
| p.lastBroadcastSnapshotMu.Unlock() | ||
|
|
||
| broadcaster(snapshot) | ||
| } | ||
|
|
||
|
|
@@ -131,7 +159,25 @@ func (p *Processor) buildFlushNotification(item *PendingDetection, status Pendin | |
| FirstDetected: item.CreatedAt.Unix(), | ||
| Source: p.getDisplayNameForSource(item.Source), | ||
| SourceID: item.Source, | ||
| HitCount: item.Count, | ||
| } | ||
| } | ||
|
|
||
| // pendingSnapshotChanged reports whether two sorted pending snapshots differ | ||
| // in species composition, hit counts, or status. | ||
| func pendingSnapshotChanged(prev, curr []SSEPendingDetection) bool { | ||
| if len(prev) != len(curr) { | ||
| return true | ||
| } | ||
| for i := range prev { | ||
| if prev[i].Species != curr[i].Species || | ||
| prev[i].SourceID != curr[i].SourceID || | ||
| prev[i].HitCount != curr[i].HitCount || | ||
| prev[i].Status != curr[i].Status { | ||
| return true | ||
| } | ||
| } | ||
| return false | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| // logPendingBroadcast logs pending broadcast activity at debug level. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -89,10 +89,12 @@ type Processor struct { | |
| sseBroadcasterMutex sync.RWMutex // Mutex to protect SSE broadcaster access | ||
|
|
||
| // Pending detection broadcast fields | ||
| PendingBroadcaster func(snapshot []SSEPendingDetection) // Function to broadcast pending detections via SSE | ||
| pendingBroadcasterMu sync.RWMutex // Mutex to protect PendingBroadcaster access | ||
| pendingFlushNotifs []SSEPendingDetection // Terminal-state notifications from last flush cycle | ||
| pendingFlushNotifsMu sync.Mutex // Mutex to protect pendingFlushNotifs | ||
| PendingBroadcaster func(snapshot []SSEPendingDetection) // Function to broadcast pending detections via SSE | ||
| pendingBroadcasterMu sync.RWMutex // Mutex to protect PendingBroadcaster access | ||
| pendingFlushNotifs []SSEPendingDetection // Terminal-state notifications from last flush cycle | ||
| pendingFlushNotifsMu sync.Mutex // Mutex to protect pendingFlushNotifs | ||
| lastBroadcastSnapshot []SSEPendingDetection // Last broadcast snapshot for change detection | ||
| lastBroadcastSnapshotMu sync.Mutex // Mutex to protect lastBroadcastSnapshot | ||
|
|
||
| // Backup system fields (optional) | ||
| backupManager any // Use interface{} to avoid import cycle | ||
|
|
@@ -1333,11 +1335,15 @@ func (p *Processor) flushPendingDetections(minDetections int) (pendingCount, flu | |
| Status: PendingStatusActive, | ||
| FirstDetected: item.CreatedAt.Unix(), | ||
| Source: p.getDisplayNameForSource(item.Source), | ||
| SourceID: item.Source, | ||
| HitCount: item.Count, | ||
| }) | ||
| } | ||
| } | ||
|
Comment on lines
1335
to
1342
This comment was marked as outdated.
Sorry, something went wrong.
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in e152e50 — extracted |
||
| logPendingBroadcast(len(broadcastSnapshot), len(terminalNotifs)) | ||
| broadcastSnapshot = append(broadcastSnapshot, terminalNotifs...) | ||
| // Sort for stable comparison in broadcastPendingSnapshot. | ||
| sortPendingSnapshot(broadcastSnapshot) | ||
| } | ||
|
|
||
| p.pendingMutex.Unlock() | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current sorting logic in
sortPendingSnapshotis not fully deterministic for the comparison performed inpendingSnapshotChanged. The comparison function checksSourceID, but the sort function does not use it as a tie-breaker.If two detections have the same
FirstDetectedandSpeciesbut differentSourceIDs, their relative order is not guaranteed. This can causependingSnapshotChangedto incorrectly report a change when the order of items happens to differ between snapshots, even if the set of items is identical.To ensure a stable comparison, you should also sort by
SourceIDas a final tie-breaker. I've also taken the liberty to simplify the comparison logic slightly for better readability.References
SourceIDas a tie-breaker, ensures consistent comparisons and avoids spurious change detections.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in latest push — added SourceID as third-level tie-breaker in sortPendingSnapshot.