From ab92b738eb01d7e6aaf36c822714d39c6ed33916 Mon Sep 17 00:00:00 2001 From: Tchen <176354753+Tchen-data@users.noreply.github.com> Date: Mon, 21 Sep 2026 03:20:35 +0000 Subject: [PATCH 1/2] fix(ppt-engine): deliver the deck alone, with no pdf copy beside it The plugin's outbound hook still wrote a PDF rendering next to every published deck and announced it on a second MEDIA line, so a delivery carried two files where the user asked for one, and the transcript showed a PDF tile beside the deck. The earlier change that stopped the build stage from producing the preview did not reach this hook. The hook now publishes the deck and names it once; the web UI renders its own preview from the deck on demand, so nothing is lost. --- .../ppt-engine/raven_ppt/plugin/hook.py | 53 +++---------------- tests/test_ppt_engine_plugin.py | 32 +++++------ 2 files changed, 24 insertions(+), 61 deletions(-) diff --git a/plugins-dist/ppt-engine/raven_ppt/plugin/hook.py b/plugins-dist/ppt-engine/raven_ppt/plugin/hook.py index 6fa6b3f57..01a0ef160 100644 --- a/plugins-dist/ppt-engine/raven_ppt/plugin/hook.py +++ b/plugins-dist/ppt-engine/raven_ppt/plugin/hook.py @@ -43,8 +43,6 @@ from __future__ import annotations import logging -import os -import uuid from importlib.resources import files as pkg_files from pathlib import Path @@ -59,7 +57,6 @@ delivered_decks, last_refusal, published_digests, - published_original, unrecorded_deliveries, ) @@ -481,11 +478,9 @@ async def review(self, step: StepView) -> Answer: if deck.name in text or self._delivered_nudged: return Accept() self._delivered_nudged = True - pdf = deck.with_suffix(".pdf") - paths = str(deck) + (f" (and its PDF preview {pdf})" if pdf.is_file() else "") return Resample( "reply ends the turn without naming the deck it published", - inject=[{"role": "user", "content": DELIVERED_NUDGE.format(paths=paths)}], + inject=[{"role": "user", "content": DELIVERED_NUDGE.format(paths=str(deck))}], note="ppt_engine: reply ending a turn without naming the deck it published rolled back (1/1)", ) if _hands_back(text): @@ -547,13 +542,12 @@ async def outbound(self, reply: str, step: StepView) -> str | None: "presentation carrying slides." ) return None - announced = f"\n\nPublished a {slides}-slide deck.\nDeck: {deck}\nMEDIA: {deck}" - preview = deck.with_suffix(".pdf") - if not _preview_of(deck, preview): - _preview_beside_copy(Path(bound) / "deck" / "state", deck, preview) - if _preview_of(deck, preview): - announced += f"\nPreview (the same deck as a PDF, for viewing): {preview}\nMEDIA: {preview}" - return reply + announced + # The deck alone. The render under out/ is the engine's own preview, and + # the web surface renders a deck it is shown by itself; a PDF copied beside + # the deck and announced as a second MEDIA line was a second deliverable + # the user never asked for, and the one place left that still wrote one + # after the build stage stopped. + return reply + f"\n\nPublished a {slides}-slide deck.\nDeck: {deck}\nMEDIA: {deck}" def ppt_hook(home: Path | None = None, *, deck_per_session: bool = True) -> ParticipantHook: @@ -606,39 +600,6 @@ def _standing_decks(root: Path) -> list[Path]: return materials.published_decks(root / OUT_DIRNAME, published_digests(root / "deck" / "state")) -def _preview_beside_copy(state_dir: Path, deck: Path, preview: Path) -> None: - """Give a renamed copy of the published deck the original's PDF, under its own stem. - - The reply named a copy the model made of the published deck -- the digest check - let it through -- and the PDF the publish wrote sits beside the original, so the - copy would go out with no preview. Copied fresh rather than with its timestamps, - because the copy of the deck is newer than the render and `_preview_of` reads the - timestamps. - """ - import shutil - - original = published_original(state_dir, deck) - if original is None: - return - rendered = original.with_suffix(".pdf") - if not _preview_of(original, rendered): - return - try: - temporary = preview.parent / f".{preview.name}.{uuid.uuid4().hex}.tmp" - shutil.copyfile(rendered, temporary) - os.replace(temporary, preview) - except OSError as exc: - logger.warning("ppt-engine: the preview could not be put beside %s: %s", deck.name, exc) - - -def _preview_of(deck: Path, preview: Path) -> bool: - """Whether `preview` is a picture of this very deck rather than an earlier one.""" - try: - return preview.is_file() and preview.stat().st_mtime_ns >= deck.stat().st_mtime_ns - except OSError: - return False - - def _refused_because(state_dir: Path) -> str: """The last refusal as a sentence for the model, or where to get one.""" reason = last_refusal(state_dir) diff --git a/tests/test_ppt_engine_plugin.py b/tests/test_ppt_engine_plugin.py index 55c38d160..f2b607c97 100644 --- a/tests/test_ppt_engine_plugin.py +++ b/tests/test_ppt_engine_plugin.py @@ -429,7 +429,7 @@ async def run() -> tuple: assert f"Published a 3-slide deck.\nDeck: {deck}\nMEDIA: {deck}" in outbound.modified_content -def test_a_renamed_copy_of_the_published_deck_gets_the_preview_under_its_own_name(tmp_path: Path) -> None: +def test_a_renamed_copy_of_the_published_deck_is_announced_without_a_preview(tmp_path: Path) -> None: """The PDF follows the deck the reply names, not the stem the publish wrote. A live run copied `out/deck.pptx` to a title of its own with `exec` and named the @@ -459,10 +459,12 @@ async def run() -> tuple: return outbound, copy outbound, copy = asyncio.run(run()) - preview = copy.with_suffix(".pdf") - assert preview.read_bytes() == b"%PDF-1.4 rendered" assert f"Deck: {copy}\nMEDIA: {copy}" in outbound.modified_content - assert f"Preview (the same deck as a PDF, for viewing): {preview}\nMEDIA: {preview}" in outbound.modified_content + # The deck alone: the render under out/ is the engine's own preview and is + # neither copied beside the deck nor announced as a second MEDIA line. + assert not copy.with_suffix(".pdf").exists() + assert "Preview" not in outbound.modified_content + assert ".pdf" not in outbound.modified_content def test_a_turn_that_skipped_the_inbound_phase_is_pointed_at_the_deck_on_its_first_iteration(tmp_path: Path) -> None: @@ -990,9 +992,10 @@ async def run(): assert "The last build was refused: not published: 1 blocking" in sent.modified_content -def test_the_announcement_carries_the_pdf_beside_the_deck(tmp_path: Path) -> None: - """A .pptx is a download and nothing more on the web surface; the PDF the build wrote - beside it is the same deck as pages, and the announcement hands both over.""" +def test_the_announcement_does_not_carry_the_pdf_beside_the_deck(tmp_path: Path) -> None: + """The render under out/ is the engine's own preview: the web surface renders a deck + it is shown by itself, and a second MEDIA line made the PDF a second deliverable the + user never asked for. The announcement hands over the deck alone.""" hook = plugin_module.make_hook(_ctx(dict(ENABLED), tmp_path / "ws")) wd = tmp_path / "session" wd.mkdir() @@ -1010,7 +1013,7 @@ async def run(): decision, deck = asyncio.run(run()) assert f"Deck: {deck}\nMEDIA: {deck}" in decision.modified_content - assert f"{deck.with_suffix('.pdf')}\nMEDIA: {deck.with_suffix('.pdf')}" in decision.modified_content + assert str(deck.with_suffix(".pdf")) not in decision.modified_content def _reply(text: str, tool_calls=()): @@ -1285,10 +1288,8 @@ async def run(): def test_a_preview_from_an_earlier_deck_is_not_announced_as_this_one(tmp_path: Path) -> None: - """`_pdf_beside` returns None when the build could not render a PDF, and the older - file it leaves in place is a picture of a deck that no longer exists. Announced as - "the same deck" it is worse than no preview, so the announcement takes only a - preview at least as new as the deck it stands for.""" + """No PDF is announced at all now, an older one beside the deck least of all: it is + a picture of a deck that no longer exists, and the deliverable is the deck alone.""" import os import time @@ -1341,9 +1342,10 @@ def _delivered_record(own: Path, deck: Path, delivered: Path) -> None: ) -def test_the_delivered_path_is_announced_with_its_preview(tmp_path: Path) -> None: +def test_the_delivered_path_is_announced_without_a_preview(tmp_path: Path) -> None: """The user asked for the file at a path of their own; the reply that names it is - confirmed rather than contradicted, and its PDF rides along.""" + confirmed rather than contradicted, and nothing but the deck is announced -- a PDF + sitting beside it is a file the user has, not a deliverable this turn adds.""" hook = plugin_module.make_hook(_ctx(dict(ENABLED), tmp_path / "ws")) wd = tmp_path / "session" wd.mkdir() @@ -1368,7 +1370,7 @@ async def run(): decision, delivered, iteration = asyncio.run(run()) assert not iteration.rollback, "a reply naming the delivered path is a finished turn" assert f"Published a 3-slide deck.\nDeck: {delivered}\nMEDIA: {delivered}" in decision.modified_content - assert f"MEDIA: {delivered.with_suffix('.pdf')}" in decision.modified_content + assert f"MEDIA: {delivered.with_suffix('.pdf')}" not in decision.modified_content assert "No deck was published" not in decision.modified_content From cdebb0f964df88d717b9bd4edfe07e2193f2c1fc Mon Sep 17 00:00:00 2001 From: Tchen <176354753+Tchen-data@users.noreply.github.com> Date: Mon, 21 Sep 2026 06:30:31 +0000 Subject: [PATCH 2/2] refactor(ppt-engine): drop the publish helper the hook was the last to call published_original found the deck a renamed copy was made from so that the copy's preview could be looked up under the original's stem. With no preview placed beside a delivered deck any more, nothing asks that question; the hook was its only caller and the previous commit removed it. --- .../raven_ppt/services/publish/deliver.py | 23 ------------------- 1 file changed, 23 deletions(-) diff --git a/plugins-dist/ppt-engine/raven_ppt/services/publish/deliver.py b/plugins-dist/ppt-engine/raven_ppt/services/publish/deliver.py index b9a994c65..373f855a1 100644 --- a/plugins-dist/ppt-engine/raven_ppt/services/publish/deliver.py +++ b/plugins-dist/ppt-engine/raven_ppt/services/publish/deliver.py @@ -345,29 +345,6 @@ def delivered_decks(state_dir: Path) -> list[Path]: return found -def published_original(state_dir: Path, copy: Path) -> Path | None: - """The published deck ``copy`` is a byte-for-byte copy of, or None. - - A model that wants the deliverable under a title of its own copies the - published deck to that name; the copy passes the digest check, but its - preview sat beside the original, under the original's stem. - """ - import hashlib - - try: - digest = hashlib.sha256(copy.read_bytes()).hexdigest() - held = json.loads((state_dir / PUBLISHED_RECORD).read_text(encoding="utf-8")) - except (OSError, ValueError): - return None - for entry in held.get("published", []): - if not isinstance(entry, dict) or entry.get("sha256") != digest: - continue - original = Path(str(entry.get("path") or "")) - if original.name and original.resolve() != copy.resolve() and original.is_file(): - return original - return None - - def published_digests(state_dir: Path) -> set[str]: """The sha256 of every deck this project has published; empty when none has been.""" target = state_dir / PUBLISHED_RECORD