From 71282c22ae302c7331e5a30886b5457ebbe9db75 Mon Sep 17 00:00:00 2001 From: Devon Jones Date: Mon, 4 May 2026 17:45:18 -0600 Subject: [PATCH 1/4] fix(loader): preserve blueprint_name and tags on file rename MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a file is renamed in Dropbox (same MD5, new path/basename), the incremental fixtures loader was leaving blueprint_name + search_text stale and re-deprecating the row in the same load — which then stripped the freshly re-inserted tags and images via _handle_deprecation. The rename branch in _handle_addition now also resyncs blueprint_name and search_text to the new file basename and clears the deprecated flag. The blueprint id is recorded so the subsequent deprecation pass in the same load skips it instead of tombstoning the live row. update_blueprint gains search_text in its allowed-fields list so the loader can push the recomputed value through. Co-Authored-By: Claude Opus 4.7 (1M context) --- openforge/db/fixtures/incremental.py | 36 ++++++++++++++++++++++++++-- openforge/db/sql/blueprints.py | 1 + 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/openforge/db/fixtures/incremental.py b/openforge/db/fixtures/incremental.py index f2bd4a46..b5a3d4c9 100644 --- a/openforge/db/fixtures/incremental.py +++ b/openforge/db/fixtures/incremental.py @@ -107,6 +107,10 @@ def __init__(self, conn: connection, verbose: bool = False): self.current_fixture_files = ( set() ) # Will be populated when processing fixture data + # Blueprint IDs that were renamed in this load — used to skip the + # subsequent deprecation step so a renamed row isn't tombstoned and + # stripped of tags/images. + self._renamed_blueprint_ids = set() def _load_existing_blueprints(self, curs: cursor = None) -> Dict[str, Dict]: """Load existing blueprints from database for comparison. @@ -752,6 +756,9 @@ def _apply_changes_with_cursor(self, curs: cursor, changes: ComparisonResult): # Load existing blueprints for modification handling existing_blueprints = self._load_existing_blueprints(curs) + # Reset rename tracking for this load. + self._renamed_blueprint_ids = set() + # Track new blueprint IDs for version change linking new_blueprint_ids = {} new_blueprint_ids_by_md5 = {} @@ -798,6 +805,17 @@ def _handle_deprecation( deprecated_bp: Blueprint to deprecate successor_id: Optional ID of the successor blueprint (for version changes) """ + # If this blueprint was renamed in this same load, the row was + # already updated in place. Deprecating it now would tombstone the + # live row and strip its freshly-inserted tags/images. + if deprecated_bp.get("id") in self._renamed_blueprint_ids: + if self.verbose: + write_output( + f"Skipping deprecation of blueprint {deprecated_bp['id']} " + f"(renamed in this load)\n" + ) + return + # Check if a deprecated blueprint with this MD5 already exists if deprecated_bp.get("file_md5"): existing_deprecated = self._find_deprecated_blueprint_by_md5( @@ -897,12 +915,26 @@ def _handle_addition(self, curs: cursor, new_item: Dict): "adding to consolidated_paths\n" ) - # Update the blueprint's full_name to the new path + # Update the blueprint's full_name to the new path. + # Also resync blueprint_name + search_text and clear + # deprecated, since the underlying file is the same + # bytes under a new name. + new_file_name = os.path.basename(new_full_name) + words = self._get_words(new_item) + search_text = blueprint_sql._blueprint_search_text( + {"blueprint_name": new_file_name}, words + ) update_data = { "full_name": new_full_name, - "file_name": os.path.basename(new_full_name), + "file_name": new_file_name, + "blueprint_name": new_file_name, + "search_text": search_text, + "deprecated": False, } blueprint_sql.update_blueprint(curs, bp["id"], update_data) + # Mark this id as renamed so a later deprecation step + # in the same load won't tombstone it. + self._renamed_blueprint_ids.add(bp["id"]) # Also update tags and images from the new fixture data # First remove old tags and images diff --git a/openforge/db/sql/blueprints.py b/openforge/db/sql/blueprints.py index 0984d823..82bccad2 100644 --- a/openforge/db/sql/blueprints.py +++ b/openforge/db/sql/blueprints.py @@ -191,6 +191,7 @@ def update_blueprint(curs: cursor, blueprint_id: uuid.UUID, data: dict) -> dict: "full_name", "file_modified_at", "storage_address", + "search_text", # Phase 1 fields "consolidated_paths", "deprecated", From 2ad8ff4436cd27e333445a97bd8d2d721647e079 Mon Sep 17 00:00:00 2001 From: Devon Jones Date: Mon, 4 May 2026 18:29:51 -0600 Subject: [PATCH 2/4] fix(loader): also recompute search_text on modify; add rename tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses review feedback on PR #219: - Promote _blueprint_search_text -> blueprint_search_text (public) to avoid cross-module access to a leading-underscore private. - _handle_modification now recomputes search_text alongside other column updates, so tag edits keep search_text consistent (latent bug, same shape as the rename one). - Add three regression tests for the same-MD5 rename branch: blueprint_name+search_text+deprecated resync, deprecation skip for renamed ids, and per-load reset of _renamed_blueprint_ids. - Document _renamed_blueprint_ids as per-apply state. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .beads/.jsonl.lock | 0 .beads/.migration-hint-ts | 1 + openforge/db/fixtures/incremental.py | 15 +++- openforge/db/sql/blueprints.py | 4 +- tests/test_incremental_fixtures.py | 128 ++++++++++++++++++++++++++- 5 files changed, 142 insertions(+), 6 deletions(-) create mode 100644 .beads/.jsonl.lock create mode 100644 .beads/.migration-hint-ts diff --git a/.beads/.jsonl.lock b/.beads/.jsonl.lock new file mode 100644 index 00000000..e69de29b diff --git a/.beads/.migration-hint-ts b/.beads/.migration-hint-ts new file mode 100644 index 00000000..3bbed3d3 --- /dev/null +++ b/.beads/.migration-hint-ts @@ -0,0 +1 @@ +1777919162 diff --git a/openforge/db/fixtures/incremental.py b/openforge/db/fixtures/incremental.py index b5a3d4c9..7e4c25b8 100644 --- a/openforge/db/fixtures/incremental.py +++ b/openforge/db/fixtures/incremental.py @@ -109,7 +109,10 @@ def __init__(self, conn: connection, verbose: bool = False): ) # Will be populated when processing fixture data # Blueprint IDs that were renamed in this load — used to skip the # subsequent deprecation step so a renamed row isn't tombstoned and - # stripped of tags/images. + # stripped of tags/images. Per-apply state: populated in + # `_handle_addition`'s rename branch and reset at the top of + # `_apply_changes_with_cursor` so it doesn't leak between fixtures + # when one loader instance processes a directory. self._renamed_blueprint_ids = set() def _load_existing_blueprints(self, curs: cursor = None) -> Dict[str, Dict]: @@ -921,7 +924,7 @@ def _handle_addition(self, curs: cursor, new_item: Dict): # bytes under a new name. new_file_name = os.path.basename(new_full_name) words = self._get_words(new_item) - search_text = blueprint_sql._blueprint_search_text( + search_text = blueprint_sql.blueprint_search_text( {"blueprint_name": new_file_name}, words ) update_data = { @@ -1046,8 +1049,14 @@ def _handle_modification( blueprint_id = existing_bp["id"] - # Update blueprint data + # Update blueprint data. Recompute search_text alongside the column + # update because tag changes are a common modify trigger and tag + # words feed into search_text — without this, search_text drifts + # whenever tags are edited. bp_data = self._munge_blueprint(modified_item) + bp_data["search_text"] = blueprint_sql.blueprint_search_text( + bp_data, self._get_words(modified_item) + ) blueprint_sql.update_blueprint(curs, blueprint_id, bp_data) # Update tags (delete old, insert new) diff --git a/openforge/db/sql/blueprints.py b/openforge/db/sql/blueprints.py index 82bccad2..c03761ee 100644 --- a/openforge/db/sql/blueprints.py +++ b/openforge/db/sql/blueprints.py @@ -94,7 +94,7 @@ def get_deprecated_blueprints(curs: cursor) -> list[dict]: return results -def _blueprint_search_text(data: dict, words: list[str]) -> str: +def blueprint_search_text(data: dict, words: list[str]) -> str: retwords = set() retwords.update(words) retwords.update(re.split(r"[^a-zA-Z0-9]", data["blueprint_name"])) @@ -104,7 +104,7 @@ def _blueprint_search_text(data: dict, words: list[str]) -> str: def insert_blueprint( curs: cursor, data: dict, rescue_md5_conflict: bool = False, words: list[str] = [] ) -> dict: - data["search_text"] = _blueprint_search_text(data, words) + data["search_text"] = blueprint_search_text(data, words) query_list = [ sql.SQL( """ diff --git a/tests/test_incremental_fixtures.py b/tests/test_incremental_fixtures.py index a7bcd0c7..5ad5f7f3 100644 --- a/tests/test_incremental_fixtures.py +++ b/tests/test_incremental_fixtures.py @@ -2,7 +2,7 @@ Tests for incremental fixtures loading functionality. """ -from unittest.mock import Mock +from unittest.mock import Mock, patch import pytest @@ -499,3 +499,129 @@ def test_comparison_result_summary(self): assert "1 modified" in summary assert "1 deprecated" in summary assert result.has_changes() + + # ------------------------------------------------------------------ + # Rename-within-same-fixture regression tests + # ------------------------------------------------------------------ + # These tests pin down the bug where renaming a file in Dropbox + # (same MD5, new full_name) corrupted the DB row: blueprint_name and + # search_text went stale, the row got tombstoned in the same load, + # and tags/images were deleted. + + def _rename_loader(self): + """Build a loader primed for the same-fixture rename branch.""" + mock_conn = create_mock_connection() + loader = IncrementalFixturesLoader(mock_conn, verbose=False) + # Trigger the is_rename=True branch in _handle_addition: both + # paths must share the subset path, and the existing path must + # not be in the current fixture files set. + loader.fixture_subset_path = "tiles/aztlan" + loader.current_fixture_files = { + "tiles/aztlan/floors/floor/openforge/aztlan#floor.1x1.openforge.stl" + } + return loader + + def test_rename_resyncs_blueprint_name_and_clears_deprecated(self): + """Rename branch must resync blueprint_name + search_text and + clear deprecated, not just full_name/file_name.""" + loader = self._rename_loader() + + # Existing DB row uses the old (typo'd) name; insert returns it + # as the rescued md5 conflict. + existing_bp = { + "id": "bp-1", + "full_name": ( + "tiles/aztlan/floors/floor/openforge/atzlan#floor.1x1.openforge.stl" + ), + } + + new_item = create_mock_fixture_item( + "tiles/aztlan/floors/floor/openforge/aztlan#floor.1x1.openforge.stl", + "md5-shared", + tags=[["texture", "aztlan"], ["shape", "floor"]], + ) + + captured = {} + + def fake_update(curs, blueprint_id, data): + captured["id"] = blueprint_id + captured["data"] = data + + with ( + patch( + "openforge.db.fixtures.incremental.blueprint_sql.insert_blueprint", + return_value=existing_bp, + ), + patch( + "openforge.db.fixtures.incremental.blueprint_sql.update_blueprint", + side_effect=fake_update, + ), + patch( + "openforge.db.fixtures.incremental.tag_sql.delete_all_blueprint_tags" + ), + patch("openforge.db.fixtures.incremental.tag_sql.insert_tag"), + patch( + "openforge.db.fixtures.incremental.image_sql" + ".delete_images_for_blueprint" + ), + patch( + "openforge.db.fixtures.incremental.image_sql.insert_image_for_blueprint" + ), + ): + loader._handle_addition(Mock(), new_item) + + # The rename id is tracked so the deprecation step skips it. + assert "bp-1" in loader._renamed_blueprint_ids + # All four corruption-prone fields must be in the update payload. + assert captured["data"]["full_name"] == ( + "tiles/aztlan/floors/floor/openforge/aztlan#floor.1x1.openforge.stl" + ) + assert captured["data"]["file_name"] == "aztlan#floor.1x1.openforge.stl" + assert captured["data"]["blueprint_name"] == ("aztlan#floor.1x1.openforge.stl") + assert captured["data"]["deprecated"] is False + # search_text reflects the new name and tag words, not the old. + search_text = captured["data"]["search_text"] + assert "aztlan" in search_text.split() + assert "atzlan" not in search_text.split() + assert "floor" in search_text.split() + + def test_renamed_id_skips_deprecation(self): + """A bp id added to _renamed_blueprint_ids during a load must + not be re-deprecated by _handle_deprecation in the same load.""" + loader = self._rename_loader() + loader._renamed_blueprint_ids.add("bp-1") + + deprecated_bp = {"id": "bp-1", "file_md5": "md5-shared"} + + with ( + patch( + "openforge.db.fixtures.incremental.blueprint_sql" + ".mark_blueprint_deprecated" + ) as mark_dep, + patch( + "openforge.db.fixtures.incremental.tag_sql.delete_all_blueprint_tags" + ) as del_tags, + patch( + "openforge.db.fixtures.incremental.image_sql" + ".delete_images_for_blueprint" + ) as del_imgs, + ): + loader._handle_deprecation(Mock(), deprecated_bp) + + # The skip path means none of the deprecation side-effects fire. + mark_dep.assert_not_called() + del_tags.assert_not_called() + del_imgs.assert_not_called() + + def test_apply_changes_resets_renamed_ids_per_load(self): + """_renamed_blueprint_ids must reset at the top of each apply so + state from one fixture doesn't leak into the next.""" + loader = self._rename_loader() + loader._renamed_blueprint_ids.add("stale-id-from-prior-load") + + empty_changes = ComparisonResult() + + with patch.object(loader, "_load_existing_blueprints", return_value={}): + loader._apply_changes_with_cursor(Mock(), empty_changes) + + assert loader._renamed_blueprint_ids == set() From 8d74ed9cb348f659f3af300f5282e4198c0be54b Mon Sep 17 00:00:00 2001 From: Devon Jones Date: Mon, 4 May 2026 18:31:57 -0600 Subject: [PATCH 3/4] chore: ignore beads workspace lockfile and migration hint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .beads/.gitignore | 2 ++ .beads/.jsonl.lock | 0 .beads/.migration-hint-ts | 1 - 3 files changed, 2 insertions(+), 1 deletion(-) delete mode 100644 .beads/.jsonl.lock delete mode 100644 .beads/.migration-hint-ts diff --git a/.beads/.gitignore b/.beads/.gitignore index 374adb81..ce9166d8 100644 --- a/.beads/.gitignore +++ b/.beads/.gitignore @@ -10,6 +10,8 @@ daemon.lock daemon.log daemon.pid bd.sock +.jsonl.lock +.migration-hint-ts # Local version tracking (prevents upgrade notification spam after git ops) .local_version diff --git a/.beads/.jsonl.lock b/.beads/.jsonl.lock deleted file mode 100644 index e69de29b..00000000 diff --git a/.beads/.migration-hint-ts b/.beads/.migration-hint-ts deleted file mode 100644 index 3bbed3d3..00000000 --- a/.beads/.migration-hint-ts +++ /dev/null @@ -1 +0,0 @@ -1777919162 From 66998604e1670b551c4b1350b09873665e35820f Mon Sep 17 00:00:00 2001 From: Devon Jones Date: Mon, 4 May 2026 18:35:31 -0600 Subject: [PATCH 4/4] fix(tests): make rename regression tests run with mocks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - _munge_blueprint reads file_metadata['file'], so add it to the fixture item used in the rename test. - _apply_changes_with_cursor runs _link_deprecated_to_successors at the end which calls cursor.fetchall(); stub it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- tests/test_incremental_fixtures.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/test_incremental_fixtures.py b/tests/test_incremental_fixtures.py index 5ad5f7f3..fce61fdf 100644 --- a/tests/test_incremental_fixtures.py +++ b/tests/test_incremental_fixtures.py @@ -540,6 +540,9 @@ def test_rename_resyncs_blueprint_name_and_clears_deprecated(self): "md5-shared", tags=[["texture", "aztlan"], ["shape", "floor"]], ) + # _munge_blueprint reads file_metadata["file"]; create_mock_fixture_item + # omits it, so add the basename here. + new_item["file_metadata"]["file"] = "aztlan#floor.1x1.openforge.stl" captured = {} @@ -621,7 +624,12 @@ def test_apply_changes_resets_renamed_ids_per_load(self): empty_changes = ComparisonResult() + # _link_deprecated_to_successors runs at the end and calls + # cursor.fetchall(); make it return an empty list. + cursor = Mock() + cursor.fetchall = Mock(return_value=[]) + with patch.object(loader, "_load_existing_blueprints", return_value={}): - loader._apply_changes_with_cursor(Mock(), empty_changes) + loader._apply_changes_with_cursor(cursor, empty_changes) assert loader._renamed_blueprint_ids == set()