From ef67ef6c72059c9e09c42e7104a1924541ab04aa Mon Sep 17 00:00:00 2001 From: Simon van Lierde <90462640+simonvanlierde@users.noreply.github.com> Date: Sat, 31 Oct 2026 05:32:21 +0100 Subject: [PATCH 1/2] docs(dataset-release): state that record ids are stable across releases - I say in the records.parquet id description and in the release README that a record id is its database primary key, so it stays the same across releases. - I add a unit test that builds twice from one in-memory database, the second time with an added product, and checks every existing record keeps its id. - I add a changelog entry for dataset users. Signed-off-by: Simon van Lierde <90462640+simonvanlierde@users.noreply.github.com> --- CHANGELOG.md | 3 + backend/scripts/build_dataset_release.py | 12 ++- .../scripts/test_build_dataset_release.py | 100 ++++++++++++++++++ 3 files changed, 114 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 79d3ddd3c..f04137ec7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,9 @@ project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - An accessibility statement at `/accessibility` on the website, backed by a WCAG 2.2 self-assessment of the website, the documentation site and the web app at `/accessibility-assessment`. +- The dataset release README and the `records.parquet` codebook state that record ids are + stable across releases. They are database primary keys, so you can match a record between + releases by its id. ### Changed diff --git a/backend/scripts/build_dataset_release.py b/backend/scripts/build_dataset_release.py index 1a632d22d..6b42fe13e 100644 --- a/backend/scripts/build_dataset_release.py +++ b/backend/scripts/build_dataset_release.py @@ -112,7 +112,13 @@ def _column(name: str, type_: pa.DataType, description: str, *, nullable: bool = # Nullable numerics stay nullable so an absent mass reads as null, not zero. RECORDS_SCHEMA = pa.schema( [ - _column("id", pa.int64(), "Record identifier, unique within the release.", nullable=False), + _column( + "id", + pa.int64(), + "Record identifier, unique within the release. It is the platform's database primary key, " + "so it is stable across releases: the same record keeps the same id in every release.", + nullable=False, + ), _column("parent_id", pa.int64(), "Record this one was taken out of; null for a whole product."), _column("amount_in_parent", pa.int64(), "How many of this component the parent holds; null if not counted."), _column( @@ -1023,6 +1029,10 @@ def render_readme(meta: ReleaseMetadata, stats: BuildStats) -> str: that points at a material or product type which is no longer in the live database would otherwise lose its meaning. +Record ids are stable across releases. Each `id` in `records.parquet` is the record's +primary key in the R9lab database, so a record keeps its id in every release and can be +matched between releases by id alone. A new release adds ids; it does not renumber old ones. + Owner identity is replaced by a stable pseudonym (`owner-<16 hex>`). The same person keeps the same pseudonym across releases and cannot be re-identified from the release alone. The salt fingerprint above identifies which salt generation produced them: two releases sharing diff --git a/backend/tests/unit/scripts/test_build_dataset_release.py b/backend/tests/unit/scripts/test_build_dataset_release.py index dc7997ec7..5a3ea8f4c 100644 --- a/backend/tests/unit/scripts/test_build_dataset_release.py +++ b/backend/tests/unit/scripts/test_build_dataset_release.py @@ -7,6 +7,7 @@ import re import tempfile from collections import Counter +from contextlib import asynccontextmanager from datetime import UTC, date, datetime from pathlib import Path from types import SimpleNamespace @@ -91,6 +92,7 @@ ) if TYPE_CHECKING: + from collections.abc import AsyncGenerator from typing import Any # The first Parquet write in a process pays a one-off setup cost; one worker pays it once. @@ -1033,6 +1035,104 @@ def test_build_refuses_a_shared_owner_before_touching_the_filesystem(tmp_path) - assert not out.exists() +def _db_product(product_id: int, name: str, parent_id: int | None = None) -> dict[str, Any]: + """Return one product row as the database holds it, owner id included.""" + moment = datetime(2026, 1, 2, 3, 4, 5, tzinfo=UTC) + return { + "id": product_id, + "parent_id": parent_id, + "amount_in_parent": None if parent_id is None else 1, + "owner_id": USER_ID, + "name": name, + "description": None, + "brand": None, + "model": None, + "product_type_id": 5, + "weight_g": 900.0, + "height_cm": None, + "width_cm": None, + "depth_cm": None, + "circularity_properties": None, + "created_at": moment, + "updated_at": moment, + } + + +def _fake_database(monkeypatch: pytest.MonkeyPatch, products: list[dict[str, Any]]) -> None: + """Point the build at an in-memory database holding *products*. + + The product read itself stays real, so the ids come from ``load_records`` exactly as the + build takes them. Everything else is stubbed down to what ``verify`` needs. + """ + rows = [SimpleNamespace(_mapping=row) for row in sorted(products, key=lambda row: row["id"])] + result = MagicMock() + result.all.return_value = rows + session = MagicMock() + session.execute = AsyncMock(return_value=result) + + @asynccontextmanager + async def session_context() -> AsyncGenerator[MagicMock]: + yield session + + async def owners(_session: object) -> dict[Any, dict[str, Any]]: + return {USER_ID: {"username": "alice", "terms_version": MINIMUM_RELEASE_TERMS_VERSION}} + + async def no_rows(_session: object, *_args: object) -> list[dict[str, Any]]: + return [] + + async def no_images(_session: object, _record_ids: set[int], images_dir: Path) -> list[dict[str, Any]]: + images_dir.mkdir(parents=True, exist_ok=True) # noqa: ASYNC240 # tmp_path, no event loop work to block + return [] + + async def reference(_session: object, reference_dir: Path) -> dict[str, set[int]]: + reference_dir.mkdir(parents=True, exist_ok=True) # noqa: ASYNC240 # tmp_path, no event loop work to block + write_parquet(reference_dir / "taxonomies.parquet", TAXONOMIES_SCHEMA, []) + write_parquet(reference_dir / "categories.parquet", CATEGORIES_SCHEMA, []) + write_parquet(reference_dir / "materials.parquet", MATERIALS_SCHEMA, []) + write_parquet(reference_dir / "product_types.parquet", PRODUCT_TYPES_SCHEMA, [{"id": 5, "name": "Drill"}]) + write_parquet(reference_dir / "units.parquet", UNITS_SCHEMA, [{"name": "kilogram", "symbol": "kg"}]) + write_parquet(reference_dir / "category_material_links.parquet", CATEGORY_MATERIAL_LINKS_SCHEMA, []) + write_parquet(reference_dir / "category_product_type_links.parquet", CATEGORY_PRODUCT_TYPE_LINKS_SCHEMA, []) + return {"taxonomy": set(), "category": set(), "material": set(), "product_type": {5}} + + monkeypatch.setattr(release, "async_session_context", session_context) + monkeypatch.setattr(release, "close_async_engine", AsyncMock()) + monkeypatch.setattr(release, "load_owners", owners) + monkeypatch.setattr(release, "export_record_materials", no_rows) + monkeypatch.setattr(release, "export_images", no_images) + monkeypatch.setattr(release, "export_reference_data", reference) + + +def _built_ids(tmp_path: Path, name: str, monkeypatch: pytest.MonkeyPatch, products: list[dict[str, Any]]) -> dict: + """Build a release from *products* and map each record name to its published id.""" + _fake_database(monkeypatch, products) + out = tmp_path / name + asyncio.run(build(out, SelectionRules(), "s" * 32, ReleaseMetadata())) + return {row["name"]: row["id"] for row in read_parquet(out / "records.parquet")} + + +def test_stable_id_survives_a_later_release_with_an_added_product(tmp_path, monkeypatch) -> None: + """A record keeps its id when a later release adds a product. + + Downstream splits hash on these ids, so a renumbered record would move between splits. + The primary keys have gaps, and the added product sorts first by name, so a builder that + numbered records by position or by name would publish different ids. + """ + database = [ + _db_product(4, "Drill"), + _db_product(9, "Sander"), + _db_product(15, "Sander motor", parent_id=9), + ] + first = _built_ids(tmp_path, "release-1", monkeypatch, database) + + database.append(_db_product(22, "Angle grinder")) + second = _built_ids(tmp_path, "release-2", monkeypatch, database) + + assert {name: second[name] for name in first} == first + assert first == {"Drill": 4, "Sander": 9, "Sander motor": 15} + assert second["Angle grinder"] == 22 + + ### Inventory ### def _product(**overrides: Any) -> dict[str, Any]: """Return one product row as collect_inventory reads it.""" From 8af90daa01332b92cb36cfdf94e8e50da53da877 Mon Sep 17 00:00:00 2001 From: Simon van Lierde <90462640+simonvanlierde@users.noreply.github.com> Date: Sat, 31 Oct 2026 05:32:22 +0100 Subject: [PATCH 2/2] fix(dataset-release): narrow the stable id wording and pin parent ids in the test - I say a record keeps its id in every release that includes it, since a later release can leave out deleted or excluded records. - I regenerate the docs codebook, which is built from RECORDS_SCHEMA. - The test now checks parent ids too and says which assertion catches which renumbering bug. Signed-off-by: Simon van Lierde <90462640+simonvanlierde@users.noreply.github.com> --- backend/scripts/build_dataset_release.py | 8 +++++--- .../unit/scripts/test_build_dataset_release.py | 18 +++++++++++------- .../datamodel/codebook.generated.mdx | 2 +- 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/backend/scripts/build_dataset_release.py b/backend/scripts/build_dataset_release.py index 6b42fe13e..064ce09ec 100644 --- a/backend/scripts/build_dataset_release.py +++ b/backend/scripts/build_dataset_release.py @@ -116,7 +116,7 @@ def _column(name: str, type_: pa.DataType, description: str, *, nullable: bool = "id", pa.int64(), "Record identifier, unique within the release. It is the platform's database primary key, " - "so it is stable across releases: the same record keeps the same id in every release.", + "so it is stable across releases: a record has the same id in every release that includes it.", nullable=False, ), _column("parent_id", pa.int64(), "Record this one was taken out of; null for a whole product."), @@ -1030,8 +1030,10 @@ def render_readme(meta: ReleaseMetadata, stats: BuildStats) -> str: otherwise lose its meaning. Record ids are stable across releases. Each `id` in `records.parquet` is the record's -primary key in the R9lab database, so a record keeps its id in every release and can be -matched between releases by id alone. A new release adds ids; it does not renumber old ones. +primary key in the R9lab database, so a record has the same id in every release that +includes it, and records can be matched between releases by id. Ids are never renumbered +or reused. A later release can add records, and it can leave out records that were deleted +or excluded since. Owner identity is replaced by a stable pseudonym (`owner-<16 hex>`). The same person keeps the same pseudonym across releases and cannot be re-identified from the release alone. The diff --git a/backend/tests/unit/scripts/test_build_dataset_release.py b/backend/tests/unit/scripts/test_build_dataset_release.py index 5a3ea8f4c..5731736dd 100644 --- a/backend/tests/unit/scripts/test_build_dataset_release.py +++ b/backend/tests/unit/scripts/test_build_dataset_release.py @@ -1103,20 +1103,23 @@ async def reference(_session: object, reference_dir: Path) -> dict[str, set[int] monkeypatch.setattr(release, "export_reference_data", reference) -def _built_ids(tmp_path: Path, name: str, monkeypatch: pytest.MonkeyPatch, products: list[dict[str, Any]]) -> dict: - """Build a release from *products* and map each record name to its published id.""" +def _built_ids( + tmp_path: Path, name: str, monkeypatch: pytest.MonkeyPatch, products: list[dict[str, Any]] +) -> dict[str, tuple[int, int | None]]: + """Build a release from *products* and map each record name to its published id and parent id.""" _fake_database(monkeypatch, products) out = tmp_path / name asyncio.run(build(out, SelectionRules(), "s" * 32, ReleaseMetadata())) - return {row["name"]: row["id"] for row in read_parquet(out / "records.parquet")} + return {row["name"]: (row["id"], row["parent_id"]) for row in read_parquet(out / "records.parquet")} def test_stable_id_survives_a_later_release_with_an_added_product(tmp_path, monkeypatch) -> None: """A record keeps its id when a later release adds a product. Downstream splits hash on these ids, so a renumbered record would move between splits. - The primary keys have gaps, and the added product sorts first by name, so a builder that - numbered records by position or by name would publish different ids. + The added product sorts first by name, so numbering by name changes ids between the two + builds. Numbering by load order would not, because the new product loads last; the gaps + in the primary keys catch that instead, through the check against the database ids. """ database = [ _db_product(4, "Drill"), @@ -1129,8 +1132,9 @@ def test_stable_id_survives_a_later_release_with_an_added_product(tmp_path, monk second = _built_ids(tmp_path, "release-2", monkeypatch, database) assert {name: second[name] for name in first} == first - assert first == {"Drill": 4, "Sander": 9, "Sander motor": 15} - assert second["Angle grinder"] == 22 + # This line catches numbering by load order, which the comparison above cannot see. + assert first == {"Drill": (4, None), "Sander": (9, None), "Sander motor": (15, 9)} + assert second["Angle grinder"] == (22, None) ### Inventory ### diff --git a/docs/src/components/datamodel/codebook.generated.mdx b/docs/src/components/datamodel/codebook.generated.mdx index 802c7db2b..8cbb3083c 100644 --- a/docs/src/components/datamodel/codebook.generated.mdx +++ b/docs/src/components/datamodel/codebook.generated.mdx @@ -6,7 +6,7 @@ One row per documented product or component. | Column | Type | Required | Description | | ------ | ---- | -------- | ----------- | -| `id` | integer | yes | Record identifier, unique within the release. | +| `id` | integer | yes | Record identifier, unique within the release. It is the platform's database primary key, so it is stable across releases: a record has the same id in every release that includes it. | | `parent_id` | integer | no | Record this one was taken out of; null for a whole product. | | `amount_in_parent` | integer | no | How many of this component the parent holds; null if not counted. | | `owner_pseudonym` | text | yes | Stable pseudonymous code for the contributing account. Salted hash, not reversible. Equal codes mean the same contributor across releases that share a salt fingerprint. |