Port #587 review follow-ups: subsystem id remap & UNASSIGNED_ID guards

Bring the integer-id model to parity with PR #587 (the pure-Julia integer-id
PR), porting the review-driven fixes that were missing here:

- assign_new_id_internal!: also remap the component's id in subsystem membership
  sets (previously left stale, breaking subsystem lookups after reassignment).
- Guard against UNASSIGNED_ID on the manager-direct path when attaching a
  supplemental attribute and when adding an association, with actionable errors
  instead of colliding at id 0.
- Fix "subystem" typos in system_data.jl and subsystems.jl.
- Tests: assert subsystem membership tracks the new id after assign_new_id!;
  assign ids in the manager-direct supplemental attribute tests.

(PR #587's f896870 put owner_category in the Julia metadata-store unique index;
here that lives in the Rust store instead — NatLabRockies/infrastore#4.)

Full test suite passes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: daniel-thom

src/component.jl

@@ -1,6 +1,7 @@
 """
-Assign a new integer id to the component, drawn from the system counter, and update any
-references to its old id in the time series store and supplemental attribute associations.
+Assign a new integer ID to the component, drawn from the system counter, and update any
+references to its old ID in the time series store, supplemental attribute associations, and
+subsystem membership sets.
 """
 function assign_new_id_internal!(data, component::InfrastructureSystemsComponent)
     old_id = get_id(component)
@@ -15,6 +16,13 @@ function assign_new_id_internal!(data, component::InfrastructureSystemsComponent
         replace_component_id!(associations, old_id, new_id)
     end
 
+    for ids in values(data.subsystems)
+        if old_id in ids
+            pop!(ids, old_id)
+            push!(ids, new_id)
+        end
+    end
+
     set_id!(get_internal(component), new_id)
     return
 end

src/subsystems.jl

@@ -7,7 +7,7 @@ function add_subsystem!(data::SystemData, subsystem_name::AbstractString)
     end
 
     data.subsystems[subsystem_name] = Set{Int}()
-    @debug "Added subystem $subsystem_name" _group = LOG_GROUP_SYSTEM
+    @debug "Added subsystem $subsystem_name" _group = LOG_GROUP_SYSTEM
     return
 end
 
@@ -33,7 +33,7 @@ Throws ArgumentError if the subsystem name is not stored.
 function remove_subsystem!(data::SystemData, subsystem_name::AbstractString)
     _throw_if_not_stored(data, subsystem_name)
     container = pop!(data.subsystems, subsystem_name)
-    @debug "Removed subystem $subsystem_name" length(container) _group = LOG_GROUP_SYSTEM
+    @debug "Removed subsystem $subsystem_name" length(container) _group = LOG_GROUP_SYSTEM
     return
 end
 
@@ -61,7 +61,7 @@ function add_component_to_subsystem!(
     end
 
     push!(container, id)
-    @debug "Added $(summary(component)) to subystem $subsystem_name" _group =
+    @debug "Added $(summary(component)) to subsystem $subsystem_name" _group =
         LOG_GROUP_SYSTEM
     return
 end
@@ -98,7 +98,7 @@ function remove_component_from_subsystem!(
     end
 
     pop!(data.subsystems[subsystem_name], get_id(component))
-    @debug "Removed $(summary(component)) from subystem $subsystem_name" _group =
+    @debug "Removed $(summary(component)) from subsystem $subsystem_name" _group =
         LOG_GROUP_SYSTEM
     return
 end
@@ -110,7 +110,7 @@ function remove_component_from_subsystems!(
     id = get_id(component)
     for (subsystem_name, ids) in data.subsystems
         pop!(ids, id, nothing)
-        @debug "Removed $(summary(component)) from subystem $subsystem_name" _group =
+        @debug "Removed $(summary(component)) from subsystem $subsystem_name" _group =
             LOG_GROUP_SYSTEM
     end
     return

src/supplemental_attribute_associations.jl

@@ -128,10 +128,20 @@ function add_association!(
     attribute::SupplementalAttribute,
 )
     TimerOutputs.@timeit_debug SYSTEM_TIMERS "add supplemental attribute association" begin
+        attribute_id = get_id(attribute)
+        component_id = get_id(component)
+        if attribute_id == UNASSIGNED_ID || component_id == UNASSIGNED_ID
+            throw(
+                ArgumentError(
+                    "cannot associate $(summary(attribute)) with $(summary(component)): " *
+                    "both must have assigned IDs (attach them to `SystemData` first).",
+                ),
+            )
+        end
         row = (
-            get_id(attribute),
+            attribute_id,
             string(nameof(typeof(attribute))),
-            get_id(component),
+            component_id,
             string(nameof(typeof(component))),
         )
         placeholder = chop(repeat("?,", length(row)))

src/supplemental_attribute_manager.jl

@@ -89,11 +89,21 @@ function _attach_attribute!(
         )
     end
 
+    id = get_id(attribute)
+    if id == UNASSIGNED_ID
+        throw(
+            ArgumentError(
+                "$(summary(attribute)) has an unassigned ID; attach it through " *
+                "`add_supplemental_attribute!` on `SystemData` so an ID is assigned first.",
+            ),
+        )
+    end
+
     T = typeof(attribute)
     if !haskey(mgr.data, T)
         mgr.data[T] = Dict{Int, T}()
     end
-    mgr.data[T][get_id(attribute)] = attribute
+    mgr.data[T][id] = attribute
 end
 
 function is_attached(attribute::SupplementalAttribute, mgr::SupplementalAttributeManager)

src/system_data.jl

@@ -27,7 +27,7 @@ mutable struct SystemData <: ComponentContainer
     next_component_id::Int
     "Next integer id to assign to a supplemental attribute. Independent of the component id stream. Starts at 1."
     next_supplemental_attribute_id::Int
-    "User-defined subystems. Components can be regular or masked."
+    "User-defined subsystems. Components can be regular or masked."
     subsystems::Dict{String, Set{Int}}
     supplemental_attribute_manager::SupplementalAttributeManager
     time_series_manager::TimeSeriesManager

test/test_supplemental_attributes.jl

@@ -184,16 +184,16 @@ end
 
 @testset "Test iterate_SupplementalAttributeManager" begin
     mgr = IS.SupplementalAttributeManager()
-    geo_supplemental_attribute = IS.GeographicInfo()
-    component = IS.TestComponent("component1", 5)
+    geo_supplemental_attribute = _id!(IS.GeographicInfo(), 2)
+    component = _id!(IS.TestComponent("component1", 5), 1)
     IS.add_supplemental_attribute!(mgr, component, geo_supplemental_attribute)
     @test length(collect(IS.iterate_supplemental_attributes(mgr))) == 1
 end
 
 @testset "Summarize SupplementalAttributeManager" begin
     mgr = IS.SupplementalAttributeManager()
-    geo_supplemental_attribute = IS.GeographicInfo()
-    component = IS.TestComponent("component1", 5)
+    geo_supplemental_attribute = _id!(IS.GeographicInfo(), 2)
+    component = _id!(IS.TestComponent("component1", 5), 1)
     IS.add_supplemental_attribute!(mgr, component, geo_supplemental_attribute)
     summary(devnull, mgr)
 end

test/test_system_data.jl

@@ -566,12 +566,20 @@ end
     component = IS.TestComponent(name, 5)
     IS.add_component!(data, component)
     id1 = IS.get_id(component)
+    subsystem_name = "subsystem1"
+    IS.add_subsystem!(data, subsystem_name)
+    IS.add_component_to_subsystem!(data, subsystem_name, component)
     IS.assign_new_id!(data, component)
     id2 = IS.get_id(component)
     @test id1 != id2
     @test IS.get_component(data, id2) === component
     @test_throws ArgumentError IS.get_component(data, id1)
     @test IS.get_component(IS.TestComponent, data, name).name == name
+    # The subsystem membership set must track the new ID after reassignment.
+    @test IS.has_component(data, subsystem_name, component)
+    @test collect(IS.get_subsystem_components(data, subsystem_name)) == [component]
+    @test id2 in IS.get_component_ids(data, subsystem_name)
+    @test !(id1 in IS.get_component_ids(data, subsystem_name))
 end
 
 @testset "Test independent integer id streams" begin