mirror of
https://github.com/sligter/LandPPT.git
synced 2026-08-30 17:34:51 +08:00
sligter
d52b969323
Audit of the outline- and slide-generation paths found four systemic defects.
All are fixed here, with 244 regression tests added across 9 new test files.
Failures were being reported as successes. Every fallback path persisted
fabricated or broken content, marked the stage completed, billed the user and
returned a success message:
- Add OutlineRepairFailedError; the repair loop now raises instead of returning
an outline that failed all 10 attempts.
- Drop the 3-page placeholder outline that replaced a failed generation.
- Track failed slides separately so they are not billed, not marked completed,
and not skipped as "already generated" on a re-run.
- Stop decoding binary uploads as latin-1, which turned PDFs into mojibake
outlines that were reported as successful.
Cancellation, reset and slide locks did nothing:
- Forward _is_slides_generation_cancelled through EnhancedPPTService, which has
no __getattr__, so the stop button was a silent no-op for the whole run.
- Catch CancelledError when awaiting the cancelled lock renewer; it is a
BaseException, so the distributed lock was never released.
- Add clear_project_outline/clear_project_slides; save_project_outline(None)
and save_project_slides("", []) were both no-ops, so stage reset kept the
old data and replayed it as a fresh result.
- Persist slide locks in slide_metadata and honour them in batch regeneration.
HTML validation was inverted. libxml2 only knows HTML 4.0, so strict parsing
rejected <header>/<section>/<svg> while accepting truncated documents:
- Add html_structure.py, an HTML5-aware structural check built on html.parser.
- Re-validate after auto-repair rather than treating "the string changed" as
success, which made the retry budget unreachable.
- Prefer the last complete document when extracting HTML, so repair responses
that echo the original no longer return the unrepaired input.
The streaming think-tag filter lost or leaked content when a marker was split
across chunks; rewrite it to buffer partial markers.
Also fixed:
- Confine client-supplied file paths to the upload roots (arbitrary file read).
- Escape model-generated content before it reaches innerHTML (stored XSS).
- Enforce ownership on the slide cancel endpoints and validate stage status.
- Give the fixed page-count mode a real branch; it behaved as ai_decide and
overwrote the user's setting.
- Stop citation markers such as "[1]" from hijacking outline JSON extraction.
- Bill only slides that were actually persisted, and price the streaming and
non-streaming outline routes identically.
- Cancel orphaned generation tasks when an SSE client disconnects.
- Release stale "running" tasks found in the database, not just in Valkey.
- Replace a 3s unconditional poll (which updated absent DOM nodes) with a
stoppable 15s poll, and add an AbortController to prevent concurrent streams.
- Implement restore_project_version, which raised AttributeError on every call.
- Declare lxml, which was imported directly but only present transitively.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
96 lines
3.3 KiB
Python
96 lines
3.3 KiB
Python
"""Regression tests for slide-generation cancellation and lock release."""
|
|
|
|
import ast
|
|
import asyncio
|
|
from pathlib import Path
|
|
|
|
import pytest
|
|
|
|
SRC = Path(__file__).resolve().parents[1] / "src" / "landppt"
|
|
|
|
|
|
def _class_methods(path: Path, class_name: str) -> set:
|
|
tree = ast.parse(path.read_text(encoding="utf-8"))
|
|
for node in ast.walk(tree):
|
|
if isinstance(node, ast.ClassDef) and node.name == class_name:
|
|
return {
|
|
m.name
|
|
for m in node.body
|
|
if isinstance(m, (ast.FunctionDef, ast.AsyncFunctionDef))
|
|
}
|
|
raise AssertionError(f"class {class_name} not found in {path}")
|
|
|
|
|
|
def test_cancellation_check_is_reachable_from_generation_service():
|
|
"""SlideGenerationService delegates to EnhancedPPTService via __getattr__.
|
|
|
|
EnhancedPPTService has no __getattr__ of its own, so the cancel check must be
|
|
forwarded explicitly or the stop button becomes a silent no-op.
|
|
"""
|
|
enhanced = _class_methods(
|
|
SRC / "services" / "enhanced_ppt_service.py", "EnhancedPPTService"
|
|
)
|
|
assert "_is_slides_generation_cancelled" in enhanced
|
|
assert "request_cancel_slides_generation" in enhanced
|
|
assert "clear_cancel_slides_generation" in enhanced
|
|
|
|
|
|
def test_generation_loop_logs_cancellation_check_failures():
|
|
"""A broken cancel check must not be swallowed by a bare `except: pass`."""
|
|
source = (
|
|
SRC / "services" / "slide" / "slide_generation_service.py"
|
|
).read_text(encoding="utf-8")
|
|
marker = "if await self._is_slides_generation_cancelled("
|
|
assert marker in source
|
|
tail = source[source.index(marker) :]
|
|
handler = tail[: tail.index("batch_end = min(")]
|
|
assert "except Exception:\n pass" not in handler
|
|
assert "logger.error" in handler
|
|
|
|
|
|
def test_await_on_cancelled_task_needs_cancellederror_in_handler():
|
|
"""Guards the actual language behaviour the lock-release fix depends on."""
|
|
|
|
async def scenario(handler_catches_cancelled: bool) -> bool:
|
|
cleanup_ran = False
|
|
|
|
async def renew():
|
|
await asyncio.sleep(60)
|
|
|
|
task = asyncio.create_task(renew())
|
|
await asyncio.sleep(0)
|
|
try:
|
|
task.cancel()
|
|
if handler_catches_cancelled:
|
|
try:
|
|
await task
|
|
except (asyncio.CancelledError, Exception):
|
|
pass
|
|
else:
|
|
try:
|
|
await task
|
|
except Exception:
|
|
pass
|
|
cleanup_ran = True
|
|
except asyncio.CancelledError:
|
|
pass
|
|
return cleanup_ran
|
|
|
|
assert asyncio.run(scenario(handler_catches_cancelled=True)) is True
|
|
# Without CancelledError in the handler the cleanup is skipped entirely.
|
|
assert asyncio.run(scenario(handler_catches_cancelled=False)) is False
|
|
|
|
|
|
def test_background_generate_slides_releases_lock_after_cancelling_renewer():
|
|
source = (
|
|
SRC / "services" / "slide" / "slide_streaming_service.py"
|
|
).read_text(encoding="utf-8")
|
|
marker = "renew_task.cancel()"
|
|
assert marker in source
|
|
tail = source[source.index(marker) :]
|
|
block = tail[: tail.index("_slides_generation_tasks.pop")]
|
|
assert "asyncio.CancelledError" in block, (
|
|
"awaiting the cancelled renew task must catch CancelledError, "
|
|
"otherwise _release_slides_generation_lock is skipped"
|
|
)
|