Skip to content

Record why the assistant failed on the message - #5070

Open
elias-ba wants to merge 1 commit into
timeout-oban-lifecyclefrom
timeout-failure-taxonomy
Open

Record why the assistant failed on the message#5070
elias-ba wants to merge 1 commit into
timeout-oban-lifecyclefrom
timeout-failure-taxonomy

Conversation

@elias-ba

@elias-ba elias-ba commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Description

Every AI chat failure looks the same, to the user and to us. A hung Apollo, a dropped connection and a deploy interrupting a run all end as one generic error, so people retry a hard-down service forever and support cannot tell from a screenshot which happened.

The reasons already exist as sentences meant for a person to read, and were logged and thrown away: the channel only ever sent a message id and the word error. They now live on the message, as a category to group by and a sentence to show.

Columns rather than a broadcast, because the failure that matters most is a deploy interrupting a run, and that is exactly when the browser reconnects to a different node with the broadcast long gone.

This is the plumbing, not the visible change. Nothing in the frontend reads either field yet. What it buys is that the reason is recorded, survives a reconnect, and reaches the client.

The sentence is user-facing prose only; raw error terms stay in the log, where they cannot leak an internal hostname or a stack trace into the chat panel. It is bounded, since it is written through change/2, which validates nothing, and re-sent on every channel join.

Validation steps

  1. Stop Apollo (or point APOLLO_ENDPOINT at a port nothing is listening on), then send a message from the assistant panel.
  2. The panel shows its usual error, unchanged. The new part is on the row, so read it in IEx:
    import Ecto.Query
    Lightning.Repo.one(
      from m in Lightning.AiAssistant.ChatMessage,
        order_by: [desc: m.inserted_at], limit: 1
    ) |> Map.take([:status, :failure_category, :failure_message])
    You should get a category and a sentence written for a person, not an error term.
  3. Make a service raise mid-answer and repeat. The exception text should be in the server log and not in failure_message.

Additional notes for the reviewer

Nothing in the frontend reads the two new columns yet, so no user sees anything different once this merges. The reason has to survive a reconnect before anything can show it.

AI Usage

Please disclose whether you've used AI anywhere in this PR (it's cool, we just want to know!):

  • I have used Claude Code
  • I have used another model
  • I have not used AI

You can read more details in our Responsible AI Policy

Pre-submission checklist

  • I have performed an AI review of my code (we recommend using /review with Claude Code)
  • I have implemented and tested all related authorization policies. (e.g., :owner, :admin, :editor, :viewer)
  • I have updated the changelog
  • I have ticked a box in "AI usage" in this PR

@github-project-automation github-project-automation Bot moved this to New Issues in Core Aug 15, 2026
@github-actions

Copy link
Copy Markdown

Based on my analysis, let me determine which checks apply and verify them.

Changed files:

  • chat_message.ex — schema fields
  • message_processor.ex — job/telemetry logic
  • application.ex — telemetry handler wiring
  • bootstrap.ex — Oban shutdown timing
  • ai_assistant_channel.ex — broadcasts new fields
  • migration — adds columns
  • test — telemetry emission test

Scoping analysis:

  • S0 (project scoping): No new queries or web-layer entrypoints. Repo.get/get! calls target ChatMessage by ID from Oban job.args (server-controlled, not user-submitted). The broadcast in AiAssistantChannel uses the already-authorized socket topic (ai_assistant:{type}:{session_id}), whose join is gated by authorize_session_access/2. Applicable but no new attack surface.
  • S1 (authorization): No new controller actions, handle_in events, or policy modules. The added put_failure helper in the channel piggybacks on an existing broadcast that was already authorized at join. N/A.
  • S2 (audit trail): ChatMessage is not a project/instance configuration resource — it's conversation data, not on the audit-tracked resource list (credentials, projects, workflows, webhook auth methods, oauth clients, etc.). N/A.

Other checks:
The failure_message field is broadcast to session subscribers. The {:upstream_error, error_message} path (message_processor.ex:217) passes strings from handle_error_response/2 (ai_assistant.ex:1260), which includes error_message_from_body(body) returning body["message"] from the Apollo response. The comment at chat_message.ex:117-118 asserts these are user-facing prose only — this relies on the Apollo contract; the hardcoded fallbacks are safe strings. Only users authorized on the session's channel receive it, so blast radius is bounded.

## Security Review ✅

- **S0 (project scoping):** No new queries or entrypoints; `Repo.get` calls key off server-controlled Oban `job.args["message_id"]`, and the channel broadcast rides an already-scoped topic authorized in `authorize_session_access/2`.
- **S1 (authorization):** N/A, no new `handle_in` events, controller actions, or policy branches — the added `put_failure` helper only augments an existing broadcast whose join is already gated.
- **S2 (audit trail):** N/A, `ChatMessage` is conversation data, not a configuration resource on the audited-domain list (credentials, projects, workflows, webhook auth methods, OAuth clients, version control).

@elias-ba
elias-ba force-pushed the timeout-oban-lifecycle branch from ed7e3fd to f742165 Compare August 15, 2026 22:50
@elias-ba
elias-ba force-pushed the timeout-failure-taxonomy branch from 3683332 to 5a28e60 Compare August 15, 2026 22:50
@elias-ba
elias-ba force-pushed the timeout-oban-lifecycle branch from f742165 to 4cff9e9 Compare August 15, 2026 23:39
@elias-ba
elias-ba force-pushed the timeout-failure-taxonomy branch 2 times, most recently from 1813938 to d16eb79 Compare August 17, 2026 02:53
@codecov

codecov Bot commented Aug 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.33333% with 3 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (timeout-oban-lifecycle@078cb15). Learn more about missing BASE report.

Files with missing lines Patch % Lines
lib/lightning_web/channels/ai_assistant_channel.ex 57.1% 3 Missing ⚠️
Additional details and impacted files
@@                   Coverage Diff                    @@
##             timeout-oban-lifecycle   #5070   +/-   ##
========================================================
  Coverage                          ?   90.5%           
========================================================
  Files                             ?     421           
  Lines                             ?   20024           
  Branches                          ?       0           
========================================================
  Hits                              ?   18131           
  Misses                            ?    1893           
  Partials                          ?       0           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@elias-ba
elias-ba force-pushed the timeout-oban-lifecycle branch from 4cff9e9 to fe286cd Compare August 17, 2026 04:10
@elias-ba
elias-ba force-pushed the timeout-failure-taxonomy branch from d16eb79 to 50ae521 Compare August 17, 2026 04:10
@elias-ba
elias-ba force-pushed the timeout-oban-lifecycle branch from fe286cd to 0797f22 Compare August 17, 2026 09:24
@elias-ba
elias-ba force-pushed the timeout-failure-taxonomy branch from 50ae521 to d340e01 Compare August 17, 2026 09:24
@elias-ba elias-ba changed the title Tell people why the assistant failed Record why the assistant failed on the message Aug 17, 2026
Every failure looks the same to a user: the message goes red and offers a
retry. Apollo being down, Apollo being slow, and a deploy interrupting the
run are indistinguishable, so people retry a hard-down service forever and
support cannot triage from a screenshot.

The reasons already exist. handle_error_response builds strings written
for a person to read, and they were logged and then dropped - the channel
only ever sent a message id and the word error.

They now live on the message as a category and a sentence. Columns
rather than a broadcast, because the failure that matters most is a
deploy interrupting a run, and that is exactly when the browser
reconnects to a different node with the broadcast long gone. It is also
frequently written by a different process than the one that failed.

Two columns rather than one blob: the category is what the frontend
switches on and what you group by when someone asks how many timeouts
there were last week.

The sentence is user-facing prose only. Raw error terms and upstream
response bodies stay in the log, where they cannot leak an internal
hostname or a stack trace to whoever is reading the chat panel. A raise
on our side is the sharpest case of that, since the exception text
carries module names and inspected payloads, and it now has a test that
fails if any of it reaches the row.

It is bounded, too. It is written through Ecto.Changeset.change/2, which
applies no validation, and read back and re-sent on every channel join,
so an unbounded string would be paid for on each one.

The internal-failure branch no longer logs a second time. Whoever raised
already logged the exception, and every extra Logger.error is another
Sentry event for one failure.

A returned reason is now a named type. The specs said String.t() |
Ecto.Changeset.t(), which this change makes untrue - the internal tuple
is neither - so they say stream_error() instead, and dialyzer can hold
the callers to it.
@elias-ba
elias-ba force-pushed the timeout-oban-lifecycle branch from 0797f22 to 078cb15 Compare August 18, 2026 07:29
@elias-ba
elias-ba force-pushed the timeout-failure-taxonomy branch from d340e01 to e815b6a Compare August 18, 2026 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: New Issues

Development

Successfully merging this pull request may close these issues.

1 participant