Use integer IDs for components and supplemental attributes instead of UUIDs#587
Open
daniel-thom wants to merge 4 commits into
Open
Use integer IDs for components and supplemental attributes instead of UUIDs#587daniel-thom wants to merge 4 commits into
daniel-thom wants to merge 4 commits into
Conversation
Components and supplemental attributes are now identified by small integer
IDs assigned by SystemData when attached, instead of random UUIDs. SystemData
tracks two independent ID streams via next_component_id and
next_supplemental_attribute_id, each starting at 1; a component and an
attribute may therefore share a numeric ID. IDs are preserved across
serialization/deserialization.
- InfrastructureSystemsInternal gains an id::Int field (UNASSIGNED_ID until
attached); identity goes through get_id/set_id!. Time series keep their UUIDs.
- SystemData.component_uuids -> component_ids::Dict{Int}; subsystems -> Set{Int};
ComponentUUIDs -> ComponentIDs.
- Supplemental attribute associations table uses component_id/attribute_id
INTEGER columns.
- Time series metadata store uses owner_id INTEGER. Because the two ID streams
can collide, owner-specific queries (has_metadata, owner where-clause,
replace_component_id!) also filter by owner_category.
- Tests updated; new tests cover ID round-trip preservation, both counters
surviving serialization, and independent ID streams.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Component and supplemental attribute ids are independent streams and may collide numerically, so the unique time-series index must key on (owner_id, owner_category), not owner_id alone. Without this, a component and an attribute that share an id and have a time series with the same name/type/params violate the unique constraint. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
daniel-thom
force-pushed
the
IS4-integer-ids
branch
from
June 18, 2026 15:55
1ab5d74 to
2707dc4
Compare
There was a problem hiding this comment.
Pull request overview
This pull request transitions component and supplemental attribute identity within InfrastructureSystems.jl from UUID-based references to integer IDs, updating core containers, association stores, and serialization/tests accordingly (while retaining UUIDs for time series objects as noted in the PR description).
Changes:
- Replace component/supplemental-attribute UUID storage and lookup paths with integer ID storage/lookup (including subsystem membership).
- Update SQLite-backed time series metadata associations and supplemental attribute associations to store
owner_id/component_idinstead of UUID strings. - Update serialization/deserialization and tests to validate ID preservation and correct behavior after ID reassignment.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| test/test_time_series.jl | Updates time series tests to use component IDs and owner_id schema fields. |
| test/test_system_data.jl | Updates SystemData tests for component/attribute ID lookup and ID reassignment. |
| test/test_supplemental_attributes.jl | Adjusts manager-direct tests to manually set integer IDs to mimic attachment. |
| test/test_serialization.jl | Adds coverage for ID and next-ID counter preservation across serialization. |
| test/test_internal.jl | Updates internal tests for integer ID semantics and serialization round-trip. |
| test/test_component_container.jl | Updates component container tests to use integer IDs in lookups. |
| src/time_series_metadata_store.jl | Migrates metadata associations from owner_uuid to (owner_id, owner_category) and updates queries/indexes. |
| src/system_data.jl | Introduces component/attribute ID streams, ID assignment, and component lookup by ID. |
| src/supplemental_attribute_manager.jl | Stores attributes keyed by integer ID rather than UUID. |
| src/supplemental_attribute_associations.jl | Updates association table schema/queries to use integer IDs. |
| src/subsystems.jl | Switches subsystem membership tracking from UUID sets to integer ID sets. |
| src/iterators.jl | Updates filtered iteration to filter by get_id and Set{Int}. |
| src/internal.jl | Adds integer id to internal storage and introduces UNASSIGNED_ID, get_id, set_id!. |
| src/InfrastructureSystems.jl | Switches include from component_uuids.jl to component_ids.jl and updates docs. |
| src/components.jl | Renames component filtering parameter to component_ids and routes to ID-based iterators. |
| src/component.jl | Replaces component UUID reassignment logic with ID reassignment and reference updates. |
| src/component_uuids.jl | Removes old ComponentUUIDs helper. |
| src/component_ids.jl | Adds new ComponentIDs helper for supplemental attribute component ID tracking. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+29
to
+30
| "User-defined subystems. Components can be regular or masked." | ||
| subsystems::Dict{String, Set{Base.UUID}} | ||
| subsystems::Dict{String, Set{Int}} |
… fixes - assign_new_id_internal!: remap the component's ID in subsystem membership sets, alongside the existing time-series metadata and supplemental-attribute association updates (previously left stale, breaking subsystem lookups after reassignment) - Guard against UNASSIGNED_ID when attaching attributes and adding associations on the manager-direct path, throwing actionable errors instead of colliding at id 0 - Fix "owner UUID" -> "owner ID" docstring in list_metadata_with_owner_id - Fix "subystem(s)" typos in system_data.jl and subsystems.jl - Tests: assert subsystem membership tracks the new ID after assign_new_id!; assign IDs in manager-direct supplemental attribute tests Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment on lines
+92
to
+106
| 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{Base.UUID, T}() | ||
| mgr.data[T] = Dict{Int, T}() | ||
| end | ||
| mgr.data[T][get_uuid(attribute)] = attribute | ||
| mgr.data[T][id] = attribute |
Comment on lines
+1070
to
+1072
| subsystems = Dict(k => Set{Int}(v) for (k, v) in raw["subsystems"]) | ||
| next_component_id = raw["next_component_id"] | ||
| next_supplemental_attribute_id = raw["next_supplemental_attribute_id"] |
Comment on lines
+111
to
112
| All subtypes must include an instance of [`ComponentIDs`](@ref) in order to track | ||
| components attached to each attribute. |
daniel-thom
added a commit
that referenced
this pull request
Jun 23, 2026
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/time-series-store#4.) Full test suite passes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note: InfrastructureSystemsInternal will continue to have a UUID field for time series until we have migrated to the new TimeSeriesStore.