Repository navigation
fix(species): source species lists from the multi-model union, not primary labels - #3504
Conversation
…imary labels
User-facing species lists were built from the primary BirdNET labels only, so
secondary-model species (bats, Perch) such as Barbastella barbastellus
("mopsilepakko" in Finnish) were missing from the settings UI and the config
filters even though the dashboard dictionary localized them correctly.
Route these surfaces through the loaded-model union (Orchestrator.AllLabels,
which includes the bat/Perch classifiers) plus OpenFauna localization:
- classifier: GetAllProbableSpeciesWithSettings now includes bat-model species
as always-active (score 1.0), deduped by scientific name and exclude-honored,
since the bat model has no geomodel range filter. This feeds the Active
Species tab.
- api/v2/species: the /species/all picker fallback and getSpeciesInfo now use
the AllLabels union (deduped by scientific name); getSpeciesInfo localizes
scientific-only labels via the OpenFauna resolver and no longer 404s for
secondary-model species; rarity now uses the union so bats are not always
reported "very rare".
- processor: the extended-capture and daylight species filters resolve against
the union and reverse-resolve localized common names through OpenFauna, so a
config entry like "mopsilepakko" matches.
Tests cover the bat always-active path, scientific-name dedup, the picker
builder, and the OpenFauna reverse-lookup resolution.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe PR makes the bat classifier model always-active (score 1.0) in ChangesMulti-model label union and bat-always-active propagation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Code Review
This pull request updates species resolution and filtering to support multi-model setups, including secondary models like bats and Perch. Key changes include resolving config entries against the full multi-model label union, reverse-resolving localized common names of secondary-model species via OpenFauna, and ensuring bat-model species are included as always-active in the probable species list. A review comment identifies a bug in internal/api/v2/species.go where the localization lookup for scientific-only labels is skipped because ParseSpeciesString sets commonName to the scientific name, making the commonName == "" check evaluate to false. The reviewer suggests checking if commonName is equal to scientificName to fix this.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if matchedLabel != "" && commonName == "" { | ||
| if resolved := bn.ResolveName(scientificName, c.currentLocale()); resolved != "" { | ||
| commonName = resolved | ||
| } | ||
| } |
There was a problem hiding this comment.
Since ParseSpeciesString returns CommonName == ScientificName for scientific-only labels (such as bats or Perch-unique species), commonName will be populated with the scientific name rather than being empty. As a result, the condition commonName == "" will evaluate to false, and the localization lookup via bn.ResolveName will be skipped entirely for these species. To ensure they are correctly localized, the condition should also check if commonName is equal to scientificName.
if if matchedLabel != "" && (commonName == "" || strings.EqualFold(commonName, scientificName)) {
if resolved := bn.ResolveName(scientificName, c.currentLocale()); resolved != "" {
commonName = resolved
}
}There was a problem hiding this comment.
Good catch, that condition never fired for scientific-only labels (ParseSpeciesString returns CommonName == ScientificName for them). Fixed in d72f18b to also localize when the parsed common name equals the scientific name.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/analysis/processor/extended_capture.go`:
- Around line 194-205: The loop that processes reverse-resolved scientific names
from LookupScientificNames currently adds all returned names to the resolved map
without filtering against the loaded model's scientificNames. You need to
intersect the reverse hits with scientificNames: for each entry in unresolved,
after calling LookupScientificNames to get sciNames, check each sci value
against scientificNames and only add to resolved if it exists in that set. If no
scientific names from the reverse lookup intersect with scientificNames, the
entry should remain in stillUnresolved instead of being resolved to species the
model cannot emit.
In `@internal/api/v2/species.go`:
- Around line 322-327: The issue is that `GetAllProbableSpeciesWithSettings()`
returns synthetic Score: 1.0 entries for species without geomodel-backed
occurrence scores (such as bats), and these synthetic scores are being fed into
`getSpeciesRarityInfo()` where `calculateRarityStatus` incorrectly classifies
them as very_common instead of unknown or unmapped. To fix this, either filter
out the synthetic scores before passing them to `getSpeciesRarityInfo()` so
rarity calculation stays probability-backed, or modify the code to distinguish
between synthetic visibility scores and real occurrence scores at the
orchestrator level before the rarity classification logic processes them. Ensure
that only genuine geomodel-backed probability scores are used for rarity
determination.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 4a20f7aa-5ece-459c-a996-2f2c29c7796f
📒 Files selected for processing (9)
internal/analysis/processor/daylight_filter.gointernal/analysis/processor/daylight_filter_test.gointernal/analysis/processor/extended_capture.gointernal/analysis/processor/extended_capture_test.gointernal/analysis/processor/species_filter_openfauna_test.gointernal/api/v2/species.gointernal/api/v2/species_all_test.gointernal/classifier/orchestrator.gointernal/classifier/orchestrator_allspecies_test.go
…and rarity - getSpeciesInfo: localize when the parsed common name equals the scientific name (scientific-only labels parse that way), not only when it is empty, so bats actually get their localized name. - resolveSpeciesFilter: constrain OpenFauna reverse-lookup hits to species the loaded models can emit; entries that only resolve to absent species stay unresolved (and still warn) instead of matching a species nothing can detect. - getSpeciesRarityInfo: keep rarity on the geomodel-backed probable list rather than the multi-model union, whose synthetic always-active scores would misclassify bats as "very common"; report "unknown" for species with no geomodel coverage instead of a misleading rarity.
|
@coderabbitai review |
|
/gemini review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Code Review
This pull request enhances species resolution and filtering for multi-model setups (such as primary BirdNET plus secondary models like bats or Perch). It updates the daylight filter, extended capture, and API endpoints to resolve species against the full multi-model label union rather than just primary labels. It integrates OpenFauna reverse lookup to resolve localized common names of secondary-model species, handles scientific-only labels correctly, and ensures bat-model species (which lack geomodels) are treated as always-active (score 1.0) rather than being misclassified or omitted. Comprehensive unit tests have been added to verify these behaviors. There are no review comments, so I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Summary
User-facing species lists were built from the primary BirdNET labels only, so secondary-model species (bats, Perch) such as Barbastella barbastellus ("mopsilepakko" in Finnish) were missing from the species settings UI and the config filters, even though the dashboard's per-locale dictionary already localizes them correctly. The species display path used the authoritative OpenFauna dictionary, but the species list/prediction paths used a narrower model-label source.
This routes those surfaces through the loaded-model union (
Orchestrator.AllLabels(), which includes the bat/Perch classifiers) plus OpenFauna localization.Changes
GetAllProbableSpeciesWithSettingsnow includes bat-model species as always-active (score1.0), deduped by scientific name and exclude-honored. The bat model has no geomodel range filter, so its species are never location-filtered. This feeds the "Active Species" tab on the settings page (previously bird-only)./species/allinclude/exclude picker fallback andgetSpeciesInfonow use theAllLabels()union (deduped by scientific name).getSpeciesInfolocalizes scientific-only labels through the OpenFauna resolver with the configured locale and no longer 404s for secondary-model species; species rarity now uses the union so bats are not always reported "very rare".Test Plan
go test -race ./internal/classifier/ ./internal/api/v2/ ./internal/analysis/processor/(all green)golangci-lint runon changed packages (0 issues)/species/allbuilder (cached-map, fallback, fallback dedup, scientific-only), and OpenFauna reverse-lookup resolution (mopsilepakko -> Barbastella barbastellus)Summary by CodeRabbit
Release Notes
New Features
Improvements
Tests