diff --git a/CHANGELOG.yaml b/CHANGELOG.yaml index 1ff055c1..c8047117 100644 --- a/CHANGELOG.yaml +++ b/CHANGELOG.yaml @@ -16,6 +16,7 @@ fixes: - "OpenRouter Client: A failed model/provider list fetch at startup no longer sticks until the server is restarted — later config saves and client status refreshes now retry it. Setting the API key for the first time during initial setup also correctly triggers the provider fetch." - "Generation Error Dialog: When several generations failed at the same time (e.g. an API throttle hitting both a summarization and a background task), only the most recent error could be answered — the earlier generation was never resumed and its result (such as a scene summary) was silently lost. Error dialogs are now queued and answered one after another, and pending dialogs are cancelled cleanly when the scene is unloaded or the frontend disconnects." + - "Timeline Rollback: Previewing a revision could fail with an 'unhashable type' error when the scene's changelog contained deltas that diverged from the base snapshot — reconstruction now repairs the affected message history (and other list data) instead of crashing the timeline." 0.38.0: features: diff --git a/src/talemate/changelog.py b/src/talemate/changelog.py index c0fc77e0..7525e294 100644 --- a/src/talemate/changelog.py +++ b/src/talemate/changelog.py @@ -119,6 +119,18 @@ EXCLUDE_FROM_DELTAS_REGEX = [ # re.compile(r"root\['some_array'\]\[\d+\]\['volatile_field'\]"), ] +# Scene-data fields that must always be lists. Delta application with +# force=True can leave an int-keyed dict in their place when an iterable op +# targets a path whose parent is missing (baseline divergence) — deepdiff +# creates ``{index: item}`` instead of a list. +_LIST_FIELDS = ( + "history", + "archived_history", + "layered_history", + "active_characters", + "game_state_watch_paths", +) + # Helper minimal scene reference compatible with this module's helpers class _SceneRef: @@ -685,6 +697,61 @@ def _get_overall_latest_revision(scene: "Scene") -> int: return latest_rev +def _coerce_forced_list(value) -> tuple[list | None, bool]: + """Convert a force-mode artifact — an int-keyed dict where a list is + expected — back into a list ordered by index. + + Returns ``(converted_list, True)`` when the value was such an artifact, + ``(None, False)`` otherwise. Dicts with any non-index key are left alone. + """ + if not isinstance(value, dict): + return None, False + + indexed: list[tuple[int, object]] = [] + for key, item in value.items(): + if isinstance(key, bool): + return None, False + if isinstance(key, int): + indexed.append((key, item)) + elif isinstance(key, str) and re.fullmatch(r"-?\d+", key): + # int keys become strings when the data round-trips through JSON + indexed.append((int(key), item)) + else: + return None, False + + indexed.sort(key=lambda pair: pair[0]) + return [item for _, item in indexed], True + + +def _repair_forced_list_fields(data: dict) -> int: + """Restore list fields that force-mode delta application turned into + int-keyed dicts (see ``_LIST_FIELDS``), including the nested layers of + ``layered_history``. + + Returns the number of fields repaired. + """ + repaired: list[str] = [] + + for field in _LIST_FIELDS: + converted, was_forced = _coerce_forced_list(data.get(field)) + if was_forced: + data[field] = converted + repaired.append(field) + + layered = data.get("layered_history") + if isinstance(layered, list): + for i, layer in enumerate(layered): + converted, was_forced = _coerce_forced_list(layer) + if was_forced: + layered[i] = converted + repaired.append(f"layered_history[{i}]") + + if repaired: + log.warning("repaired_forced_list_fields", fields=repaired) + + return len(repaired) + + def _repair_history(data: dict) -> int: """Backfill required SceneMessage fields on bare-fragment history entries. @@ -729,6 +796,7 @@ async def reconstruct_cleanup(data: dict) -> dict: ) data["shared_context"] = "" + _repair_forced_list_fields(data) _repair_history(data) return data diff --git a/tests/test_changelog.py b/tests/test_changelog.py index 8d2ffda6..9d973071 100644 --- a/tests/test_changelog.py +++ b/tests/test_changelog.py @@ -1294,6 +1294,132 @@ async def test_reconstruct_cleanup_repairs_bare_history_entries(): assert repaired["asset_type"] == "scene_illustration" +@pytest.mark.asyncio +async def test_reconstruct_cleanup_repairs_forced_dict_history(): + """An int-keyed dict where history should be a list (Delta force=True + artifact) is converted back to a list ordered by index — issue #81.""" + data = { + "history": { + 2: {"message": "third", "typ": "narrator", "source": "ai", "flags": 0}, + 1: {"message": "second", "typ": "narrator", "source": "ai", "flags": 0}, + } + } + + result = await reconstruct_cleanup(data) + + history = result["history"] + assert isinstance(history, list) + assert [entry["message"] for entry in history] == ["second", "third"] + # the preview handler slices this — must not raise + assert history[-20:] == history + + +@pytest.mark.asyncio +async def test_reconstruct_cleanup_repairs_forced_dict_history_string_keys(): + """Index keys arrive as strings when the corrupted data round-tripped + through JSON — those convert too, and bare entries still get repaired.""" + data = {"history": {"1": {"asset_id": "abc123"}, "0": {"m": 0}}} + + result = await reconstruct_cleanup(data) + + history = result["history"] + assert isinstance(history, list) + assert len(history) == 2 + assert history[1]["asset_id"] == "abc123" + # _repair_history runs after the coercion and backfills required fields + for entry in history: + assert entry["typ"] == "narrator" + assert "message" in entry + + +@pytest.mark.asyncio +async def test_reconstruct_cleanup_repairs_all_forced_list_fields(): + """Every list field is normalized, including nested layered_history layers.""" + data = { + "history": {0: {"message": "a", "typ": "narrator"}}, + "archived_history": {0: {"text": "summary"}}, + "layered_history": {0: [{"text": "layer0"}], 1: {0: {"text": "layer1"}}}, + "active_characters": {0: "Alice", 1: "Bob"}, + "game_state_watch_paths": {0: "game.state.foo"}, + } + + result = await reconstruct_cleanup(data) + + assert isinstance(result["history"], list) + assert result["archived_history"] == [{"text": "summary"}] + assert result["layered_history"] == [[{"text": "layer0"}], [{"text": "layer1"}]] + assert result["active_characters"] == ["Alice", "Bob"] + assert result["game_state_watch_paths"] == ["game.state.foo"] + + +@pytest.mark.asyncio +async def test_reconstruct_cleanup_leaves_legit_dicts_and_lists_alone(): + """Dicts with non-index keys and healthy lists pass through untouched.""" + data = { + "history": [{"message": "a", "typ": "narrator", "source": "ai", "flags": 0}], + # malformed digit-ish key — must not be treated as an index (and must + # not raise from int()) + "archived_history": {"--5": {"text": "x"}}, + "character_data": {"Alice": {"name": "Alice"}}, + "agent_state": {"0_custom": True, "narrator": {}}, + } + + result = await reconstruct_cleanup(data) + + assert result["history"] == data["history"] + assert result["archived_history"] == {"--5": {"text": "x"}} + assert result["character_data"] == {"Alice": {"name": "Alice"}} + assert result["agent_state"] == {"0_custom": True, "narrator": {}} + + +@pytest.mark.asyncio +async def test_reconstruct_scene_data_repairs_forced_dict_history(mock_scene): + """End-to-end issue #81 repro: an iterable_item_added delta targeting a + missing history parent makes Delta(force=True) build history as an + int-keyed dict — reconstruction must still return a sliceable list.""" + base_data = {"characters": [], "entries": []} # no history at all + base_path = _base_path(mock_scene) + os.makedirs(os.path.dirname(base_path), exist_ok=True) + with open(base_path, "w") as f: + json.dump(base_data, f) + + log_data = { + "version": 1, + "base": f"{mock_scene.filename}.base.json", + "start_rev": 0, + "deltas": [ + { + "rev": 1, + "ts": 1672531200, + "delta": { + "iterable_item_added": { + "root['history'][1]": { + "message": "hello", + "typ": "narrator", + "source": "ai", + "flags": 0, + } + } + }, + "meta": {}, + } + ], + "latest_rev": 1, + } + log_path = _changelog_log_path(mock_scene, 0) + with open(log_path, "w") as f: + json.dump(log_data, f) + + result = await reconstruct_scene_data(mock_scene, to_rev=1) + + history = result["history"] + assert isinstance(history, list) + assert len(history) == 1 + assert history[0]["message"] == "hello" + # the timeline preview slice that crashed in issue #81 + assert history[-20:] == history + + @pytest.mark.asyncio async def test_reconstruct_scene_data_repairs_force_true_corruption(mock_scene): """End-to-end: orphan dict-add on a non-existent history index reconstructs to a loadable entry."""