Add detections_count to sequence and alert responses by ThbltLmr · Pull Request #559 · pyronear/pyro-api

ThbltLmr · 2026-04-07T21:29:44Z

Closes #551

Summary

add a shared helper to batch count detections by sequence
expose detections_count on sequence responses returned by sequence endpoints
expose detections_count on sequence payloads returned by alert endpoints
cover the new field in alert and sequence endpoint tests

Testing

Screen.Recording.2026-04-07.at.23.22.03.mov

Notes

I do not have the access rights to the API repo so I had to fork it sorry
I added the detection count to all the endpoint that were listed in the issue Provide the number of detections associated with a sequence in multiple endpoints. #551, maybe they're not all needed. Let me know if we need to remove the property from some endpoints

codecov · 2026-04-08T11:19:02Z

Codecov Report

❌ Patch coverage is 91.89189% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.00%. Comparing base (1f67522) to head (a011104).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
src/app/api/api_v1/endpoints/alerts.py	87.50%	2 Missing ⚠️
src/app/api/api_v1/endpoints/sequences.py	88.88%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #559      +/-   ##
==========================================
+ Coverage   88.75%   89.00%   +0.25%     
==========================================
  Files          52       53       +1     
  Lines        2196     2219      +23     
==========================================
+ Hits         1949     1975      +26     
+ Misses        247      244       -3

Flag	Coverage Δ
backend	`88.92% <91.89%> (+0.26%)`	⬆️
client	`90.40% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

fe51

Hi @ThbltLmr , Thank you a lot for your PR !

I have a few comments, but no big changes !

Also, about the consideration of adding detection_counts to each sequences related endpoints, here is my views :

frontend actually uses only /alerts/unlabeled/latest and /alerts/all/fromdate and we will need detections count (later or perform pagination and improve front endplayer)
Elsewhere, we do not require performance, and the query added is no expensive, so lets add it. Could be deleted later if not needed and an issue.

Thanks again, I think after answering this comments we will be able to merge quite quickly :)

fe51 · 2026-04-10T14:32:31Z

+    )
+    res = await session.exec(stmt)
+    return {
+        int(sequence_id): int(detections_count)


Why the int() wrapping? After a DB fetch, sequence.id/alert.id are already int per the model annotation, this seems redundant. it is about test/style/ruff ?

Comment here apply in other occurence of int(sequence.id) and int(alert.id)

done in d8b1d5b!

fe51 · 2026-04-10T15:06:29Z



-def _serialize_alert(alert: Alert, sequences: List[Sequence]) -> AlertReadWithSequences:
+def _serialize_sequence(sequence: Sequence, detection_counts: Dict[int, int]) -> SequenceRead:


_serialize_sequence has two different signatures across alerts.py and sequences.py. Could you align them? The sequences.py version (sequence, detections_count: int) seems simpler — the caller does the dict lookup, the function just serializes.

done in 65c3734!

fe51 · 2026-04-10T15:08:55Z

    await session.commit()
    for seq in sequences:
        await session.refresh(seq)
+    for sequence, detections_count in zip(sequences, detections_count_by_sequence, strict=False):


Suggested change

for sequence, detections_count in zip(sequences, detections_count_by_sequence, strict=False):

for sequence, detections_count in zip(sequences, detections_count_by_sequence, strict=True):

strict=False silently ignores a length mismatch between sequences and detections_count_by_sequence. strict=True would catch it immediately if the fixture is ever changed.

done in b598e19!

Per review feedback on PR pyronear#559: get_detection_counts_by_sequence_ids is an aggregation query, not a CRUD/route helper, so it belongs in services/ where it can be reused outside of endpoint modules.

Per review feedback on PR pyronear#559: _serialize_sequence in alerts.py took the full Dict[int, int] of counts and did the lookup inside. Match the sequences.py version (scalar int param, caller does the lookup) so the helper has one shape repo-wide and only does serialization.

Per review feedback on PR pyronear#559: Sequence.id and Alert.id are typed int on the model, so int(sequence.id) / int(alert.id) is redundant. Same for the dict comprehension in get_detection_counts_by_sequence_ids where the row values are already int. The int(alert_id) at alerts.py:46 stays because alert_id there is unpacked from a raw SQL row tuple, not a model attribute.

Per review feedback on PR pyronear#559: strict=False silently swallows length mismatches. Both lists have length 3 today, so flipping to strict=True guards against future fixture edits drifting out of sync.

Per review feedback on PR pyronear#559: Sequence.id and Alert.id are typed int on the model, so int(sequence.id) / int(alert.id) is redundant. Same for the dict comprehension in get_detection_counts_by_sequence_ids where the row values are already int. The int(alert_id) at alerts.py:46 stays because alert_id there is unpacked from a raw SQL row tuple, not a model attribute.

Per review feedback on PR pyronear#559: strict=False silently swallows length mismatches. Both lists have length 3 today, so flipping to strict=True guards against future fixture edits drifting out of sync.

Rebase fallout from pyronear#574 (datetime.utcnow deprecation): one new test function still called datetime.utcnow() while the datetime import had been removed from the file, causing NameError on collection.

fe51

Hi @ThbltLmr, thank you for taking comments into accounts ! All good to merge !

ThbltLmr force-pushed the feat/issue-551 branch from 85766f5 to 1e8ae77 Compare April 8, 2026 00:42

ThbltLmr marked this pull request as ready for review April 8, 2026 00:54

fe51 requested changes Apr 10, 2026

View reviewed changes

ThbltLmr and others added 7 commits May 3, 2026 12:10

feat(schemas): add detection_count to SequenceRead schema

8f8cd89

feat(endpoints): add helper function to get sequence count

c3587bd

feat(endpoints): add sequence count to alerts endpoints

747c755

feat(endpoints): add sequence count to sequences endpoint

687e938

Fix local CI ordering and ruff issues

71460a7

refactor(services): move detection-count helper to services/

d59fc46

Per review feedback on PR pyronear#559: get_detection_counts_by_sequence_ids is an aggregation query, not a CRUD/route helper, so it belongs in services/ where it can be reused outside of endpoint modules.

ThbltLmr force-pushed the feat/issue-551 branch from 41c38ce to 0f684c2 Compare May 3, 2026 10:32

ThbltLmr added 3 commits May 3, 2026 12:36

test(alerts): use strict=True when zipping fixture sequences and counts

b598e19

Per review feedback on PR pyronear#559: strict=False silently swallows length mismatches. Both lists have length 3 today, so flipping to strict=True guards against future fixture edits drifting out of sync.

test(sequences): use utcnow() helper in detections-count test

a011104

Rebase fallout from pyronear#574 (datetime.utcnow deprecation): one new test function still called datetime.utcnow() while the datetime import had been removed from the file, causing NameError on collection.

ThbltLmr force-pushed the feat/issue-551 branch from 0f684c2 to a011104 Compare May 3, 2026 10:37

ThbltLmr requested a review from fe51 May 3, 2026 12:12

fe51 approved these changes May 3, 2026

View reviewed changes

fe51 merged commit 8917c95 into pyronear:main May 3, 2026
24 of 26 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add detections_count to sequence and alert responses#559

Add detections_count to sequence and alert responses#559
fe51 merged 10 commits intopyronear:mainfrom
ThbltLmr:feat/issue-551

ThbltLmr commented Apr 7, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Apr 8, 2026 •

edited

Loading

Uh oh!

fe51 left a comment

Uh oh!

fe51 Apr 10, 2026

Uh oh!

fe51 Apr 10, 2026

Uh oh!

ThbltLmr May 3, 2026

Uh oh!

Uh oh!

fe51 Apr 10, 2026

Uh oh!

ThbltLmr May 3, 2026

Uh oh!

fe51 Apr 10, 2026

Uh oh!

ThbltLmr May 3, 2026

Uh oh!

fe51 left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants



		def _serialize_alert(alert: Alert, sequences: List[Sequence]) -> AlertReadWithSequences:
		def _serialize_sequence(sequence: Sequence, detection_counts: Dict[int, int]) -> SequenceRead:

	for sequence, detections_count in zip(sequences, detections_count_by_sequence, strict=False):
	for sequence, detections_count in zip(sequences, detections_count_by_sequence, strict=True):

Uh oh!

Conversation

ThbltLmr commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Testing

Notes

Uh oh!

codecov Bot commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

fe51 left a comment

Choose a reason for hiding this comment

Uh oh!

fe51 Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

fe51 Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

ThbltLmr May 3, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fe51 Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

ThbltLmr May 3, 2026

Choose a reason for hiding this comment

Uh oh!

fe51 Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

ThbltLmr May 3, 2026

Choose a reason for hiding this comment

Uh oh!

fe51 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ThbltLmr commented Apr 7, 2026 •

edited

Loading

codecov Bot commented Apr 8, 2026 •

edited

Loading