diff --git a/pkg-py/CHANGELOG.md b/pkg-py/CHANGELOG.md index c4a3a63c..a79f07ac 100644 --- a/pkg-py/CHANGELOG.md +++ b/pkg-py/CHANGELOG.md @@ -33,6 +33,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Bug fixes +* Rich tool-display HTML now uses htmltools' source-preserving persistence + codec. Local dependency definitions can be registered again after restoring + a conversation in a new session. Persisted dependency dictionaries written + by earlier shinychat versions remain readable on a best-effort basis. + * Fixed conversation history and bookmarks failing to serialize a chatlas `ContentToolResult` when its supported dictionary-form `extra["display"]` contains HTML. Dictionary displays are now normalized through `ToolResultDisplay` before JSON serialization. (Related to #295) * Fixed `MarkdownStream` permanently stopping following new content after the user scrolled back to the bottom. Pinning was decided only from `scroll` events, which browsers dispatch asynchronously; if a chunk grew the container first, the user's at-bottom position no longer read as at-bottom and auto-scroll silently disengaged for good. (#282) diff --git a/pkg-py/src/shinychat/_chat_normalize_chatlas.py b/pkg-py/src/shinychat/_chat_normalize_chatlas.py index d929f868..f772b6d7 100644 --- a/pkg-py/src/shinychat/_chat_normalize_chatlas.py +++ b/pkg-py/src/shinychat/_chat_normalize_chatlas.py @@ -11,6 +11,7 @@ MetadataNode, RenderedHTML, ReprHtml, + SerializedHTML, Tag, Tagifiable, TagList, @@ -20,7 +21,10 @@ from typing_extensions import TypeAliasType from ._chat_types import ChatMessage -from ._htmltools_serialization import SerializedHTML, serialize_htmltools +from ._htmltools_serialization import ( + deserialize_htmltools, + serialize_htmltools, +) if TYPE_CHECKING: from chatlas.types import ContentToolRequest, ContentToolResult @@ -77,7 +81,7 @@ def _serialize_icon(self, value: TagChild) -> SerializedHTML: @classmethod def _validate_icon(cls, value: TagChild) -> TagChild: if isinstance(value, dict): - return restore_rendered_html(value) + return deserialize_htmltools(value) else: return value @@ -379,7 +383,7 @@ def _serialize_html_icon(self, value: TagChild) -> SerializedHTML: @classmethod def _validate_html_icon(cls, value: TagChild) -> TagChild: if isinstance(value, dict): - return restore_rendered_html(value) + return deserialize_htmltools(value) else: return value @@ -644,38 +648,3 @@ def tool_display_override() -> Literal["none", "basic", "rich"]: raise ValueError( 'The `SHINYCHAT_TOOL_DISPLAY` env var must be one of: "none", "basic", or "rich"' ) - - -def restore_rendered_html(x: dict[str, Any]): - from htmltools import HTMLDependency - - if "html" not in x or "dependencies" not in x: - raise ValueError(f"Don't know how to restore HTML from {x}") - - deps: list[HTMLDependency] = [] - for d in x["dependencies"]: - if not isinstance(d, dict): - continue - name = d["name"] - version = d["version"] - other = {k: v for k, v in d.items() if k not in ("name", "version")} - # TODO: warn if the source is a tempdir? - deps.append(HTMLDependency(name=name, version=version, **other)) - - res = TagList(HTML(x["html"]), *deps) - if not deps: - return res - - session = None - try: - from shiny.session import get_current_session - - session = get_current_session() - except Exception: - pass - - # De-dupe dependencies for the current Shiny session - if session: - session._process_ui(res) - - return res diff --git a/pkg-py/src/shinychat/_htmltools_serialization.py b/pkg-py/src/shinychat/_htmltools_serialization.py index ebe9030f..d868c9b8 100644 --- a/pkg-py/src/shinychat/_htmltools_serialization.py +++ b/pkg-py/src/shinychat/_htmltools_serialization.py @@ -1,35 +1,73 @@ -"""Temporary JSON adapter for htmltools values. +"""Persistence adapters for htmltools values. -Keep shinychat serialization behind this module until htmltools exposes a -source-preserving rendered-HTML codec; migration should then be confined to -this boundary. +New values use htmltools' source-preserving codec. The legacy reader remains +here because shinychat versions before the htmltools 0.8.0 migration persisted +browser-oriented dependency dictionaries. """ from __future__ import annotations -from typing import Any +from typing import Any, cast -from htmltools import TagList, is_tag_child +from htmltools import ( + HTML, + HTMLDependency, + SerializedHTML, + TagChild, + TagList, + deserialize_html, + is_tag_child, + serialize_html, +) from pydantic_core import PydanticSerializationError -from typing_extensions import TypedDict - - -class SerializedHTML(TypedDict): - html: str - dependencies: list[dict[str, Any]] def serialize_htmltools(value: object) -> SerializedHTML: - """Convert an htmltools node to shinychat's current JSON wire format.""" + """Serialize an htmltools value for durable persistence.""" if not is_tag_child(value): raise PydanticSerializationError( f"Unable to serialize unknown type: {type(value)}" ) + return serialize_html(value) + + +def deserialize_htmltools(value: object) -> TagChild: + if not isinstance(value, dict): + if is_tag_child(value): + return value + raise TypeError(f"Expected an htmltools value, got {type(value)}") + + if "html" not in value or "dependencies" not in value: + raise ValueError(f"Don't know how to restore HTML from {value}") + + dependencies = value["dependencies"] + if not isinstance(dependencies, list): + raise ValueError(f"Don't know how to restore HTML from {value}") + + if all(is_durable_dependency(dependency) for dependency in dependencies): + return deserialize_html(cast(SerializedHTML, value)) + + return deserialize_legacy_html(value) + + +def is_durable_dependency(value: object) -> bool: + return ( + isinstance(value, dict) and "source" in value and "all_files" in value + ) + + +def deserialize_legacy_html(value: dict[str, Any]) -> TagList: + dependencies: list[HTMLDependency] = [] + for dependency in value["dependencies"]: + if not isinstance(dependency, dict): + continue + name = dependency["name"] + version = dependency["version"] + other = { + key: item + for key, item in dependency.items() + if key not in ("name", "version") + } + dependencies.append(HTMLDependency(name=name, version=version, **other)) - rendered = TagList(value).render() - return { - "html": rendered["html"], - "dependencies": [ - dependency.as_dict() for dependency in rendered["dependencies"] - ], - } + return TagList(HTML(value["html"]), *dependencies) diff --git a/pkg-py/tests/pytest/test_bookmark_serialization.py b/pkg-py/tests/pytest/test_bookmark_serialization.py index 32ed24fd..c3f7867c 100644 --- a/pkg-py/tests/pytest/test_bookmark_serialization.py +++ b/pkg-py/tests/pytest/test_bookmark_serialization.py @@ -9,12 +9,14 @@ from __future__ import annotations import json +from pathlib import Path from typing import Any import pytest from chatlas import ChatOpenAI, ContentToolResult, Turn -from htmltools import HTMLDependency, tags +from htmltools import HTMLDependency, TagList, tags from pydantic_core import PydanticSerializationError +from shiny import App from shinychat._chat_bookmark import get_chatlas_state from shinychat.types import ToolResultDisplay @@ -38,7 +40,14 @@ async def test_turn_serialization_with_htmldep_in_tool_result(as_dict: bool): typed_display = ToolResultDisplay( html=tags.div( "Widget output", - HTMLDependency("my-dep", "1.0", source={"subdir": "."}), + HTMLDependency( + "my-dep", + "1.0", + source={"subdir": "."}, + script={"src": "widget.js"}, + stylesheet={"href": "widget.css"}, + all_files=True, + ), ), title="My Widget", ) @@ -69,16 +78,118 @@ async def test_turn_serialization_with_htmldep_in_tool_result(as_dict: bool): assert display_data["application_metadata"] == { "widget_id": "my-widget" } - deps = display_data["html"]["dependencies"] - assert len(deps) == 1 - assert deps[0]["name"] == "my-dep" - assert deps[0]["version"] == "1.0" + dependencies = display_data["html"]["dependencies"] + assert len(dependencies) == 1 + assert dependencies[0] == { + "name": "my-dep", + "version": "1.0", + "source": {"subdir": "."}, + "script": [{"src": "widget.js"}], + "stylesheet": [{"href": "widget.css", "rel": "stylesheet"}], + "meta": [], + "all_files": True, + "head": None, + } # Must round-trip back to a valid Turn restored = Turn.model_validate(json.loads(json_str)) assert restored.role == "user" assert len(restored.contents) == 1 assert isinstance(restored.contents[0], ContentToolResult) + restored_display = ToolResultDisplay.model_validate( + restored.contents[0].extra["display"] + ) + restored_dependency = TagList(restored_display.html).render()[ + "dependencies" + ][0] + + assert restored_dependency.source == {"subdir": "."} + assert restored_dependency.script == [{"src": "widget.js"}] + assert restored_dependency.stylesheet == [ + {"href": "widget.css", "rel": "stylesheet"} + ] + assert restored_dependency.all_files is True + + +def test_tool_result_display_restores_legacy_dependency_payload(): + legacy_display = { + "html": { + "html": '
Widget output
', + "dependencies": [ + { + "name": "my-dep", + "version": "1.0", + "script": [{"src": "lib/my-dep-1.0/widget.js"}], + "stylesheet": [ + { + "href": "lib/my-dep-1.0/widget.css", + "rel": "stylesheet", + } + ], + "meta": [], + "head": None, + } + ], + } + } + + display = ToolResultDisplay.model_validate(legacy_display) + rendered = TagList(display.html).render() + dependency = rendered["dependencies"][0] + + assert rendered["html"] == '
Widget output
' + assert dependency.name == "my-dep" + assert str(dependency.version) == "1.0" + assert dependency.source is None + assert dependency.script == [{"src": "lib/my-dep-1.0/widget.js"}] + assert dependency.stylesheet == [ + { + "href": "lib/my-dep-1.0/widget.css", + "rel": "stylesheet", + } + ] + + new_value = display.model_dump(mode="json") + new_dependency = new_value["html"]["dependencies"][0] + + assert "source" in new_dependency + assert new_dependency["source"] is None + assert new_dependency["all_files"] is False + + +def test_restored_dependency_can_register_with_new_shiny_app(tmp_path: Path): + (tmp_path / "widget.css").write_text( + ".widget { color: red; }", + encoding="utf-8", + ) + display = ToolResultDisplay( + html=tags.div( + {"class": "widget"}, + "Widget", + HTMLDependency( + "my-dep", + "1.0", + source={"subdir": str(tmp_path)}, + stylesheet={"href": "widget.css"}, + ), + ) + ) + + restored = ToolResultDisplay.model_validate( + json.loads(display.model_dump_json()) + ) + dependency = TagList(restored.html).render()["dependencies"][0] + + app = App( + tags.div(), + lambda input_, output, session: None, + ) + app._register_web_dependency(dependency) + + assert any( + getattr(route, "path", None) == "/lib/my-dep-1.0" + for route in app._dependency_handler.routes + ) @pytest.mark.anyio diff --git a/pkg-py/tests/test_history_store.py b/pkg-py/tests/test_history_store.py index e4e19cf9..62a3acd6 100644 --- a/pkg-py/tests/test_history_store.py +++ b/pkg-py/tests/test_history_store.py @@ -92,7 +92,12 @@ async def test_file_store_round_trips_dict_tool_result_display( adapter.set_turns_json(restored.path_turns()) display = adapter.get_turns_json()[0]["contents"][0]["extra"]["display"] assert display["html"]["html"] == "
Widget output
" - assert display["html"]["dependencies"][0]["name"] == "my-dep" + dependency = display["html"]["dependencies"][0] + assert dependency["name"] == "my-dep" + assert dependency["source"] == {"subdir": "."} + assert dependency["script"] == [] + assert dependency["stylesheet"] == [] + assert dependency["all_files"] is False @pytest.mark.anyio diff --git a/pyproject.toml b/pyproject.toml index 3206d7a2..517a979e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -5,7 +5,7 @@ readme = "pkg-py/README.md" requires-python = ">=3.10" license = { text = "MIT" } dependencies = [ - "htmltools>=0.7.0", + "htmltools>=0.8.0", "shiny>=1.4.0", "pydantic>=2.11" ]