Skip to content

feat: replace UUID-based identifiers with stable integer IDs#156

Open
pesap wants to merge 9 commits into
mainfrom
feat/ids
Open

feat: replace UUID-based identifiers with stable integer IDs#156
pesap wants to merge 9 commits into
mainfrom
feat/ids

Conversation

@pesap

@pesap pesap commented May 24, 2026

Copy link
Copy Markdown
Collaborator

Components, supplemental attributes, and time series metadata now use monotonically-increasing integer IDs as their primary identifier. Legacy UUIDs are preserved as legacy_uuid for backward compatibility.

Key changes:

  • InfraSysBaseModelWithIdentifiers: new id field (int), legacy_uuid for BC
  • IDManager: tracks next available ID, supports advance_past() for serialized data with pre-assigned IDs
  • SQLite tables use INTEGER foreign keys with referential integrity
  • ComponentAssociations, SupplementalAttributeAssociations, and TimeSeriesMetadataStore all migrated to integer ID columns
  • Migration logic for legacy serialized data and existing SQLite schemas
  • New get_by_id() methods on ComponentManager, SupplementalAttributeManager
  • SerializedComponentReference uses integer id instead of UUID
  • Fixed double-negative bug in metadata_store_needs_migration
  • copy.deepcopy replaces manual model_dump() reconstruction
  • Backward-compat alias InfraSysBaseModelWithIdentifers preserved

Components, supplemental attributes, and time series metadata now use
monotonically-increasing integer IDs as their primary identifier.
Legacy UUIDs are preserved as `legacy_uuid` for backward compatibility.

Key changes:
- InfraSysBaseModelWithIdentifiers: new id field (int), legacy_uuid for BC
- IDManager: tracks next available ID, supports advance_past() for
  serialized data with pre-assigned IDs
- SQLite tables use INTEGER foreign keys with referential integrity
- ComponentAssociations, SupplementalAttributeAssociations, and
  TimeSeriesMetadataStore all migrated to integer ID columns
- Migration logic for legacy serialized data and existing SQLite schemas
- New get_by_id() methods on ComponentManager, SupplementalAttributeManager
- SerializedComponentReference uses integer id instead of UUID
- Fixed double-negative bug in metadata_store_needs_migration
- copy.deepcopy replaces manual model_dump() reconstruction
- Backward-compat alias InfraSysBaseModelWithIdentifers preserved
@codecov-commenter

codecov-commenter commented May 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.23299% with 50 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.78%. Comparing base (1971984) to head (7fb5d05).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...rc/infrasys/supplemental_attribute_associations.py 69.30% 31 Missing ⚠️
src/infrasys/time_series_metadata_store.py 93.23% 9 Missing ⚠️
src/infrasys/component_manager.py 92.59% 6 Missing ⚠️
src/infrasys/component_associations.py 92.30% 2 Missing ⚠️
src/infrasys/supplemental_attribute_manager.py 93.33% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #156      +/-   ##
==========================================
- Coverage   94.82%   94.78%   -0.05%     
==========================================
  Files          61       62       +1     
  Lines        6072     6802     +730     
==========================================
+ Hits         5758     6447     +689     
- Misses        314      355      +41     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR migrates InfraSys object identifiers from UUIDs to stable, monotonically-increasing integer IDs across components, supplemental attributes, and time series metadata while retaining legacy UUIDs for backward compatibility.

Changes:

  • Introduces integer id fields (with legacy_uuid retained for BC) and updates serialization/deserialization to use integer IDs for references.
  • Migrates SQLite schemas and association tables (components, supplemental attributes, time series metadata) to integer foreign keys and adds migration helpers for legacy UUID-based data.
  • Updates public accessors/managers and test suite expectations to use get_by_id() / owner_id / time_series_id paths.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
tests/test_system.py Updates record and SQL assertions to validate integer IDs and storage-key UUIDs.
tests/test_supplemental_attributes.py Makes assertions resilient to ordering by comparing coordinate sets.
tests/test_serialization.py Switches component lookups to get_component_by_id() and adds explicit ID-serialization coverage.
src/infrasys/utils/metadata_utils.py Updates SQLite table schemas/indexes to integer FK columns and enables foreign keys.
src/infrasys/time_series_models.py Adds time_series_id into metadata models and sets it during from_data().
src/infrasys/time_series_metadata_store.py Migrates association columns to integer IDs + storage keys and adds legacy table migration/remapping logic.
src/infrasys/time_series_manager.py Assigns integer IDs to time series and exposes metadata schema migration hook.
src/infrasys/system.py Adds legacy upgrade path for serialized references, new get_*_by_id() APIs, and triggers metadata/schema migrations on load.
src/infrasys/supplemental_attribute_manager.py Adds ID-based attribute indexing/lookup and association schema migration call.
src/infrasys/supplemental_attribute_associations.py Converts association storage and queries from UUID columns to integer ID columns.
src/infrasys/serialization.py Updates SerializedComponentReference to store integer IDs with legacy UUID as excluded BC field.
src/infrasys/models.py Replaces UUID base identifier with integer id + legacy_uuid, and extends label parsing to handle ints.
src/infrasys/migrations/db_migrations.py Fixes migration predicate and routes legacy inserts through the updated metadata store insert path.
src/infrasys/id_manager.py Adds advance_past() to support externally-supplied IDs during deserialization/migration.
src/infrasys/component.py Serializes composed component references using integer IDs and errors if ID is missing.
src/infrasys/component_manager.py Adds ID indexing/lookup, updates composed-component association logic to use IDs, and switches deepcopy to copy.deepcopy.
src/infrasys/component_associations.py Converts component association storage/querying from UUIDs to integer IDs with parent-ID insertion.
src/infrasys/init.py Removes duplicate constant definition.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/infrasys/supplemental_attribute_manager.py Outdated
Comment thread src/infrasys/component_manager.py Outdated
Comment thread src/infrasys/time_series_metadata_store.py Outdated
Comment thread src/infrasys/utils/metadata_utils.py
Comment thread src/infrasys/models.py Outdated
Comment thread src/infrasys/system.py Outdated
Comment thread src/infrasys/system.py Outdated
Comment thread src/infrasys/time_series_manager.py
Comment thread src/infrasys/supplemental_attribute_associations.py
Comment thread src/infrasys/supplemental_attribute_associations.py Outdated
Comment thread src/infrasys/serialization.py Outdated
Comment thread src/infrasys/utils/metadata_utils.py
pesap added 2 commits May 23, 2026 20:04
- Fix ComponentManager.remove() index cleanup only triggering on
  empty container (P1 bug)
- Fix get_by_label() to try name-based lookup before ID for
  numeric labels (P1 ambiguity)
- Add owner_category to time series unique index to prevent false
  collisions across owner types
- Extract migration functions to src/infrasys/utils/migrations.py
- Raise NotImplementedError for deprecated
  get_component_uuids_with_attribute
- Merge duplicate query strings in supplemental attribute associations
- Add Field descriptions to SerializedComponentReference
- Add cascade warning when removing components with time series
- Add migration path tests and remove-index cleanup test
…nent associations

Covers:
- SerializedComponentReference.uuid property (success + error)
- get_class_and_name_from_label with UUID and unknown string labels
- ComponentAssociations.clear operation
- list_child_components/list_parent_components with type filter
- get_by_label with UUID-based labels
@pesap

pesap commented May 24, 2026

Copy link
Copy Markdown
Collaborator Author

@daniel-thom, is this what you had in mind?

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated 6 comments.

Comment thread src/infrasys/models.py Outdated
Comment thread src/infrasys/time_series_manager.py
Comment thread src/infrasys/time_series_metadata_store.py
Comment thread src/infrasys/component_manager.py Outdated
Comment thread src/infrasys/utils/metadata_utils.py Outdated
Comment thread src/infrasys/migrations/db_migrations.py

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 5 comments.

Comment thread src/infrasys/models.py Outdated
Comment thread src/infrasys/time_series_metadata_store.py Outdated
Comment thread src/infrasys/time_series_manager.py Outdated
Comment thread src/infrasys/time_series_metadata_store.py Outdated
Comment thread src/infrasys/system.py Outdated
- fix duplicate list_time_series_metadata call in remove_component
- widen sql() params type from Sequence[str] to Sequence[Any]
- guard MAX(*_id) queries with column existence checks for legacy DBs
- add 17 tests covering component associations, ID management, migrations,
  supplemental attribute queries, and time series metadata normalization

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 4 comments.

Comment thread src/infrasys/models.py Outdated
Comment on lines +94 to +99
class_name, name = label.split(".", maxsplit=1)
name_or_uuid: str | UUID = name
name_or_uuid: str | int | UUID = name
try:
name_or_uuid = int(name)
return class_name, name_or_uuid
except ValueError:
query = f"SELECT 1 FROM {TIME_SERIES_ASSOCIATIONS_TABLE} WHERE time_series_uuid = ?"
query = f"SELECT 1 FROM {TIME_SERIES_ASSOCIATIONS_TABLE} WHERE time_series_storage_key = ?"
row = execute(cur, query, params=(str(time_series_uuid),)).fetchone()
return row
Comment thread src/infrasys/time_series_metadata_store.py Outdated
Comment thread src/infrasys/supplemental_attribute_associations.py
… type, replace asserts with domain exceptions

Agent-Logs-Url: https://github.com/NatLabRockies/infrasys/sessions/070e4fb1-22bd-4d30-8a82-321e1f62f210

Co-authored-by: pesap <2238996+pesap@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.

Comment thread src/infrasys/component_manager.py Outdated
Comment thread src/infrasys/system.py Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.

Comment on lines 470 to 474
def raise_if_attached(self, component: Component):
"""Raise an exception if this component is attached to a system."""
if component.uuid in self._components_by_uuid:
if component.id is not None and component.id in self._components_by_id:
msg = f"{component.label} is already attached to the system"
raise ISAlreadyAttached(msg)
Comment on lines +394 to +400
if component.id is None:
component.id = self._id_manager.get_next_id()
elif component.id in self._components_by_id:
msg = f"{component.label} with id={component.id} is already stored"
raise ISAlreadyAttached(msg)
else:
self._id_manager.advance_past(component.id)
Comment on lines 437 to 439
if not owners:
msg = "At least one component must be passed."
raise ISOperationNotAllowed(msg)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants