feat: add species cards and hourly heatmap views to detections page#7
feat: add species cards and hourly heatmap views to detections page#7
Conversation
Split the detections page into three switchable views: Table (existing), Species (card grid with aggregated stats), and Grid (hourly detection heatmap). Adds new IPC handlers, database queries, and shared types to support per-species aggregation and hourly detection grouping. The heatmap includes dark/light theme-aware color palettes matching BirdNET-Go style. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
WalkthroughAdds species-aggregation and hourly-detection features: new DB query helpers, IPC handlers, shared types, renderer IPC clients, localization keys, two UI components (SpeciesCards, DetectionHeatmap), and refactors DetectionsPage to support table/species/grid views with per-view loading and filtering. Changes
Sequence DiagramsequenceDiagram
participant User as User/UI
participant Renderer as Renderer
participant Preload as Preload / IPC Bridge
participant Main as Main Process
participant DB as Database
participant Catalog as Label Service
User->>Renderer: switch to "Species" view
Renderer->>Renderer: buildBaseFilter(...)
Renderer->>Preload: invoke('catalog:get-run-species', filter)
Preload->>Main: catalog:get-run-species
Main->>DB: getRunSpeciesAggregation(filter)
DB-->>Main: RawRunSpeciesAggregation[]
Main->>Catalog: resolveAll(scientific_names)
Catalog-->>Main: common_name mappings
Main-->>Preload: RunSpeciesAggregation[]
Preload-->>Renderer: aggregated species data
Renderer->>Renderer: render SpeciesCards
User->>Renderer: switch to "Grid" view
Renderer->>Preload: invoke('catalog:get-hourly-detections', filter)
Preload->>Main: catalog:get-hourly-detections
Main->>DB: getDetectionsForGrid(filter)
DB-->>Main: RawGridDetection[]
Main->>Main: computeDetectionHour(...) & aggregate by (species,hour)
Main->>Catalog: resolveAll(scientific_names)
Catalog-->>Main: common_name mappings
Main-->>Preload: HourlyDetectionCell[]
Preload-->>Renderer: hourly detection cells
Renderer->>Renderer: render DetectionHeatmap
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @tphakala, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the user experience on the detections page by introducing new ways to visualize and interact with detection data. Users can now switch between a detailed table, an aggregated species card view, and an hourly heatmap, providing more comprehensive insights into their analysis runs. The changes involve substantial additions to the data layer for efficient aggregation and a complete overhaul of the detections page UI to support these new interactive views, all while maintaining compatibility with existing filtering capabilities. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces two new views on the detections page: a species card grid and an hourly detection heatmap. The changes include new database queries, IPC handlers, and Svelte components for the new views. The code is well-structured, with good extraction of reusable logic like buildWhereClause. I've found a few issues: a potential timezone bug in the heatmap data calculation, some code duplication in the IPC handlers, and a data-loading bug in the main page component that can lead to stale data being displayed. My review includes suggestions to fix these issues.
src/main/ipc/catalog.ts
Outdated
| const match = /^(\d{4})(\d{2})(\d{2})_(\d{2})(\d{2})(\d{2})$/.exec(base); | ||
| if (match) { | ||
| const [, y, mo, d, h, mi, s] = match; | ||
| const date = new Date(+y, +mo - 1, +d, +h, +mi, +s); |
There was a problem hiding this comment.
The current implementation of computeDetectionHour assumes the timestamp in the AudioMoth filename is in the system's local timezone. These timestamps are often in UTC. This can lead to incorrect hour calculations for the heatmap if the user's system is not in the UTC timezone. To ensure correctness, you should parse the timestamp as UTC.
| const date = new Date(+y, +mo - 1, +d, +h, +mi, +s); | |
| const date = new Date(Date.UTC(+y, +mo - 1, +d, +h, +mi, +s)); |
There was a problem hiding this comment.
Fixed in c391e8f. Changed to Date.UTC() + getUTCHours() so AudioMoth timestamps are correctly parsed as UTC.
| function loadActiveView() { | ||
| void loadRunDetections(); | ||
| if (activeView === 'species') void loadSpeciesView(); | ||
| if (activeView === 'grid') void loadGridView(); | ||
| } | ||
|
|
||
| function switchView(view: DetectionView) { | ||
| if (view === activeView) return; | ||
| activeView = view; | ||
| if (view === 'species' && speciesData.length === 0) void loadSpeciesView(); | ||
| if (view === 'grid' && gridData.length === 0) void loadGridView(); | ||
| } |
There was a problem hiding this comment.
The data loading logic across the different views has a potential bug and is confusing.
- Bug: When filters are changed while on one view (e.g., 'Table'), data for other views is not updated. Switching to another view (e.g., 'Species') will then display stale, unfiltered data because
switchViewonly reloads data if it's empty. - Clarity: The function
loadActiveViewis misleadingly named. It always reloads data for the 'Table' view, regardless of which view is active, and then conditionally loads for other views.
I suggest refactoring this to a clearer and more robust approach where changing filters or switching views reliably loads fresh data for the active view. You would also need to replace all calls to loadActiveView() with loadActiveViewData() throughout the component.
function loadActiveViewData() {
if (!appState.selectedRunId) return;
switch (activeView) {
case 'table':
void loadRunDetections();
break;
case 'species':
void loadSpeciesView();
break;
case 'grid':
void loadGridView();
break;
}
}
function switchView(view: DetectionView) {
if (view === activeView) return;
activeView = view;
loadActiveViewData();
}
There was a problem hiding this comment.
Fixed in c391e8f. Refactored both loadActiveView and switchView to use a switch statement that always reloads data for the active view, eliminating stale data after filter changes.
src/main/ipc/catalog.ts
Outdated
| ipcMain.handle('catalog:get-run-species', (_event, filter: DetectionFilter): RunSpeciesAggregation[] => { | ||
| if (filter.species) { | ||
| const matchingScientific = searchByCommonName(filter.species); | ||
| if (matchingScientific.length > 0) { | ||
| filter = { ...filter, scientific_names: matchingScientific }; | ||
| } | ||
| } |
There was a problem hiding this comment.
This logic for resolving a species filter query to a list of scientific names is duplicated across catalog:get-detections, catalog:get-run-species, and catalog:get-hourly-detections. To improve maintainability and reduce code duplication, consider extracting this into a shared helper function.
There was a problem hiding this comment.
Fixed in c391e8f. Extracted resolveSpeciesFilter() helper and replaced the duplicated pattern in all three handlers.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| function computeDetectionHour(sourceFile: string, startTime: number): number { | ||
| const base = sourceFile.replace(/^.*[\\/]/, '').replace(/\.[^.]+$/, ''); | ||
| const match = /^(\d{4})(\d{2})(\d{2})_(\d{2})(\d{2})(\d{2})$/.exec(base); | ||
| if (match) { | ||
| const [, y, mo, d, h, mi, s] = match; | ||
| const date = new Date(+y, +mo - 1, +d, +h, +mi, +s); | ||
| const actual = new Date(date.getTime() + startTime * 1000); | ||
| return actual.getHours(); | ||
| } | ||
| // Fallback: treat start_time as offset from midnight (hour within recording) | ||
| return Math.floor(startTime / 3600) % 24; |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Fixed in c391e8f. Changed to new Date(Date.UTC(...)) and getUTCHours().
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/main/ipc/catalog.ts`:
- Around line 41-52: computeDetectionHour currently falls back to
Math.floor(startTime/3600)%24 when the filename doesn't match the
YYYYMMDD_HHMMSS pattern, which can produce a misleading wall-clock hour; change
computeDetectionHour to return null (or undefined) on unmatched filenames
instead of a computed hour, update its signature/usage to allow null, and adjust
callers (e.g., heatmap generation or any code that consumes
computeDetectionHour) to treat null as "unknown/skip" or bucket into an
"unknown" column, and add a brief comment in computeDetectionHour explaining
that accurate hours require AudioMoth-style filenames.
In `@src/renderer/src/pages/DetectionsPage.svelte`:
- Around line 157-168: The bug is stale species/grid data after filter changes
because switchView skips reload when speciesData/gridData are non-empty; change
switchView (and adjust loadActiveView) so that switchView always calls
loadSpeciesView() when view === 'species' and always calls loadGridView() when
view === 'grid' (do not gate on speciesData.length or gridData.length), and
update loadActiveView to only load table data when activeView === 'table'
(remove eager loadRunDetections() when not on table) so views are reloaded with
current filters and redundant DB calls are avoided.
🧹 Nitpick comments (5)
src/main/db/detections.ts (1)
135-142: Unbounded result set may cause memory pressure on large runs.
getDetectionsForGridfetches every matching detection row into memory so the IPC handler can compute hourly buckets in JS. For runs with tens or hundreds of thousands of detections this is fine, but very large datasets could cause noticeable memory spikes.Consider adding a SQL-side aggregation (e.g., grouping by
scientific_nameand extracting the hour viasubstr(source_file, …)for the common AudioMoth path) or at least documenting the expected upper bound. Not urgent if current dataset sizes are modest.src/main/ipc/catalog.ts (2)
85-107: Extract the repeated species-filter resolution into a helper.The pattern of resolving
filter.speciesviasearchByCommonNameand attachingscientific_namesis now duplicated in three handlers (lines 74-79, 86-91, 102-107). A small helper would reduce repetition:♻️ Suggested helper extraction
+function resolveSpeciesFilter(filter: DetectionFilter): DetectionFilter { + if (filter.species) { + const matchingScientific = searchByCommonName(filter.species); + if (matchingScientific.length > 0) { + return { ...filter, scientific_names: matchingScientific }; + } + } + return filter; +}Then each handler becomes:
filter = resolveSpeciesFilter(filter);
123-131:parseIntwithout explicit radix.Nitpick:
parseInt(hourStr)on line 128 works correctly here since hour strings are"0"–"23", but passing radix 10 explicitly is a common best practice.♻️ Suggested fix
- hour: parseInt(hourStr), + hour: parseInt(hourStr, 10),src/renderer/src/lib/components/DetectionHeatmap.svelte (1)
73-85:SvelteMapis unnecessary inside$derived.by()— plainMapsuffices.Since these maps are recreated from scratch every time
cellschanges (the$derived.byblock re-runs entirely), there's no need for Svelte's reactiveSvelteMap. A regularMapavoids the reactive-proxy overhead without any loss of reactivity.♻️ Suggested change
const speciesList = $derived.by(() => { - const seen = new SvelteMap<string, string>(); + const seen = new Map<string, string>(); for (const c of cells) seen.set(c.scientific_name, c.common_name); return [...seen.entries()] .sort((a, b) => a[1].localeCompare(b[1])) .map(([scientific_name, common_name]) => ({ scientific_name, common_name })); }); const cellMap = $derived.by(() => { - const lookup = new SvelteMap<string, number>(); + const lookup = new Map<string, number>(); for (const c of cells) lookup.set(`${c.scientific_name}:${c.hour}`, c.detection_count); return lookup; });src/renderer/src/lib/components/SpeciesCards.svelte (1)
21-25: Sort option labels are computed once and won't update on runtime language change.
sortOptionsis a plainconstinitialized at component creation, som.species_card_sortCount()etc. are evaluated once. If the UI language can change at runtime without remounting this component, these labels will be stale. Converting to a$derivedwould fix this, though it's a non-issue if language changes trigger a full page reload.♻️ Reactive alternative
- const sortOptions: { id: SortOption; label: string }[] = [ - { id: 'count', label: m.species_card_sortCount() }, - { id: 'name', label: m.species_card_sortName() }, - { id: 'confidence', label: m.species_card_sortConfidence() }, - ]; + const sortOptions = $derived<{ id: SortOption; label: string }[]>([ + { id: 'count', label: m.species_card_sortCount() }, + { id: 'name', label: m.species_card_sortName() }, + { id: 'confidence', label: m.species_card_sortConfidence() }, + ]);
- Fix stale data bug: loadActiveView/switchView now always reload the active view's data instead of skipping when data exists - Fix UTC timezone bug: computeDetectionHour uses Date.UTC and getUTCHours for correct AudioMoth timestamp parsing - Extract resolveSpeciesFilter helper to deduplicate species name resolution across three IPC handlers - Add explicit radix to parseInt call Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
/gemini review |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/renderer/src/pages/DetectionsPage.svelte`:
- Around line 67-81: The species/grid branches of the headerCount derived value
always call m.pagination_speciesCount and never handle the singular case; update
the species and grid cases (inside the headerCount const derived block) to
detect when the count is exactly 1 (use speciesData.length for 'species' and the
uniqueSpecies count for 'grid') and call a singular i18n key (e.g.,
m.pagination_speciesCountSingular) when count === 1, otherwise call
m.pagination_speciesCount; alternatively, use your existing pluralization helper
if available to produce the correct singular/plural string for species.
🧹 Nitpick comments (3)
src/main/ipc/catalog.ts (1)
101-128: In-memory aggregation of all raw detections may not scale for large runs.
getDetectionsForGridfetches every detection row for the run, then aggregates in JS. For runs with hundreds of thousands of detections, this could cause significant memory pressure and latency. The filename-based hour derivation prevents pushing this to SQL, so the approach is understandable, but consider adding aLIMITsafety cap or a note about expected dataset sizes.The aggregation logic itself (null-char key, split, parseInt) is correct and clean.
src/renderer/src/pages/DetectionsPage.svelte (2)
83-91:run_id: appState.selectedRunId ?? 0silently defaults to an invalid run ID.All callers guard with
if (!appState.selectedRunId) returnbefore reachingbuildBaseFilter, sorun_idwill never actually be0at runtime. However, defaulting to0instead of throwing or returningundefinedmasks potential misuse if a future caller omits the guard. Consider making this explicit:Suggested tightening
function buildBaseFilter() { + const runId = appState.selectedRunId; + if (!runId) throw new Error('buildBaseFilter called without a selected run'); return { - run_id: appState.selectedRunId ?? 0, + run_id: runId, min_confidence: ignoreConfidence ? undefined : appState.minConfidence, species: speciesQuery || undefined, species_list_id: speciesListFilterId || undefined, }; }
284-297: Species-list cross-tab effect:loadActiveView()call is notvoid-prefixed like other fire-and-forget sites.Other call sites (e.g., lines 160, 176, 212) use
void loadRunDetections()orvoid loadSpeciesView()to explicitly discard the promise. Here,loadActiveView()(which itself usesvoidinternally) is called bare. Consistency-wise, consider prefixing withvoidor — sinceloadActiveViewis synchronous and returnsundefined— this is technically fine but differs from the surrounding pattern.
| // --- Contextual header count --- | ||
| const headerCount = $derived.by(() => { | ||
| switch (activeView) { | ||
| case 'table': | ||
| return total === 1 | ||
| ? m.pagination_detectionCountSingular({ count: formatNumber(total) }) | ||
| : m.pagination_detectionCount({ count: formatNumber(total) }); | ||
| case 'species': | ||
| return m.pagination_speciesCount({ count: formatNumber(speciesData.length) }); | ||
| case 'grid': { | ||
| const uniqueSpecies = new Set(gridData.map((c) => c.scientific_name)).size; | ||
| return m.pagination_speciesCount({ count: formatNumber(uniqueSpecies) }); | ||
| } | ||
| } | ||
| }); |
There was a problem hiding this comment.
No singular form for species count in species/grid views.
The table view distinguishes singular vs plural (pagination_detectionCountSingular / pagination_detectionCount), but both species and grid cases always use pagination_speciesCount even when the count is 1. If the i18n key doesn't handle pluralization internally, the header could read "1 species" (correct in English) or awkwardly in other locales.
This is a very minor i18n nit — only matters when exactly one species is present.
🤖 Prompt for AI Agents
In `@src/renderer/src/pages/DetectionsPage.svelte` around lines 67 - 81, The
species/grid branches of the headerCount derived value always call
m.pagination_speciesCount and never handle the singular case; update the species
and grid cases (inside the headerCount const derived block) to detect when the
count is exactly 1 (use speciesData.length for 'species' and the uniqueSpecies
count for 'grid') and call a singular i18n key (e.g.,
m.pagination_speciesCountSingular) when count === 1, otherwise call
m.pagination_speciesCount; alternatively, use your existing pluralization helper
if available to produce the correct singular/plural string for species.
There was a problem hiding this comment.
Valid point. Currently English only and "1 species" reads correctly, but noted for when we add more locales.
There was a problem hiding this comment.
@tphakala, sounds good! You're absolutely right that "1 species" is grammatically correct in English since "species" is one of those words that's both singular and plural. Deferring the singular/plural handling for other locales until you actually add them is a pragmatic approach—no need to over-engineer for hypothetical cases.
Thanks for noting it for future i18n work! 👍
There was a problem hiding this comment.
Code Review
This pull request introduces significant new functionality by adding 'Species' and 'Grid' (heatmap) views to the detections page, complementing the existing table view. The implementation is well-structured, with good separation of concerns between the backend (database queries, IPC handlers) and the frontend (new Svelte components). The use of Svelte 5's new reactivity model is also well done.
My review includes a correction for a potential bug in date parsing within an IPC handler that could lead to incorrect data in the heatmap view, and a suggestion to refactor a small piece of duplicated code in the main page component for better maintainability. Overall, this is a great addition to the application.
| function computeDetectionHour(sourceFile: string, startTime: number): number { | ||
| const base = sourceFile.replace(/^.*[\\/]/, '').replace(/\.[^.]+$/, ''); | ||
| const match = /^(\d{4})(\d{2})(\d{2})_(\d{2})(\d{2})(\d{2})$/.exec(base); | ||
| if (match) { | ||
| const [, y, mo, d, h, mi, s] = match; | ||
| const date = new Date(Date.UTC(+y, +mo - 1, +d, +h, +mi, +s)); | ||
| const actual = new Date(date.getTime() + startTime * 1000); | ||
| return actual.getUTCHours(); | ||
| } | ||
| // Fallback: treat start_time as offset from midnight (hour within recording) | ||
| return Math.floor(startTime / 3600) % 24; | ||
| } |
There was a problem hiding this comment.
The computeDetectionHour function parses a date from the filename but doesn't validate if the parsed date components (year, month, day) are valid. The Date constructor can silently accept invalid values (e.g., month 13) and adjust the date, which could lead to incorrect hour calculations. This is inconsistent with parseRecordingStart in the renderer code, which does perform this validation. It's recommended to add validation to handle potentially malformed filenames gracefully, for example by using the fallback logic.
function computeDetectionHour(sourceFile: string, startTime: number): number {
const base = sourceFile.replace(/^.*[\\/]/, '').replace(/\.[^.]+$/, '');
const match = /^(\d{4})(\d{2})(\d{2})_(\d{2})(\d{2})(\d{2})$/.exec(base);
if (match) {
const [, y, mo, d, h, mi, s] = match;
const date = new Date(Date.UTC(+y, +mo - 1, +d, +h, +mi, +s));
// Validate that the parsed date is not an automatic correction of invalid parts
if (date.getUTCFullYear() !== +y || date.getUTCMonth() !== +mo - 1 || date.getUTCDate() !== +d) {
// Malformed date in filename, use fallback
return Math.floor(startTime / 3600) % 24;
}
const actual = new Date(date.getTime() + startTime * 1000);
return actual.getUTCHours();
}
// Fallback: treat start_time as offset from midnight (hour within recording)
return Math.floor(startTime / 3600) % 24;
}There was a problem hiding this comment.
Fixed in 71d4fe4. Added date component validation - if the parsed date doesn't match the input components (e.g. month 13 silently becoming next year), we fall back to the start_time-based calculation.
| function switchView(view: DetectionView) { | ||
| if (view === activeView) return; | ||
| activeView = view; | ||
| switch (view) { | ||
| case 'table': | ||
| void loadRunDetections(); | ||
| break; | ||
| case 'species': | ||
| void loadSpeciesView(); | ||
| break; | ||
| case 'grid': | ||
| void loadGridView(); | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
The switch statement inside switchView duplicates the logic from loadActiveView. You can simplify this by calling loadActiveView from within switchView. This will make the code more maintainable and reduce redundancy.
function switchView(view: DetectionView) {
if (view === activeView) return;
activeView = view;
loadActiveView();
}
There was a problem hiding this comment.
Fixed in 71d4fe4. switchView now delegates to loadActiveView() instead of duplicating the switch.
- switchView now delegates to loadActiveView instead of duplicating the switch statement - Add date component validation in computeDetectionHour to handle malformed filenames (e.g. month 13) that Date silently corrects Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 participant
Summary
catalog:get-run-species,catalog:get-hourly-detections) with database queries using extractedbuildWhereClausehelperFiles changed
shared/types.ts—RunSpeciesAggregation,HourlyDetectionCellsrc/main/db/detections.ts— extractedbuildWhereClause, addedgetRunSpeciesAggregation,getDetectionsForGridsrc/main/ipc/catalog.ts— two new handlers with label enrichment and AudioMoth filename hour parsingsrc/preload/index.ts— whitelisted new channelssrc/renderer/src/lib/utils/ipc.ts— two new IPC wrapperssrc/renderer/src/lib/components/SpeciesCards.svelte(new),DetectionHeatmap.svelte(new)src/renderer/src/pages/DetectionsPage.svelte— header/filter bar split, view state, switching logicmessages/en.json— 16 new keys for views, cards, and heatmapTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit