diff --git a/app/features/demo/routes.py b/app/features/demo/routes.py index 6d3284c4..d2881acb 100644 --- a/app/features/demo/routes.py +++ b/app/features/demo/routes.py @@ -3,8 +3,10 @@ Exposes: - ``POST /demo/run`` -- synchronous; runs the whole pipeline, returns a result. - ``WS /demo/stream`` -- streams one StepEvent per step for the live UI. -- ``GET /demo/workspaces`` -- E4 (#393): list saved workspaces. -- ``GET /demo/workspaces/{workspace_id}`` -- E4 (#393): one workspace's detail. +- ``GET /demo/workspaces`` -- E4 (#393): list saved workspaces. +- ``GET /demo/workspaces/{workspace_id}`` -- E4 (#393): one workspace's detail. +- ``DELETE /demo/workspaces/{workspace_id}`` -- delete the workspace METADATA + row only; the run's created objects are soft references and stay untouched. The run/stream handlers obtain the live FastAPI app from ``request.app`` / ``websocket.app`` and pass it into the pipeline -- the slice never imports @@ -16,7 +18,15 @@ import json -from fastapi import APIRouter, Depends, Query, Request, WebSocket, WebSocketDisconnect +from fastapi import ( + APIRouter, + Depends, + Query, + Request, + WebSocket, + WebSocketDisconnect, + status, +) from pydantic import ValidationError from sqlalchemy.ext.asyncio import AsyncSession @@ -125,6 +135,34 @@ async def get_showcase_workspace( return WorkspaceDetailResponse.model_validate(row) +@router.delete( + "/workspaces/{workspace_id}", + status_code=status.HTTP_204_NO_CONTENT, + summary="Delete a saved showcase workspace", + description=( + "Delete one saved workspace METADATA row. Everything the run created " + "(model runs, scenario plans, aliases, jobs, artifacts) is a soft " + "reference and is NOT deleted." + ), +) +async def delete_showcase_workspace( + workspace_id: str, + db: AsyncSession = Depends(get_db), +) -> None: + """Delete a saved showcase workspace metadata row. + + Args: + workspace_id: External identifier of the workspace. + db: Async database session from dependency. + + Raises: + NotFoundError: When no workspace matches ``workspace_id``. + """ + deleted = await workspace.delete_workspace(db, workspace_id) + if not deleted: + raise NotFoundError(message=f"Workspace not found: {workspace_id}") + + @router.websocket("/stream") async def stream_demo_pipeline(websocket: WebSocket) -> None: """Stream one StepEvent per pipeline step over a WebSocket. diff --git a/app/features/demo/tests/test_routes.py b/app/features/demo/tests/test_routes.py index 016049db..6fd5b84a 100644 --- a/app/features/demo/tests/test_routes.py +++ b/app/features/demo/tests/test_routes.py @@ -316,6 +316,41 @@ async def fake_get(_db, workspace_id: str) -> SimpleNamespace: assert body["date_end"] == "2026-03-31" +# ============================================================================= +# DELETE /demo/workspaces/{workspace_id} (unit) +# ============================================================================= + + +async def test_delete_workspace_204(client, monkeypatch): + """A deleted workspace row yields 204 with an empty body.""" + seen: dict[str, str] = {} + + async def fake_delete(_db, workspace_id: str) -> bool: + seen["workspace_id"] = workspace_id + return True + + monkeypatch.setattr(workspace, "delete_workspace", fake_delete) + + resp = await client.delete("/demo/workspaces/" + "c" * 32) + assert resp.status_code == 204 + assert resp.content == b"" + assert seen["workspace_id"] == "c" * 32 + + +async def test_delete_workspace_404(client, monkeypatch): + """An unknown workspace_id is a 404 problem+json.""" + + async def fake_delete(_db, _workspace_id: str) -> bool: + return False + + monkeypatch.setattr(workspace, "delete_workspace", fake_delete) + + resp = await client.delete("/demo/workspaces/" + "0" * 32) + assert resp.status_code == 404 + assert resp.headers["content-type"].startswith("application/problem+json") + assert "Workspace not found" in resp.json()["detail"] + + # ============================================================================= # E4 (#393) -- workspace GET routes against real Postgres (integration) # ============================================================================= @@ -368,3 +403,64 @@ async def test_get_workspace_integration_round_trip(client, db_session: AsyncSes missing = await client.get("/demo/workspaces/" + "f" * 32) assert missing.status_code == 404 assert missing.headers["content-type"].startswith("application/problem+json") + + +@pytest.mark.integration +async def test_delete_workspace_integration_round_trip(client, db_session: AsyncSession): + """DELETE removes exactly the target metadata row; a re-delete is 404.""" + kept = await workspace.create_workspace( + DemoRunRequest.model_validate({"preservation": "keep", "workspace_name": "del-kept"}) + ) + target = await workspace.create_workspace( + DemoRunRequest.model_validate({"preservation": "keep", "workspace_name": "del-target"}) + ) + assert kept is not None and target is not None + + resp = await client.delete(f"/demo/workspaces/{target}") + assert resp.status_code == 204 + + # The deleted row is gone; the sibling row is untouched. + assert (await client.get(f"/demo/workspaces/{target}")).status_code == 404 + survivors = await client.get("/demo/workspaces") + assert [w["workspace_id"] for w in survivors.json()["workspaces"]] == [kept] + + # Deleting again is a 404 problem+json (no idempotent 204). + again = await client.delete(f"/demo/workspaces/{target}") + assert again.status_code == 404 + assert again.headers["content-type"].startswith("application/problem+json") + + +@pytest.mark.integration +async def test_delete_workspace_integration_keeps_created_objects(client, db_session: AsyncSession): + """Deleting a workspace never deletes (or resolves) its soft references. + + The workspace references one REAL cross-slice object (an agent session) + plus one dangling run id -- the delete must succeed without touching the + former or resolving the latter (no-FK soft-reference contract). + """ + session_resp = await client.post("/agents/sessions", json={"agent_type": "experiment"}) + assert session_resp.status_code == 201 + agent_session_id = session_resp.json()["session_id"] + try: + workspace_id = await workspace.create_workspace( + DemoRunRequest.model_validate({"preservation": "keep", "workspace_name": "del-softref"}) + ) + assert workspace_id is not None + row = await workspace.get_workspace(db_session, workspace_id) + assert row is not None + row.created_objects = { + "agent_session_id": agent_session_id, + "winning_run_id": "run-dangling-never-created", + } + await db_session.commit() + + resp = await client.delete(f"/demo/workspaces/{workspace_id}") + assert resp.status_code == 204 + + # The metadata row is gone... + assert (await client.get(f"/demo/workspaces/{workspace_id}")).status_code == 404 + # ...but the soft-referenced agent session still exists. + still_there = await client.get(f"/agents/sessions/{agent_session_id}") + assert still_there.status_code == 200 + finally: + await client.delete(f"/agents/sessions/{agent_session_id}") diff --git a/app/features/demo/tests/test_workspace.py b/app/features/demo/tests/test_workspace.py index 110254c4..0b002be3 100644 --- a/app/features/demo/tests/test_workspace.py +++ b/app/features/demo/tests/test_workspace.py @@ -164,3 +164,21 @@ async def test_list_workspaces_newest_first_limit_offset(db_session: AsyncSessio async def test_get_workspace_missing_returns_none(db_session: AsyncSession) -> None: """get_workspace returns None for an unknown id.""" assert await workspace.get_workspace(db_session, "0" * 32) is None + + +async def test_delete_workspace_removes_only_target_row(db_session: AsyncSession) -> None: + """delete_workspace removes exactly the matching metadata row.""" + id_a = await workspace.create_workspace(_keep_request(workspace_name="it-del-a")) + id_b = await workspace.create_workspace(_keep_request(workspace_name="it-del-b")) + assert id_a is not None and id_b is not None + + assert await workspace.delete_workspace(db_session, id_a) is True + + assert await workspace.get_workspace(db_session, id_a) is None + remaining = await workspace.list_workspaces(db_session) + assert [r.workspace_id for r in remaining] == [id_b] + + +async def test_delete_workspace_missing_returns_false(db_session: AsyncSession) -> None: + """delete_workspace returns False (no raise) for an unknown id.""" + assert await workspace.delete_workspace(db_session, "0" * 32) is False diff --git a/app/features/demo/workspace.py b/app/features/demo/workspace.py index 40b20807..b0e65dad 100644 --- a/app/features/demo/workspace.py +++ b/app/features/demo/workspace.py @@ -14,7 +14,8 @@ :func:`get_workspace` / :func:`list_workspaces` / :func:`count_workspaces` are routed since E4 (epic #393) by ``GET /demo/workspaces`` and -``GET /demo/workspaces/{workspace_id}`` in ``app/features/demo/routes.py``. +``GET /demo/workspaces/{workspace_id}`` in ``app/features/demo/routes.py``; +:func:`delete_workspace` backs ``DELETE /demo/workspaces/{workspace_id}``. """ from __future__ import annotations @@ -195,6 +196,31 @@ async def list_workspaces( return list(result.scalars().all()) +async def delete_workspace(db: AsyncSession, workspace_id: str) -> bool: + """Delete a workspace METADATA row; return ``True`` when a row was removed. + + Deletes ONLY the ``showcase_workspace`` row. Everything the run created -- + model runs, scenario plans, aliases, jobs, agent sessions, artifacts -- is + carried as OPAQUE SOFT REFERENCES in ``created_objects`` (no ForeignKeys + by design, see ``app/features/demo/models.py``) and is deliberately left + untouched: the workspace is an audit record, never an ownership root. + + Args: + db: An open async session (caller-owned). + workspace_id: The external id of the row to delete. + + Returns: + ``True`` when a row was deleted, ``False`` when none matched. + """ + row = await get_workspace(db, workspace_id) + if row is None: + return False + await db.delete(row) + await db.commit() + logger.info("demo.workspace_deleted", workspace_id=workspace_id) + return True + + async def count_workspaces(db: AsyncSession) -> int: """Count all workspace rows (E4, issue #393). diff --git a/docs/_base/API_CONTRACTS.md b/docs/_base/API_CONTRACTS.md index 7947f13a..d307d2a9 100644 --- a/docs/_base/API_CONTRACTS.md +++ b/docs/_base/API_CONTRACTS.md @@ -62,6 +62,7 @@ All endpoints serve JSON; error responses use `application/problem+json` (RFC 78 | demo | WS | `/demo/stream` | Stream one `StepEvent` per pipeline step for the live Showcase page | | demo | GET | `/demo/workspaces` | **E4 (#393)** — list saved showcase workspaces, newest first (`limit` 1-100 default 20 / `offset`); `200` + empty list on an empty table | | demo | GET | `/demo/workspaces/{workspace_id}` | **E4 (#393)** — full workspace row incl. `created_objects` soft references + grain/window columns; `404 application/problem+json` when missing | +| demo | DELETE | `/demo/workspaces/{workspace_id}` | Delete one saved workspace METADATA row; `204` on success, `404 application/problem+json` when missing. The run's created objects (model runs, scenario plans, aliases, jobs, artifacts) are soft references and are NOT deleted | | config | GET | `/config/ai` | Effective AI-model config (agent LLM + RAG embeddings); API keys masked, never raw | | config | PATCH | `/config/ai` | Persist + apply AI-model changes live (no restart). `409` if an embedding-dimension change would orphan indexed RAG chunks (resend with `force=true`) | | config | GET | `/config/providers/health` | Per-provider connectivity — Ollama probed live, cloud providers by API-key presence | diff --git a/docs/_base/DOMAIN_MODEL.md b/docs/_base/DOMAIN_MODEL.md index 06ba2d30..1ec3200b 100644 --- a/docs/_base/DOMAIN_MODEL.md +++ b/docs/_base/DOMAIN_MODEL.md @@ -56,14 +56,18 @@ - An agent-saved plan (`source='agent'`) is persisted ONLY after the human approves it through the HITL gate — it always carries the approval audit trail. ### `showcase_workspace` (Demo) -- **Root:** `ShowcaseWorkspace(workspace_id: str, status: str)` — one row = one preserved (`preservation="keep"`) showcase run. +- **Root:** `ShowcaseWorkspace(workspace_id: str, status: str)` — one row = one preserved (`preservation="keep"`) showcase run. Ephemeral runs (the default) write no row; a `workspace_name` merely labels a keep-run row (names are non-unique). - **Status state machine:** `running` → `completed` | `failed` (CHECK-constrained; the finalize hook settles the row even on mid-run failure). +- **Stored metadata:** replay config (`seed`, `scenario`, `reset`, `skip_seed`), showcase grain + window (`store_id`, `product_id`, `date_start`, `date_end` — NULL on early failure), lifecycle (`status`, `created_at`/`updated_at`), and the two JSONB payloads below. - **JSONB fields:** `created_objects` (sparse soft-reference keys — `winning_run_id`, `v2_run_id`, `v2_model_path`, `alias`, `agent_session_id`, `batch_id`, `scenario_plan_ids`, `scenario_artifact_key`, `train_model_types`, `stale_alias_run_id`) and `result_summary` (winner / WAPE / wall-clock display payload). +- **Relationship to demo pipeline runs:** one workspace row per kept pipeline run — `create_workspace` inserts it as `running` before the first step; `finalize_workspace` settles it with the run's collected ids. NOT a seeder `scenario`: a preset is a reusable data-generation recipe; a workspace is the record of ONE concrete run (which preset it used, with what seed, and what it produced). - **Invariants:** - The config columns (`seed`, `scenario`, `reset`, `skip_seed`) are sufficient for a verbatim Replay through the normal run path — replay never mutates the original row; it creates a NEW row. - `name` is deliberately NON-unique; `workspace_id` (UUID hex) is the unique handle. - `created_objects` carries SOFT references only — **no ForeignKeys by design**. The workspace row is an audit record, not an ownership root: the referenced runs/plans/aliases are independently operator-deletable, and a workspace must never block (or cascade) their deletion. + - Deletion is METADATA-ONLY, symmetric with the no-FK design: `DELETE /demo/workspaces/{id}` removes the `showcase_workspace` row and nothing else — the soft-referenced model runs, scenario plans, aliases, jobs, agent sessions, and artifacts survive, and a workspace whose references already dangle still deletes cleanly. - Persistence is warn-and-continue: a workspace write failure must never break the demo pipeline (the run completes with `workspace_id: null`). +- **Out of scope (deliberately not modeled yet):** a `replayed_from` provenance column, export bundles under `artifacts/showcase//`, RAG-event / approval-decision capture, advanced seed config, and per-phase interactive configuration — see `docs/_base/RUNBOOKS.md` § Showcase workspace. ## Key Invariants — NEVER violate diff --git a/docs/_base/RUNBOOKS.md b/docs/_base/RUNBOOKS.md index b54bf7e1..007176be 100644 --- a/docs/_base/RUNBOOKS.md +++ b/docs/_base/RUNBOOKS.md @@ -144,16 +144,21 @@ uv run python scripts/run_demo.py --seed 42 --quiet 2>&1 | tee demo.log **Notes:** the `POST /demo/run` body and `WS /demo/stream` events are documented in `docs/_base/API_CONTRACTS.md`. The pipeline mirrors `scripts/run_demo.py`; the per-step diagnosis for `make demo` above applies to the same steps. PRP-38 added the `scenario` field on `DemoRunRequest` (defaults to `demo_minimal`) and the additive `phase_name` / `phase_index` / `phase_total` fields on every `StepEvent`. PRP-39 added four new steps (`champion_compat_compare`, `stale_alias_trigger`, `safer_promote_flow`, `batch_preset`) and a new `portfolio` phase between `decision` and `verify`. PRP-40 added the `planning` + `knowledge` phases (5 steps inserted after `portfolio`, before `verify`) and the additive `IndexProjectDocsRequest.path_prefix` field on the RAG slice. PRP-41 — design Z renames the legacy `agent` phase to `agents`, swaps the legacy `step_agent` for `agent_hitl_flow` (HITL approval round-trip), and appends a new `ops` phase carrying `ops_snapshot` immediately before `cleanup`. Total: 24 rows / 10 phases on `showcase_rich`; demo_minimal / sparse keep the 11-row layout under the unified `agents` phase id. The frontend's `DemoPhasePanel.tsx` now carries `onValueChange` (issue #311) and the Showcase page adds a KPI strip + Run-history strip + Stop button + Inspect-Artifacts panel + one-click Approve button on the HITL step card. E2 (#391) — the Scenario control is a card grid exposing all 8 `ScenarioPreset` values with per-preset demo seed profiles (`_SCENARIO_SEED_PROFILE` is exhaustive over the enum; `holiday_rush` seeds a pinned Oct–Dec 2024 window); the 5 newly exposed presets keep the legacy 11-row layout. -### Showcase workspace — preserve/restore/replay semantics (E1–E4, umbrella #389) -**Surface:** the `/showcase` "Save as workspace" controls + **Saved workspaces** panel; `GET /demo/workspaces(/{id})`; `showcase_workspace` table. Endpoint contracts live in `docs/_base/API_CONTRACTS.md` — this entry covers the operational traps only. +### Showcase workspace — preserve/restore/replay/delete semantics (E1–E4, umbrella #389) +**Surface:** the `/showcase` "Save as workspace" controls + **Saved workspaces** panel; `GET/DELETE /demo/workspaces(/{id})`; `showcase_workspace` table. Endpoint contracts live in `docs/_base/API_CONTRACTS.md` — this entry covers the operational traps only. + +**Lifecycle modes:** an **ephemeral** run (the default) writes no workspace row — it lives only in the localStorage Run-history strip. A **keep** run (`preservation="keep"` / the "Save as workspace" checkbox) records a `showcase_workspace` row with the run's replay config and soft references to what it created. A **named** keep run additionally carries the operator-supplied `workspace_name` label (non-unique). Kept rows back the panel's **Load** (restore config + artifact links, read-only), **Replay** (re-run verbatim), and **Delete** (remove the saved record) actions. 1. **Replay is verbatim — replaying a `reset=true` workspace WIPES the database.** Replay re-submits the recorded config exactly (`seed`/`scenario`/`reset`/`skip_seed`) with `preservation="keep"`. A workspace saved from a Reset-database run therefore wipes + reseeds on every Replay; the panel styles such rows with a `DESTRUCTIVE` marker. This is designed E4 semantics (#393), not a bug — there is deliberately no confirm dialog (consistency with the Reset checkbox's severity styling). 2. **Names are non-unique by design.** Every Replay creates a NEW `showcase_workspace` row; same-named rows accumulate (the replay regression test itself leaves two `replay-regression` rows). Disambiguate by `workspace_id` or `created_at` (panel lists newest first). -3. **Rows accumulate — there is no DELETE endpoint yet** (a future epic; deletion was out of #393's scope). Rows are harmless audit records. `created_objects` ids are SOFT references (deliberately no FKs): an operator-issued `DELETE /registry/runs/{id}` or scenario-plan delete leaves dangling deep links on a loaded workspace's artifact cards — expected; the workspace row records what WAS created, not what still exists. -4. **`holiday_rush` workspaces replay the pinned 2024 window.** The preset seeds a fixed Oct–Dec 2024 window (incident 28 above); a Replay with `reset=false` ADDS those rows to a today-anchored dataset, so `/seeder/status` reports the union range afterwards. For a clean pinned window, save the workspace from a run with **Reset database** ticked — its (destructive) Replay then reproduces the pinned window exactly. +3. **Rows accumulate unless deleted.** `DELETE /demo/workspaces/{workspace_id}` (and the panel's per-row **Delete** button, behind a confirmation dialog) removes a saved row; a missing id is an RFC 7807 404. Undeleted rows are harmless audit records. +4. **Deleting a workspace deletes METADATA ONLY.** The delete removes just the `showcase_workspace` row — the model runs, scenario plans, aliases, jobs, agent sessions, and on-disk artifacts the run created are NOT touched (and the seeded data is not reverted). `created_objects` ids are SOFT references (deliberately no FKs), so deletion in either direction never cascades: an operator-issued `DELETE /registry/runs/{id}` or scenario-plan delete leaves dangling deep links on a loaded workspace's artifact cards — expected; the workspace row records what WAS created, not what still exists. +5. **`holiday_rush` workspaces replay the pinned 2024 window.** The preset seeds a fixed Oct–Dec 2024 window (incident 28 above); a Replay with `reset=false` ADDS those rows to a today-anchored dataset, so `/seeder/status` reports the union range afterwards. For a clean pinned window, save the workspace from a run with **Reset database** ticked — its (destructive) Replay then reproduces the pinned window exactly. **Notes:** keep-runs are recorded by warn-and-continue hooks — a DB hiccup during `create_workspace` yields a green pipeline with `workspace_id: null` and no row (check uvicorn logs for `demo.workspace_create_failed`). Ephemeral runs write no workspace rows and stay in the localStorage Run-history strip; kept runs appear ONLY in the server-backed panel. On `showcase_rich` keep-runs, the planning-phase scenario plans carry the `workspace:` tag (E3 #392) — retrieve them via `GET /scenarios?tags=workspace: