feat: add a stop control to the Library Assistant (sc-42230)

[stevekaplan123]

What this does

Adds a stop button to the Library Assistant. Today, once you send a question there is no way to take it back — you wait for an answer you may not want, and we pay Anthropic for every token of it.

While the assistant is generating, the send arrow becomes a stop button. Clicking it abandons the turn, shuts down the agent, and puts your question back in the input box so you can edit it and resend.

How it works

The cancel flag lives in Postgres, not in memory. POST /api/v2/chat/cancel writes processing_state = "cancelled" onto the user's message row; the streaming pod polls that row once a second and relays it to the agent thread via a threading.Event.

Stopping the model costs money until the subprocess dies. sdk_runner.py checks the flag once per streamed message and raises TurnCancelled, which unwinds out of the SDK's async with block — that close is what terminates the agent subprocess and stops the billing. turn_orchestrator.py adds checkpoints between phases so a stop is honored even when the turn is between model calls.

Nothing is persisted for a cancelled turn. No assistant message is saved. The persistence step re-reads the database rather than trusting the in-memory result, because the agent can still return an answer after a cancel (an early return that skipped the checkpoints, or a turn that finished a moment before the click). On reload, the client rebuilds the stopped note from the processingState now returned with history.

Tests

~450 new lines covering the cancel endpoint (auth, 404, idempotency, completed turns left alone), the streaming endpoint's cancelled path, end-to-end propagation, an answer arriving after a cancel being discarded, and SDK-runner tests proving the client actually closes.

Note for reviewers

In models.py the processing_* fields moved a few lines up and the hand-written choices=[…] list was replaced with ProcessingState.choices. The diff makes it look like fields were deleted — they were only relocated. The one behavioral change is the added cancelled choice.

🤖 Generated with Claude Code

[gitvelocity-reviewer]

📊 Code Quality Score: 75/100

Base Score 75 × ESF 1.0 (962 effective lines, Extra Large tier; 15 files, Small file tier; gap is negative, no bump) = 75

Category	Score	Factors
🔭 Scope	17/20	15 files touched across frontend (LCChatbot.svelte, api.js), backend (views.py, models.py, serializers.py, urls.py), agent layer (contracts.py, claude_service.py, sdk_runner.py, turn_orchestrator.py, init.py), and a Django migration. Adds one new public endpoint (POST /api/v2/chat/cancel), one new SSE event type (cancelled), one new DB state (CANCELLED), and one new serializer field (processingState on HistoryMessageSerializer).
🏗️ Architecture	15/20	Introduces a cross-process cancel mechanism: chat_cancel_v2 writes to Postgres, the SSE loop in chat_stream_v2 polls _is_turn_cancelled and sets a threading.Event, and the async agent thread reads cancel_event.is_set via the CancelCheck protocol. TurnCancelled exception unwinds the claude_agent_sdk async-with block to terminate the subprocess. Adds raise_if_cancelled checkpoints at four distinct phases in turn_orchestrator.py. No new external service dependency; the DB is already present.
⚙️ Implementation	16/20	The SSE generator thread and the agent asyncio thread share state via a threading.Event; the SSE thread is the only writer (sync context, ORM legal) and the agent thread is the only reader (async context). _mark_turn_cancelled uses a conditional UPDATE filtered to STARTED/RUNNING states so a completed row is never overwritten. handleStop in LCChatbot.svelte sequences cancelStream before streamAbortController.abort() and guards against the answer landing during the cancel round trip. withStoppedNotes reconstructs stopped-turn display rows from the processingState field on history load. buildStoppedMessage carries the appetizer snapshot so content already rendered is preserved.
⚠️ Risk	12/20	The processing_state field definition block is removed from models.py in this diff without a visible destination; if it is not defined in a parent class the model is broken, though the AlterField migration implies the field exists. The SSE loop now queries Postgres every STREAM_CANCEL_POLL_SECONDS (1.0s) per active connection rather than every 60s, which increases DB query rate proportionally to concurrent users. The cancel endpoint has no rate limiting. No feature flag gates the new behaviour.
✅ Quality	13/15	test_sdk_runner_streaming.py adds three tests: normal completion with should_cancel=False, TurnCancelled raised after two messages with client.closed verified, and cancel before the first message. test_streaming_integration.py adds TestChatCancelV2 (7 cases: running, started, completed, double-cancel, 404, cross-user auth, missing fields), TestStreamingEndpointCancellation (3 cases: no assistant row persisted, user message flagged cancelled, should_cancel callable passed to agent), TestStreamingCancelPropagation (transaction=True end-to-end with real DB flag and slow_send_message coroutine), and TestStreamingCancelWinsOverLateResponse (regression for the guardrail race). test_serializers.py updates the field set assertion for processingState. No frontend tests accompany the Svelte changes.
🔒 Perf / Security	2/5	chat_cancel_v2 runs _authenticate_actor_or_response and filters ChatMessage by user_id__in=actor.user_id_candidates, matching the auth pattern of other endpoints. cancelStream uses a hand-rolled 5s timeout rather than AbortSignal.timeout() to preserve compatibility with pre-mid-2022 browsers. No rate limiting on the cancel endpoint and no benchmark accompanies the 1s DB poll change.

Was this score accurate? 👍 Yes · 👎 No

How this was scored →

_{Scored by GitVelocity · How are scores calculated?}

[Copilot]

🟡 Changes recommended

Several lifecycle races can lose acknowledged cancellations, persist completed answers, or leave the agent running and billing.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds a stop control that cancels in-flight Library Assistant turns across the Svelte UI, Django API, and Claude agent lifecycle.

Changes:

• Adds stop-state UI, prompt restoration, and cancellation-aware streaming.
• Persists cancellation state and propagates it to the agent subprocess.
• Adds backend cancellation and SDK lifecycle tests.

File summaries

File	Description
src/lib/api.js	Adds stream abort support and cancellation API client.
src/components/LCChatbot.svelte	Implements stop UI and stopped-message rendering.
server/chat/V2/views.py	Adds cancellation endpoint and stream propagation.
server/chat/V2/agent/turn_orchestrator.py	Adds cancellation checkpoints between phases.
server/chat/V2/agent/sdk_runner.py	Stops SDK processing when cancellation is observed.
server/chat/V2/agent/contracts.py	Defines cancellation contracts and exception.
server/chat/V2/agent/claude_service.py	Passes cancellation checks into orchestration.
server/chat/V2/agent/__init__.py	Exports the cancellation exception.
server/chat/urls.py	Registers the cancellation endpoint.
server/chat/tests/test_streaming_integration.py	Tests endpoint and streaming cancellation behavior.
server/chat/tests/test_serializers.py	Updates history serializer expectations.
server/chat/tests/test_sdk_runner_streaming.py	Tests SDK cancellation and client closure.
server/chat/serializers.py	Adds cancel requests and history processing state.
server/chat/models.py	Adds the cancelled processing state.
server/chat/migrations/0012_alter_chatmessage_processing_state.py	Updates processing-state choices.

Review details

• Files reviewed: 15/15 changed files
• Comments generated: 7
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

The allowed guardrail path can start a billable router request after cancellation has already been observed.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 15/15 changed files
• Comments generated: 1
• Review effort level: Balanced

[YishaiGlasner]

Nit: per PEP 8, exceptions get an Error suffix when they represent an error condition. Python's own cancellation exceptions (asyncio.CancelledError, concurrent.futures.CancelledError) keep that suffix even though a cancellation isn't a bug — worth following that precedent here for consistency: TurnCancelledError.

[YishaiGlasner]

Follow-up on the naming nit: since every call site does if should_cancel and should_cancel():, consider defaulting this to a no-op instead of None so the call sites can drop the should_cancel and guard and just do if should_cancel():. The default has to return False (never cancel) — a lambda: True default would cancel every turn on its first poll. Prefer a named function over an inline lambda per PEP 8 (E731):

def _never_cancel() -> bool:
    return False

then should_cancel: CancelCheck = _never_cancel.

This also lets the type annotation drop the | None — with a real default, should_cancel is never None, so it can just be should_cancel: CancelCheck = _never_cancel instead of CancelCheck | None = None.

The if should_cancel and should_cancel(): guard is already duplicated three times in the codebase as-is: twice inline in sdk_runner.py, and once wrapped in raise_if_cancelled() in turn_orchestrator.py. If this default isn't adopted, at least don't add further copies of the repeated guard — keep new checkpoints going through raise_if_cancelled() rather than re-inlining the should_cancel and should_cancel() pattern.

[YishaiGlasner]

This function adds the 5th copy of the serializer-validation block and the 4th copy of the actor-authentication block in this file (chat_stream_v2, chat_recover_v2, chat_client_event_v2, chat_feedback_v2 all repeat one or both verbatim). Extract a decorator that does both and short-circuits before the view body runs:

def validated_view(serializer_cls, *, authenticate=True):
    def decorator(view_func):
        @wraps(view_func)
        def wrapper(request):
            serializer = serializer_cls(data=request.data)
            if not serializer.is_valid():
                return Response(
                    {"error": "Invalid request", "details": serializer.errors},
                    status=status.HTTP_400_BAD_REQUEST,
                )
            data = serializer.validated_data
            if not authenticate:
                return view_func(request, data)
            actor = _authenticate_actor_or_response(request, data)
            if isinstance(actor, Response):
                return actor
            return view_func(request, data, actor)
        return wrapper
    return decorator

Apply it to chat_cancel_v2 here and to the other four views in this file. chat_cancel_v2 becomes:

@api_view(["POST"])
@validated_view(CancelRequestSerializer)
def chat_cancel_v2(request, data, actor):
    ...

Cut the ~30 lines of repeated boilerplate across the file now rather than adding a 5th and 6th copy of it later.

[YishaiGlasner]

.stop-btn re-declares most of .send-btn's layout instead of sharing it: display: flex; align-items: center; justify-content: center; width: 40px; height: 40px; border-radius: var(--lc-radius-sm); cursor: pointer; are identical between the two, and :active:not(:disabled) { transform: scale(0.95); } is byte-for-byte the same rule in both places. Only the fill (background/border/color) should actually differ between a filled and an outlined button.

Merge the shared shape/interaction rules into one selector:

.send-btn, .stop-btn {
  display: flex;
  align-items: center;
  justify-content: center;
  width: 40px;
  height: 40px;
  border-radius: var(--lc-radius-sm);
  cursor: pointer;
}

.send-btn:active:not(:disabled),
.stop-btn:active:not(:disabled) {
  transform: scale(0.95);
}

and leave each button's own rule with only what actually differs (fill vs. outline, its own :hover/:disabled colors).

Separately: .stop-btn has a :focus-visible outline and .send-btn doesn't, even though they occupy the same spot in the footer. Add the same :focus-visible rule to .send-btn while touching this block.

[YishaiGlasner]

Both exit paths of this loop — validation passing on the first try, and validation still failing after the repair attempt is exhausted (MAX_RESPONSE_LINK_REPAIR_ATTEMPTS = 1) — go straight from the last await validator.validate_response(...) to build_agent_response(...) with no raise_if_cancelled() in between. The loop's own comment above explains why that call is exactly the kind of gap this pattern is meant to close ("Link validation resolves every ref in the answer over the network... exactly when a waiting user gives up and hits stop"), but the checkpoint only guards the way into the repair branch, not the exit on either path.

This doesn't cause a persistence bug — views.py's independent DB-flag check before persisting (if result_holder["cancelled"] or _is_turn_cancelled(...)) catches it regardless of whether TurnCancelled was raised here, which is exactly what test_response_returned_after_cancel_is_discarded verifies. But it does mean a turn that's about to be discarded still runs trace_logger.log_success() and finishes computing metrics for an answer nobody will see, purely because this one checkpoint is missing. A raise_if_cancelled() right after the while loop, before the metrics/logging block, closes it.

Separately: this file (turn_orchestrator.py) has no dedicated test file at all — nothing in this PR or before it exercises run_turn's checkpoint placement directly. Every cancellation test in this PR goes one layer below (sdk_runner.py's per-message check) or one layer above (views.py, where agent.send_message is mocked with AsyncMock(side_effect=TurnCancelled()), bypassing this file's internals entirely). That's why this gap isn't caught by the suite — nothing here can catch it.

— Claude

[YishaiGlasner]

cancelStream()'s return value is discarded here, and isSending is the only guard left before abort() fires. cancelStream tells you exactly what happened server-side — 'cancelling' vs 'not_running' (the latter meaning _mark_turn_cancelled found the row already outside STARTED/RUNNING, i.e. the turn already completed). Throwing that away means the only thing standing between this abort() and a finished turn is isSending, which only flips false once the SSE event: message has been fully received and parsed client-side.

There's a real window between those two signals. In views.py, _mark_turn_completed() runs (line ~776) well before event: message is actually yielded (line ~867) — building the payload, stats, and trace logging happens in between, plus normal network transit on top. If stop is clicked inside that window: the server correctly reports not_running, but isSending is still true client-side, so this falls through to abort(). sendMessageStream's reader loop then re-throws the resulting AbortError immediately, without attempting recovery (recovery is deliberately skipped on abort — "a response nobody wants"). A fully generated, already-persisted answer gets discarded and replaced with the synthetic "Stopped generating..." note, even though chat_recover_v2 would return it as status: 'complete' if asked. It's self-correcting on a full reload (history sync reads processingState: 'completed', not 'cancelled'), but nothing re-triggers that mid-session.

Suggest checking cancelStream's result: if it comes back not_running, don't abort — clear isStopping and let the in-flight message land naturally.

Separately: no .test.js/.spec.js files are touched anywhere in this PR, even though it adds substantial client logic (handleStop, cancelStream, the abort/recovery interaction). This race — and the rest of the stop-button flow — has zero test coverage on the frontend.

— Claude