feat(pilot): PATCH /suggested-fixes/:id/outcome endpoint + tests
Records engineer-reported outcome (applied_success|applied_failed|
applied_partial|dismissed). Enforces transition rules (partial → success/
failed allowed; terminal outcomes return 409) and notes requirements
(applied_partial requires notes).
Sets verified_at on success/failure, stamps applied_at if not already
set (handles the case where the AI [FIX_OUTCOME] marker fires before
the engineer clicks Apply).
Also fixes pre-existing test-infrastructure bug: network_diagram.py used
bare string server_default="'[]'" for JSONB columns, which asyncpg
rejects during test schema creation. Changed to text("'[]'::jsonb") to
match the pattern used by script_template.py.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -30,6 +30,7 @@ from app.schemas.session_suggested_fix import (
|
|||||||
ResolutionPostResponse,
|
ResolutionPostResponse,
|
||||||
SessionSuggestedFixDecisionRequest,
|
SessionSuggestedFixDecisionRequest,
|
||||||
SessionSuggestedFixDecisionResponse,
|
SessionSuggestedFixDecisionResponse,
|
||||||
|
SessionSuggestedFixOutcomeRequest,
|
||||||
SessionSuggestedFixResponse,
|
SessionSuggestedFixResponse,
|
||||||
)
|
)
|
||||||
from app.models.draft_template import DraftTemplate
|
from app.models.draft_template import DraftTemplate
|
||||||
@@ -216,6 +217,70 @@ async def record_decision(
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
# ── Suggested fix: outcome ────────────────────────────────────────────────
|
||||||
|
|
||||||
|
@router.patch(
|
||||||
|
"/suggested-fixes/{fix_id}/outcome",
|
||||||
|
response_model=SessionSuggestedFixResponse,
|
||||||
|
)
|
||||||
|
async def patch_suggested_fix_outcome(
|
||||||
|
session_id: UUID,
|
||||||
|
fix_id: UUID,
|
||||||
|
body: SessionSuggestedFixOutcomeRequest,
|
||||||
|
current_user: Annotated[User, Depends(get_current_active_user)],
|
||||||
|
db: Annotated[AsyncSession, Depends(get_db)],
|
||||||
|
_: None = Depends(require_engineer_or_admin),
|
||||||
|
) -> SessionSuggestedFixResponse:
|
||||||
|
"""Record the engineer's outcome for an applied fix.
|
||||||
|
|
||||||
|
See `SessionSuggestedFixOutcomeRequest` for transition rules.
|
||||||
|
"""
|
||||||
|
await _load_session_or_404(db, session_id)
|
||||||
|
now = datetime.now(timezone.utc)
|
||||||
|
|
||||||
|
result = await db.execute(
|
||||||
|
select(SessionSuggestedFix).where(
|
||||||
|
SessionSuggestedFix.id == fix_id,
|
||||||
|
SessionSuggestedFix.session_id == session_id,
|
||||||
|
)
|
||||||
|
)
|
||||||
|
fix = result.scalar_one_or_none()
|
||||||
|
if fix is None:
|
||||||
|
raise HTTPException(
|
||||||
|
status_code=status.HTTP_404_NOT_FOUND, detail="Suggested fix not found"
|
||||||
|
)
|
||||||
|
|
||||||
|
if body.outcome == "applied_partial" and not (body.notes and body.notes.strip()):
|
||||||
|
raise HTTPException(
|
||||||
|
status_code=status.HTTP_400_BAD_REQUEST,
|
||||||
|
detail="notes are required when outcome is applied_partial",
|
||||||
|
)
|
||||||
|
|
||||||
|
TERMINAL = {"applied_success", "applied_failed", "dismissed"}
|
||||||
|
if fix.status in TERMINAL:
|
||||||
|
raise HTTPException(
|
||||||
|
status_code=status.HTTP_409_CONFLICT,
|
||||||
|
detail=f"Fix is already in terminal status {fix.status!r}",
|
||||||
|
)
|
||||||
|
|
||||||
|
fix.status = body.outcome
|
||||||
|
if body.outcome == "applied_partial":
|
||||||
|
fix.partial_notes = (body.notes or "").strip() or None
|
||||||
|
elif body.outcome == "applied_failed":
|
||||||
|
fix.failure_reason = (body.notes or "").strip() or None
|
||||||
|
fix.verified_at = now
|
||||||
|
elif body.outcome == "applied_success":
|
||||||
|
fix.verified_at = now
|
||||||
|
# dismissed: no timestamp/notes stamping
|
||||||
|
|
||||||
|
if fix.applied_at is None and body.outcome != "dismissed":
|
||||||
|
fix.applied_at = now
|
||||||
|
|
||||||
|
await db.commit()
|
||||||
|
await db.refresh(fix)
|
||||||
|
return SessionSuggestedFixResponse.model_validate(fix)
|
||||||
|
|
||||||
|
|
||||||
async def _summarize_session_for_extraction(
|
async def _summarize_session_for_extraction(
|
||||||
db: AsyncSession, session_id: UUID,
|
db: AsyncSession, session_id: UUID,
|
||||||
) -> str:
|
) -> str:
|
||||||
|
|||||||
@@ -3,7 +3,7 @@ import uuid
|
|||||||
from datetime import datetime, timezone
|
from datetime import datetime, timezone
|
||||||
from typing import Any, TYPE_CHECKING
|
from typing import Any, TYPE_CHECKING
|
||||||
|
|
||||||
from sqlalchemy import String, Text, Boolean, DateTime, ForeignKey
|
from sqlalchemy import String, Text, Boolean, DateTime, ForeignKey, text
|
||||||
from sqlalchemy.orm import Mapped, mapped_column, relationship
|
from sqlalchemy.orm import Mapped, mapped_column, relationship
|
||||||
from sqlalchemy.dialects.postgresql import UUID, JSONB
|
from sqlalchemy.dialects.postgresql import UUID, JSONB
|
||||||
|
|
||||||
@@ -30,8 +30,8 @@ class NetworkDiagram(Base):
|
|||||||
client_name: Mapped[str | None] = mapped_column(String(255), nullable=True)
|
client_name: Mapped[str | None] = mapped_column(String(255), nullable=True)
|
||||||
asset_name: Mapped[str | None] = mapped_column(String(255), nullable=True)
|
asset_name: Mapped[str | None] = mapped_column(String(255), nullable=True)
|
||||||
description: Mapped[str | None] = mapped_column(Text, nullable=True)
|
description: Mapped[str | None] = mapped_column(Text, nullable=True)
|
||||||
nodes: Mapped[list[dict[str, Any]]] = mapped_column(JSONB, nullable=False, server_default="'[]'")
|
nodes: Mapped[list[dict[str, Any]]] = mapped_column(JSONB, nullable=False, server_default=text("'[]'::jsonb"))
|
||||||
edges: Mapped[list[dict[str, Any]]] = mapped_column(JSONB, nullable=False, server_default="'[]'")
|
edges: Mapped[list[dict[str, Any]]] = mapped_column(JSONB, nullable=False, server_default=text("'[]'::jsonb"))
|
||||||
thumbnail_url: Mapped[str | None] = mapped_column(Text, nullable=True)
|
thumbnail_url: Mapped[str | None] = mapped_column(Text, nullable=True)
|
||||||
is_archived: Mapped[bool] = mapped_column(
|
is_archived: Mapped[bool] = mapped_column(
|
||||||
Boolean, nullable=False, default=False,
|
Boolean, nullable=False, default=False,
|
||||||
|
|||||||
@@ -103,9 +103,11 @@ class SessionSuggestedFixOutcomeRequest(BaseModel):
|
|||||||
engineer took); outcome captures whether the fix actually worked.
|
engineer took); outcome captures whether the fix actually worked.
|
||||||
|
|
||||||
Allowed transitions:
|
Allowed transitions:
|
||||||
- from `proposed`: any of applied_success | applied_failed | applied_partial | dismissed
|
- from `proposed` or `applied_partial`: any outcome is valid
|
||||||
- from `applied_partial`: applied_success | applied_failed (partial is not terminal)
|
(partial is parked, not terminal — the engineer may update notes,
|
||||||
- from any terminal outcome: no change (server returns 409)
|
abandon via dismiss, or advance to success/failed)
|
||||||
|
- from any terminal outcome (`applied_success`, `applied_failed`,
|
||||||
|
`dismissed`): server returns 409
|
||||||
"""
|
"""
|
||||||
outcome: FixOutcome
|
outcome: FixOutcome
|
||||||
# Required for applied_partial, optional for applied_failed, ignored otherwise.
|
# Required for applied_partial, optional for applied_failed, ignored otherwise.
|
||||||
|
|||||||
199
backend/tests/test_fix_outcome_endpoint.py
Normal file
199
backend/tests/test_fix_outcome_endpoint.py
Normal file
@@ -0,0 +1,199 @@
|
|||||||
|
"""Integration tests for PATCH /ai-sessions/{sid}/suggested-fixes/{fid}/outcome.
|
||||||
|
|
||||||
|
Fixture style follows test_session_suggested_fixes_api.py:
|
||||||
|
client, test_user, auth_headers, test_db
|
||||||
|
"""
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
from httpx import AsyncClient
|
||||||
|
|
||||||
|
from app.models.ai_session import AISession
|
||||||
|
from app.models.session_suggested_fix import SessionSuggestedFix
|
||||||
|
|
||||||
|
|
||||||
|
# ── shared helper ────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
async def _make_session_with_fix(test_db, user) -> tuple[str, str]:
|
||||||
|
"""Create an AISession + active proposed SessionSuggestedFix.
|
||||||
|
|
||||||
|
Returns (session_id_str, fix_id_str).
|
||||||
|
"""
|
||||||
|
session = AISession(
|
||||||
|
user_id=user["user_data"]["id"],
|
||||||
|
account_id=user["user_data"]["account_id"],
|
||||||
|
session_type="chat",
|
||||||
|
intake_type="free_text",
|
||||||
|
intake_content={"text": "outcome test"},
|
||||||
|
status="active",
|
||||||
|
confidence_tier="discovery",
|
||||||
|
conversation_messages=[],
|
||||||
|
)
|
||||||
|
test_db.add(session)
|
||||||
|
await test_db.flush()
|
||||||
|
|
||||||
|
fix = SessionSuggestedFix(
|
||||||
|
session_id=session.id,
|
||||||
|
account_id=session.account_id,
|
||||||
|
title="Reset credential cache",
|
||||||
|
description="Clear stale credentials from the domain cache.",
|
||||||
|
confidence_pct=82,
|
||||||
|
)
|
||||||
|
test_db.add(fix)
|
||||||
|
await test_db.commit()
|
||||||
|
await test_db.refresh(fix)
|
||||||
|
|
||||||
|
return str(session.id), str(fix.id)
|
||||||
|
|
||||||
|
|
||||||
|
# ── tests ────────────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_patch_outcome_marks_success(
|
||||||
|
client: AsyncClient, test_user, auth_headers, test_db
|
||||||
|
):
|
||||||
|
session_id, fix_id = await _make_session_with_fix(test_db, test_user)
|
||||||
|
|
||||||
|
r = await client.patch(
|
||||||
|
f"/api/v1/ai-sessions/{session_id}/suggested-fixes/{fix_id}/outcome",
|
||||||
|
headers=auth_headers,
|
||||||
|
json={"outcome": "applied_success"},
|
||||||
|
)
|
||||||
|
assert r.status_code == 200, r.text
|
||||||
|
body = r.json()
|
||||||
|
assert body["status"] == "applied_success"
|
||||||
|
assert body["verified_at"] is not None
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_patch_outcome_partial_requires_notes(
|
||||||
|
client: AsyncClient, test_user, auth_headers, test_db
|
||||||
|
):
|
||||||
|
session_id, fix_id = await _make_session_with_fix(test_db, test_user)
|
||||||
|
|
||||||
|
r = await client.patch(
|
||||||
|
f"/api/v1/ai-sessions/{session_id}/suggested-fixes/{fix_id}/outcome",
|
||||||
|
headers=auth_headers,
|
||||||
|
json={"outcome": "applied_partial"},
|
||||||
|
)
|
||||||
|
assert r.status_code == 400
|
||||||
|
assert "notes" in r.text.lower()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_partial_to_success_allowed(
|
||||||
|
client: AsyncClient, test_user, auth_headers, test_db
|
||||||
|
):
|
||||||
|
session_id, fix_id = await _make_session_with_fix(test_db, test_user)
|
||||||
|
|
||||||
|
r1 = await client.patch(
|
||||||
|
f"/api/v1/ai-sessions/{session_id}/suggested-fixes/{fix_id}/outcome",
|
||||||
|
headers=auth_headers,
|
||||||
|
json={"outcome": "applied_partial", "notes": "ran cred clear only"},
|
||||||
|
)
|
||||||
|
assert r1.status_code == 200, r1.text
|
||||||
|
|
||||||
|
r2 = await client.patch(
|
||||||
|
f"/api/v1/ai-sessions/{session_id}/suggested-fixes/{fix_id}/outcome",
|
||||||
|
headers=auth_headers,
|
||||||
|
json={"outcome": "applied_success"},
|
||||||
|
)
|
||||||
|
assert r2.status_code == 200
|
||||||
|
assert r2.json()["status"] == "applied_success"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_terminal_outcome_is_locked(
|
||||||
|
client: AsyncClient, test_user, auth_headers, test_db
|
||||||
|
):
|
||||||
|
session_id, fix_id = await _make_session_with_fix(test_db, test_user)
|
||||||
|
|
||||||
|
r1 = await client.patch(
|
||||||
|
f"/api/v1/ai-sessions/{session_id}/suggested-fixes/{fix_id}/outcome",
|
||||||
|
headers=auth_headers,
|
||||||
|
json={"outcome": "applied_failed", "notes": "no change"},
|
||||||
|
)
|
||||||
|
assert r1.status_code == 200
|
||||||
|
|
||||||
|
r2 = await client.patch(
|
||||||
|
f"/api/v1/ai-sessions/{session_id}/suggested-fixes/{fix_id}/outcome",
|
||||||
|
headers=auth_headers,
|
||||||
|
json={"outcome": "applied_success"},
|
||||||
|
)
|
||||||
|
assert r2.status_code == 409
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_partial_notes_can_be_updated(
|
||||||
|
client: AsyncClient, test_user, auth_headers, test_db
|
||||||
|
):
|
||||||
|
"""partial→partial with new notes updates the stored notes."""
|
||||||
|
session_id, fix_id = await _make_session_with_fix(test_db, test_user)
|
||||||
|
|
||||||
|
r1 = await client.patch(
|
||||||
|
f"/api/v1/ai-sessions/{session_id}/suggested-fixes/{fix_id}/outcome",
|
||||||
|
json={"outcome": "applied_partial", "notes": "ran cred clear only"},
|
||||||
|
headers=auth_headers,
|
||||||
|
)
|
||||||
|
assert r1.status_code == 200
|
||||||
|
assert r1.json()["partial_notes"] == "ran cred clear only"
|
||||||
|
|
||||||
|
r2 = await client.patch(
|
||||||
|
f"/api/v1/ai-sessions/{session_id}/suggested-fixes/{fix_id}/outcome",
|
||||||
|
json={"outcome": "applied_partial", "notes": "also finished the rebuild; not verified yet"},
|
||||||
|
headers=auth_headers,
|
||||||
|
)
|
||||||
|
assert r2.status_code == 200
|
||||||
|
assert r2.json()["partial_notes"] == "also finished the rebuild; not verified yet"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_dismissed_sets_no_timestamps(
|
||||||
|
client: AsyncClient, test_user, auth_headers, test_db
|
||||||
|
):
|
||||||
|
"""dismissed outcome does not stamp applied_at or verified_at."""
|
||||||
|
session_id, fix_id = await _make_session_with_fix(test_db, test_user)
|
||||||
|
r = await client.patch(
|
||||||
|
f"/api/v1/ai-sessions/{session_id}/suggested-fixes/{fix_id}/outcome",
|
||||||
|
json={"outcome": "dismissed"},
|
||||||
|
headers=auth_headers,
|
||||||
|
)
|
||||||
|
assert r.status_code == 200
|
||||||
|
body = r.json()
|
||||||
|
assert body["status"] == "dismissed"
|
||||||
|
assert body["applied_at"] is None
|
||||||
|
assert body["verified_at"] is None
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_applied_at_auto_stamped_on_first_outcome(
|
||||||
|
client: AsyncClient, test_user, auth_headers, test_db
|
||||||
|
):
|
||||||
|
"""If applied_at is null when the engineer sets outcome, server stamps it."""
|
||||||
|
session_id, fix_id = await _make_session_with_fix(test_db, test_user)
|
||||||
|
r = await client.patch(
|
||||||
|
f"/api/v1/ai-sessions/{session_id}/suggested-fixes/{fix_id}/outcome",
|
||||||
|
json={"outcome": "applied_success"},
|
||||||
|
headers=auth_headers,
|
||||||
|
)
|
||||||
|
assert r.status_code == 200
|
||||||
|
body = r.json()
|
||||||
|
assert body["applied_at"] is not None
|
||||||
|
assert body["verified_at"] is not None
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_failed_outcome_stores_notes_as_failure_reason(
|
||||||
|
client: AsyncClient, test_user, auth_headers, test_db
|
||||||
|
):
|
||||||
|
"""applied_failed stores notes under failure_reason (not partial_notes)."""
|
||||||
|
session_id, fix_id = await _make_session_with_fix(test_db, test_user)
|
||||||
|
r = await client.patch(
|
||||||
|
f"/api/v1/ai-sessions/{session_id}/suggested-fixes/{fix_id}/outcome",
|
||||||
|
json={"outcome": "applied_failed", "notes": "user reports no change"},
|
||||||
|
headers=auth_headers,
|
||||||
|
)
|
||||||
|
assert r.status_code == 200
|
||||||
|
body = r.json()
|
||||||
|
assert body["failure_reason"] == "user reports no change"
|
||||||
|
assert body["partial_notes"] is None
|
||||||
@@ -238,9 +238,11 @@ class SessionSuggestedFixOutcomeRequest(BaseModel):
|
|||||||
engineer took); outcome captures whether the fix actually worked.
|
engineer took); outcome captures whether the fix actually worked.
|
||||||
|
|
||||||
Allowed transitions:
|
Allowed transitions:
|
||||||
- from `proposed`: any of applied_success | applied_failed | applied_partial | dismissed
|
- from `proposed` or `applied_partial`: any outcome is valid
|
||||||
- from `applied_partial`: applied_success | applied_failed (partial is not terminal)
|
(partial is parked, not terminal — the engineer may update notes,
|
||||||
- from any terminal outcome: no change (server returns 409)
|
abandon via dismiss, or advance to success/failed)
|
||||||
|
- from any terminal outcome (`applied_success`, `applied_failed`,
|
||||||
|
`dismissed`): server returns 409
|
||||||
"""
|
"""
|
||||||
outcome: Literal[
|
outcome: Literal[
|
||||||
"applied_success", "applied_failed", "applied_partial", "dismissed"
|
"applied_success", "applied_failed", "applied_partial", "dismissed"
|
||||||
@@ -769,9 +771,9 @@ Add a new method on `sessionSuggestedFixesApi` (after `recordDecision`):
|
|||||||
```ts
|
```ts
|
||||||
/**
|
/**
|
||||||
* Record the outcome of applying a suggested fix. Transitions:
|
* Record the outcome of applying a suggested fix. Transitions:
|
||||||
* - from 'proposed': any non-proposed outcome
|
* - from 'proposed' or 'applied_partial': any outcome is valid
|
||||||
* - from 'applied_partial': applied_success | applied_failed
|
* (partial→partial updates notes, partial→dismissed abandons)
|
||||||
* - terminal statuses are locked (server returns 409)
|
* - terminal statuses (applied_success, applied_failed, dismissed) are locked (server returns 409)
|
||||||
*/
|
*/
|
||||||
async patchOutcome(
|
async patchOutcome(
|
||||||
sessionId: string,
|
sessionId: string,
|
||||||
|
|||||||
Reference in New Issue
Block a user