Return null from RegistryCache.GetLabelById for unknown labels - #1448
Conversation
|
if you are using an agent for these fixes please make it use correct label |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1448 +/- ##
====================================
Coverage 83% 83%
====================================
Files 201 201
Lines 4170 4170
Branches 486 486
====================================
+ Hits 3468 3470 +2
- Misses 497 499 +2
+ Partials 205 201 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
helto4real
left a comment
There was a problem hiding this comment.
The GetValueOrDefault fix is correct and the unknown-label regression test works: all 153 HassModel tests passed locally, and reverting the production change made the new test fail with KeyNotFoundException. Before approval, please add a focused assertion/test that RegistryCache.GetLabelById(null) returns null (or exercise that path through the concrete HaRegistry.GetLabel method). The modified line currently has only partial branch coverage: local coverage recorded 19 executions but zero hits for the null-id branch, consistent with the failing codecov/patch check. Keep the existing known-label and unknown-label assertions, rerun the HassModel suite with coverage, and ensure codecov/patch is green. No production-code change or coverage-threshold reduction is needed; this request is for the missing null-input test.
GetLabelById used the dictionary indexer while every sibling lookup uses GetValueOrDefault. Each registry is reloaded independently, so an entity, device or area can reference a label the label cache has not loaded yet, which made any access to Registry or entity.Registration throw a KeyNotFoundException. HassObjectMapper already filters null labels. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DGjT5J93Q8hpyZ8RzrCJhk
Covers the null-id branch of RegistryCache.GetLabelById, as requested in review. Claude-Session: https://claude.ai/code/session_01X4cpEyF6m8EX2EMNweyy1M
3796bc0 to
e355bc2
Compare
Proposed change
RegistryCache.GetLabelByIdused the dictionary indexer, so looking up a label id that is not in the label cache threw aKeyNotFoundException. Every sibling lookup (GetHassEntityById,GetDeviceById,GetAreaById,GetFloorById) already returnsnullfor unknown ids, andHassObjectMapperfilters those nulls out withOfType<Label>()when it maps theLabelscollections of entities, devices and areas.Each registry (entities, devices, areas, floors, labels) is reloaded independently and each has its own 5-minute throttle on its
*_registry_updatedevent. This means the entity or area registry can reference a label the label registry has not loaded yet, for example right after a label is created and assigned in the Home Assistant UI. Any app that then touchedIHaContext.Registry,entity.Registration, or navigated to anArea/Devicethrew instead of seeing the entity without that label.This PR:
GetLabelByIdto useGetValueOrDefault, in line with the other lookups. An unknown label is now simply omitted from the mappedLabelslists, andIHaRegistry.GetLabelreturnsnullfor it.TestUnknownLabelIsIgnoredtoRegistryNavigationTest. An entity and an area reference a label id that is not in the label registry; the test assertsGetLabelreturnsnull, the entity registration keeps only the known label, and the area has no labels. Without the fix it fails withKeyNotFoundException: The given key 'unknown_label' was not present in the dictionary.Type of change
Additional information
Checklist
If user exposed functionality or configuration are added/changed: