From 657a1cfc2275153e79438b520da096150d2afc32 Mon Sep 17 00:00:00 2001 From: Ruge Li Date: Tue, 14 Apr 2026 12:32:40 -0700 Subject: [PATCH 1/6] normalize data order before hashing and tests --- cellpack/autopack/DBRecipeHandler.py | 36 +++++++++++++- cellpack/tests/test_data_doc.py | 70 ++++++++++++++++++++++++++++ 2 files changed, 105 insertions(+), 1 deletion(-) diff --git a/cellpack/autopack/DBRecipeHandler.py b/cellpack/autopack/DBRecipeHandler.py index 03558df47..944733c2d 100644 --- a/cellpack/autopack/DBRecipeHandler.py +++ b/cellpack/autopack/DBRecipeHandler.py @@ -48,9 +48,43 @@ def is_obj(comp_or_obj): # in resolved DB data, if the top level of a downloaded comp doesn't have the key `name`, it's an obj return not comp_or_obj.get("name") and "object" in comp_or_obj + @staticmethod + def _is_positional(item): + if isinstance(item, bool): + return False + if isinstance(item, (int, float)): + return True + if isinstance(item, list): + return all(DataDoc._is_positional(x) for x in item) + return False + + @staticmethod + def _normalize_for_hashing(data): + """ + Recursively normalize the input json data so that dedup hashes are + stable across semantically equivalent inputs. + + Categories (see `cellpack/tests/test_data_doc.py` for worked shapes): + - dict key order — sorted by `json.dumps(sort_keys=True)` + - string lists (e.g. region composition refs) — sorted + - mixed string + inline-dict lists — sorted + - pure-dict lists (partners, inline composition entries) — sorted + - positional numeric lists (vectors, colors) — leave as is + - positional nested-numeric lists (bounding boxes) — leave as is + """ + if isinstance(data, dict): + return {k: DataDoc._normalize_for_hashing(v) for k, v in data.items()} + if isinstance(data, list): + normalized = [DataDoc._normalize_for_hashing(item) for item in data] + if all(DataDoc._is_positional(item) for item in normalized): + return normalized + return sorted(normalized, key=lambda x: json.dumps(x, sort_keys=True)) + return data + @staticmethod def generate_hash(doc_data): - doc_str = json.dumps(doc_data, sort_keys=True) + normalized_data = DataDoc._normalize_for_hashing(doc_data) + doc_str = json.dumps(normalized_data, sort_keys=True) return hashlib.sha256(doc_str.encode()).hexdigest() diff --git a/cellpack/tests/test_data_doc.py b/cellpack/tests/test_data_doc.py index 072f5c894..b8e630c5a 100644 --- a/cellpack/tests/test_data_doc.py +++ b/cellpack/tests/test_data_doc.py @@ -45,3 +45,73 @@ def test_generate_hash(): generated_hash = DataDoc.generate_hash(input_data) assert isinstance(generated_hash, str) assert generated_hash == DataDoc.generate_hash(input_data) + + +def test_generate_hash_is_stable_across_key_order(): + recipe_a = {"name": "test", "version": "1.0", "count": 1} + recipe_b = {"count": 1, "version": "1.0", "name": "test"} + assert DataDoc.generate_hash(recipe_a) == DataDoc.generate_hash(recipe_b) + + nested_a = {"outer": {"a": 1, "b": 2}, "meta": {"x": True, "y": False}} + nested_b = {"meta": {"y": False, "x": True}, "outer": {"b": 2, "a": 1}} + assert DataDoc.generate_hash(nested_a) == DataDoc.generate_hash(nested_b) + + +def test_generate_hash_is_stable_across_string_list_order(): + recipe_a = { + "composition": { + "space": {"regions": {"interior": ["A", "B", "C", "D", "E"]}}, + "A": {"object": "sphere_100", "count": 6}, + "B": {"object": "sphere_200", "count": 2}, + "C": {"object": "sphere_50", "count": 15}, + } + } + recipe_b = { + "composition": { + "A": {"count": 6, "object": "sphere_100"}, + "C": {"object": "sphere_50", "count": 15}, + "B": {"object": "sphere_200", "count": 2}, + "space": {"regions": {"interior": ["E", "C", "A", "D", "B"]}}, + } + } + assert DataDoc.generate_hash(recipe_a) == DataDoc.generate_hash(recipe_b) + + +def test_generate_hash_preserves_positional_list_order(): + # numeric/nested lists encode positional data (bounding boxes, vectors, colors) and must remain order-sensitive. + bbox_a = {"bounding_box": [[0, 0, 0], [1000, 1000, 1]]} + bbox_b = {"bounding_box": [[1000, 1000, 1], [0, 0, 0]]} + assert DataDoc.generate_hash(bbox_a) != DataDoc.generate_hash(bbox_b) + + axis_a = {"rotation_axis": [0, 0, 1]} + axis_b = {"rotation_axis": [1, 0, 0]} + assert DataDoc.generate_hash(axis_a) != DataDoc.generate_hash(axis_b) + + +def test_generate_hash_is_stable_across_mixed_list_order(): + # region lists that mix string refs with inline dicts should dedup regardless of element order. + recipe_a = { + "composition": { + "bounding_area": { + "regions": { + "interior": [ + "outer_sphere", + {"object": "green_sphere", "count": 5}, + ] + } + } + } + } + recipe_b = { + "composition": { + "bounding_area": { + "regions": { + "interior": [ + {"object": "green_sphere", "count": 5}, + "outer_sphere", + ] + } + } + } + } + assert DataDoc.generate_hash(recipe_a) == DataDoc.generate_hash(recipe_b) From fefc98464671b27970a121838f74d4525483e5d6 Mon Sep 17 00:00:00 2001 From: Ruge Li Date: Tue, 14 Apr 2026 13:33:11 -0700 Subject: [PATCH 2/6] test pack cli --- cellpack/tests/test_pack_cli.py | 83 +++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 cellpack/tests/test_pack_cli.py diff --git a/cellpack/tests/test_pack_cli.py b/cellpack/tests/test_pack_cli.py new file mode 100644 index 000000000..57285837d --- /dev/null +++ b/cellpack/tests/test_pack_cli.py @@ -0,0 +1,83 @@ +""" +Integration tests for the `pack` CLI entry point (`cellpack/bin/pack.py`). + +Two input types are covered: + + 1. A string file path — the existing local CLI workflow invoked as + `pack -r RECIPE_PATH -c CONFIG_PATH`. Accepting a recipe dict must not change anything about this + path. + + 2. A json dict — the new flow used by the docker server's + `pack_handler` when it receives a json body. The dict must flow + through `RecipeLoader` and the rest of the pipeline remains the same to a + recipe loaded. + +The packing config can be supplied as a file path or omitted entirely, in +which case `ConfigLoader` falls back to its built-in default values. +""" + +import json +from pathlib import Path + +from cellpack.bin.pack import pack + + +MINIMAL_RECIPE = { + "version": "1.0.0", + "format_version": "2.0", + "name": "test_pack_cli", + "bounding_box": [[0, 0, 0], [50, 50, 1]], + "objects": { + "sphere_5": { + "type": "single_sphere", + "radius": 5, + "max_jitter": [1, 1, 0], + "place_method": "jitter", + } + }, + "composition": { + "space": {"regions": {"interior": ["A"]}}, + "A": {"object": "sphere_5", "count": 1}, + }, +} + + +def _write_minimal_config(tmp_path: Path) -> Path: + config = { + "name": "test_pack_cli", + "out": f"{tmp_path}/", + "place_method": "jitter", + "inner_grid_method": "raytrace", + "save_analyze_result": False, + "save_plot_figures": False, + "number_of_packings": 1, + "show_progress_bar": False, + "load_from_grid_file": False, + } + config_path = tmp_path / "config.json" + config_path.write_text(json.dumps(config)) + return config_path + + +def test_pack_with_recipe_path(tmp_path): + recipe_path = tmp_path / "recipe.json" + recipe_path.write_text(json.dumps(MINIMAL_RECIPE)) + config_path = _write_minimal_config(tmp_path) + pack(recipe=str(recipe_path), config_path=str(config_path)) + + +def test_pack_with_recipe_dict(tmp_path): + """ + `pack()` also accepts a recipe dict directly, so + the docker server can forward a parsed JSON body without writing it to + db first. + """ + config_path = _write_minimal_config(tmp_path) + pack(recipe=MINIMAL_RECIPE, config_path=str(config_path)) + + +def test_pack_with_default_config(tmp_path, monkeypatch): + """Omitting `config_path` falls back to `ConfigLoader.default_values`.""" + # default `out: "out/"` is relative, monkeypatch.chdir keeps outputs inside tmp_path. + monkeypatch.chdir(tmp_path) + pack(recipe=MINIMAL_RECIPE) From 1657cc0e5cb43f16477b6033e627c83cc47902f3 Mon Sep 17 00:00:00 2001 From: Ruge Li Date: Tue, 14 Apr 2026 13:46:22 -0700 Subject: [PATCH 3/6] refactor test file: use tmp_path instead of writing to a working dir --- cellpack/tests/test_aws_handler.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/cellpack/tests/test_aws_handler.py b/cellpack/tests/test_aws_handler.py index e1514e993..ed3d0f37b 100644 --- a/cellpack/tests/test_aws_handler.py +++ b/cellpack/tests/test_aws_handler.py @@ -31,7 +31,7 @@ def test_get_aws_object_key(): assert object_key == "test_folder/test_file" -def test_upload_file(): +def test_upload_file(tmp_path): with mock_aws(): aws_handler = AWSHandler( bucket_name="test_bucket", @@ -43,13 +43,13 @@ def test_upload_file(): Bucket="test_bucket", CreateBucketConfiguration={"LocationConstraint": "us-west-2"}, ) - with open("test_file.txt", "w") as file: - file.write("test file") - file_name = aws_handler.upload_file("test_file.txt") + test_file = tmp_path / "test_file.txt" + test_file.write_text("test file") + file_name = aws_handler.upload_file(str(test_file)) assert file_name == "test_folder/test_file.txt" -def test_create_presigned_url(): +def test_create_presigned_url(tmp_path): with mock_aws(), patch.object(AWSHandler, "_s3_client") as mock_client: presigned_url = "https://s3.us-west-2.amazonaws.com/test_bucket/test_folder/test_file.txt?query=string" mock_client.generate_presigned_url.return_value = presigned_url @@ -63,9 +63,9 @@ def test_create_presigned_url(): Bucket="test_bucket", CreateBucketConfiguration={"LocationConstraint": "us-west-2"}, ) - with open("test_file.txt", "w") as file: - file.write("test file") - aws_handler.upload_file("test_file.txt") + test_file = tmp_path / "test_file.txt" + test_file.write_text("test file") + aws_handler.upload_file(str(test_file)) url = aws_handler.create_presigned_url("test_file.txt") assert url is not None assert url.startswith( @@ -73,7 +73,7 @@ def test_create_presigned_url(): ) -def test_is_url_valid(): +def test_is_url_valid(tmp_path): with mock_aws(), patch.object(AWSHandler, "_s3_client") as mock_client: presigned_url = "https://s3.us-west-2.amazonaws.com/test_bucket/test_folder/test_file.txt?query=string" mock_client.generate_presigned_url.return_value = presigned_url @@ -87,9 +87,9 @@ def test_is_url_valid(): Bucket="test_bucket", CreateBucketConfiguration={"LocationConstraint": "us-west-2"}, ) - with open("test_file.txt", "w") as file: - file.write("test file") - aws_handler.upload_file("test_file.txt") + test_file = tmp_path / "test_file.txt" + test_file.write_text("test file") + aws_handler.upload_file(str(test_file)) url = aws_handler.create_presigned_url("test_file.txt") assert aws_handler.is_url_valid(url) is True assert aws_handler.is_url_valid("invalid_url") is False From b5ca2f794cce5aec2140fb55746526001a5f5026 Mon Sep 17 00:00:00 2001 From: Ruge Li Date: Tue, 14 Apr 2026 14:09:48 -0700 Subject: [PATCH 4/6] cleanup --- cellpack/tests/test_data_doc.py | 4 ---- cellpack/tests/test_pack_cli.py | 14 +++++++------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/cellpack/tests/test_data_doc.py b/cellpack/tests/test_data_doc.py index b8e630c5a..6f1e563fb 100644 --- a/cellpack/tests/test_data_doc.py +++ b/cellpack/tests/test_data_doc.py @@ -52,10 +52,6 @@ def test_generate_hash_is_stable_across_key_order(): recipe_b = {"count": 1, "version": "1.0", "name": "test"} assert DataDoc.generate_hash(recipe_a) == DataDoc.generate_hash(recipe_b) - nested_a = {"outer": {"a": 1, "b": 2}, "meta": {"x": True, "y": False}} - nested_b = {"meta": {"y": False, "x": True}, "outer": {"b": 2, "a": 1}} - assert DataDoc.generate_hash(nested_a) == DataDoc.generate_hash(nested_b) - def test_generate_hash_is_stable_across_string_list_order(): recipe_a = { diff --git a/cellpack/tests/test_pack_cli.py b/cellpack/tests/test_pack_cli.py index 57285837d..74eaf96eb 100644 --- a/cellpack/tests/test_pack_cli.py +++ b/cellpack/tests/test_pack_cli.py @@ -22,7 +22,7 @@ from cellpack.bin.pack import pack -MINIMAL_RECIPE = { +recipe_data = { "version": "1.0.0", "format_version": "2.0", "name": "test_pack_cli", @@ -42,7 +42,7 @@ } -def _write_minimal_config(tmp_path: Path) -> Path: +def _write_config(tmp_path: Path) -> Path: config = { "name": "test_pack_cli", "out": f"{tmp_path}/", @@ -61,8 +61,8 @@ def _write_minimal_config(tmp_path: Path) -> Path: def test_pack_with_recipe_path(tmp_path): recipe_path = tmp_path / "recipe.json" - recipe_path.write_text(json.dumps(MINIMAL_RECIPE)) - config_path = _write_minimal_config(tmp_path) + recipe_path.write_text(json.dumps(recipe_data)) + config_path = _write_config(tmp_path) pack(recipe=str(recipe_path), config_path=str(config_path)) @@ -72,12 +72,12 @@ def test_pack_with_recipe_dict(tmp_path): the docker server can forward a parsed JSON body without writing it to db first. """ - config_path = _write_minimal_config(tmp_path) - pack(recipe=MINIMAL_RECIPE, config_path=str(config_path)) + config_path = _write_config(tmp_path) + pack(recipe=recipe_data, config_path=str(config_path)) def test_pack_with_default_config(tmp_path, monkeypatch): """Omitting `config_path` falls back to `ConfigLoader.default_values`.""" # default `out: "out/"` is relative, monkeypatch.chdir keeps outputs inside tmp_path. monkeypatch.chdir(tmp_path) - pack(recipe=MINIMAL_RECIPE) + pack(recipe=recipe_data) From b84f1f188f6cb43c5681425d4a5641677d448494 Mon Sep 17 00:00:00 2001 From: Ruge Li Date: Tue, 14 Apr 2026 14:29:48 -0700 Subject: [PATCH 5/6] formatting --- cellpack/tests/test_pack_cli.py | 1 - 1 file changed, 1 deletion(-) diff --git a/cellpack/tests/test_pack_cli.py b/cellpack/tests/test_pack_cli.py index 74eaf96eb..589aeb87e 100644 --- a/cellpack/tests/test_pack_cli.py +++ b/cellpack/tests/test_pack_cli.py @@ -21,7 +21,6 @@ from cellpack.bin.pack import pack - recipe_data = { "version": "1.0.0", "format_version": "2.0", From aeb65e2c6669052d7d6b33266b27aa33fb9554b4 Mon Sep 17 00:00:00 2001 From: Ruge Li Date: Tue, 14 Apr 2026 15:27:49 -0700 Subject: [PATCH 6/6] add recipe fixture --- cellpack/tests/test_pack_cli.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/cellpack/tests/test_pack_cli.py b/cellpack/tests/test_pack_cli.py index 589aeb87e..add6c0a5d 100644 --- a/cellpack/tests/test_pack_cli.py +++ b/cellpack/tests/test_pack_cli.py @@ -18,10 +18,12 @@ import json from pathlib import Path +import copy +import pytest from cellpack.bin.pack import pack -recipe_data = { +_RECIPE = { "version": "1.0.0", "format_version": "2.0", "name": "test_pack_cli", @@ -41,6 +43,12 @@ } +@pytest.fixture +def recipe_data(): + # RecipeLoader mutates the dict it receives, so each test gets a fresh copy to stay independent of run order. + return copy.deepcopy(_RECIPE) + + def _write_config(tmp_path: Path) -> Path: config = { "name": "test_pack_cli", @@ -58,14 +66,14 @@ def _write_config(tmp_path: Path) -> Path: return config_path -def test_pack_with_recipe_path(tmp_path): +def test_pack_with_recipe_path(tmp_path, recipe_data): recipe_path = tmp_path / "recipe.json" recipe_path.write_text(json.dumps(recipe_data)) config_path = _write_config(tmp_path) pack(recipe=str(recipe_path), config_path=str(config_path)) -def test_pack_with_recipe_dict(tmp_path): +def test_pack_with_recipe_dict(tmp_path, recipe_data): """ `pack()` also accepts a recipe dict directly, so the docker server can forward a parsed JSON body without writing it to @@ -75,7 +83,7 @@ def test_pack_with_recipe_dict(tmp_path): pack(recipe=recipe_data, config_path=str(config_path)) -def test_pack_with_default_config(tmp_path, monkeypatch): +def test_pack_with_default_config(tmp_path, monkeypatch, recipe_data): """Omitting `config_path` falls back to `ConfigLoader.default_values`.""" # default `out: "out/"` is relative, monkeypatch.chdir keeps outputs inside tmp_path. monkeypatch.chdir(tmp_path)