From a693bed0890d3312d499e2b01e7e0cd81ea70b39 Mon Sep 17 00:00:00 2001 From: bordumb Date: Mon, 28 Sep 2026 18:53:19 +0100 Subject: [PATCH 01/18] docs(core): every investigation lives in an issue (spec 0001 M5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Revise spec 0001 with the owner's decisions from 2026-09-28: - D12: every run belongs to an issue; API, SDK, webhook and rule runs open or reuse one - D13: Investigate… opens the brief editor where you are and lands on the new issue's thread; /investigations/new goes - D14: /investigations/:id becomes the run's details page - D15: LLM failures fail the run with the reason; the API checks the key and models at startup and every page shows a banner Adds §7.11 (one starter, open_issue(), every start path), §7.12 (LLM error table, failing the run, key check), §8.1-8.4 (the mockup as the acceptance reference) and milestone M5, filed as fn-70.18-25. fn-70.17 is closed: PR #211 shipped it. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Claude --- .flow/tasks/fn-70.17.json | 20 ++- .flow/tasks/fn-70.17.md | 9 +- .flow/tasks/fn-70.18.json | 14 +++ .flow/tasks/fn-70.18.md | 17 +++ .flow/tasks/fn-70.19.json | 16 +++ .flow/tasks/fn-70.19.md | 21 ++++ .flow/tasks/fn-70.20.json | 16 +++ .flow/tasks/fn-70.20.md | 19 +++ .flow/tasks/fn-70.21.json | 16 +++ .flow/tasks/fn-70.21.md | 23 ++++ .flow/tasks/fn-70.22.json | 16 +++ .flow/tasks/fn-70.22.md | 19 +++ .flow/tasks/fn-70.23.json | 16 +++ .flow/tasks/fn-70.23.md | 20 +++ .flow/tasks/fn-70.24.json | 17 +++ .flow/tasks/fn-70.24.md | 18 +++ .flow/tasks/fn-70.25.json | 21 ++++ .flow/tasks/fn-70.25.md | 18 +++ docs/specs/0001_issue_chat.md | 229 +++++++++++++++++++++++++++++++++- 19 files changed, 534 insertions(+), 11 deletions(-) create mode 100644 .flow/tasks/fn-70.18.json create mode 100644 .flow/tasks/fn-70.18.md create mode 100644 .flow/tasks/fn-70.19.json create mode 100644 .flow/tasks/fn-70.19.md create mode 100644 .flow/tasks/fn-70.20.json create mode 100644 .flow/tasks/fn-70.20.md create mode 100644 .flow/tasks/fn-70.21.json create mode 100644 .flow/tasks/fn-70.21.md create mode 100644 .flow/tasks/fn-70.22.json create mode 100644 .flow/tasks/fn-70.22.md create mode 100644 .flow/tasks/fn-70.23.json create mode 100644 .flow/tasks/fn-70.23.md create mode 100644 .flow/tasks/fn-70.24.json create mode 100644 .flow/tasks/fn-70.24.md create mode 100644 .flow/tasks/fn-70.25.json create mode 100644 .flow/tasks/fn-70.25.md diff --git a/.flow/tasks/fn-70.17.json b/.flow/tasks/fn-70.17.json index 7d82faa9..940ae770 100644 --- a/.flow/tasks/fn-70.17.json +++ b/.flow/tasks/fn-70.17.json @@ -1,14 +1,26 @@ { - "assignee": null, + "assignee": "bordumbb@gmail.com", "claim_note": "", - "claimed_at": null, + "claimed_at": "2026-09-28T17:52:29.670737Z", "created_at": "2026-09-28T15:16:44.759472Z", "depends_on": [], "epic": "fn-70", + "evidence": { + "commits": [ + "1a31082c" + ], + "prs": [ + "https://github.com/bordumb/dataing/pull/211" + ], + "tests": [ + "tests/unit/test_config.py::test_chat_agent_defaults_to_claude_opus_5_5", + "tests/unit/agents/test_chat_agent.py::TestBriefDrafting::test_draft_is_parsed_from_a_json_reply_without_forcing_a_tool" + ] + }, "id": "fn-70.17", "priority": null, "spec_path": ".flow/tasks/fn-70.17.md", - "status": "todo", + "status": "done", "title": "Run the issue chat agent on Claude Opus 5.5", - "updated_at": "2026-09-28T15:16:44.762271Z" + "updated_at": "2026-09-28T17:52:29.952602Z" } diff --git a/.flow/tasks/fn-70.17.md b/.flow/tasks/fn-70.17.md index f03a5a15..b4a120ec 100644 --- a/.flow/tasks/fn-70.17.md +++ b/.flow/tasks/fn-70.17.md @@ -11,9 +11,8 @@ TBD ## Done summary -TBD - +CHAT_AGENT_MODEL defaults to claude-opus-5-5; brief drafting returns JSON text (PromptedOutput) instead of a forced output tool, which Opus 5.5 rejects. Merged as PR #211 (1a31082c). ## Evidence -- Commits: -- Tests: -- PRs: +- Commits: 1a31082c +- Tests: tests/unit/test_config.py::test_chat_agent_defaults_to_claude_opus_5_5, tests/unit/agents/test_chat_agent.py::TestBriefDrafting::test_draft_is_parsed_from_a_json_reply_without_forcing_a_tool +- PRs: https://github.com/bordumb/dataing/pull/211 diff --git a/.flow/tasks/fn-70.18.json b/.flow/tasks/fn-70.18.json new file mode 100644 index 00000000..1a7aa54a --- /dev/null +++ b/.flow/tasks/fn-70.18.json @@ -0,0 +1,14 @@ +{ + "assignee": null, + "claim_note": "", + "claimed_at": null, + "created_at": "2026-09-28T17:51:58.959592Z", + "depends_on": [], + "epic": "fn-70", + "id": "fn-70.18", + "priority": null, + "spec_path": ".flow/tasks/fn-70.18.md", + "status": "todo", + "title": "M5: spec revision: every run in the hub, one-step start, details page, LLM failures", + "updated_at": "2026-09-28T17:51:58.959798Z" +} diff --git a/.flow/tasks/fn-70.18.md b/.flow/tasks/fn-70.18.md new file mode 100644 index 00000000..9350cc0a --- /dev/null +++ b/.flow/tasks/fn-70.18.md @@ -0,0 +1,17 @@ +# fn-70.18 M5: spec revision: every run in the hub, one-step start, details page, LLM failures + +## Description +TBD + +## Acceptance +- docs/specs/0001_issue_chat.md records D12–D15, the §2 "found after M1–M4" table, §7.11 (one starter, open_issue, every path), §7.12 (LLM error table, failing the run, key check), §8.1–8.4 (mockup fidelity, Investigate…, details page, banner), M5 in §10 and its tests in §11 +- Owner decisions (2026-09-28): one-click Investigate…, details page off the card, always open an issue, fail the run + key check + + +## Done summary +TBD + +## Evidence +- Commits: +- Tests: +- PRs: diff --git a/.flow/tasks/fn-70.19.json b/.flow/tasks/fn-70.19.json new file mode 100644 index 00000000..6729e5d1 --- /dev/null +++ b/.flow/tasks/fn-70.19.json @@ -0,0 +1,16 @@ +{ + "assignee": null, + "claim_note": "", + "claimed_at": null, + "created_at": "2026-09-28T17:51:59.221401Z", + "depends_on": [ + "fn-70.18" + ], + "epic": "fn-70", + "id": "fn-70.19", + "priority": null, + "spec_path": ".flow/tasks/fn-70.19.md", + "status": "todo", + "title": "M5: LLM failures fail the run (classify, activities raise, workflow fails)", + "updated_at": "2026-09-28T17:51:59.221589Z" +} diff --git a/.flow/tasks/fn-70.19.md b/.flow/tasks/fn-70.19.md new file mode 100644 index 00000000..3eaca1df --- /dev/null +++ b/.flow/tasks/fn-70.19.md @@ -0,0 +1,21 @@ +# fn-70.19 M5: LLM failures fail the run (classify, activities raise, workflow fails) + +## Description +TBD + +## Acceptance +- `agents/errors.py` `classify_llm_error(exc)` maps the real exception chain (LLMError → ModelHTTPError/ModelAPIError → anthropic errors, UserError for a missing key) to the §7.12 codes, retryability and messages; unit-tested per row +- generate_hypotheses, generate_query, interpret_evidence, synthesize and counter_analyze raise ApplicationError `LLMRejected` (non-retryable) or `LLMUnavailable` with `{code, message}` details; AgentClient.interpret_evidence no longer swallows errors +- Every LLM activity call has an explicit RetryPolicy (4 attempts, 5 s, ×2, max 60 s, LLMRejected non-retryable) +- Behind `workflow.patched("llm-failures-v1")`, the workflow fails the run on: generation failure or no hypotheses, any subagent LLM error (others cancelled), all hypotheses untested from errors, synthesis failure. Counter-analysis failure keeps the synthesis and records counter_analysis.error +- A failed run publishes `{"status":"failed","error":{code,message,step}}` via publish_investigation_outcome (outcome, run row completed_at, thread card), then raises a non-retryable ApplicationError +- Workflow tests with fake activities cover each case; Replayer tests pass on the recorded histories + + +## Done summary +TBD + +## Evidence +- Commits: +- Tests: +- PRs: diff --git a/.flow/tasks/fn-70.20.json b/.flow/tasks/fn-70.20.json new file mode 100644 index 00000000..897d72d3 --- /dev/null +++ b/.flow/tasks/fn-70.20.json @@ -0,0 +1,16 @@ +{ + "assignee": null, + "claim_note": "", + "claimed_at": null, + "created_at": "2026-09-28T17:51:59.475477Z", + "depends_on": [ + "fn-70.19" + ], + "epic": "fn-70", + "id": "fn-70.20", + "priority": null, + "spec_path": ".flow/tasks/fn-70.20.md", + "status": "todo", + "title": "M5: LLM key check at startup, GET /system/llm, readable chat errors", + "updated_at": "2026-09-28T17:51:59.475730Z" +} diff --git a/.flow/tasks/fn-70.20.md b/.flow/tasks/fn-70.20.md new file mode 100644 index 00000000..919947d6 --- /dev/null +++ b/.flow/tasks/fn-70.20.md @@ -0,0 +1,19 @@ +# fn-70.20 M5: LLM key check at startup, GET /system/llm, readable chat errors + +## Description +TBD + +## Acceptance +- API startup checks `GET /v1/models/{id}` for LLM_MODEL and CHAT_AGENT_MODEL in the background (10 s timeout, no SDK retries); an empty key is reported without a request +- `GET /api/v1/system/llm` (ANY_USER, POLICY entry) returns {state, message, models, checked_at}; stale `unreachable` re-checks on read +- run_agent_turn classifies model errors: the reply/brief shows the §7.12 message, not the raw ModelHTTPError; non-retryable errors aren't retried +- Tests fake the Anthropic client for missing, rejected (401), unknown model (404) and ok + + +## Done summary +TBD + +## Evidence +- Commits: +- Tests: +- PRs: diff --git a/.flow/tasks/fn-70.21.json b/.flow/tasks/fn-70.21.json new file mode 100644 index 00000000..498b0411 --- /dev/null +++ b/.flow/tasks/fn-70.21.json @@ -0,0 +1,16 @@ +{ + "assignee": null, + "claim_note": "", + "claimed_at": null, + "created_at": "2026-09-28T17:51:59.729817Z", + "depends_on": [ + "fn-70.18" + ], + "epic": "fn-70", + "id": "fn-70.21", + "priority": null, + "spec_path": ".flow/tasks/fn-70.21.md", + "status": "todo", + "title": "M5: one starter and open_issue() for every start path", + "updated_at": "2026-09-28T17:51:59.730075Z" +} diff --git a/.flow/tasks/fn-70.21.md b/.flow/tasks/fn-70.21.md new file mode 100644 index 00000000..5c8b468b --- /dev/null +++ b/.flow/tasks/fn-70.21.md @@ -0,0 +1,23 @@ +# fn-70.21 M5: one starter and open_issue() for every start path + +## Description +TBD + +## Acceptance +- `adapters/db/issues.py` `open_issue()` is the only issue insert (POST /issues, CE + EE webhooks, the starter) and posts the thread's first entry via the `created` event +- `InvestigationStarterService.start()` resolves or opens the issue, builds the missing brief/alert, writes the investigation row, run row (trigger_type human/api/webhook/rule) and start card, starts the workflow with alert.issue_id, and records a failed outcome if the start fails +- POST /investigations takes exactly one of brief/alert (+ datasource_id, execution_profile, issue_id); bad alert → 422; response adds run_id, issue_id, issue_number +- POST /issues/{id}/investigation-runs, CE webhook-generic AUTO, EE provider webhook AUTO and the EE rule action all use the starter +- InvestigationRunResponse gains number, status, error; InvestigationStateResponse gains issue_id, issue_number, issue_title, run_number, brief, execution_profile, error +- ToolCallRecord stores duration_ms and row_count for run_query +- SDK Investigation gains issue_id/issue_number; `dataing run start` prints the issue URL +- Integration tests on migrated_db for each path in the §7.11 table + + +## Done summary +TBD + +## Evidence +- Commits: +- Tests: +- PRs: diff --git a/.flow/tasks/fn-70.22.json b/.flow/tasks/fn-70.22.json new file mode 100644 index 00000000..f15d4220 --- /dev/null +++ b/.flow/tasks/fn-70.22.json @@ -0,0 +1,16 @@ +{ + "assignee": null, + "claim_note": "", + "claimed_at": null, + "created_at": "2026-09-28T17:51:59.979116Z", + "depends_on": [ + "fn-70.21" + ], + "epic": "fn-70", + "id": "fn-70.22", + "priority": null, + "spec_path": ".flow/tasks/fn-70.22.md", + "status": "todo", + "title": "M5: frontend: Investigate\u2026 everywhere, remove /investigations/new", + "updated_at": "2026-09-28T17:51:59.979327Z" +} diff --git a/.flow/tasks/fn-70.22.md b/.flow/tasks/fn-70.22.md new file mode 100644 index 00000000..f439db96 --- /dev/null +++ b/.flow/tasks/fn-70.22.md @@ -0,0 +1,19 @@ +# fn-70.22 M5: frontend: Investigate… everywhere, remove /investigations/new + +## Description +TBD + +## Acceptance +- /investigations/new route, NewInvestigation.tsx and the dead components/Layout.tsx are gone; no link points at /investigations/new +- Investigate… (sidebar quick action, dashboard header + empty state, investigations list header + empty state, dataset page header "Investigate this dataset") opens the brief editor in new mode, pre-filled from the page +- New mode: "Start an investigation", symptom + ≥1 scope table required, datasource required only with >1 datasource; Start → POST /investigations → navigate to /issues/{issue_id} +- vitest covers each entry point and the new-mode submit + navigation + + +## Done summary +TBD + +## Evidence +- Commits: +- Tests: +- PRs: diff --git a/.flow/tasks/fn-70.23.json b/.flow/tasks/fn-70.23.json new file mode 100644 index 00000000..a2fbddbc --- /dev/null +++ b/.flow/tasks/fn-70.23.json @@ -0,0 +1,16 @@ +{ + "assignee": null, + "claim_note": "", + "claimed_at": null, + "created_at": "2026-09-28T17:52:00.239444Z", + "depends_on": [ + "fn-70.21" + ], + "epic": "fn-70", + "id": "fn-70.23", + "priority": null, + "spec_path": ".flow/tasks/fn-70.23.md", + "status": "todo", + "title": "M5: frontend: issue page matches the mockup", + "updated_at": "2026-09-28T17:52:00.239659Z" +} diff --git a/.flow/tasks/fn-70.23.md b/.flow/tasks/fn-70.23.md new file mode 100644 index 00000000..9eb82d74 --- /dev/null +++ b/.flow/tasks/fn-70.23.md @@ -0,0 +1,20 @@ +# fn-70.23 M5: frontend: issue page matches the mockup + +## Description +TBD + +## Acceptance +- Issue page matches 0001_issue_chat_mockup.html: one-row top bar; description as the thread's first entry; tabs Shared thread / My scratch chats (N) + "N watching · live"; one sidebar panel (Status with note, Details, Dataset + open dataset page, Investigations, Watchers, Your scratch chats); mockup pill colours +- Tool calls collapse to "Ran N queries · X ms · Y rows"; footer "Snapshot saved with this message · copy SQL" +- Investigation card: "Investigation #N", details → link, failed pill + reason + Retry (reopens the editor with the same brief); headless runs attributed to dataing +- Sidebar runs show number, depth and status (failed no longer "running") +- vitest updated/added; screenshots compared against the mockup + + +## Done summary +TBD + +## Evidence +- Commits: +- Tests: +- PRs: diff --git a/.flow/tasks/fn-70.24.json b/.flow/tasks/fn-70.24.json new file mode 100644 index 00000000..d76f08c7 --- /dev/null +++ b/.flow/tasks/fn-70.24.json @@ -0,0 +1,17 @@ +{ + "assignee": null, + "claim_note": "", + "claimed_at": null, + "created_at": "2026-09-28T17:52:00.495921Z", + "depends_on": [ + "fn-70.20", + "fn-70.21" + ], + "epic": "fn-70", + "id": "fn-70.24", + "priority": null, + "spec_path": ".flow/tasks/fn-70.24.md", + "status": "todo", + "title": "M5: frontend: run details page and LLM banner", + "updated_at": "2026-09-28T17:52:00.496129Z" +} diff --git a/.flow/tasks/fn-70.24.md b/.flow/tasks/fn-70.24.md new file mode 100644 index 00000000..383da363 --- /dev/null +++ b/.flow/tasks/fn-70.24.md @@ -0,0 +1,18 @@ +# fn-70.24 M5: frontend: run details page and LLM banner + +## Description +TBD + +## Acceptance +- /investigations/:id is the run's details page: back link to the issue thread, "Investigation #N" + status/depth pills, Share copies the link (mock removed), Export snapshot downloads GET /investigations/{id}/snapshot, Add as check (renamed from Codify Test), Cancel while running; brief; hypotheses with status and evidence (queries collapse like the thread's); outcome card; failed runs show the reason +- A banner under the header on every page shows GET /system/llm problems and what to fix +- vitest covers the back link, export, failed state and the banner + + +## Done summary +TBD + +## Evidence +- Commits: +- Tests: +- PRs: diff --git a/.flow/tasks/fn-70.25.json b/.flow/tasks/fn-70.25.json new file mode 100644 index 00000000..abf3a138 --- /dev/null +++ b/.flow/tasks/fn-70.25.json @@ -0,0 +1,21 @@ +{ + "assignee": null, + "claim_note": "", + "claimed_at": null, + "created_at": "2026-09-28T17:52:00.750051Z", + "depends_on": [ + "fn-70.19", + "fn-70.20", + "fn-70.21", + "fn-70.22", + "fn-70.23", + "fn-70.24" + ], + "epic": "fn-70", + "id": "fn-70.25", + "priority": null, + "spec_path": ".flow/tasks/fn-70.25.md", + "status": "todo", + "title": "M5: OpenAPI client refresh, full checks, demo walk-through", + "updated_at": "2026-09-28T17:52:00.750371Z" +} diff --git a/.flow/tasks/fn-70.25.md b/.flow/tasks/fn-70.25.md new file mode 100644 index 00000000..834320ae --- /dev/null +++ b/.flow/tasks/fn-70.25.md @@ -0,0 +1,18 @@ +# fn-70.25 M5: OpenAPI client refresh, full checks, demo walk-through + +## Description +TBD + +## Acceptance +- python-packages/dataing/openapi.json and the orval client carry the new/changed operations only (surgical: no unrelated drift) +- CE + EE pytest, ruff, ruff format --check, mypy, frontend vitest + typecheck + lint all green +- Demo stack walk-through: Investigate… from the dashboard lands in the issue thread; a bad key shows the banner and a failed card with the reason + + +## Done summary +TBD + +## Evidence +- Commits: +- Tests: +- PRs: diff --git a/docs/specs/0001_issue_chat.md b/docs/specs/0001_issue_chat.md index 27b893f4..0be9627f 100644 --- a/docs/specs/0001_issue_chat.md +++ b/docs/specs/0001_issue_chat.md @@ -1,6 +1,6 @@ # 0001: Issue hub: shared agent chat, handoff and steering -**Status:** Draft, 2026-09-27 +**Status:** M1–M4 shipped in PR #209. Revised 2026-09-28 with D12–D15 and M5: every run reaches the hub, one-step start, the run's details page, LLM failures, and the mockup as the acceptance reference. **Edition:** CE. Nothing here is EE-only. @@ -23,6 +23,7 @@ The issue page becomes the place where a team works a data problem: - People can **steer a running investigation** (add context, rule out or add a hypothesis, stop and conclude) without restarting it. - Each person can keep **private scratch chats** and publish from them. - Results, confirmation and the resolution land back in the thread. +- **Every investigation lives in an issue**, however it started, and starting one is a single step from any page (D12, D13). "Manager" is `InvestigationWorkflow`. "Subagents" are the `EvaluateHypothesisWorkflow` children it starts, one per hypothesis (`python-packages/dataing/src/dataing/temporal/workflows/`). @@ -42,6 +43,17 @@ Verified on main at `6e8812c8`. Paths are under `python-packages/dataing/src/dat | Results | Nothing writes `issue_investigation_runs.synthesis_summary`. The summary card never renders, and "resolved via a linked investigation" can never pass. | `migrations/018_issues.sql`; `routes/issues.py` | | Ad-hoc questions | No agent can answer them. The investigation agent is built with no tools. | `agents/client.py` | +**Found after M1–M4 shipped** (main at `245c9d2e`, 2026-09-28): + +| Area | Today | Where | +|---|---|---| +| Starting a run | Every "New investigation" button opens `/investigations/new`. That page starts a run with no issue and no thread, then lands on the old run page, so nobody who starts there sees the hub. | `features/investigation/NewInvestigation.tsx` and 7 links to it | +| Runs outside the UI | `POST /investigations` (SDK, CLI, notebook) and both webhooks insert the run inline, with no run row and no start card. The EE rule action never writes its outcome back. Only one of 8 creation paths links fully to an issue. | `routes/investigations.py`, `routes/integrations.py`, EE `routes/integrations.py`, EE `core/automation/executor.py` | +| LLM failures | With a rejected key, every LLM step fails. The activities return empty results and the workflow logs warnings. The run "completes" at 0% confidence with "Unable to determine a definitive root cause". An interpretation failure reads as *refuted* evidence. The chat shows the raw `ModelHTTPError`. | `temporal/activities/*.py`, `agents/client.py`, `temporal/workflows/investigation.py` | +| Key check | Nothing checks the key. The first sign of a bad key is a run that did nothing. | `entrypoints/api/deps.py` | +| The issue page | It is close to the mockup but not the same: a separate description card, no Shared thread / scratch tabs, the sidebar split into three cards, tool calls without totals, and a card link labelled "Open". | `features/issues/` | +| The run page | Its Share menu is mocked. It has no hypotheses list, no snapshot export and no link back to the issue. | `features/investigation/InvestigationDetail.tsx` | + --- ## 3. Goals and non-goals @@ -55,6 +67,9 @@ Verified on main at `6e8812c8`. Paths are under `python-packages/dataing/src/dat 5. Private scratch chats with explicit publishing. 6. Results, confirmation and resolution flow back into the thread. 7. A sidebar that only offers moves that will succeed, with inline editing. +8. Every investigation reaches the hub: one step to start from any page, and runs started by the API, SDK, webhooks or checks open an issue. +9. A run that can't reach the model fails with the reason, and a broken key is visible before anyone starts a run. +10. The issue page follows the mockup (`0001_issue_chat_mockup.html`), which is the acceptance reference for §8. **Non-goals for v1** @@ -96,6 +111,10 @@ At any point, Maya can explore in a private scratch chat, then publish the usefu | D9 | Comments, agent replies and system events share **one timeline table**. Issue events also append a system entry. | One cursor for streaming, and no merging of sources in the UI. | | D10 | Every result the agent saw is **snapshotted with its message**, and every query is in the gateway's audit log. | Data changes; the thread must show what the agent actually saw. | | D11 | Model: `claude-opus-5-5` (Claude Opus 5.5) with adaptive thinking, which it can't turn off. Effort is `low` for chat turns and `medium` for brief drafting, configurable per route. Brief drafting returns JSON text instead of calling an output tool, because Claude Opus 5.5 rejects a forced `tool_choice`. | Speed comes from effort, not from a smaller model. The owner chose Claude Opus 5.5 over Claude Opus 5 for cost: $4 / $20 per MTok against $5 / $25 (§12). | +| D12 | **Every investigation belongs to an issue.** A run started without one (API, SDK, webhook, check) opens an issue, or reuses the open one for the same alert, and reports into its shared thread. There is no issue-less run. | One place to follow every run. The hub (thread, card, steering, outcome review) works for every run, not only the ones started from an issue. | +| D13 | **One step from intent to a running investigation.** Every start button opens the brief editor where the person already is, pre-filled with what that page knows (the dataset, the alert). Start opens the issue and the run together and lands on the issue's thread. `/investigations/new` is removed. | The old page was a middle layer that started runs outside the hub. Opening the issue on Start, not on click, means a cancelled editor leaves no empty issue, and the issue's title is the symptom the person typed. | +| D14 | **The thread's card is the main view of a run.** `/investigations/:id` becomes the run's details page, linked from the card and the sidebar. It keeps what the card has no room for: the evidence, every query with its result, share, snapshot export and Add as check. | The team works in the thread; the details page is for digging in. Nothing the old page did is lost. | +| D15 | **LLM failures fail the run, loudly.** Errors a retry can't fix (a missing or rejected key, an unknown model, a rejected request) fail the run at once with the reason on its card. Rate limits, overload and server errors retry with backoff first. A run never synthesizes a conclusion from steps that all failed. The API checks the key and models at startup, and every page shows a banner while they don't work. | A run that "completes" at 0% confidence after doing nothing hides the real problem. The fix is usually one environment variable, so say which. | --- @@ -212,6 +231,8 @@ All paths are under `/api/v1`. Gates use the names from the route authorization | `POST /investigations/{investigation_id}/steers` | SCOPE_WRITE | Body: `kind`, `text`, `hypothesis_id` | | `GET /investigations/{investigation_id}/steers` | ANY_USER | Includes each steer's status and outcome | | `POST /investigations/{investigation_id}/outcome-review` | SCOPE_WRITE | Body: `verdict` (`confirmed` or `rejected`), `note` | +| `POST /investigations` | SCOPE_WRITE | Starts a run from a `brief` or an `alert`, opening an issue unless `issue_id` is given (§7.11) | +| `GET /system/llm` | ANY_USER | Whether the key and models work (§7.12) | **Removed:** `GET/POST /issues/{issue_id}/comments`, `POST /investigations/{investigation_id}/messages` and `POST /investigations/{investigation_id}/input`. @@ -246,7 +267,7 @@ If polling load becomes a problem, switch to Postgres `LISTEN/NOTIFY` behind the 1. Builds the prompt (§7.6) and a `BondAgent` with the chat tools (§7.5). 2. Creates the reply message with `status = 'streaming'` and `requested_by_user_id` set to the asker. 3. Streams text into `body_md`, flushing every 250 ms or 200 characters. -4. Records each tool call in `payload.tool_calls` as `{id, tool, input, status, summary, query_result_id}`. +4. Records each tool call in `payload.tool_calls` as `{id, tool, input, status, summary, query_result_id, duration_ms, row_count}`. The last two are set for `run_query` only. 5. Ends in `complete`, `error` (with the message) or `cancelled`, and records token usage, including cache reads, in `payload.usage`. **Limits** (configuration now, per tenant later): @@ -391,6 +412,119 @@ The brief schema is `InvestigationBrief` (Pydantic, versioned): - **Add as check** works as described in checks as code §7.6. - **Downstream:** resolving an issue with a confirmed outcome emits `issue.resolved_with_cause`. 0002 lists these on the dataset, and 0003 can export them. +### 7.11 Starting investigations (D12, D13) + +**One starter.** `InvestigationStarterService.start()` (`services/investigation.py`) is the only code that starts an investigation. It: +1. **Resolves the issue.** + - Given an `issue_id`, it uses that issue, which must belong to the tenant. + - Otherwise it opens one with `open_issue()`. The title is the brief's symptom, the dataset is the first scope table, and the severity comes from the alert. `created_by` is the person, or NULL for API keys and webhooks. +2. **Builds the missing half** of the input: + - Given a brief, it builds the workflow's alert from it, as the spawn route does today. + - Given an alert (SDK, webhooks), it builds the brief: + - the symptom from the alert's display name and values + - the scope from its dataset ids and datasource + - the time window as the anomaly date ± 1 day +3. **Writes the run.** It inserts the `investigations` row (with `created_by`), the `issue_investigation_runs` row and the thread's `investigation` card. + - The run row's `trigger_type` is `human`, `api`, `webhook` or `rule`. + - A run without a person is attributed to dataing ("dataing started an investigation"). +4. **Starts the workflow** with `alert.issue_id` set, so the outcome is always written back. + - If the start fails, the run gets a failed outcome ("Couldn't start the investigation: …") and the route returns 503. +5. **Returns** `investigation_id`, `run_id`, `issue_id` and `issue_number`. + +**Opening issues.** `open_issue()` (`adapters/db/issues.py`) is the one way to insert an issue: `POST /issues`, both webhooks and the starter all use it. +- It records the `created` event, which also posts the thread's first entry: "Maya opened the issue", or "Issue opened by dataing from Monte Carlo". +- Today, webhook-opened issues get no thread entry. + +**Every path, after this change:** + +| Path | Today | After | +|---|---|---| +| `POST /issues/{id}/investigation-runs` | Starter, then the run row and card inline | Starter, with the issue | +| `POST /investigations` (UI, SDK, CLI, notebook) | Inline insert; no issue, run row or card | Starter; opens an issue unless `issue_id` is given | +| CE `webhook-generic` and EE provider webhooks, policy AUTO | Inline insert and a direct Temporal start. Only `alert.issue_id` links it: no run row, no start card, and outcome review returns 404. | Starter, with the issue the webhook opened, `trigger_type = webhook` | +| EE rule action `spawn_investigation` | Starter, but no `alert.issue_id`, so the outcome is never written back | Starter, with the issue, `trigger_type = rule` | +| `POST /investigations/import` | Inserts a finished replay record | Unchanged. It is a record, not a run, so it has no issue; its details page has no back link. | + +The dormant creation paths have no callers: the Redis queue worker (`adapters/queue/investigation_worker.py`) and `InvestigationService.start_investigation`. They are removed in a follow-up. + +**`POST /investigations`** (SCOPE_WRITE): + +```json +{ + "brief": {"symptom": "…", "scope": {"tables": ["public.orders"]}}, + "alert": null, + "datasource_id": null, + "execution_profile": "standard", + "issue_id": null +} +``` + +- **Body:** + - Exactly one of `brief` and `alert`. A bad alert is a 422, not today's 500. + - The datasource resolves as it does today; an ambiguous one is a 409 with `ambiguous_datasource`. +- **Response:** `investigation_id`, `run_id`, `issue_id`, `issue_number`, `status: "queued"`, and `main_branch_id` for the SDK. +- **SDK and CLI:** the SDK's `Investigation` gains `issue_id` and `issue_number`. `dataing run start` prints the issue's URL. + +**Run numbers and status:** +- `InvestigationRunResponse` gains `number` (the run's position among the issue's runs, by start time), `status` (`running`, `completed` or `failed`, from the outcome) and `error`. Today a failed run shows "running" forever. +- `InvestigationStateResponse` (`GET /investigations/{id}`) gains `issue_id`, `issue_number`, `issue_title`, `run_number`, `brief`, `execution_profile` and `error`. + +### 7.12 LLM failures and the key check (D15) + +**Today:** a rejected key never fails a run. +1. Every LLM activity catches the exception and returns empty results. +2. `AgentClient.interpret_evidence` turns an LLM error into evidence, so the hypothesis reads as *refuted*. +3. The workflow logs each error as a warning, synthesizes from nothing, and publishes `status: "completed"` at confidence 0. + +The chat agent shows the raw `ModelHTTPError` text. + +**Classifying errors.** `classify_llm_error(exc)` (`agents/errors.py`) follows the exception chain (`LLMError` → pydantic-ai `ModelHTTPError`/`ModelAPIError` → `anthropic.*Error`) and returns a code, whether a retry can help, and a message that says what to fix: + +| Code | Cause | Retry? | Message | +|---|---|---|---| +| `missing_key` | `ANTHROPIC_API_KEY` is empty | no | "ANTHROPIC_API_KEY isn't set. Set it and restart the API and the worker." | +| `invalid_key` | 401 | no | "Anthropic rejected the API key (401). Set a valid ANTHROPIC_API_KEY and restart the API and the worker." | +| `forbidden` | 403 | no | "The API key isn't allowed to use {model} (403)." | +| `unknown_model` | 404 | no | "Anthropic doesn't know the model {model} (404). Check LLM_MODEL and CHAT_AGENT_MODEL." | +| `bad_request` | 400, 413, 422 | no | "Anthropic rejected the request ({status}): {detail}" | +| `rate_limited` | 429 | yes | "Anthropic rate-limited the request (429)." | +| `overloaded` | 529 | yes | "Anthropic is overloaded (529)." | +| `server_error` | other 5xx | yes | "Anthropic returned an error ({status})." | +| `unreachable` | connection error, timeout | yes | "Couldn't reach Anthropic: {detail}" | + +**Activities raise instead of returning an error.** +- On an LLM error, `generate_hypotheses`, `generate_query`, `interpret_evidence`, `synthesize` and `counter_analyze` raise a Temporal `ApplicationError`: + - type `LLMRejected` and `non_retryable=True` when a retry can't help + - type `LLMUnavailable` otherwise + - details `{code, message}` in both cases +- Every LLM activity call gets an explicit retry policy: 4 attempts, 5 s initial backoff, ×2, capped at 60 s. `LLMRejected` is non-retryable. The Anthropic SDK already retries 429/5xx twice inside each attempt. +- `AgentClient` stops swallowing errors in `interpret_evidence`. + +**The workflow fails the run.** New code paths are guarded with `workflow.patched("llm-failures-v1")`. The run fails when: +- hypothesis generation fails, or proposes nothing and no person added a hypothesis; +- any hypothesis evaluation fails with an LLM error. The key or model is broken for every subagent, so the others are cancelled. +- every hypothesis ended untested because of errors. Hypotheses a person ruled out or stopped don't count. +- synthesis fails. + +Counter-analysis failing doesn't fail a run whose synthesis succeeded. The outcome records `counter_analysis.error` and the card says the check didn't run. + +**What a failed run does:** +1. It publishes a failed outcome through `publish_investigation_outcome`: `{"status": "failed", "error": {"code", "message", "step"}}`. This writes `investigations.outcome`, the run row (`completed_at`) and the thread card. +2. It raises a non-retryable `ApplicationError`, so Temporal shows the execution as failed too. + +The card shows a red "failed" pill, the message and **Retry**. + +**Chat turns and brief drafts.** `run_agent_turn` classifies the error the same way. The reply shows the message, not the raw exception, and a turn that can't succeed on retry isn't retried. + +**The key check.** +- At startup the API calls `GET /v1/models/{id}` for each configured model (`LLM_MODEL`, `CHAT_AGENT_MODEL`). This costs no tokens. + - It runs in the background with a 10-second timeout and no SDK retries, so a slow or missing network never delays startup. + - An empty key is reported without a request. +- `GET /system/llm` (ANY_USER) returns `{state, message, models, checked_at}`. + - `state` is `ok`, `checking` or one of the codes above. + - An `unreachable` result older than 60 seconds is checked again on read. +- The key is read from the environment, so a fixed key takes effect when the API and the worker restart; the check reruns at startup. + --- ## 8. Frontend @@ -412,6 +546,64 @@ The brief schema is `InvestigationBrief` (Pydantic, versioned): - **Removed:** "Ask a question" and "Collaborate → Create Branch" on the investigation page, plus `BranchTree` and `MergeIndicator`. - **API client:** regenerate the orval client for the new routes. The committed `openapi.json` has drifted from the app, so either regenerate only these operations or do the full refresh as a separate PR. +### 8.1 Following the mockup + +`0001_issue_chat_mockup.html` is the acceptance reference for the issue page. Where the page and the mockup differ, the mockup wins: + +- **Top bar:** the `#N` pill, the title, the status and priority pills, and the dataset on the right, in one row. +- **No separate description card.** The description is the thread's first entry: "Maya opened the issue", with the description as its body and Edit for its author. An issue opened by dataing starts with the event line instead ("Check … failed · issue opened by dataing"). +- **Thread head:** two tabs, **Shared thread** and **My scratch chats (N)**, and "N watching · live" on the right. The scratch tab opens the scratch drawer. +- **One sidebar panel,** in this order: + 1. Status, with **Change ▾**. The menu starts with the note "Only moves that will succeed are shown", and Resolved says it asks for a note pre-filled from the confirmed cause. + 2. Details: assignee, priority, severity, labels, observed, column. + 3. Dataset, with "open dataset page →". + 4. Investigations: one row per run ("#2 · standard" and a status pill), linking to the run's details page. + 5. Watchers, by name. + 6. Your scratch chats, and **＋ New scratch chat**. + + The Timeline section goes; the thread already records when things happened. +- **Tool calls** collapse to one line with the totals: "▸ Ran 1 query · 212 ms · 4 rows" or "▸ Ran 2 queries · 480 ms". Expanded, the footer reads "Snapshot saved with this message · copy SQL". The tool-call record stores `duration_ms` and `row_count` so the line needs no extra request. +- **Pills** use the mockup's colours: purple for the agent and running work, green for supported and done, amber for in progress and untested, red for refuted and failed, blue for people's additions. +- **Investigation card:** "Investigation #N", where N counts the issue's runs in start order. It has a **details →** link to the run's page. A failed run shows a red "failed" pill, the reason and **Retry**, which reopens the brief editor with the same brief. + +### 8.2 Starting an investigation (D13) + +- **Removed:** the `investigations/new` route, `NewInvestigation.tsx`, and the unused `components/Layout.tsx`. +- **Every start button becomes Investigate…** and opens the brief editor in *new* mode where the person is: + - the sidebar's quick action + - the dashboard header and its empty recent-investigations card + - the investigations list header and its empty state + - the dataset page, as **Investigate this dataset** in the header, shown whether or not the dataset has runs +- **New mode** is the same editor, titled "Start an investigation": + - It is pre-filled from the page. The dataset page supplies the table's `native_path` as the scope table and its `datasource_id`; the other pages supply nothing. + - Symptom and at least one scope table are required. The datasource is required only when the tenant has more than one. + - Findings, ruled out and leads start empty; there is no thread to draft from. + - **Start investigation** calls `POST /investigations` (§7.11) and navigates to `/issues/{issue_id}`, where the card is already live. +- **Hand off** mode (from a thread) is unchanged, except that its datasource option names the issue's datasource instead of "The issue's datasource". + +### 8.3 The run's details page (D14) + +`/investigations/:id` keeps its URL and becomes the run's details page: + +- **Header:** + - a back link to the issue's thread ("← #42 Completed orders dropped") + - "Investigation #N" with its status and depth pills + - **Share**, which copies the link (the mocked user picker goes) + - **Export snapshot**, which downloads `GET /investigations/{id}/snapshot` + - **Add as check** (renamed from "Codify Test", same gating) + - Cancel, while the run is running +- **Brief:** the brief the run was given. +- **Hypotheses:** each with its status pill (including "ruled out by a person" and "untested") and its evidence. Queries collapse like the thread's tool calls and expand to the SQL, the result summary and the interpretation. +- **Outcome:** the root cause card in the thread's style, with the causal chain, onset, affected scope and recommendations, plus the existing feedback buttons. +- **A failed run** shows the failure reason and what to fix in place of the outcome. +- **Backend:** `InvestigationStateResponse` gains `issue_id`, `issue_number` and `issue_title`. + +### 8.4 LLM status banner (D15) + +- The app polls `GET /system/llm` (§7.12) once a minute. +- While it reports a problem, a banner under the header on every page names it and says what to fix, for example "Anthropic rejected the API key. Investigations and the agent can't run until ANTHROPIC_API_KEY is fixed and the API and worker are restarted." +- It can't be dismissed while the problem lasts. + --- ## 9. Security and privacy @@ -436,6 +628,7 @@ The brief schema is `InvestigationBrief` (Pydantic, versioned): | M2 | Handoff and results:
  • brief drafting and editor
  • brief-aware manager and prompts
  • investigation card and outcome write-back
  • confirm/reject and resolution pre-fill
  • Continue investigating
| About 1.5 weeks | | M3 | Steering:
  • signal and checkpoint application
  • evaluation-loop rewrite
  • steers API and UI, `propose_steer`
  • Replayer tests
| About 1.5 weeks | | M4 | Scratch chats, publishing, investigate from a scratch chat | About 1 week | +| M5 | Every run in the hub:
  • one starter and `open_issue()` for every path, and `POST /investigations` with a brief
  • LLM failures fail the run, plus the key check and banner
  • Investigate… from every page, and `/investigations/new` removed
  • the issue page matching the mockup
  • the run's details page
| About 1 week | `run_query` in M1 needs `UserPrincipal` through `QueryGateway` (checks as code §5.2). If that hasn't landed yet, M1 builds the user path as specified there. Tools from 0002 and 0003 plug in as those specs land. @@ -473,6 +666,27 @@ The brief schema is `InvestigationBrief` (Pydantic, versioned): - the brief editor - the steer controls - **End to end,** on the `null_spike` demo fixture: open an issue, ask the agent, hand off with a brief, rule out a hypothesis, get the outcome, confirm, resolve. +- **M5:** + - `classify_llm_error` for each row of the §7.12 table, through the real exception chain + - each LLM activity raises `LLMRejected` or `LLMUnavailable` and never returns empty results + - workflow tests with fake activities: + - a rejected key fails the run with a failed outcome and a failed Temporal execution + - one subagent's LLM error cancels the others and fails the run + - all hypotheses untested from errors fails the run + - a counter-analysis failure keeps the synthesis + - Replayer tests still pass on the recorded histories + - the starter: + - each path in the §7.11 table writes the run row and the start card, and sets `alert.issue_id` + - `POST /investigations` without `issue_id` opens exactly one issue + - a failed workflow start leaves a failed outcome + - `GET /system/llm` for missing, rejected and valid keys and an unknown model, with the Anthropic client faked + - frontend: + - Investigate… on each page opens the editor pre-filled and navigates to the new issue + - the banner + - the failed card with Retry + - the tool-call totals + - the tabs + - the details page's back link and export --- @@ -501,4 +715,15 @@ The brief schema is `InvestigationBrief` (Pydantic, versioned): - `frontend/app/src/features/issues/IssueWorkspace.tsx`, `IssueCreate.tsx`, `IssueList.tsx` - `frontend/app/src/features/investigation/InvestigationDetail.tsx`, `components/index.ts` (`BranchTree`, `MergeIndicator`) - `python-packages/dataing/tests/unit/entrypoints/api/routes/test_route_authorization.py`: `POLICY` +- M5: + - `python-packages/dataing/src/dataing/services/investigation.py`: `InvestigationStarterService` + - `python-packages/dataing/src/dataing/entrypoints/api/routes/investigations.py`: `start_investigation`, `InvestigationStateResponse` + - `python-packages/dataing/src/dataing/entrypoints/api/routes/integrations.py`: `_start_auto_investigation` + - `python-packages/dataing-ee/src/dataing_ee/entrypoints/api/routes/integrations.py`: `_evaluate_and_start_investigation` + - `python-packages/dataing-ee/src/dataing_ee/core/automation/executor.py`: `spawn_investigation` + - `python-packages/dataing/src/dataing/temporal/activities/`: `generate_hypotheses.py`, `generate_query.py`, `interpret_evidence.py`, `synthesize.py`, `counter_analyze.py`, `publish_outcome.py` + - `python-packages/dataing/src/dataing/agents/client.py`: `LLMError` wrapping; `interpret_evidence` swallows errors today + - `frontend/app/src/features/investigation/NewInvestigation.tsx`, `InvestigationDetail.tsx` + - `frontend/app/src/features/issues/brief/BriefEditor.tsx`, `thread/ToolCalls.tsx`, `thread/InvestigationCard.tsx`, `IssueSidebar.tsx`, `IssueWorkspace.tsx` + - `0001_issue_chat_mockup.html`: the acceptance reference for §8 - [Checks as code](../plans/2026-09-26-checks-as-code-design.md): §5.2 principals, §7 failure loop, §7.6 codify From bf2f46191fa3deec582aedab1203a9542a078d76 Mon Sep 17 00:00:00 2001 From: bordumb Date: Mon, 28 Sep 2026 19:21:45 +0100 Subject: [PATCH 02/18] fix(temporal): LLM failures fail the investigation with the reason MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With a rejected key, every LLM activity caught the error and returned empty results. The workflow logged warnings, synthesized from nothing and published "completed" at confidence 0. AgentClient also turned interpretation errors into evidence that read as refuted. Now (spec 0001 §7.12): - classify_llm_error maps the real exception chain (anthropic → pydantic-ai → LLMError) to a code, retryability and what to fix - LLM activities raise LLMRejected (non-retryable) or LLMUnavailable and retry with LLM_RETRY_POLICY (4 attempts, 5-60 s backoff) - behind the llm-failures-v1 patch, the run fails when: - hypothesis generation fails or proposes nothing - a subagent hits an LLM error (the others are cancelled) - every hypothesis is left untested by errors - synthesis fails - a failed counter-analysis keeps the conclusion and records the error - a failed run publishes {"status": "failed", "error": {code, message, step}} to the investigation, run row and thread, then fails its Temporal execution as InvestigationFailed Recorded histories still replay. The datasource-down test now expects a failed run instead of a conclusion drawn from no evidence. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Claude --- .flow/tasks/fn-70.18.json | 15 +- .flow/tasks/fn-70.18.md | 5 +- .flow/tasks/fn-70.19.json | 8 +- .../dataing/src/dataing/agents/client.py | 36 ++- .../dataing/src/dataing/agents/errors.py | 120 +++++++++ .../temporal/activities/counter_analyze.py | 6 + .../activities/generate_hypotheses.py | 6 + .../temporal/activities/generate_query.py | 6 + .../temporal/activities/interpret_evidence.py | 6 + .../temporal/activities/publish_outcome.py | 56 +++-- .../dataing/temporal/activities/synthesize.py | 6 + .../dataing/src/dataing/temporal/errors.py | 78 ++++++ .../temporal/workflows/evaluate_hypothesis.py | 8 + .../temporal/workflows/investigation.py | 193 ++++++++++++-- .../tests/fixtures/investigation_env.py | 26 +- .../dataing/tests/fixtures/llm_errors.py | 37 +++ .../tests/integration/test_publish_outcome.py | 53 ++++ .../dataing/tests/unit/agents/test_client.py | 37 +++ .../tests/unit/agents/test_llm_errors.py | 134 ++++++++++ .../temporal/test_investigation_failures.py | 238 ++++++++++++++++++ .../temporal/test_investigation_workflow.py | 27 +- .../unit/temporal/test_llm_activity_errors.py | 156 ++++++++++++ 22 files changed, 1174 insertions(+), 83 deletions(-) create mode 100644 python-packages/dataing/src/dataing/agents/errors.py create mode 100644 python-packages/dataing/src/dataing/temporal/errors.py create mode 100644 python-packages/dataing/tests/fixtures/llm_errors.py create mode 100644 python-packages/dataing/tests/unit/agents/test_llm_errors.py create mode 100644 python-packages/dataing/tests/unit/temporal/test_investigation_failures.py create mode 100644 python-packages/dataing/tests/unit/temporal/test_llm_activity_errors.py diff --git a/.flow/tasks/fn-70.18.json b/.flow/tasks/fn-70.18.json index 1a7aa54a..1acd0c8e 100644 --- a/.flow/tasks/fn-70.18.json +++ b/.flow/tasks/fn-70.18.json @@ -1,14 +1,21 @@ { - "assignee": null, + "assignee": "bordumbb@gmail.com", "claim_note": "", - "claimed_at": null, + "claimed_at": "2026-09-28T17:54:28.759022Z", "created_at": "2026-09-28T17:51:58.959592Z", "depends_on": [], "epic": "fn-70", + "evidence": { + "commits": [ + "f292971a" + ], + "prs": [], + "tests": [] + }, "id": "fn-70.18", "priority": null, "spec_path": ".flow/tasks/fn-70.18.md", - "status": "todo", + "status": "done", "title": "M5: spec revision: every run in the hub, one-step start, details page, LLM failures", - "updated_at": "2026-09-28T17:51:58.959798Z" + "updated_at": "2026-09-28T17:54:29.035901Z" } diff --git a/.flow/tasks/fn-70.18.md b/.flow/tasks/fn-70.18.md index 9350cc0a..ad8941af 100644 --- a/.flow/tasks/fn-70.18.md +++ b/.flow/tasks/fn-70.18.md @@ -9,9 +9,8 @@ TBD ## Done summary -TBD - +Spec revision committed: D12-D15, §7.11, §7.12, §8.1-8.4, M5. ## Evidence -- Commits: +- Commits: f292971a - Tests: - PRs: diff --git a/.flow/tasks/fn-70.19.json b/.flow/tasks/fn-70.19.json index 6729e5d1..1a34a745 100644 --- a/.flow/tasks/fn-70.19.json +++ b/.flow/tasks/fn-70.19.json @@ -1,7 +1,7 @@ { - "assignee": null, + "assignee": "bordumbb@gmail.com", "claim_note": "", - "claimed_at": null, + "claimed_at": "2026-09-28T17:55:54.224028Z", "created_at": "2026-09-28T17:51:59.221401Z", "depends_on": [ "fn-70.18" @@ -10,7 +10,7 @@ "id": "fn-70.19", "priority": null, "spec_path": ".flow/tasks/fn-70.19.md", - "status": "todo", + "status": "in_progress", "title": "M5: LLM failures fail the run (classify, activities raise, workflow fails)", - "updated_at": "2026-09-28T17:51:59.221589Z" + "updated_at": "2026-09-28T17:55:54.224228Z" } diff --git a/python-packages/dataing/src/dataing/agents/client.py b/python-packages/dataing/src/dataing/agents/client.py index f4376a10..49a41a6f 100644 --- a/python-packages/dataing/src/dataing/agents/client.py +++ b/python-packages/dataing/src/dataing/agents/client.py @@ -227,6 +227,10 @@ async def interpret_evidence( Returns: Evidence with validated interpretation. + + Raises: + LLMError: If interpretation fails. A failed interpretation is not evidence: + returned as such, its hypothesis would read as refuted. """ prompt = interpretation.build_user(hypothesis=hypothesis, query=sql, results=results) system = interpretation.build_system() @@ -239,28 +243,18 @@ async def interpret_evidence( instructions=system, handlers=handlers, ) - - return Evidence( - hypothesis_id=hypothesis.id, - query=sql, - result_summary=results.to_summary(), - row_count=results.row_count, - supports_hypothesis=result.supports_hypothesis, - confidence=result.confidence, - interpretation=result.interpretation, - ) - except Exception as e: - # Return low-confidence evidence on failure rather than crashing - return Evidence( - hypothesis_id=hypothesis.id, - query=sql, - result_summary=results.to_summary(), - row_count=results.row_count, - supports_hypothesis=None, - confidence=0.3, - interpretation=f"Interpretation failed: {e}", - ) + raise LLMError(f"Interpretation failed: {e}", retryable=True) from e + + return Evidence( + hypothesis_id=hypothesis.id, + query=sql, + result_summary=results.to_summary(), + row_count=results.row_count, + supports_hypothesis=result.supports_hypothesis, + confidence=result.confidence, + interpretation=result.interpretation, + ) async def synthesize_findings( self, diff --git a/python-packages/dataing/src/dataing/agents/errors.py b/python-packages/dataing/src/dataing/agents/errors.py new file mode 100644 index 00000000..453bd6b8 --- /dev/null +++ b/python-packages/dataing/src/dataing/agents/errors.py @@ -0,0 +1,120 @@ +"""Classify LLM errors by what fixes them (docs/specs/0001_issue_chat.md §7.12). + +The investigation and the chat agent reach Anthropic through pydantic-ai, which maps +the SDK's errors to ModelHTTPError and ModelAPIError; the key check calls the SDK +directly. classify_llm_error follows an exception's cause chain to whichever of those +it finds and says whether a retry can help and what the person should change. +""" + +from __future__ import annotations + +from collections.abc import Iterator +from dataclasses import dataclass +from typing import Any + +import anthropic +from pydantic_ai.exceptions import ModelAPIError, ModelHTTPError, UserError + +RESTART = "restart the API and the worker" + + +@dataclass(frozen=True) +class LLMFailure: + """Why an LLM call failed, and whether trying again can help.""" + + code: str + message: str + retryable: bool + status_code: int | None = None + + def to_dict(self) -> dict[str, Any]: + """Return the code and message, as error details and outcome payloads carry them.""" + return {"code": self.code, "message": self.message} + + +def classify_llm_error(error: BaseException, *, model: str | None = None) -> LLMFailure | None: + """Return why an LLM call failed, or None when the error didn't come from the API. + + Args: + error: The exception raised by the call, however deeply it wraps the cause. + model: The model called, for messages when the error doesn't name it. + """ + for cause in _causes(error): + if isinstance(cause, ModelHTTPError): + return _from_status(cause.status_code, cause.model_name or model, cause.body) + if isinstance(cause, anthropic.APIStatusError): + return _from_status(cause.status_code, model, cause.body) + if isinstance(cause, anthropic.APIConnectionError): + return _unreachable(cause.message) + if isinstance(cause, ModelAPIError): + return _unreachable(cause.message) + if isinstance(cause, UserError) and "ANTHROPIC_API_KEY" in str(cause): + return LLMFailure( + code="missing_key", + message=f"ANTHROPIC_API_KEY isn't set. Set it and {RESTART}.", + retryable=False, + ) + return None + + +def _causes(error: BaseException) -> Iterator[BaseException]: + """Yield the error and each exception it was raised from, outermost first.""" + seen: set[int] = set() + current: BaseException | None = error + while current is not None and id(current) not in seen: + seen.add(id(current)) + yield current + current = current.__cause__ or current.__context__ + + +def _from_status(status: int, model: str | None, body: object) -> LLMFailure: + """Map an HTTP status from the API to a failure.""" + named = model or "the configured model" + if status == 401: + return _rejected( + "invalid_key", + f"Anthropic rejected the API key (401). Set a valid ANTHROPIC_API_KEY and {RESTART}.", + status, + ) + if status == 403: + return _rejected("forbidden", f"The API key isn't allowed to use {named} (403).", status) + if status == 404: + return _rejected( + "unknown_model", + f"Anthropic doesn't know the model {named} (404). Check LLM_MODEL and " + "CHAT_AGENT_MODEL.", + status, + ) + if status == 429: + return _retryable("rate_limited", "Anthropic rate-limited the request (429).", status) + if status == 529: + return _retryable("overloaded", "Anthropic is overloaded (529).", status) + if status >= 500 or status in (408, 409): + return _retryable("server_error", f"Anthropic returned an error ({status}).", status) + detail = _api_message(body) + suffix = f": {detail}" if detail else "" + return _rejected("bad_request", f"Anthropic rejected the request ({status}){suffix}", status) + + +def _api_message(body: object) -> str | None: + """Return the API's own error message from a response body, if it has one.""" + if isinstance(body, dict): + error = body.get("error") + if isinstance(error, dict) and isinstance(error.get("message"), str): + message: str = error["message"] + return message + return None + + +def _rejected(code: str, message: str, status: int) -> LLMFailure: + return LLMFailure(code=code, message=message, retryable=False, status_code=status) + + +def _retryable(code: str, message: str, status: int) -> LLMFailure: + return LLMFailure(code=code, message=message, retryable=True, status_code=status) + + +def _unreachable(detail: str) -> LLMFailure: + return LLMFailure( + code="unreachable", message=f"Couldn't reach Anthropic: {detail}", retryable=True + ) diff --git a/python-packages/dataing/src/dataing/temporal/activities/counter_analyze.py b/python-packages/dataing/src/dataing/temporal/activities/counter_analyze.py index 7113b587..fcaa3faf 100644 --- a/python-packages/dataing/src/dataing/temporal/activities/counter_analyze.py +++ b/python-packages/dataing/src/dataing/temporal/activities/counter_analyze.py @@ -7,6 +7,9 @@ from temporalio import activity +from dataing.agents.errors import classify_llm_error +from dataing.temporal.errors import llm_activity_error + if TYPE_CHECKING: from dataing.temporal.adapters import TemporalAgentAdapter @@ -52,6 +55,9 @@ async def counter_analyze(input: CounterAnalyzeInput) -> CounterAnalyzeResult: hypotheses=input.hypotheses, ) except Exception as e: + failure = classify_llm_error(e) + if failure is not None: + raise llm_activity_error(failure) from e return CounterAnalyzeResult( alternative_explanations=[], weaknesses=[], diff --git a/python-packages/dataing/src/dataing/temporal/activities/generate_hypotheses.py b/python-packages/dataing/src/dataing/temporal/activities/generate_hypotheses.py index 437c7b56..46d57144 100644 --- a/python-packages/dataing/src/dataing/temporal/activities/generate_hypotheses.py +++ b/python-packages/dataing/src/dataing/temporal/activities/generate_hypotheses.py @@ -7,6 +7,9 @@ from temporalio import activity +from dataing.agents.errors import classify_llm_error +from dataing.temporal.errors import llm_activity_error + if TYPE_CHECKING: from dataing.temporal.adapters import TemporalAgentAdapter @@ -63,6 +66,9 @@ async def generate_hypotheses(input: GenerateHypothesesInput) -> GenerateHypothe code_changes=input.code_changes, ) except Exception as e: + failure = classify_llm_error(e) + if failure is not None: + raise llm_activity_error(failure) from e return GenerateHypothesesResult( hypotheses=[], error=f"Hypothesis generation failed: {e}", diff --git a/python-packages/dataing/src/dataing/temporal/activities/generate_query.py b/python-packages/dataing/src/dataing/temporal/activities/generate_query.py index 2b0f90d7..3523f307 100644 --- a/python-packages/dataing/src/dataing/temporal/activities/generate_query.py +++ b/python-packages/dataing/src/dataing/temporal/activities/generate_query.py @@ -7,6 +7,9 @@ from temporalio import activity +from dataing.agents.errors import classify_llm_error +from dataing.temporal.errors import llm_activity_error + if TYPE_CHECKING: from dataing.temporal.adapters import TemporalAgentAdapter @@ -54,6 +57,9 @@ async def generate_query(input: GenerateQueryInput) -> GenerateQueryResult: alert=input.alert, ) except Exception as e: + failure = classify_llm_error(e) + if failure is not None: + raise llm_activity_error(failure) from e return GenerateQueryResult( query="", hypothesis_id=hypothesis_id, diff --git a/python-packages/dataing/src/dataing/temporal/activities/interpret_evidence.py b/python-packages/dataing/src/dataing/temporal/activities/interpret_evidence.py index b7d36906..7eb70e01 100644 --- a/python-packages/dataing/src/dataing/temporal/activities/interpret_evidence.py +++ b/python-packages/dataing/src/dataing/temporal/activities/interpret_evidence.py @@ -7,6 +7,9 @@ from temporalio import activity +from dataing.agents.errors import classify_llm_error +from dataing.temporal.errors import llm_activity_error + if TYPE_CHECKING: from dataing.temporal.adapters import TemporalAgentAdapter @@ -59,6 +62,9 @@ async def interpret_evidence(input: InterpretEvidenceInput) -> InterpretEvidence alert_summary=input.alert_summary, ) except Exception as e: + failure = classify_llm_error(e) + if failure is not None: + raise llm_activity_error(failure) from e return InterpretEvidenceResult( hypothesis_id=hypothesis_id, supports_hypothesis=None, diff --git a/python-packages/dataing/src/dataing/temporal/activities/publish_outcome.py b/python-packages/dataing/src/dataing/temporal/activities/publish_outcome.py index 1e66ece2..8d80f24a 100644 --- a/python-packages/dataing/src/dataing/temporal/activities/publish_outcome.py +++ b/python-packages/dataing/src/dataing/temporal/activities/publish_outcome.py @@ -1,9 +1,10 @@ -"""Activity that publishes a finished investigation's outcome (spec 0001 §7.7). +"""Activity that publishes a finished investigation's outcome (spec 0001 §7.7, §7.12). It writes the outcome to the investigation (the 036 trigger stamps completed_at), fills the linked issue run's summary fields, and posts the result card to the -issue's shared thread. Every step is idempotent, so a retried attempt changes -nothing twice. +issue's shared thread. A failed run publishes its failure the same way, as +{"status": "failed", "error": {code, message, step}}. Every step is idempotent, so a +retried attempt changes nothing twice. """ from __future__ import annotations @@ -37,6 +38,11 @@ def outcome_markdown(synthesis: dict[str, Any], hypotheses: list[dict[str, Any]] return "\n".join(lines) +def failure_markdown(failure: dict[str, Any]) -> str: + """Render a failed run's card text for the thread.""" + return f"**Investigation failed**\n\n{failure.get('message') or 'The run failed.'}" + + def make_publish_investigation_outcome_activity(app_db: AppDatabase) -> Any: """Return the publish_investigation_outcome activity with its database bound.""" @@ -47,15 +53,20 @@ async def publish_investigation_outcome(payload: dict[str, Any]) -> dict[str, An tenant_id = UUID(str(payload["tenant_id"])) synthesis: dict[str, Any] = payload.get("synthesis") or {} hypotheses: list[dict[str, Any]] = payload.get("hypotheses") or [] - outcome = { - "status": "completed", - "root_cause": synthesis.get("root_cause"), - "confidence": synthesis.get("confidence"), - "recommendations": synthesis.get("recommendations") or [], - "supporting_evidence": synthesis.get("supporting_evidence") or [], - "hypotheses": hypotheses, - "counter_analysis": payload.get("counter_analysis"), - } + failure: dict[str, Any] | None = payload.get("failure") + outcome: dict[str, Any] + if failure: + outcome = {"status": "failed", "error": failure, "hypotheses": hypotheses} + else: + outcome = { + "status": "completed", + "root_cause": synthesis.get("root_cause"), + "confidence": synthesis.get("confidence"), + "recommendations": synthesis.get("recommendations") or [], + "supporting_evidence": synthesis.get("supporting_evidence") or [], + "hypotheses": hypotheses, + "counter_analysis": payload.get("counter_analysis"), + } await app_db.execute( "UPDATE investigations SET outcome = $3 WHERE id = $1 AND tenant_id = $2", investigation_id, @@ -98,7 +109,11 @@ async def publish_investigation_outcome(payload: dict[str, Any]) -> dict[str, An thread["id"], author_kind="agent", kind="investigation", - body_md=outcome_markdown(synthesis, hypotheses), + body_md=( + failure_markdown(failure) + if failure + else outcome_markdown(synthesis, hypotheses) + ), payload={ "phase": "outcome", "outcome_for": str(investigation_id), @@ -107,18 +122,19 @@ async def publish_investigation_outcome(payload: dict[str, Any]) -> dict[str, An "outcome": outcome, }, ) + event_type, event = ( + ("investigation_failed", {"code": failure.get("code")}) + if failure + else ("investigation_completed", {"confidence": synthesis.get("confidence")}) + ) await app_db.execute( """ INSERT INTO issue_events (issue_id, event_type, payload) - VALUES ($1, 'investigation_completed', $2) + VALUES ($1, $2, $3) """, issue_id, - to_json_string( - { - "investigation_id": str(investigation_id), - "confidence": synthesis.get("confidence"), - } - ), + event_type, + to_json_string({"investigation_id": str(investigation_id), **event}), ) return {"published": True, "thread_message": existing is None} diff --git a/python-packages/dataing/src/dataing/temporal/activities/synthesize.py b/python-packages/dataing/src/dataing/temporal/activities/synthesize.py index af2fbc42..03311f7d 100644 --- a/python-packages/dataing/src/dataing/temporal/activities/synthesize.py +++ b/python-packages/dataing/src/dataing/temporal/activities/synthesize.py @@ -7,6 +7,9 @@ from temporalio import activity +from dataing.agents.errors import classify_llm_error +from dataing.temporal.errors import llm_activity_error + if TYPE_CHECKING: from dataing.temporal.adapters import TemporalAgentAdapter @@ -69,6 +72,9 @@ async def synthesize(input: SynthesizeInput) -> SynthesizeResult: ruled_out_hypotheses=input.ruled_out_hypotheses, ) except Exception as e: + failure = classify_llm_error(e) + if failure is not None: + raise llm_activity_error(failure) from e return SynthesizeResult( root_cause="", confidence=0.0, diff --git a/python-packages/dataing/src/dataing/temporal/errors.py b/python-packages/dataing/src/dataing/temporal/errors.py new file mode 100644 index 00000000..230ce4e6 --- /dev/null +++ b/python-packages/dataing/src/dataing/temporal/errors.py @@ -0,0 +1,78 @@ +"""How LLM failures travel through Temporal (docs/specs/0001_issue_chat.md §7.12). + +An LLM activity that the API refuses raises an ApplicationError whose type says +whether a retry can help: LLMRejected (a missing or rejected key, an unknown model, +a rejected request) is never retried; LLMUnavailable (rate limits, overload, server +and connection errors) is, by LLM_RETRY_POLICY. Its details carry the failure's code +and message, which the workflow reads back to fail the run with the reason. + +Workflows import this module, so it imports nothing beyond temporalio. +""" + +from __future__ import annotations + +from datetime import timedelta +from typing import TYPE_CHECKING, Any + +from temporalio.common import RetryPolicy +from temporalio.exceptions import ActivityError, ApplicationError, ChildWorkflowError + +if TYPE_CHECKING: + from dataing.agents.errors import LLMFailure + +LLM_REJECTED = "LLMRejected" +LLM_UNAVAILABLE = "LLMUnavailable" +# The error a failed investigation run ends with +INVESTIGATION_FAILED = "InvestigationFailed" + +LLM_MAX_ATTEMPTS = 4 +LLM_RETRY_POLICY = RetryPolicy( + initial_interval=timedelta(seconds=5), + backoff_coefficient=2.0, + maximum_interval=timedelta(seconds=60), + maximum_attempts=LLM_MAX_ATTEMPTS, + non_retryable_error_types=[LLM_REJECTED], +) + + +def llm_activity_error(failure: LLMFailure) -> ApplicationError: + """Return the error an activity raises for an LLM failure.""" + return ApplicationError( + failure.message, + failure.to_dict(), + type=LLM_UNAVAILABLE if failure.retryable else LLM_REJECTED, + non_retryable=not failure.retryable, + ) + + +def llm_failure_details(error: BaseException) -> dict[str, Any] | None: + """Return an LLM failure's code and message from an activity or child failure. + + Temporal wraps the activity's ApplicationError in ActivityError, and a child + workflow's failure in ChildWorkflowError; this unwraps both. `retried` is True + when the failure was retryable, so the retry policy already gave up on it, and + `activity` names the activity that failed. + + Returns: + {"code", "message", "retried", "activity"}, or None when the failure wasn't + the LLM's. + """ + current: BaseException | None = error + activity = None + while isinstance(current, ChildWorkflowError | ActivityError): + if isinstance(current, ActivityError): + activity = current.activity_type + current = current.cause + if not isinstance(current, ApplicationError): + return None + if current.type not in (LLM_REJECTED, LLM_UNAVAILABLE): + return None + details = current.details[0] if current.details else {} + if not isinstance(details, dict): + details = {} + return { + "code": str(details.get("code") or "llm_error"), + "message": str(details.get("message") or current.message), + "retried": current.type == LLM_UNAVAILABLE, + "activity": activity, + } diff --git a/python-packages/dataing/src/dataing/temporal/workflows/evaluate_hypothesis.py b/python-packages/dataing/src/dataing/temporal/workflows/evaluate_hypothesis.py index 4758032c..3b5a89a2 100644 --- a/python-packages/dataing/src/dataing/temporal/workflows/evaluate_hypothesis.py +++ b/python-packages/dataing/src/dataing/temporal/workflows/evaluate_hypothesis.py @@ -12,6 +12,7 @@ GenerateQueryInput, InterpretEvidenceInput, ) + from dataing.temporal.errors import LLM_RETRY_POLICY @dataclass @@ -60,6 +61,11 @@ async def run(self, input: EvaluateHypothesisInput) -> EvaluateHypothesisResult: EvaluateHypothesisResult with evidence gathered. """ hypothesis_id = input.hypothesis.get("id", f"h-{input.hypothesis_index}") + # With llm-failures-v1, LLM activities retry per LLM_RETRY_POLICY; a failure the + # model caused fails this child, and the parent fails the run + llm_options: dict[str, Any] = ( + {"retry_policy": LLM_RETRY_POLICY} if workflow.patched("llm-failures-v1") else {} + ) # Step 1: Generate SQL query to test this hypothesis query_input = GenerateQueryInput( @@ -73,6 +79,7 @@ async def run(self, input: EvaluateHypothesisInput) -> EvaluateHypothesisResult: "generate_query", query_input, start_to_close_timeout=timedelta(minutes=2), + **llm_options, ) if query_result.get("error"): @@ -128,6 +135,7 @@ async def run(self, input: EvaluateHypothesisInput) -> EvaluateHypothesisResult: "interpret_evidence", interpret_input, start_to_close_timeout=timedelta(minutes=2), + **llm_options, ) # A failed interpretation is not a refutation: report it, never use it as evidence diff --git a/python-packages/dataing/src/dataing/temporal/workflows/investigation.py b/python-packages/dataing/src/dataing/temporal/workflows/investigation.py index cf5e8d9a..7710a5bc 100644 --- a/python-packages/dataing/src/dataing/temporal/workflows/investigation.py +++ b/python-packages/dataing/src/dataing/temporal/workflows/investigation.py @@ -7,7 +7,12 @@ from temporalio import workflow from temporalio.common import RetryPolicy -from temporalio.exceptions import ActivityError, CancelledError, ChildWorkflowError +from temporalio.exceptions import ( + ActivityError, + ApplicationError, + CancelledError, + ChildWorkflowError, +) from temporalio.workflow import ChildWorkflowCancellationType, ChildWorkflowHandle with workflow.unsafe.imports_passed_through(): @@ -20,6 +25,12 @@ GenerateHypothesesInput, SynthesizeInput, ) + from dataing.temporal.errors import ( + INVESTIGATION_FAILED, + LLM_MAX_ATTEMPTS, + LLM_RETRY_POLICY, + llm_failure_details, + ) from dataing.temporal.workflows.evaluate_hypothesis import ( EvaluateHypothesisInput, EvaluateHypothesisWorkflow, @@ -86,6 +97,25 @@ class InvestigationQueryStatus: pending_steers: list[dict[str, Any]] = field(default_factory=list) +class _RunFailed(Exception): + """Ends a run as failed, with the reason (runs with the llm-failures-v1 patch). + + Raised inside the workflow and turned into an InvestigationFailed error by run(), + after the failure is published. See docs/specs/0001_issue_chat.md §7.12. + """ + + def __init__(self, code: str, message: str, step: str) -> None: + """Initialize with the failure's code, what to fix, and the step that failed.""" + super().__init__(message) + self.code = code + self.message = message + self.step = step + + def details(self) -> dict[str, str]: + """Return the failure as outcome payloads and error details carry it.""" + return {"code": self.code, "message": self.message, "step": self.step} + + @workflow.defn class InvestigationWorkflow: """Main investigation workflow that orchestrates the full investigation process. @@ -102,6 +132,9 @@ class InvestigationWorkflow: - cancel_investigation: Gracefully cancel the investigation - steer: A person's steer, applied at the next checkpoint (runs with the steering-v1 patch; see steering.py) + + With the llm-failures-v1 patch, a run the model can't serve fails with the reason + instead of concluding from nothing (docs/specs/0001_issue_chat.md §7.12). """ def __init__(self) -> None: @@ -138,6 +171,9 @@ def __init__(self) -> None: self._input: InvestigationInput | None = None self._schema_info: dict[str, Any] = {} self._alert_summary = "" + # LLM failures fail the run (llm-failures-v1) + self._fail_runs = False + self._stopped: set[str] = set() # Hypotheses a person's stop left untested @workflow.signal def cancel_investigation(self) -> None: @@ -273,7 +309,16 @@ async def run(self, input: InvestigationInput) -> InvestigationResult: Returns: InvestigationResult with status and findings. """ - result = await self._investigate(input) + try: + result = await self._investigate(input) + except _RunFailed as failure: + await self._fail(input, failure) + raise ApplicationError( + failure.message, + failure.details(), + type=INVESTIGATION_FAILED, + non_retryable=True, + ) from None if self._steering: # Steers the run never reached end here, so none stays pending rejection = "The investigation was cancelled" if result.status == "cancelled" else None @@ -292,6 +337,7 @@ async def _investigate(self, input: InvestigationInput) -> InvestigationResult: self._snapshot_paths = [] self._max_hypotheses = input.max_hypotheses self._steering = workflow.patched("steering-v1") + self._fail_runs = workflow.patched("llm-failures-v1") alert_summary = input.alert_summary or str(input.alert_data) @@ -390,15 +436,21 @@ async def _investigate(self, input: InvestigationInput) -> InvestigationResult: matched_patterns=matched_patterns, max_hypotheses=input.max_hypotheses, ) - hypotheses_result = await workflow.execute_activity( - "generate_hypotheses", - hypotheses_input, - start_to_close_timeout=timedelta(minutes=5), + hypotheses_result = await self._llm_activity( + "generate_hypotheses", hypotheses_input, timedelta(minutes=5) ) hypotheses = hypotheses_result.get("hypotheses", []) if hypotheses_result.get("error"): err = hypotheses_result["error"] workflow.logger.warning(f"Hypothesis generation warning: {err}") + if self._fail_runs: + raise _RunFailed("hypotheses_failed", err, "generate_hypotheses") + if self._fail_runs and not hypotheses and not self._additions_queued(): + raise _RunFailed( + "no_hypotheses", + "The model proposed no hypotheses to test.", + "generate_hypotheses", + ) except CancelledError: return InvestigationResult( investigation_id=input.investigation_id, @@ -463,6 +515,8 @@ async def _investigate(self, input: InvestigationInput) -> InvestigationResult: evidence=evidence, snapshot_paths=self._snapshot_paths, ) + if self._fail_runs: + self._require_evidence(evidence, untested_hypotheses) # Step 5: Synthesize findings self._current_step = "synthesize" @@ -490,6 +544,8 @@ async def _investigate(self, input: InvestigationInput) -> InvestigationResult: } if synthesize_result.get("error"): workflow.logger.warning(f"Synthesis warning: {synthesize_result['error']}") + if self._fail_runs: + raise _RunFailed("synthesis_failed", synthesize_result["error"], "synthesize") except CancelledError: return InvestigationResult( investigation_id=input.investigation_id, @@ -558,11 +614,14 @@ async def _investigate(self, input: InvestigationInput) -> InvestigationResult: evidence=evidence, hypotheses=hypotheses, ) - counter_result = await workflow.execute_activity( - "counter_analyze", - counter_input, - start_to_close_timeout=timedelta(minutes=5), - ) + try: + counter_result = await self._llm_activity( + "counter_analyze", counter_input, timedelta(minutes=5) + ) + except _RunFailed as failure: + # Counter-analysis only checks the conclusion: keep the conclusion + workflow.logger.warning(f"Counter-analysis failed: {failure.message}") + counter_result = {"error": failure.message, "error_code": failure.code} # Build counter_analysis dict from result fields counter_analysis = { "alternative_explanations": counter_result.get("alternative_explanations", []), @@ -572,6 +631,13 @@ async def _investigate(self, input: InvestigationInput) -> InvestigationResult: } if counter_result.get("error"): workflow.logger.warning(f"Counter-analysis warning: {counter_result['error']}") + if self._fail_runs: + counter_analysis = { + "error": { + "code": counter_result.get("error_code", "counter_analysis_failed"), + "message": counter_result["error"], + } + } except CancelledError: return InvestigationResult( investigation_id=input.investigation_id, @@ -689,13 +755,93 @@ async def _synthesize( alert=self._alert(input.alert_data), ruled_out_hypotheses=ruled_out, ) - result: dict[str, Any] = await workflow.execute_activity( - "synthesize", - synthesize_input, - start_to_close_timeout=timedelta(minutes=5), - ) + return await self._llm_activity("synthesize", synthesize_input, timedelta(minutes=5)) + + async def _llm_activity(self, name: str, arg: Any, timeout: timedelta) -> dict[str, Any]: + """Run an LLM activity. + + With llm-failures-v1 it retries per LLM_RETRY_POLICY, and a failure the model + caused fails the run with the reason. + """ + if not self._fail_runs: + result: dict[str, Any] = await workflow.execute_activity( + name, arg, start_to_close_timeout=timeout + ) + return result + try: + result = await workflow.execute_activity( + name, arg, start_to_close_timeout=timeout, retry_policy=LLM_RETRY_POLICY + ) + except ActivityError as e: + failure = _llm_run_failure(e, step=name) + if failure is None: + raise + raise failure from None return result + def _additions_queued(self) -> bool: + """Return whether a person has asked for a hypothesis the run hasn't added yet.""" + return any(steer.kind == "add_hypothesis" for steer in self._steers) + + def _require_evidence( + self, evidence: list[dict[str, Any]], untested: list[dict[str, Any]] + ) -> None: + """Fail the run when errors left every hypothesis untested. + + Hypotheses a person ruled out or stopped don't count: concluding without them + was that person's choice. + """ + if evidence: + return + failed = [ + u + for u in untested + if u["hypothesis_id"] not in self._stopped and u["hypothesis_id"] not in self._ruled_out + ] + if failed: + raise _RunFailed( + "no_evidence", + f"No hypothesis could be tested. The first error: {failed[0]['error']}", + "evaluate_hypotheses", + ) + + async def _fail(self, input: InvestigationInput, failure: _RunFailed) -> None: + """End a failed run: stop its subagents, publish the failure, answer steers.""" + for handle in self._running.values(): + handle.cancel() + self._running.clear() + for hypothesis_id, status in list(self._statuses.items()): + if status in ("pending", "running"): + self._statuses[hypothesis_id] = "untested" + self._current_step = "failed" + self._is_complete = True + payload = { + "investigation_id": input.investigation_id, + "tenant_id": input.tenant_id, + "issue_id": (input.alert_data or {}).get("issue_id"), + "failure": failure.details(), + "hypotheses": [ + { + "id": _hypothesis_id(hypothesis, index), + "title": hypothesis.get("title", ""), + "status": self._statuses.get(_hypothesis_id(hypothesis, index), "untested"), + } + for index, hypothesis in enumerate(self._hypotheses) + ], + } + try: + await workflow.execute_activity( + "publish_investigation_outcome", + payload, + start_to_close_timeout=timedelta(minutes=1), + retry_policy=RetryPolicy(maximum_attempts=5), + ) + except Exception as e: + workflow.logger.warning(f"Publishing the failure failed (non-fatal): {e}") + if self._steering: + while self._steers: + await self._apply_steers(Phase.FINISHED, rejection="The investigation failed") + def _run_state(self) -> RunState: """Return what steer decisions need to know about the run.""" return RunState( @@ -751,6 +897,7 @@ async def _carry_out(self, steer: Steer, decision: Decision) -> tuple[bool, str] for running_id, handle in list(self._running.items()): handle.cancel() self._statuses[running_id] = "untested" + self._stopped.add(running_id) index = self._index(running_id) self._untested.append( _untested( @@ -852,6 +999,9 @@ def _collect(self, hypothesis_id: str, handle: ChildWorkflowHandle[Any, Any]) -> workflow.logger.warning(f"Child workflow failed: {reason}") self._untested.append(_untested(hypothesis, index, f"Evaluation failed: {reason}")) self._statuses[hypothesis_id] = "untested" + # The model fails every subagent the same way, so the run stops here + if self._fail_runs and (failure := _llm_run_failure(e)) is not None: + raise failure from None return self._hypotheses_evaluated += 1 if result.error: @@ -1045,6 +1195,17 @@ def _untested(hypothesis: dict[str, Any], index: int, error: str) -> dict[str, A } +def _llm_run_failure(error: BaseException, step: str | None = None) -> _RunFailed | None: + """Return the run failure for an activity or child failure the model caused.""" + details = llm_failure_details(error) + if details is None: + return None + message = details["message"] + if details["retried"]: + message = f"{message} It kept failing after {LLM_MAX_ATTEMPTS} attempts; try again later." + return _RunFailed(details["code"], message, step or details["activity"] or "evaluate") + + def _describe_failure(error: BaseException) -> str: """Describe why a child evaluation failed, without Temporal's wrapper errors.""" while isinstance(error, ChildWorkflowError | ActivityError): diff --git a/python-packages/dataing/tests/fixtures/investigation_env.py b/python-packages/dataing/tests/fixtures/investigation_env.py index 4b7fe93b..87193f7b 100644 --- a/python-packages/dataing/tests/fixtures/investigation_env.py +++ b/python-packages/dataing/tests/fixtures/investigation_env.py @@ -1,8 +1,9 @@ """Fake activities for running InvestigationWorkflow on Temporal's test server. Each fake returns the dict shape the workflow reads. Hypothesis evaluations can be -held open (to steer a run while its subagents are working) and every call is -recorded, so tests can assert what the manager and its subagents did. +held open (to steer a run while its subagents are working), calls can be scripted to +fail, and every call is recorded, so tests can assert what the manager and its +subagents did. """ from __future__ import annotations @@ -42,6 +43,11 @@ class FakeInvestigation: # Hypothesis ids whose query generation, or activity names, wait for `released` hold: set[str] = field(default_factory=set) released: asyncio.Event = field(default_factory=asyncio.Event) + # Activity names, or ":" for a subagent's activity, mapped + # to the errors their calls raise in turn; once a list runs out, calls succeed + failures: dict[str, list[BaseException]] = field(default_factory=dict) + # Hypothesis ids whose query fails, with the error execute_query reports + query_errors: dict[str, str] = field(default_factory=dict) calls: list[tuple[str, dict[str, Any]]] = field(default_factory=list) def names(self) -> list[str]: @@ -52,6 +58,13 @@ def inputs(self, name: str) -> list[dict[str, Any]]: """Return the inputs of every call to one activity.""" return [payload for called, payload in self.calls if called == name] + def fail_if_scripted(self, *keys: str) -> None: + """Raise the next scripted error for the first key that still has one.""" + for key in keys: + errors = self.failures.get(key) + if errors: + raise errors.pop(0) + async def wait_if_held(self, name: str) -> None: """Keep an activity running until released, heartbeating so it can be cancelled.""" if name in self.hold: @@ -80,6 +93,7 @@ async def check_patterns(payload: dict[str, Any]) -> dict[str, Any]: @activity.defn(name="generate_hypotheses") async def generate_hypotheses(payload: dict[str, Any]) -> dict[str, Any]: record("generate_hypotheses", payload) + fake.fail_if_scripted("generate_hypotheses") return {"hypotheses": fake.hypotheses} @activity.defn(name="capture_snapshot") @@ -91,16 +105,22 @@ async def generate_query(payload: dict[str, Any]) -> dict[str, Any]: record("generate_query", payload) hypothesis_id = payload["hypothesis"]["id"] await fake.wait_if_held(hypothesis_id) + fake.fail_if_scripted("generate_query", f"generate_query:{hypothesis_id}") return {"query": f"SELECT 1 -- {hypothesis_id}"} @activity.defn(name="execute_query") async def execute_query(payload: dict[str, Any]) -> dict[str, Any]: record("execute_query", payload) + error = fake.query_errors.get(payload["hypothesis_id"]) + if error: + return {"error": error} return {"columns": ["n"], "rows": [{"n": 1}], "row_count": 1} @activity.defn(name="interpret_evidence") async def interpret_evidence(payload: dict[str, Any]) -> dict[str, Any]: record("interpret_evidence", payload) + hypothesis_id = payload["hypothesis"]["id"] + fake.fail_if_scripted("interpret_evidence", f"interpret_evidence:{hypothesis_id}") return { "supports_hypothesis": payload["hypothesis"]["id"] == "h1", "confidence": 0.8, @@ -112,6 +132,7 @@ async def interpret_evidence(payload: dict[str, Any]) -> dict[str, Any]: async def synthesize(payload: dict[str, Any]) -> dict[str, Any]: record("synthesize", payload) await fake.wait_if_held("synthesize") + fake.fail_if_scripted("synthesize") return { "root_cause": "app_v2 writes COMPLETE instead of completed", "confidence": fake.confidence, @@ -128,6 +149,7 @@ async def finalize_evidence_chain(payload: dict[str, Any]) -> dict[str, Any]: async def counter_analyze(payload: dict[str, Any]) -> dict[str, Any]: record("counter_analyze", payload) await fake.wait_if_held("counter_analyze") + fake.fail_if_scripted("counter_analyze") return {"alternative_explanations": [], "weaknesses": [], "recommendation": "accept"} @activity.defn(name="publish_investigation_outcome") diff --git a/python-packages/dataing/tests/fixtures/llm_errors.py b/python-packages/dataing/tests/fixtures/llm_errors.py new file mode 100644 index 00000000..bd6020eb --- /dev/null +++ b/python-packages/dataing/tests/fixtures/llm_errors.py @@ -0,0 +1,37 @@ +"""LLM errors as the code really sees them, for tests. + +A failed Anthropic call reaches dataing as the SDK's error, wrapped by pydantic-ai, +wrapped again by AgentClient's LLMError. +""" + +from __future__ import annotations + +from typing import Any + +import anthropic +import httpx2 +from pydantic_ai.exceptions import ModelHTTPError + +from dataing.core.exceptions import LLMError + +MODEL = "claude-sonnet-4-20250514" +REQUEST = httpx2.Request("POST", "https://api.anthropic.com/v1/messages") + + +def api_body(error_type: str, message: str) -> dict[str, Any]: + """Return an Anthropic error response body.""" + return {"type": "error", "error": {"type": error_type, "message": message}} + + +def anthropic_error(status: int, body: dict[str, Any] | None = None) -> LLMError: + """Return the error AgentClient raises when Anthropic answers with this status.""" + sdk_error = anthropic.APIStatusError( + f"Error code: {status}", + response=httpx2.Response(status, request=REQUEST, json=body or {}), + body=body, + ) + model_error = ModelHTTPError(status_code=status, model_name=MODEL, body=body) + model_error.__cause__ = sdk_error + wrapped = LLMError(f"LLM call failed: {model_error}", retryable=False) + wrapped.__cause__ = model_error + return wrapped diff --git a/python-packages/dataing/tests/integration/test_publish_outcome.py b/python-packages/dataing/tests/integration/test_publish_outcome.py index 72661723..fe52975b 100644 --- a/python-packages/dataing/tests/integration/test_publish_outcome.py +++ b/python-packages/dataing/tests/integration/test_publish_outcome.py @@ -130,3 +130,56 @@ async def test_runs_without_an_issue_only_complete_the_investigation( ) assert investigation is not None assert investigation["status"] == "completed" + + +async def test_a_failed_run_is_published_with_its_reason(migrated_db: AppDatabase) -> None: + """A failed run ends with the reason on the investigation, the run and the thread.""" + payload = await _issue_with_run(migrated_db) + failure = { + "code": "invalid_key", + "message": "Anthropic rejected the API key (401). Set a valid ANTHROPIC_API_KEY and " + "restart the API and the worker.", + "step": "generate_hypotheses", + } + del payload["synthesis"], payload["counter_analysis"] + payload["failure"] = failure + payload["hypotheses"] = [] + publish = make_publish_investigation_outcome_activity(migrated_db) + + await ActivityEnvironment().run(publish, payload) + + investigation = await migrated_db.fetch_one( + "SELECT outcome, completed_at FROM investigations WHERE id = $1", + uuid.UUID(payload["investigation_id"]), + ) + assert investigation is not None + assert json.loads(investigation["outcome"]) == { + "status": "failed", + "error": failure, + "hypotheses": [], + } + assert investigation["completed_at"] is not None + + run = await migrated_db.fetch_one( + "SELECT synthesis_summary, confidence, completed_at FROM issue_investigation_runs " + "WHERE investigation_id = $1", + uuid.UUID(payload["investigation_id"]), + ) + assert run is not None + assert (run["synthesis_summary"], run["confidence"]) == (None, None) + assert run["completed_at"] is not None + + threads = IssueThreadRepository(migrated_db) + thread = await threads.ensure_shared_thread(uuid.UUID(payload["issue_id"])) + (card,) = (m for m in await threads.list_messages(thread["id"]) if m["kind"] == "investigation") + assert card["payload"]["phase"] == "outcome" + assert card["payload"]["outcome"]["status"] == "failed" + assert card["body_md"] == f"**Investigation failed**\n\n{failure['message']}" + + event = await migrated_db.fetch_one( + "SELECT payload FROM issue_events WHERE issue_id = $1 AND event_type = " + "'investigation_failed'", + uuid.UUID(payload["issue_id"]), + ) + assert event is not None + assert json.loads(event["payload"])["code"] == "invalid_key" diff --git a/python-packages/dataing/tests/unit/agents/test_client.py b/python-packages/dataing/tests/unit/agents/test_client.py index d81718b9..a0e88304 100644 --- a/python-packages/dataing/tests/unit/agents/test_client.py +++ b/python-packages/dataing/tests/unit/agents/test_client.py @@ -15,6 +15,7 @@ import pytest from bond import BondAgent from pydantic_ai import models +from pydantic_ai.exceptions import ModelHTTPError from pydantic_ai.messages import ( ModelMessage, ModelMessagesTypeAdapter, @@ -36,6 +37,7 @@ ) from dataing.agents import client as client_module from dataing.agents.client import AgentClient +from dataing.agents.errors import classify_llm_error from dataing.core.domain_types import ( AnomalyAlert, Evidence, @@ -44,6 +46,7 @@ InvestigationContext, MetricSpec, ) +from dataing.core.exceptions import LLMError LlmCall = Callable[[AgentClient, str], Awaitable[object]] RecordedRequests = list[list[ModelMessage]] @@ -229,3 +232,37 @@ def test_client_holds_no_bond_agents(self) -> None: held = [name for name, value in vars(client).items() if isinstance(value, BondAgent)] assert held == [] + + +class TestAgentClientErrors: + """A failed model call surfaces as LLMError that keeps the API error as its cause.""" + + @pytest.mark.parametrize( + "call", + [ + pytest.param(_generate_hypotheses, id="generate_hypotheses"), + pytest.param(_generate_query, id="generate_query"), + pytest.param(_interpret_evidence, id="interpret_evidence"), + pytest.param(_synthesize_findings, id="synthesize_findings"), + pytest.param(_counter_analyze, id="counter_analyze"), + ], + ) + async def test_a_rejected_key_is_raised_never_turned_into_a_result( + self, monkeypatch: pytest.MonkeyPatch, call: LlmCall + ) -> None: + """No call turns an API error into a result, e.g. evidence that reads as refuted.""" + + def reject(messages: list[ModelMessage], info: AgentInfo) -> ModelResponse: + raise ModelHTTPError(status_code=401, model_name="claude-x", body=None) + + model = FunctionModel(reject) + monkeypatch.setattr(client_module, "AnthropicModel", lambda *args, **kwargs: model) + monkeypatch.setattr(models, "ALLOW_MODEL_REQUESTS", False) + client = AgentClient(api_key="test-key") + + with pytest.raises(LLMError) as caught: + await call(client, "tenant_a") + + failure = classify_llm_error(caught.value) + assert failure is not None + assert failure.code == "invalid_key" diff --git a/python-packages/dataing/tests/unit/agents/test_llm_errors.py b/python-packages/dataing/tests/unit/agents/test_llm_errors.py new file mode 100644 index 00000000..e84d8390 --- /dev/null +++ b/python-packages/dataing/tests/unit/agents/test_llm_errors.py @@ -0,0 +1,134 @@ +"""Classifying LLM errors (docs/specs/0001_issue_chat.md §7.12). + +Each case builds the exception chain the code really sees: the Anthropic SDK's error, +wrapped by pydantic-ai, wrapped again by AgentClient's LLMError. +""" + +from __future__ import annotations + +import anthropic +import httpx2 +import pytest +from fixtures.llm_errors import MODEL, REQUEST, anthropic_error, api_body +from pydantic_ai.exceptions import ModelAPIError, UnexpectedModelBehavior +from pydantic_ai.providers.anthropic import AnthropicProvider + +from dataing.agents.errors import classify_llm_error +from dataing.core.exceptions import LLMError + + +@pytest.mark.parametrize( + ("status", "code", "retryable", "message"), + [ + ( + 401, + "invalid_key", + False, + "Anthropic rejected the API key (401). Set a valid ANTHROPIC_API_KEY and " + "restart the API and the worker.", + ), + (403, "forbidden", False, f"The API key isn't allowed to use {MODEL} (403)."), + ( + 404, + "unknown_model", + False, + f"Anthropic doesn't know the model {MODEL} (404). " + "Check LLM_MODEL and CHAT_AGENT_MODEL.", + ), + (429, "rate_limited", True, "Anthropic rate-limited the request (429)."), + (529, "overloaded", True, "Anthropic is overloaded (529)."), + (500, "server_error", True, "Anthropic returned an error (500)."), + (503, "server_error", True, "Anthropic returned an error (503)."), + ], +) +def test_status_codes_map_to_a_code_and_what_to_fix( + status: int, code: str, retryable: bool, message: str +) -> None: + """Each HTTP status the API returns maps to one code, retryability and message.""" + failure = classify_llm_error(anthropic_error(status)) + + assert failure is not None + assert (failure.code, failure.retryable, failure.message) == (code, retryable, message) + assert failure.status_code == status + + +@pytest.mark.parametrize("status", [400, 413, 422]) +def test_rejected_requests_carry_the_api_detail(status: int) -> None: + """A request the API refuses says why, in the API's words, and isn't retried.""" + body = api_body("invalid_request_error", "Your credit balance is too low") + + failure = classify_llm_error(anthropic_error(status, body)) + + assert failure is not None + assert failure.code == "bad_request" + assert not failure.retryable + assert failure.message == ( + f"Anthropic rejected the request ({status}): Your credit balance is too low" + ) + + +def test_connection_errors_are_retryable() -> None: + """pydantic-ai maps a connection error to ModelAPIError; it is worth retrying.""" + sdk_error = anthropic.APIConnectionError(request=REQUEST) + model_error = ModelAPIError(model_name=MODEL, message=sdk_error.message) + model_error.__cause__ = sdk_error + + failure = classify_llm_error(model_error) + + assert failure is not None + assert (failure.code, failure.retryable) == ("unreachable", True) + assert failure.message == "Couldn't reach Anthropic: Connection error." + + +def test_direct_sdk_errors_are_classified_too() -> None: + """The key check calls the SDK directly, without pydantic-ai in between.""" + response = httpx2.Response(401, request=REQUEST, json=api_body("authentication_error", "x")) + sdk_error = anthropic.AuthenticationError("Error code: 401", response=response, body=None) + + failure = classify_llm_error(sdk_error) + + assert failure is not None + assert failure.code == "invalid_key" + + +def test_sdk_timeouts_are_unreachable() -> None: + """A timeout is a connection problem: retry it.""" + failure = classify_llm_error(anthropic.APITimeoutError(request=REQUEST)) + + assert failure is not None + assert (failure.code, failure.retryable) == ("unreachable", True) + + +def test_a_missing_key_is_reported_without_a_request() -> None: + """pydantic-ai refuses to build a provider without a key.""" + with pytest.raises(Exception) as caught: + AnthropicProvider(api_key="") + + failure = classify_llm_error(caught.value) + + assert failure is not None + assert (failure.code, failure.retryable) == ("missing_key", False) + assert failure.message == ( + "ANTHROPIC_API_KEY isn't set. Set it and restart the API and the worker." + ) + + +@pytest.mark.parametrize( + "error", + [ + RuntimeError("LLM request timed out"), + LLMError("Synthesis failed: bad output"), + UnexpectedModelBehavior("Exceeded maximum retries (3) for output validation"), + ], +) +def test_other_errors_are_not_llm_api_failures(error: Exception) -> None: + """Only errors from the API or its client are classified; the rest keep their path.""" + assert classify_llm_error(error) is None + + +def test_to_dict_carries_code_and_message() -> None: + """The dict form travels in Temporal error details and outcome payloads.""" + failure = classify_llm_error(anthropic_error(401)) + + assert failure is not None + assert failure.to_dict() == {"code": "invalid_key", "message": failure.message} diff --git a/python-packages/dataing/tests/unit/temporal/test_investigation_failures.py b/python-packages/dataing/tests/unit/temporal/test_investigation_failures.py new file mode 100644 index 00000000..a41a1b2e --- /dev/null +++ b/python-packages/dataing/tests/unit/temporal/test_investigation_failures.py @@ -0,0 +1,238 @@ +"""A run that can't reach the model fails with the reason (docs/specs/0001_issue_chat.md §7.12). + +Runs execute on Temporal's test server with fake activities. The fakes raise the +errors the real LLM activities raise: LLMRejected, which fails the run at once, and +LLMUnavailable, which the retry policy tries again before the run fails. +""" + +from __future__ import annotations + +import asyncio +from collections.abc import AsyncIterator, Awaitable, Callable +from typing import Any + +import pytest +from fixtures.investigation_env import INVESTIGATION_TASK_QUEUE, FakeInvestigation +from fixtures.llm_errors import anthropic_error +from fixtures.record_investigation_histories import investigation_input +from temporalio.client import WorkflowFailureError, WorkflowHandle +from temporalio.exceptions import ApplicationError +from temporalio.testing import WorkflowEnvironment +from temporalio.worker import Worker + +from dataing.agents.errors import classify_llm_error +from dataing.temporal.errors import llm_activity_error +from dataing.temporal.sandbox import workflow_runner +from dataing.temporal.workflows import ( + EvaluateHypothesisWorkflow, + InvestigationWorkflow, +) + +INVALID_KEY = ( + "Anthropic rejected the API key (401). Set a valid ANTHROPIC_API_KEY and " + "restart the API and the worker." +) + + +@pytest.fixture +async def env() -> AsyncIterator[WorkflowEnvironment]: + """Return a time-skipping Temporal test environment.""" + async with await WorkflowEnvironment.start_time_skipping() as environment: + yield environment + + +def rejected() -> ApplicationError: + """Return the error an LLM activity raises when Anthropic rejects the key.""" + failure = classify_llm_error(anthropic_error(401)) + assert failure is not None + return llm_activity_error(failure) + + +def overloaded() -> ApplicationError: + """Return the error an LLM activity raises when Anthropic is overloaded.""" + failure = classify_llm_error(anthropic_error(529)) + assert failure is not None + return llm_activity_error(failure) + + +async def run( + env: WorkflowEnvironment, + fake: FakeInvestigation, + during: Callable[[WorkflowHandle[Any, Any]], Awaitable[None]] | None = None, +) -> Any: + """Run an investigation and return its result, or the failure it ended with.""" + workflow_id = f"fail-{id(fake)}" + async with Worker( + env.client, + workflow_runner=workflow_runner(), + task_queue=INVESTIGATION_TASK_QUEUE, + workflows=[InvestigationWorkflow, EvaluateHypothesisWorkflow], + activities=fake.activities(), + ): + handle = await env.client.start_workflow( + InvestigationWorkflow.run, + investigation_input(workflow_id), + id=workflow_id, + task_queue=INVESTIGATION_TASK_QUEUE, + ) + try: + if during is not None: + with env.auto_time_skipping_disabled(): + await during(handle) + return await handle.result() + except WorkflowFailureError as e: + return e + finally: + fake.released.set() + + +def failure_of(outcome: Any) -> dict[str, Any]: + """Return the code, message and step a failed run ended with.""" + assert isinstance(outcome, WorkflowFailureError), f"the run didn't fail: {outcome}" + cause = outcome.cause + assert isinstance(cause, ApplicationError) + assert cause.type == "InvestigationFailed" + assert cause.non_retryable + (details,) = cause.details + assert details["message"] == cause.message + return dict(details) + + +def published_failure(fake: FakeInvestigation) -> dict[str, Any]: + """Return the failure the run published for the app and the issue thread.""" + (published,) = fake.inputs("publish_investigation_outcome") + failure: dict[str, Any] = published["failure"] + return failure + + +async def until(condition: Callable[[], bool]) -> None: + """Wait (in real time) until a condition over the fake's calls holds.""" + for _ in range(500): + if condition(): + return + await asyncio.sleep(0.02) + raise AssertionError("condition never held") + + +async def test_a_rejected_key_fails_the_run_with_what_to_fix(env: WorkflowEnvironment) -> None: + """A 401 at hypothesis generation ends the run at once: no retry, no synthesis.""" + fake = FakeInvestigation(failures={"generate_hypotheses": [rejected()]}) + + outcome = await run(env, fake) + + expected = {"code": "invalid_key", "message": INVALID_KEY, "step": "generate_hypotheses"} + assert failure_of(outcome) == expected + assert published_failure(fake) == expected + assert len(fake.inputs("generate_hypotheses")) == 1 + assert "synthesize" not in fake.names() + + +async def test_an_overloaded_api_is_retried_before_the_run_moves_on( + env: WorkflowEnvironment, +) -> None: + """Overload is transient: the activity is retried and the run completes.""" + fake = FakeInvestigation(failures={"generate_hypotheses": [overloaded(), overloaded()]}) + + result = await run(env, fake) + + assert result.status == "completed" + assert len(fake.inputs("generate_hypotheses")) == 3 + + +async def test_an_api_that_stays_overloaded_fails_the_run(env: WorkflowEnvironment) -> None: + """After the retry policy's four attempts, the run fails and says to try later.""" + fake = FakeInvestigation(failures={"synthesize": [overloaded() for _ in range(4)]}) + + outcome = await run(env, fake) + + failure = failure_of(outcome) + assert (failure["code"], failure["step"]) == ("overloaded", "synthesize") + assert failure["message"] == ( + "Anthropic is overloaded (529). It kept failing after 4 attempts; try again later." + ) + assert len(fake.inputs("synthesize")) == 4 + + +async def test_a_subagent_llm_error_stops_the_others_and_fails_the_run( + env: WorkflowEnvironment, +) -> None: + """The key is broken for every subagent, so one LLM error ends the whole run.""" + fake = FakeInvestigation(hold={"h3"}, failures={"interpret_evidence:h2": [rejected()]}) + + outcome = await run(env, fake) + + failure = failure_of(outcome) + assert (failure["code"], failure["step"]) == ("invalid_key", "interpret_evidence") + assert "synthesize" not in fake.names() + (published,) = fake.inputs("publish_investigation_outcome") + statuses = {h["id"]: h["status"] for h in published["hypotheses"]} + # h3 was still held, so the run ended without waiting for it + assert (statuses["h2"], statuses["h3"]) == ("untested", "untested") + + +async def test_a_run_where_no_hypothesis_could_be_tested_fails( + env: WorkflowEnvironment, +) -> None: + """With every query failing there is nothing to conclude from, so the run fails.""" + fake = FakeInvestigation( + query_errors={ + "h1": 'Query execution failed: relation "orders" does not exist', + "h2": 'Query execution failed: relation "orders" does not exist', + "h3": 'Query execution failed: relation "orders" does not exist', + } + ) + + outcome = await run(env, fake) + + failure = failure_of(outcome) + assert (failure["code"], failure["step"]) == ("no_evidence", "evaluate_hypotheses") + assert failure["message"] == ( + "No hypothesis could be tested. The first error: " + 'Query execution failed: relation "orders" does not exist' + ) + assert "synthesize" not in fake.names() + + +async def test_a_run_without_hypotheses_fails(env: WorkflowEnvironment) -> None: + """A model that proposes nothing leaves nothing to test.""" + fake = FakeInvestigation(hypotheses=[]) + + outcome = await run(env, fake) + + failure = failure_of(outcome) + assert (failure["code"], failure["step"]) == ("no_hypotheses", "generate_hypotheses") + assert failure["message"] == "The model proposed no hypotheses to test." + + +async def test_a_counter_analysis_failure_keeps_the_conclusion( + env: WorkflowEnvironment, +) -> None: + """Counter-analysis only checks a conclusion; losing it doesn't lose the conclusion.""" + fake = FakeInvestigation(confidence=0.5, failures={"counter_analyze": [rejected()]}) + + result = await run(env, fake) + + assert result.status == "completed" + assert result.synthesis["root_cause"] == "app_v2 writes COMPLETE instead of completed" + (published,) = fake.inputs("publish_investigation_outcome") + assert "failure" not in published + assert published["counter_analysis"] == { + "error": {"code": "invalid_key", "message": INVALID_KEY} + } + + +async def test_stopping_before_any_evidence_still_concludes(env: WorkflowEnvironment) -> None: + """Hypotheses a person stopped don't count as failures: the run concludes.""" + fake = FakeInvestigation(hold={"h1", "h2", "h3"}) + + async def stop(handle: WorkflowHandle[Any, Any]) -> None: + await until(lambda: len(fake.inputs("generate_query")) == 3) + await handle.signal( + InvestigationWorkflow.steer, + {"steer_id": "s1", "kind": "stop_and_synthesize", "actor_user_id": "user-1"}, + ) + + result = await run(env, fake, during=stop) + + assert result.status == "completed" + assert len(fake.inputs("synthesize")) == 1 diff --git a/python-packages/dataing/tests/unit/temporal/test_investigation_workflow.py b/python-packages/dataing/tests/unit/temporal/test_investigation_workflow.py index 54e661f6..9e749b82 100644 --- a/python-packages/dataing/tests/unit/temporal/test_investigation_workflow.py +++ b/python-packages/dataing/tests/unit/temporal/test_investigation_workflow.py @@ -11,6 +11,7 @@ import pytest from fixtures.temporal import FakeAgent, FakeDatasource, InProcessTemporal, sql_for +from temporalio.exceptions import ApplicationError from dataing.adapters.datasource.errors import ConnectionFailedError, QuerySyntaxError from dataing.adapters.datasource.types import QueryResult @@ -147,26 +148,26 @@ async def test_failed_interpretation_reaches_synthesis_as_untested_not_refuted( ] -async def test_datasource_down_reaches_synthesis_as_all_untested( +async def test_datasource_down_fails_the_run_with_the_query_error( monkeypatch: pytest.MonkeyPatch, ) -> None: - """When every query fails, synthesis gets no evidence and every hypothesis as untested.""" + """When every query fails there is nothing to conclude from: the run fails and says why. + + Synthesizing from no evidence would "complete" at confidence 0 and hide the cause + (docs/specs/0001_issue_chat.md §7.12). + """ down = ConnectionFailedError("connection refused") datasource = FakeDatasource({sql_for("h-1"): down, sql_for("h-2"): down}) agent = FakeAgent() - await _investigate(monkeypatch, datasource, agent) + with pytest.raises(ApplicationError) as caught: + await _investigate(monkeypatch, datasource, agent) - [synthesis] = agent.synthesized - assert synthesis["evidence"] == [] - assert synthesis["untested_hypotheses"] == [ - { - "hypothesis_id": hypothesis["id"], - "title": hypothesis["title"], - "error": "Query execution failed: connection refused", - } - for hypothesis in HYPOTHESES - ] + assert caught.value.type == "InvestigationFailed" + assert caught.value.message == ( + "No hypothesis could be tested. The first error: Query execution failed: connection refused" + ) + assert agent.synthesized == [] async def test_terminated_evaluation_reaches_synthesis_as_untested( diff --git a/python-packages/dataing/tests/unit/temporal/test_llm_activity_errors.py b/python-packages/dataing/tests/unit/temporal/test_llm_activity_errors.py new file mode 100644 index 00000000..67b1a048 --- /dev/null +++ b/python-packages/dataing/tests/unit/temporal/test_llm_activity_errors.py @@ -0,0 +1,156 @@ +"""LLM activities fail instead of returning empty results (docs/specs/0001_issue_chat.md §7.12). + +An error the API returns becomes a Temporal ApplicationError: LLMRejected, which no +retry can fix, or LLMUnavailable, which the activity's retry policy tries again. +Other errors keep coming back in the result's `error` field, as before. +""" + +from __future__ import annotations + +from collections.abc import Awaitable, Callable +from typing import Any + +import pytest +from fixtures.llm_errors import anthropic_error +from temporalio.exceptions import ApplicationError + +from dataing.temporal.activities import ( + CounterAnalyzeInput, + GenerateHypothesesInput, + GenerateQueryInput, + InterpretEvidenceInput, + SynthesizeInput, + make_counter_analyze_activity, + make_generate_hypotheses_activity, + make_generate_query_activity, + make_interpret_evidence_activity, + make_synthesize_activity, +) + +HYPOTHESIS = {"id": "h1", "title": "app_v2 writes a different status"} +INVALID_KEY = ( + "Anthropic rejected the API key (401). Set a valid ANTHROPIC_API_KEY and " + "restart the API and the worker." +) + + +class FailingAdapter: + """TemporalAgentAdapter stub whose every LLM call raises the same error.""" + + def __init__(self, error: Exception) -> None: + self.error = error + + async def generate_hypotheses_for_temporal(self, **_: Any) -> list[dict[str, Any]]: + raise self.error + + async def generate_query(self, **_: Any) -> str: + raise self.error + + async def interpret_evidence(self, **_: Any) -> dict[str, Any]: + raise self.error + + async def synthesize_findings_for_temporal(self, **_: Any) -> dict[str, Any]: + raise self.error + + async def counter_analyze(self, **_: Any) -> dict[str, Any]: + raise self.error + + +Run = Callable[[FailingAdapter], Awaitable[Any]] + +ACTIVITIES: list[tuple[str, Run]] = [ + ( + "generate_hypotheses", + lambda adapter: make_generate_hypotheses_activity(adapter)( # type: ignore[arg-type] + GenerateHypothesesInput( + investigation_id="inv-1", + alert_summary="orders dropped", + alert=None, + schema_info=None, + lineage_info=None, + matched_patterns=[], + ) + ), + ), + ( + "generate_query", + lambda adapter: make_generate_query_activity(adapter)( # type: ignore[arg-type] + GenerateQueryInput( + investigation_id="inv-1", + hypothesis=HYPOTHESIS, + schema_info={}, + alert_summary="orders dropped", + ) + ), + ), + ( + "interpret_evidence", + lambda adapter: make_interpret_evidence_activity(adapter)( # type: ignore[arg-type] + InterpretEvidenceInput( + investigation_id="inv-1", + hypothesis=HYPOTHESIS, + query_result={"query": "SELECT 1", "rows": [], "row_count": 0}, + alert_summary="orders dropped", + ) + ), + ), + ( + "synthesize", + lambda adapter: make_synthesize_activity(adapter)( # type: ignore[arg-type] + SynthesizeInput( + investigation_id="inv-1", + evidence=[], + hypotheses=[HYPOTHESIS], + alert_summary="orders dropped", + ) + ), + ), + ( + "counter_analyze", + lambda adapter: make_counter_analyze_activity(adapter)( # type: ignore[arg-type] + CounterAnalyzeInput( + investigation_id="inv-1", + synthesis={"root_cause": "x", "confidence": 0.5}, + evidence=[], + hypotheses=[HYPOTHESIS], + ) + ), + ), +] +NAMES = [name for name, _ in ACTIVITIES] + + +@pytest.mark.parametrize(("name", "run"), ACTIVITIES, ids=NAMES) +async def test_a_rejected_key_fails_the_activity_for_good(name: str, run: Run) -> None: + """A 401 is LLMRejected and non-retryable, with the code and what to fix.""" + with pytest.raises(ApplicationError) as caught: + await run(FailingAdapter(anthropic_error(401))) + + error = caught.value + assert error.type == "LLMRejected" + assert error.non_retryable + assert error.message == INVALID_KEY + assert list(error.details) == [{"code": "invalid_key", "message": INVALID_KEY}] + + +@pytest.mark.parametrize(("name", "run"), ACTIVITIES, ids=NAMES) +async def test_an_overloaded_api_fails_the_activity_for_a_retry(name: str, run: Run) -> None: + """A 529 is LLMUnavailable, which the retry policy tries again.""" + with pytest.raises(ApplicationError) as caught: + await run(FailingAdapter(anthropic_error(529))) + + error = caught.value + assert error.type == "LLMUnavailable" + assert not error.non_retryable + assert list(error.details) == [ + {"code": "overloaded", "message": "Anthropic is overloaded (529)."} + ] + + +@pytest.mark.parametrize(("name", "run"), ACTIVITIES, ids=NAMES) +async def test_other_errors_still_come_back_in_the_result(name: str, run: Run) -> None: + """An error that isn't from the API keeps its old path: the result's error field.""" + result = await run(FailingAdapter(RuntimeError("output didn't validate"))) + + assert result.error is not None + assert "output didn't validate" in result.error From e9f99c5f09ff7ebbf9d853f85bbe2e275ec6ad8c Mon Sep 17 00:00:00 2001 From: bordumb Date: Mon, 28 Sep 2026 20:32:22 +0100 Subject: [PATCH 03/18] feat(api): every investigation starts in an issue MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Of the 8 ways a run was created, only starting it from an issue page linked it fully. POST /investigations (UI, SDK, CLI, notebook) and the AUTO webhooks inserted runs inline, with no run row or start card, and the EE rule action never wrote its outcome back. Now (spec 0001 §7.11): - open_issue() (adapters/db/issues.py) is the only issue insert. The API, both webhooks and the starter use it, and every thread starts with the event that opened it - InvestigationStarterService.start(): - resolves the given issue or opens one - builds the missing brief or alert - writes the investigation, the run row (trigger_type human/api/ webhook/rule) and the thread card - starts the workflow with alert.issue_id - on a failed start, records the failure on the card - POST /investigations takes a brief or an alert (a bad alert is a 422, not a 500) and returns run_id, issue_id and issue_number - the issue spawn route, both webhooks and the EE rule action go through the starter - runs report their number, status and error; GET /investigations/{id} adds the issue, run number, brief and failure for the details page - tool calls record duration_ms and row_count for "Ran 1 query · 212 ms" - the SDK's Investigation gains issue_id and issue_number; `dataing run start` links the issue Co-Authored-By: Claude Opus 5.5 Signed-off-by: Claude --- .flow/tasks/fn-70.19.json | 18 +- .flow/tasks/fn-70.19.md | 7 +- .flow/tasks/fn-70.21.json | 8 +- .../src/dataing_cli/commands/run.py | 10 +- .../dataing-cli/tests/test_commands_run.py | 31 ++ .../dataing_ee/core/automation/executor.py | 42 +- .../entrypoints/api/routes/integrations.py | 146 ++----- .../automation/test_spawn_investigation.py | 16 + .../api/routes/test_integrations.py | 52 ++- .../dataing-sdk/src/dataing_sdk/client.py | 4 + .../dataing-sdk/src/dataing_sdk/types.py | 4 + .../dataing-sdk/tests/test_client.py | 7 +- .../src/dataing/adapters/db/issue_threads.py | 3 + .../dataing/src/dataing/adapters/db/issues.py | 167 ++++++++ .../dataing/src/dataing/agents/chat/deps.py | 5 + .../dataing/src/dataing/agents/chat/tools.py | 14 +- .../src/dataing/core/investigation/brief.py | 46 +- .../entrypoints/api/routes/integrations.py | 172 +++----- .../api/routes/investigation_outcomes.py | 16 +- .../entrypoints/api/routes/investigations.py | 283 ++++++------ .../entrypoints/api/routes/issue_threads.py | 5 +- .../dataing/entrypoints/api/routes/issues.py | 281 +++--------- .../src/dataing/services/investigation.py | 340 ++++++++++++--- .../temporal/activities/publish_outcome.py | 175 ++++---- .../api/test_start_investigation.py | 401 ++++++++++++++++++ .../tests/integration/test_open_issue.py | 112 +++++ .../tests/unit/agents/test_chat_agent.py | 3 + .../unit/api/test_integrations_routes.py | 17 +- .../tests/unit/api/test_issues_routes.py | 3 + .../api/routes/test_investigations.py | 64 ++- .../api/routes/test_issue_sidebar.py | 4 +- .../api/routes/test_issue_threads.py | 6 + 32 files changed, 1639 insertions(+), 823 deletions(-) create mode 100644 python-packages/dataing/src/dataing/adapters/db/issues.py create mode 100644 python-packages/dataing/tests/integration/api/test_start_investigation.py create mode 100644 python-packages/dataing/tests/integration/test_open_issue.py diff --git a/.flow/tasks/fn-70.19.json b/.flow/tasks/fn-70.19.json index 1a34a745..5de08f7b 100644 --- a/.flow/tasks/fn-70.19.json +++ b/.flow/tasks/fn-70.19.json @@ -7,10 +7,24 @@ "fn-70.18" ], "epic": "fn-70", + "evidence": { + "commits": [ + "b54b8725" + ], + "prs": [], + "tests": [ + "tests/unit/agents/test_llm_errors.py", + "tests/unit/temporal/test_llm_activity_errors.py", + "tests/unit/temporal/test_investigation_failures.py", + "tests/unit/temporal/test_investigation_replay.py", + "tests/integration/test_publish_outcome.py", + "CE unit suite: 3169 passed" + ] + }, "id": "fn-70.19", "priority": null, "spec_path": ".flow/tasks/fn-70.19.md", - "status": "in_progress", + "status": "done", "title": "M5: LLM failures fail the run (classify, activities raise, workflow fails)", - "updated_at": "2026-09-28T17:55:54.224228Z" + "updated_at": "2026-09-28T18:22:06.119415Z" } diff --git a/.flow/tasks/fn-70.19.md b/.flow/tasks/fn-70.19.md index 3eaca1df..4e88b4de 100644 --- a/.flow/tasks/fn-70.19.md +++ b/.flow/tasks/fn-70.19.md @@ -13,9 +13,8 @@ TBD ## Done summary -TBD - +classify_llm_error (agents/errors.py) maps anthropic → pydantic-ai → LLMError chains to the §7.12 codes. LLM activities raise LLMRejected/LLMUnavailable with an explicit retry policy; AgentClient.interpret_evidence no longer swallows errors. Behind llm-failures-v1 the workflow fails the run (generation failure/no hypotheses, subagent LLM error cancelling the others, nothing testable, synthesis failure), publishes {"status":"failed","error":{code,message,step}} and fails the Temporal execution as InvestigationFailed. Counter-analysis failure keeps the conclusion. ## Evidence -- Commits: -- Tests: +- Commits: b54b8725 +- Tests: tests/unit/agents/test_llm_errors.py, tests/unit/temporal/test_llm_activity_errors.py, tests/unit/temporal/test_investigation_failures.py, tests/unit/temporal/test_investigation_replay.py, tests/integration/test_publish_outcome.py, CE unit suite: 3169 passed - PRs: diff --git a/.flow/tasks/fn-70.21.json b/.flow/tasks/fn-70.21.json index 498b0411..b49294a0 100644 --- a/.flow/tasks/fn-70.21.json +++ b/.flow/tasks/fn-70.21.json @@ -1,7 +1,7 @@ { - "assignee": null, + "assignee": "bordumbb@gmail.com", "claim_note": "", - "claimed_at": null, + "claimed_at": "2026-09-28T18:22:06.382549Z", "created_at": "2026-09-28T17:51:59.729817Z", "depends_on": [ "fn-70.18" @@ -10,7 +10,7 @@ "id": "fn-70.21", "priority": null, "spec_path": ".flow/tasks/fn-70.21.md", - "status": "todo", + "status": "in_progress", "title": "M5: one starter and open_issue() for every start path", - "updated_at": "2026-09-28T17:51:59.730075Z" + "updated_at": "2026-09-28T18:22:06.382808Z" } diff --git a/python-packages/dataing-cli/src/dataing_cli/commands/run.py b/python-packages/dataing-cli/src/dataing_cli/commands/run.py index fe4ca72e..45d364be 100644 --- a/python-packages/dataing-cli/src/dataing_cli/commands/run.py +++ b/python-packages/dataing-cli/src/dataing_cli/commands/run.py @@ -138,10 +138,16 @@ def start_run( ) frontend_url = get_frontend_url() - inv_url = f"{frontend_url}/investigations/{investigation.investigation_id}" inv_id = investigation.investigation_id console.print(f"[green]+[/green] Started investigation: [cyan]{inv_id}[/cyan]") - console.print(f"[green]+[/green] View at: [link={inv_url}]{inv_url}[/link]") + if investigation.issue_id: + # Every run lives in an issue; its thread is where to follow and steer it + issue_url = f"{frontend_url}/issues/{investigation.issue_id}" + console.print(f"[green]+[/green] In issue #{investigation.issue_number}") + console.print(f"[green]+[/green] View at: [link={issue_url}]{issue_url}[/link]") + else: + inv_url = f"{frontend_url}/investigations/{inv_id}" + console.print(f"[green]+[/green] View at: [link={inv_url}]{inv_url}[/link]") # Handle --no-watch (deprecated): exit immediately after showing URL if no_watch: diff --git a/python-packages/dataing-cli/tests/test_commands_run.py b/python-packages/dataing-cli/tests/test_commands_run.py index 1173e5cd..173797d0 100644 --- a/python-packages/dataing-cli/tests/test_commands_run.py +++ b/python-packages/dataing-cli/tests/test_commands_run.py @@ -52,6 +52,37 @@ def test_run_start_success( assert "Started investigation" in result.output assert "inv-xyz123" in result.output + def test_run_start_links_the_issue_it_runs_in( + self, + runner: CliRunner, + configured_env: Path, + mock_client_patch: MagicMock, + ) -> None: + """The run's issue thread is where to follow it.""" + mock_investigation = MagicMock() + mock_investigation.investigation_id = "inv-xyz123" + mock_investigation.issue_id = "issue-7" + mock_investigation.issue_number = 7 + mock_client_patch.start_investigation.return_value = mock_investigation + + result = runner.invoke( + app, + [ + "run", + "start", + "schema.table", + "--anomaly-type", + "null_rate", + "--goal", + "investigate null spike", + "--no-watch", + ], + ) + + assert result.exit_code == 0 + assert "In issue #7" in result.output + assert "/issues/issue-7" in result.output + def test_run_start_with_datasource( self, runner: CliRunner, diff --git a/python-packages/dataing-ee/src/dataing_ee/core/automation/executor.py b/python-packages/dataing-ee/src/dataing_ee/core/automation/executor.py index f45a891a..be02ea98 100644 --- a/python-packages/dataing-ee/src/dataing_ee/core/automation/executor.py +++ b/python-packages/dataing-ee/src/dataing_ee/core/automation/executor.py @@ -422,32 +422,24 @@ async def _spawn_investigation( error="No single datasource to investigate; set the datasource_id param", ) - alert = _issue_alert(issue_data, dataset_id) - started = await ctx.investigation_starter.start_investigation( - tenant_id=ctx.tenant_id, - datasource_id=datasource_id, - alert_data=alert.model_dump(mode="json"), - alert_summary=f"Issue #{issue_data['number']} on {dataset_id}: {issue_data['title']}", - ) - - await ctx.db.execute( - """ - INSERT INTO issue_investigation_runs ( - issue_id, investigation_id, trigger_type, trigger_ref, execution_profile + try: + started = await ctx.investigation_starter.start( + tenant_id=ctx.tenant_id, + datasource_id=datasource_id, + trigger_type="rule", + alert=_issue_alert(issue_data, dataset_id), + issue_id=ctx.issue_id, + dataset_id=dataset_id, + trigger_ref={"rule_id": str(ctx.rule_id)}, + execution_profile=profile, + ) + except Exception as e: + logger.error(f"Rule {ctx.rule_id} could not start an investigation: {e}") + return ActionResult( + action_type=ActionType.SPAWN_INVESTIGATION, + success=False, + error=f"Could not start the investigation: {e}", ) - VALUES ($1, $2, 'rule', $3, $4) - """, - ctx.issue_id, - started.investigation_id, - to_json_string({"rule_id": str(ctx.rule_id)}), - profile, - ) - - await self._record_event( - ctx, - "investigation_started", - {"investigation_id": str(started.investigation_id), "source": "automation"}, - ) return ActionResult( action_type=ActionType.SPAWN_INVESTIGATION, diff --git a/python-packages/dataing-ee/src/dataing_ee/entrypoints/api/routes/integrations.py b/python-packages/dataing-ee/src/dataing_ee/entrypoints/api/routes/integrations.py index a024954e..52196da7 100644 --- a/python-packages/dataing-ee/src/dataing_ee/entrypoints/api/routes/integrations.py +++ b/python-packages/dataing-ee/src/dataing_ee/entrypoints/api/routes/integrations.py @@ -15,17 +15,20 @@ from collections.abc import Awaitable, Callable from datetime import UTC, datetime from typing import Annotated, Any -from uuid import UUID, uuid4 +from uuid import UUID import httpx from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException, Request, Response, status from pydantic import BaseModel, Field from dataing.adapters.db.app_db import AppDatabase +from dataing.adapters.db.issues import open_issue from dataing.adapters.db.team_policy_repository import PolicyAction, TeamPolicyRepository +from dataing.core.domain_types import AnomalyAlert from dataing.core.json_utils import to_json_string from dataing.entrypoints.api.deps import get_app_db, resolve_datasource_id from dataing.entrypoints.api.middleware.auth import ApiKeyContext, require_scope, verify_api_key +from dataing.services.investigation import InvestigationStarterService from dataing.services.policy import IssueContext, PolicyService from dataing_ee.adapters.integrations.base import IntegrationAdapter, IssueData, WebhookRequest from dataing_ee.adapters.integrations.registry import get_adapter @@ -731,64 +734,26 @@ async def _process_webhook_event( # Create issue tenant_id = integration["tenant_id"] - # Get next issue number - number_row = await db.fetch_one( - "SELECT next_issue_number($1) as num", - tenant_id, - ) - issue_number = number_row["num"] if number_row else 1 - - issue_row = await db.fetch_one( - """ - INSERT INTO issues ( - tenant_id, number, title, description, status, - priority, severity, dataset_id, - author_type, source_provider, source_external_id - ) - VALUES ($1, $2, $3, $4, 'open', $5, $6, $7, 'integration', $8, $9) - RETURNING id, number - """, - tenant_id, - issue_number, - issue_data.get("title"), - issue_data.get("description"), - issue_data.get("priority"), - issue_data.get("severity"), - issue_data.get("dataset_id"), - provider, - idempotency_key, + issue_row = await open_issue( + db, + tenant_id=tenant_id, + title=str(issue_data["title"]), + description=issue_data.get("description"), + priority=issue_data.get("priority"), + severity=issue_data.get("severity"), + dataset_id=issue_data.get("dataset_id"), + labels=issue_data.get("labels", []), + author_type="integration", + source_provider=provider, + source_external_id=idempotency_key, + event_payload={ + "source": "webhook", + "provider": provider, + "integration_id": str(integration_id), + }, ) - - if not issue_row: - raise HTTPException( - status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, - detail="Failed to create issue from webhook", - ) issue_id = issue_row["id"] - - # Add labels if present - labels = issue_data.get("labels", []) - for label in labels: - await db.execute( - "INSERT INTO issue_labels (issue_id, label) VALUES ($1, $2)", - issue_id, - label, - ) - - # Record creation event - event_payload = { - "source": "webhook", - "provider": provider, - "integration_id": str(integration_id), - } - await db.execute( - """ - INSERT INTO issue_events (issue_id, event_type, actor_user_id, payload) - VALUES ($1, 'created', NULL, $2) - """, - issue_id, - to_json_string(event_payload), - ) + issue_number = issue_row["number"] # Evaluate policy and start auto-investigation if applicable investigation_id = await _evaluate_and_start_investigation( @@ -979,8 +944,6 @@ async def _evaluate_and_start_investigation( "source_alert_id": idempotency_key, } - investigation_id = uuid4() - # Resolve the tenant's own datasource. There is no fallback ID: a fixed ID # would point the investigation at a datasource owned by another tenant. try: @@ -992,59 +955,30 @@ async def _evaluate_and_start_investigation( ) return None - # Add issue_id to alert data for back-linking - alert_data["issue_id"] = str(issue_id) - alert_data["datasource_id"] = str(datasource_id) - + starter = InvestigationStarterService(db=db, temporal_client=temporal_client) try: - # Create investigation record - await db.execute( - """ - INSERT INTO investigations (id, tenant_id, alert) - VALUES ($1, $2, $3) - """, - investigation_id, - tenant_id, - json.dumps(alert_data), - ) - - # Start Temporal workflow - alert_summary = f"Auto investigation: {issue_data.get('title', 'Webhook alert')}" - await temporal_client.start_investigation( - investigation_id=str(investigation_id), - tenant_id=str(tenant_id), - datasource_id=str(datasource_id), - alert_data=alert_data, - alert_summary=alert_summary, - ) - - # Record investigation started event on issue - await db.execute( - """ - INSERT INTO issue_events (issue_id, event_type, actor_user_id, payload) - VALUES ($1, 'investigation_started', NULL, $2) - """, - issue_id, - to_json_string( - { - "investigation_id": str(investigation_id), - "trigger": "auto_policy", - "source_system": provider, - } - ), - ) - - logger.info( - f"Auto investigation started: investigation={investigation_id}, " - f"issue={issue_id}, provider={provider}" + started = await starter.start( + tenant_id=tenant_id, + datasource_id=datasource_id, + trigger_type="webhook", + alert=AnomalyAlert.model_validate(alert_data), + issue_id=issue_id, + trigger_ref={ + "source": "auto_policy", + "provider": provider, + "idempotency_key": idempotency_key, + }, ) - - return investigation_id - except Exception as e: logger.error(f"Failed to start auto investigation for issue={issue_id}: {e}") return None + logger.info( + f"Auto investigation started: investigation={started.investigation_id}, " + f"issue={issue_id}, provider={provider}" + ) + return started.investigation_id + def _extract_idempotency_key(payload: dict[str, Any], provider: str, request: Request) -> str: """Extract idempotency key from provider payload.""" diff --git a/python-packages/dataing-ee/tests/integration/core/automation/test_spawn_investigation.py b/python-packages/dataing-ee/tests/integration/core/automation/test_spawn_investigation.py index 5cfd9544..ac2e361f 100644 --- a/python-packages/dataing-ee/tests/integration/core/automation/test_spawn_investigation.py +++ b/python-packages/dataing-ee/tests/integration/core/automation/test_spawn_investigation.py @@ -116,6 +116,11 @@ async def test_spawn_starts_an_investigation_on_a_valid_anomaly_alert( assert (row["tenant_id"], row["status"]) == (tenant_id, "active") alert_data = json.loads(row["alert"]) assert alert_data.pop("datasource_id") == str(datasource_id) + # Linked to the issue, so the outcome is written back to its thread + assert alert_data.pop("issue_id") == str(issue["id"]) + assert alert_data.pop("brief")["symptom"] == ( + "customer_id is null on a quarter of today's orders" + ) alert = AnomalyAlert.model_validate(alert_data) assert alert.model_dump(mode="json") == alert_data assert alert.dataset_ids == ["public.orders"] @@ -125,6 +130,17 @@ async def test_spawn_starts_an_investigation_on_a_valid_anomaly_alert( assert [(w["investigation_id"], w["datasource_id"]) for w in temporal.started] == [ (str(investigation_id), str(datasource_id)) ] + card = await migrated_db.fetch_one( + """ + SELECT m.author_kind, m.payload FROM issue_thread_messages m + JOIN issue_threads t ON t.id = m.thread_id + WHERE t.issue_id = $1 AND m.kind = 'investigation' + """, + issue["id"], + ) + assert card is not None + assert card["author_kind"] == "system" + assert json.loads(card["payload"])["investigation_id"] == str(investigation_id) async def test_spawn_links_the_issue_and_reuses_its_investigation( diff --git a/python-packages/dataing-ee/tests/unit/entrypoints/api/routes/test_integrations.py b/python-packages/dataing-ee/tests/unit/entrypoints/api/routes/test_integrations.py index f7e041a8..ebbd9f71 100644 --- a/python-packages/dataing-ee/tests/unit/entrypoints/api/routes/test_integrations.py +++ b/python-packages/dataing-ee/tests/unit/entrypoints/api/routes/test_integrations.py @@ -37,6 +37,7 @@ from dataing.entrypoints.api.middleware.auth import ApiKeyContext, verify_api_key TENANT_ID = UUID("5f0c2a9e-0000-0000-0000-000000000001") +ISSUE_ID = UUID("5f0c2a9e-0000-0000-0000-000000000042") class TestIntegrationCreateSchema: @@ -878,19 +879,25 @@ def _post_raw(db: AsyncMock, provider: str, body: bytes, headers: dict[str, str] def _processing_db(provider: str) -> AsyncMock: - """App DB that takes a signed webhook all the way to new issue #7.""" + """App DB that takes a signed webhook up to opening its issue.""" db = AsyncMock() db.fetch_one.side_effect = [ _integration_row(provider, WEBHOOK_SECRET), None, # not delivered before {"id": uuid4()}, # integration event recorded - {"num": 7}, # next issue number - {"id": uuid4(), "number": 7}, # issue created ] db.fetch_all.return_value = [] return db +@pytest.fixture +def opened(monkeypatch: pytest.MonkeyPatch) -> AsyncMock: + """Open webhook issues as #7, capturing what open_issue was given.""" + open_issue = AsyncMock(return_value={"id": uuid4(), "number": 7}) + monkeypatch.setattr("dataing_ee.entrypoints.api.routes.integrations.open_issue", open_issue) + return open_issue + + @pytest.fixture def no_auto_investigation(monkeypatch: pytest.MonkeyPatch) -> None: """Skip the policy evaluation that can start an investigation.""" @@ -912,7 +919,7 @@ class TestFormEncodedWebhooks: """Webhook bodies are decoded once authenticated, JSON or form-encoded.""" @pytest.mark.usefixtures("no_auto_investigation") - def test_processes_signed_block_action(self) -> None: + def test_processes_signed_block_action(self, opened: AsyncMock) -> None: body = urlencode({"payload": json.dumps(SLACK_BLOCK_ACTION)}).encode() db = _processing_db("slack") @@ -921,10 +928,14 @@ def test_processes_signed_block_action(self) -> None: assert response.status_code == 200 assert response.json()["status"] == "processed" - assert "Slack Action: flag_issue = orders" in db.fetch_one.await_args.args + issue = opened.await_args.kwargs + assert issue["title"] == "Slack Action: flag_issue = orders" + assert (issue["author_type"], issue["source_provider"]) == ("integration", "slack") @pytest.mark.usefixtures("no_auto_investigation") - def test_slash_command_is_acknowledged_then_answered(self, replies: AsyncMock) -> None: + def test_slash_command_is_acknowledged_then_answered( + self, replies: AsyncMock, opened: AsyncMock + ) -> None: # Slack gives a slash command 3 seconds: acknowledge, then report back body = urlencode(SLACK_SLASH_COMMAND).encode() db = _processing_db("slack") @@ -935,7 +946,7 @@ def test_slash_command_is_acknowledged_then_answered(self, replies: AsyncMock) - ack = {"response_type": "ephemeral", "text": "Creating a dataing issue..."} assert response.status_code == 200 assert response.json() == ack - assert "orders has nulls" in db.fetch_one.await_args.args + assert opened.await_args.kwargs["title"] == "orders has nulls" replies.assert_awaited_once_with( SLACK_SLASH_COMMAND["response_url"], {"response_type": "ephemeral", "text": "Created dataing issue #7."}, @@ -962,7 +973,7 @@ def test_slash_command_failure_is_reported(self, replies: AsyncMock) -> None: }, ) - @pytest.mark.usefixtures("no_auto_investigation") + @pytest.mark.usefixtures("no_auto_investigation", "opened") def test_untrusted_response_url_is_never_posted_to(self, replies: AsyncMock) -> None: command = {**SLACK_SLASH_COMMAND, "response_url": "https://attacker.example/hook"} body = urlencode(command).encode() @@ -1029,7 +1040,9 @@ class TestEvaluateAndStartInvestigation: MODULE = "dataing_ee.entrypoints.api.routes.integrations" - async def _run(self, resolve_datasource: AsyncMock) -> tuple[object, AsyncMock, AsyncMock]: + async def _run( + self, resolve_datasource: AsyncMock + ) -> tuple[object, AsyncMock, AsyncMock, MagicMock]: from dataing_ee.entrypoints.api.routes.integrations import ( _evaluate_and_start_investigation, ) @@ -1047,7 +1060,9 @@ async def _run(self, resolve_datasource: AsyncMock) -> tuple[object, AsyncMock, patch(f"{self.MODULE}.TeamPolicyRepository") as repo, patch(f"{self.MODULE}.PolicyService") as policy_service, patch(f"{self.MODULE}.resolve_datasource_id", resolve_datasource), + patch(f"{self.MODULE}.InvestigationStarterService") as starter, ): + starter.return_value.start = AsyncMock(return_value=MagicMock(investigation_id=uuid4())) repo.return_value.get_default_team_for_tenant = AsyncMock(return_value=team_id) policy_service.return_value.evaluate = AsyncMock( return_value=PolicyResult( @@ -1061,26 +1076,26 @@ async def _run(self, resolve_datasource: AsyncMock) -> tuple[object, AsyncMock, request=request, db=db, tenant_id=TENANT_ID, - issue_id=uuid4(), + issue_id=ISSUE_ID, issue_data={"title": "orders volume drop", "severity": "high"}, adapter=None, webhook_request=None, idempotency_key="jira_1_created", provider="jira", ) - return investigation_id, db, temporal + return investigation_id, db, temporal, starter async def test_starts_investigation_on_tenant_datasource(self) -> None: - """The resolved tenant datasource is what the workflow receives.""" + """The run starts in the webhook's issue, on the tenant's resolved datasource.""" datasource_id = uuid4() - investigation_id, _, temporal = await self._run(AsyncMock(return_value=datasource_id)) + investigation_id, _, _, starter = await self._run(AsyncMock(return_value=datasource_id)) assert investigation_id is not None - temporal.start_investigation.assert_awaited_once() - kwargs = temporal.start_investigation.await_args.kwargs - assert kwargs["tenant_id"] == str(TENANT_ID) - assert kwargs["datasource_id"] == str(datasource_id) + start = starter.return_value.start.await_args.kwargs + assert (start["tenant_id"], start["datasource_id"]) == (TENANT_ID, datasource_id) + assert (start["issue_id"], start["trigger_type"]) == (ISSUE_ID, "webhook") + assert start["alert"].metric_spec.expression == "orders volume drop" @pytest.mark.parametrize( "resolve_error", @@ -1091,11 +1106,12 @@ async def test_starts_investigation_on_tenant_datasource(self) -> None: ) async def test_unresolvable_datasource_skips_investigation(self, resolve_error: str) -> None: """No datasource of the tenant's own means no investigation (no fallback ID).""" - investigation_id, db, temporal = await self._run( + investigation_id, db, temporal, starter = await self._run( AsyncMock(side_effect=ValueError(resolve_error)) ) assert investigation_id is None + starter.return_value.start.assert_not_called() temporal.start_investigation.assert_not_called() executed_sql = [call.args[0] for call in db.execute.call_args_list] assert not any("INSERT INTO investigations" in sql for sql in executed_sql) diff --git a/python-packages/dataing-sdk/src/dataing_sdk/client.py b/python-packages/dataing-sdk/src/dataing_sdk/client.py index dbe049a0..f52c4dd9 100644 --- a/python-packages/dataing-sdk/src/dataing_sdk/client.py +++ b/python-packages/dataing-sdk/src/dataing_sdk/client.py @@ -899,6 +899,8 @@ def start_investigation( investigation_id=str(data["investigation_id"]), main_branch_id=str(data["main_branch_id"]), status=data.get("status", "queued"), + issue_id=str(data["issue_id"]) if data.get("issue_id") else None, + issue_number=data.get("issue_number"), ) async def async_start_investigation( @@ -976,6 +978,8 @@ async def async_start_investigation( investigation_id=str(data["investigation_id"]), main_branch_id=str(data["main_branch_id"]), status=data.get("status", "queued"), + issue_id=str(data["issue_id"]) if data.get("issue_id") else None, + issue_number=data.get("issue_number"), ) def get_investigation(self, investigation_id: str) -> Any: diff --git a/python-packages/dataing-sdk/src/dataing_sdk/types.py b/python-packages/dataing-sdk/src/dataing_sdk/types.py index e425faaa..c3cdf957 100644 --- a/python-packages/dataing-sdk/src/dataing_sdk/types.py +++ b/python-packages/dataing-sdk/src/dataing_sdk/types.py @@ -1052,12 +1052,16 @@ class Investigation(BaseModel): investigation_id: Unique identifier for the investigation. main_branch_id: ID of the main investigation branch. status: Current status (queued, running, completed, failed). + issue_id: The issue the run lives in; its thread is where to follow it. + issue_number: That issue's number. run_id: Alias for investigation_id (for compatibility with Run type). """ investigation_id: str = Field(..., description="Unique investigation identifier") main_branch_id: str = Field(..., description="Main branch identifier") status: str = Field(default="queued", description="Current investigation status") + issue_id: str | None = Field(default=None, description="The issue the run lives in") + issue_number: int | None = Field(default=None, description="That issue's number") @property def run_id(self) -> str: diff --git a/python-packages/dataing-sdk/tests/test_client.py b/python-packages/dataing-sdk/tests/test_client.py index 4fa784b5..f67474e8 100644 --- a/python-packages/dataing-sdk/tests/test_client.py +++ b/python-packages/dataing-sdk/tests/test_client.py @@ -204,11 +204,14 @@ def test_start_investigation_uses_v1_endpoint(self) -> None: "investigation_id": "test-inv-123", "main_branch_id": "test-branch-123", "status": "queued", + "run_id": "test-run-1", + "issue_id": "test-issue-7", + "issue_number": 7, } mock_response.status_code = 200 with patch.object(client, "_request", return_value=mock_response) as mock_request: - client.start_investigation( + investigation = client.start_investigation( dataset="main.orders", anomaly_type="null_rate", goal="test investigation", @@ -219,6 +222,8 @@ def test_start_investigation_uses_v1_endpoint(self) -> None: call_args = mock_request.call_args assert call_args[0][0] == "POST" assert call_args[0][1] == "/api/v1/investigations" + # Every run lives in an issue; its thread is where to follow it + assert (investigation.issue_id, investigation.issue_number) == ("test-issue-7", 7) def test_stream_run_uses_v1_investigations_events_endpoint(self) -> None: """Test that stream_run() uses /api/v1/investigations/{id}/events endpoint.""" diff --git a/python-packages/dataing/src/dataing/adapters/db/issue_threads.py b/python-packages/dataing/src/dataing/adapters/db/issue_threads.py index f5505251..3aebcd78 100644 --- a/python-packages/dataing/src/dataing/adapters/db/issue_threads.py +++ b/python-packages/dataing/src/dataing/adapters/db/issue_threads.py @@ -46,6 +46,9 @@ def _decode_message(row: dict[str, Any]) -> dict[str, Any]: def describe_event(event_type: str, payload: dict[str, Any]) -> str: """Return a one-line, human-readable description of an issue event.""" + if event_type == "created": + provider = payload.get("source_provider") + return f"Issue opened from {provider}" if provider else "Issue opened" if event_type == "status_changed": return f"Status changed from {payload.get('from', '?')} to {payload.get('to', '?')}" if event_type in ("assigned", "assignee_changed"): diff --git a/python-packages/dataing/src/dataing/adapters/db/issues.py b/python-packages/dataing/src/dataing/adapters/db/issues.py new file mode 100644 index 00000000..0b2eb4ef --- /dev/null +++ b/python-packages/dataing/src/dataing/adapters/db/issues.py @@ -0,0 +1,167 @@ +"""Issues and their investigation runs in the app database. + +open_issue() is the one way to insert an issue: the API, both webhooks and the +investigation starter use it, so every issue's shared thread starts with the event +that opened it (docs/specs/0001_issue_chat.md §7.11). +""" + +from __future__ import annotations + +from collections.abc import Sequence +from typing import Any +from uuid import UUID + +from dataing.adapters.db.app_db import AppDatabase +from dataing.adapters.db.issue_threads import IssueThreadRepository +from dataing.core.json_utils import to_json_string + +ISSUE_COLUMNS = """id, number, title, description, status, priority, severity, + dataset_id, due_at, assignee_user_id, acknowledged_by, created_by_user_id, + author_type, source_provider, source_external_id, source_external_url, + resolution_note, context, created_at, updated_at, closed_at""" + +RUN_COLUMNS = """ + id, issue_id, investigation_id, trigger_type, brief, source_thread_id, parent_run_id, + execution_profile, approval_status, confidence, root_cause_tag, synthesis_summary, + created_at, completed_at, outcome_verdict, outcome_note, outcome_reviewed_by, + outcome_reviewed_at +""" + +# A run with its number among the issue's runs, and how it ended so far +_RUNS = f""" + SELECT {", ".join(f"r.{column.strip()}" for column in RUN_COLUMNS.split(","))}, + ROW_NUMBER() OVER (PARTITION BY r.issue_id ORDER BY r.created_at, r.id) AS number, + CASE WHEN i.outcome IS NULL THEN 'running' + ELSE COALESCE(i.outcome->>'status', 'completed') END AS status, + i.outcome->'error'->>'message' AS error + FROM issue_investigation_runs r + JOIN investigations i ON i.id = r.investigation_id +""" + +# A new comment and a new investigation are already messages in the thread (a +# comment, an investigation card), so those events are only in the event log. +THREAD_SILENT_EVENTS = frozenset({"comment_added", "investigation_spawned"}) + + +async def record_issue_event( + db: AppDatabase, + issue_id: UUID, + event_type: str, + actor_user_id: UUID | None, + payload: dict[str, Any] | None = None, +) -> None: + """Record an issue event and show it in the issue's shared thread.""" + await db.execute( + """ + INSERT INTO issue_events (issue_id, event_type, actor_user_id, payload) + VALUES ($1, $2, $3, $4) + """, + issue_id, + event_type, + actor_user_id, + to_json_string(payload or {}), + ) + if event_type not in THREAD_SILENT_EVENTS: + await IssueThreadRepository(db).append_event(issue_id, event_type, actor_user_id, payload) + + +async def open_issue( + db: AppDatabase, + *, + tenant_id: UUID, + title: str, + description: str | None = None, + priority: str | None = None, + severity: str | None = None, + dataset_id: str | None = None, + labels: Sequence[str] = (), + context: dict[str, Any] | None = None, + created_by: UUID | None = None, + author_type: str = "human", + source_provider: str | None = None, + source_external_id: str | None = None, + source_external_url: str | None = None, + event_payload: dict[str, Any] | None = None, +) -> dict[str, Any]: + """Insert an open issue, with its labels and the event that opens its thread. + + Args: + db: The app database. + tenant_id: The issue's tenant. + title: What is wrong, in a line. + description: More detail, as markdown. + priority: P0-P3. + severity: low, medium, high or critical. + dataset_id: The affected table, as a native path. + labels: Labels to add. + context: Where the problem was seen, e.g. observed_at and column. + created_by: The person who opened it; None when an integration or dataing did. + author_type: human or integration. + source_provider: The integration it came from, e.g. monte_carlo. + source_external_id: The source's id for it, the webhook dedup key. + source_external_url: A link back to the source. + event_payload: Extra fields for the opening event. + + Returns: + The issue row. + """ + number_row = await db.fetch_one("SELECT next_issue_number($1) AS number", tenant_id) + number = number_row["number"] if number_row else 1 + row = await db.execute_returning( + f""" + INSERT INTO issues ( + tenant_id, number, title, description, status, priority, severity, dataset_id, + created_by_user_id, author_type, source_provider, source_external_id, + source_external_url, context + ) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14) + RETURNING {ISSUE_COLUMNS} + """, + tenant_id, + number, + title, + description, + "open", + priority, + severity, + dataset_id, + created_by, + author_type, + source_provider, + source_external_id, + source_external_url, + to_json_string(context or {}), + ) + if row is None: + raise RuntimeError("Failed to create issue") + for label in labels: + await db.execute( + "INSERT INTO issue_labels (issue_id, label) VALUES ($1, $2)", row["id"], label + ) + opened: dict[str, Any] = {"title": title} + if source_provider: + opened["source_provider"] = source_provider + await record_issue_event( + db, row["id"], "created", created_by, {**opened, **(event_payload or {})} + ) + return row + + +async def list_runs(db: AppDatabase, issue_id: UUID) -> list[dict[str, Any]]: + """Return an issue's investigation runs, newest first.""" + return await db.fetch_all( + f"{_RUNS} WHERE r.issue_id = $1 ORDER BY r.created_at DESC, r.id DESC", issue_id + ) + + +async def get_run(db: AppDatabase, run_id: UUID) -> dict[str, Any] | None: + """Return one investigation run, numbered among its issue's runs.""" + return await db.fetch_one( + f""" + SELECT * FROM ({_RUNS} + WHERE r.issue_id = (SELECT issue_id FROM issue_investigation_runs WHERE id = $1) + ) runs + WHERE id = $1 + """, + run_id, + ) diff --git a/python-packages/dataing/src/dataing/agents/chat/deps.py b/python-packages/dataing/src/dataing/agents/chat/deps.py index c15f1acc..8863b5aa 100644 --- a/python-packages/dataing/src/dataing/agents/chat/deps.py +++ b/python-packages/dataing/src/dataing/agents/chat/deps.py @@ -59,6 +59,9 @@ class ToolCallRecord: summary: str query_result_id: UUID | None = None error_code: str | None = None + # A query's totals, for the collapsed line ("Ran 1 query · 212 ms · 4 rows") + duration_ms: int | None = None + row_count: int | None = None def to_payload(self) -> dict[str, Any]: """Return the record as JSON-safe payload data.""" @@ -70,6 +73,8 @@ def to_payload(self) -> dict[str, Any]: "summary": self.summary, "query_result_id": str(self.query_result_id) if self.query_result_id else None, "error_code": self.error_code, + "duration_ms": self.duration_ms, + "row_count": self.row_count, } diff --git a/python-packages/dataing/src/dataing/agents/chat/tools.py b/python-packages/dataing/src/dataing/agents/chat/tools.py index c09ab35a..f8afdd49 100644 --- a/python-packages/dataing/src/dataing/agents/chat/tools.py +++ b/python-packages/dataing/src/dataing/agents/chat/tools.py @@ -27,6 +27,8 @@ def _record( *, error: AgentQueryError | None = None, query_result_id: Any = None, + duration_ms: int | None = None, + row_count: int | None = None, ) -> None: ctx.deps.tool_calls.append( ToolCallRecord( @@ -37,6 +39,8 @@ def _record( summary=summary, query_result_id=query_result_id, error_code=error.code.value if error else None, + duration_ms=duration_ms, + row_count=row_count, ) ) @@ -107,7 +111,15 @@ async def run_query(ctx: RunContext[ChatDeps], sql: str, purpose: str) -> dict[s return e.to_dict() snapshot_id = await services.save_query_result(call_id, result.sql, result, None) summary = _plural(result.row_count, "row") + (" (truncated)" if result.truncated else "") - _record(ctx, "run_query", args, summary, query_result_id=snapshot_id) + _record( + ctx, + "run_query", + args, + summary, + query_result_id=snapshot_id, + duration_ms=result.duration_ms, + row_count=result.row_count, + ) view = result.model_view.to_dict() view["query_result_id"] = str(snapshot_id) view["sql"] = result.sql diff --git a/python-packages/dataing/src/dataing/core/investigation/brief.py b/python-packages/dataing/src/dataing/core/investigation/brief.py index 4cd872fa..1f2a14b8 100644 --- a/python-packages/dataing/src/dataing/core/investigation/brief.py +++ b/python-packages/dataing/src/dataing/core/investigation/brief.py @@ -11,12 +11,15 @@ from __future__ import annotations -from datetime import datetime -from typing import Literal +from datetime import UTC, date, datetime, time, timedelta +from typing import TYPE_CHECKING, Literal from uuid import UUID from pydantic import BaseModel, ConfigDict, Field +if TYPE_CHECKING: + from dataing.core.domain_types import AnomalyAlert + class BriefClaim(BaseModel): """A finding or exclusion, optionally backed by a thread message and a query.""" @@ -152,6 +155,45 @@ def claim(item: DraftClaim) -> BriefClaim: ) +def brief_from_alert(alert: AnomalyAlert, datasource_id: UUID | None = None) -> InvestigationBrief: + """Describe an alert as the brief its run starts from (SDK, webhooks, rules). + + A measured alert's symptom names the metric, the expected and actual values and + the date; a described one (a webhook title, an issue) is its own text. The time + window is the anomaly's day, with a day either side. + """ + tables = [table for table in alert.dataset_ids if table and table != "unknown"] + return InvestigationBrief( + symptom=_alert_symptom(alert, tables)[:1000], + scope=BriefScope( + datasource_id=datasource_id, + tables=tables[:20], + time_window=_around(alert.anomaly_date), + ), + ) + + +def _alert_symptom(alert: AnomalyAlert, tables: list[str]) -> str: + spec = alert.metric_spec + measured = spec.metric_type != "description" and (alert.expected_value or alert.actual_value) + if not measured: + return spec.expression or spec.display_name or "Investigate this alert" + where = f" on {tables[0]}" if tables else "" + return ( + f"{spec.display_name or spec.expression}{where}: expected {alert.expected_value:g}, " + f"got {alert.actual_value:g} ({alert.deviation_pct:+.1f}%) on {alert.anomaly_date}" + ) + + +def _around(anomaly_date: str) -> TimeWindow | None: + try: + day = date.fromisoformat(anomaly_date[:10]) + except ValueError: + return None + start = datetime.combine(day - timedelta(days=1), time(), tzinfo=UTC) + return TimeWindow(from_=start, to=start + timedelta(days=2)) + + def _section(title: str, items: list[str]) -> str: return f"{title}:\n" + "\n".join(f"- {item}" for item in items) diff --git a/python-packages/dataing/src/dataing/entrypoints/api/routes/integrations.py b/python-packages/dataing/src/dataing/entrypoints/api/routes/integrations.py index 81491abb..681e9a25 100644 --- a/python-packages/dataing/src/dataing/entrypoints/api/routes/integrations.py +++ b/python-packages/dataing/src/dataing/entrypoints/api/routes/integrations.py @@ -16,7 +16,7 @@ from datetime import UTC, datetime from functools import lru_cache from typing import Annotated, Any -from uuid import UUID, uuid4 +from uuid import UUID from fastapi import APIRouter, Depends, Header, HTTPException, Request, status from jsonschema import Draft7Validator @@ -24,10 +24,13 @@ from pydantic import BaseModel, Field from dataing.adapters.db.app_db import AppDatabase +from dataing.adapters.db.issues import open_issue from dataing.adapters.db.team_policy_repository import PolicyAction, TeamPolicyRepository +from dataing.core.domain_types import AnomalyAlert, MetricSpec from dataing.core.json_utils import to_json_string from dataing.entrypoints.api.deps import get_app_db from dataing.entrypoints.api.middleware.auth import ApiKeyContext, require_scope +from dataing.services.investigation import InvestigationStarterService from dataing.services.notification import NotificationEvent, NotificationService from dataing.services.policy import IssueContext, PolicyService @@ -312,67 +315,23 @@ async def receive_generic_webhook( created=False, ) - # Get next issue number - number_row = await db.fetch_one( - "SELECT next_issue_number($1) as num", - auth.tenant_id, - ) - issue_number = number_row["num"] if number_row else 1 - - # Create the issue - row = await db.fetch_one( - """ - INSERT INTO issues ( - tenant_id, number, title, description, status, - priority, severity, dataset_id, - author_type, source_provider, source_external_id, source_external_url - ) - VALUES ($1, $2, $3, $4, 'open', $5, $6, $7, 'integration', $8, $9, $10) - RETURNING id, number, status - """, - auth.tenant_id, - issue_number, - payload.title, - payload.description, - payload.priority, - payload.severity, - payload.dataset_id, - payload.source_provider, - payload.source_external_id, - payload.source_external_url, + row = await open_issue( + db, + tenant_id=auth.tenant_id, + title=payload.title, + description=payload.description, + priority=payload.priority, + severity=payload.severity, + dataset_id=payload.dataset_id, + labels=payload.labels or [], + author_type="integration", + source_provider=payload.source_provider, + source_external_id=payload.source_external_id, + source_external_url=payload.source_external_url, + event_payload={"source": "webhook", "provider": payload.source_provider}, ) - - if not row: - raise HTTPException( - status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, - detail="Failed to create issue", - ) - issue_id = row["id"] - - # Add labels if provided - if payload.labels: - for label in payload.labels: - await db.execute( - "INSERT INTO issue_labels (issue_id, label) VALUES ($1, $2)", - issue_id, - label, - ) - - # Record creation event - await db.execute( - """ - INSERT INTO issue_events (issue_id, event_type, actor_user_id, payload) - VALUES ($1, 'created', NULL, $2) - """, - issue_id, - to_json_string( - { - "source": "webhook", - "provider": payload.source_provider, - } - ), - ) + issue_number = row["number"] logger.info( f"Webhook issue created: id={issue_id}, number={issue_number}, " @@ -508,9 +467,6 @@ async def _start_auto_investigation( ) return None - investigation_id = uuid4() - now = datetime.now(UTC) - # Resolve the tenant's own datasource. There is no fallback ID: a fixed ID # would point the investigation at a datasource owned by another tenant. try: @@ -522,72 +478,42 @@ async def _start_auto_investigation( ) return None - # Build alert data - alert_data: dict[str, Any] = { - "dataset_ids": [payload.dataset_id] if payload.dataset_id else [], - "metric_spec": { - "metric_type": "description", - "expression": payload.title, - "display_name": "Integration Alert", - "columns_referenced": [], - }, - "anomaly_type": "integration_alert", - "expected_value": 0.0, - "actual_value": 0.0, - "deviation_pct": 0.0, - "anomaly_date": now.date().isoformat(), - "severity": payload.severity or "medium", - "datasource_id": str(datasource_id), - "issue_id": str(issue_id), - } - + alert = AnomalyAlert( + dataset_ids=[payload.dataset_id] if payload.dataset_id else [], + metric_spec=MetricSpec( + metric_type="description", + expression=payload.title, + display_name="Integration Alert", + ), + anomaly_type="integration_alert", + expected_value=0.0, + actual_value=0.0, + deviation_pct=0.0, + anomaly_date=datetime.now(UTC).date().isoformat(), + severity=payload.severity or "medium", + source_system=payload.source_provider, + source_alert_id=payload.source_external_id, + source_url=payload.source_external_url, + ) + starter = InvestigationStarterService(db=db, temporal_client=temporal_client) try: - # Create investigation record (issue_id is stored in alert JSONB) - await db.execute( - """ - INSERT INTO investigations (id, tenant_id, alert) - VALUES ($1, $2, $3) - """, - investigation_id, - auth.tenant_id, - json.dumps(alert_data), - ) - - # Start Temporal workflow - alert_summary = f"Auto investigation: {payload.title}" - await temporal_client.start_investigation( - investigation_id=str(investigation_id), - tenant_id=str(auth.tenant_id), - datasource_id=str(datasource_id), - alert_data=alert_data, - alert_summary=alert_summary, - ) - - # Record event on issue - await db.execute( - """ - INSERT INTO issue_events (issue_id, event_type, actor_user_id, payload) - VALUES ($1, 'investigation_started', NULL, $2) - """, - issue_id, - to_json_string( - { - "investigation_id": str(investigation_id), - "trigger": "auto_policy", - } - ), - ) - - logger.info( - f"Auto investigation started: investigation={investigation_id}, issue={issue_id}" + started = await starter.start( + tenant_id=auth.tenant_id, + datasource_id=datasource_id, + trigger_type="webhook", + alert=alert, + issue_id=issue_id, + trigger_ref={"source": "auto_policy", "provider": payload.source_provider}, ) - - return investigation_id - except Exception as e: logger.error(f"Failed to start auto investigation for issue={issue_id}: {e}") return None + logger.info( + f"Auto investigation started: investigation={started.investigation_id}, issue={issue_id}" + ) + return started.investigation_id + async def _send_review_notification( db: AppDatabase, diff --git a/python-packages/dataing/src/dataing/entrypoints/api/routes/investigation_outcomes.py b/python-packages/dataing/src/dataing/entrypoints/api/routes/investigation_outcomes.py index 46d2b3eb..919047de 100644 --- a/python-packages/dataing/src/dataing/entrypoints/api/routes/investigation_outcomes.py +++ b/python-packages/dataing/src/dataing/entrypoints/api/routes/investigation_outcomes.py @@ -12,16 +12,12 @@ from pydantic import BaseModel, Field, model_validator from dataing.adapters.db.app_db import AppDatabase +from dataing.adapters.db.issues import get_run, record_issue_event from dataing.adapters.investigation_feedback import EventType, InvestigationFeedbackAdapter from dataing.entrypoints.api.deps import get_app_db, get_feedback_adapter from dataing.entrypoints.api.middleware.auth import ApiKeyContext, require_scope from dataing.entrypoints.api.routes.investigations import TenantInvestigationId -from dataing.entrypoints.api.routes.issues import ( - RUN_COLUMNS, - InvestigationRunResponse, - _record_issue_event, - _run_response, -) +from dataing.entrypoints.api.routes.issues import InvestigationRunResponse, _run_response router = APIRouter(prefix="/investigations", tags=["investigations"]) @@ -68,19 +64,19 @@ async def review_outcome( if run["outcome"] is None: raise HTTPException(status_code=409, detail="The investigation has no outcome yet") - updated = await db.execute_returning( - f""" + await db.execute( + """ UPDATE issue_investigation_runs SET outcome_verdict = $2, outcome_note = $3, outcome_reviewed_by = $4, outcome_reviewed_at = NOW() WHERE id = $1 - RETURNING {RUN_COLUMNS} """, run["id"], body.verdict, body.note, auth.user_id, ) + updated = await get_run(db, run["id"]) assert updated is not None await feedback.emit( @@ -96,7 +92,7 @@ async def review_outcome( actor_id=auth.user_id, actor_type="user", ) - await _record_issue_event( + await record_issue_event( db, run["issue_id"], "outcome_reviewed", diff --git a/python-packages/dataing/src/dataing/entrypoints/api/routes/investigations.py b/python-packages/dataing/src/dataing/entrypoints/api/routes/investigations.py index 82e853fd..cccac1df 100644 --- a/python-packages/dataing/src/dataing/entrypoints/api/routes/investigations.py +++ b/python-packages/dataing/src/dataing/entrypoints/api/routes/investigations.py @@ -13,18 +13,26 @@ from collections.abc import AsyncIterator, Iterator from datetime import UTC, datetime from enum import Enum -from typing import Annotated, Any +from typing import Annotated, Any, Literal from uuid import UUID, uuid4 from fastapi import APIRouter, Depends, File, Header, HTTPException, Query, Request from fastapi.responses import Response -from pydantic import BaseModel, Field +from pydantic import BaseModel, Field, model_validator from sse_starlette.sse import EventSourceResponse from dataing.adapters.db.app_db import AppDatabase -from dataing.core.domain_types import AnomalyAlert, MetricSpec +from dataing.core.domain_types import AnomalyAlert +from dataing.core.investigation.brief import InvestigationBrief from dataing.core.json_utils import to_json_string +from dataing.entrypoints.api.deps import get_investigation_starter from dataing.entrypoints.api.middleware.auth import ApiKeyContext, require_scope, verify_api_key +from dataing.services.investigation import ( + DatasetRequiredError, + InvestigationStarterService, + InvestigationStartFailed, + IssueNotFoundError, +) from dataing.temporal.client import TemporalInvestigationClient logger = logging.getLogger(__name__) @@ -34,21 +42,39 @@ # Annotated types for dependency injection AuthDep = Annotated[ApiKeyContext, Depends(verify_api_key)] WriteScopeDep = Annotated[ApiKeyContext, Depends(require_scope("write"))] +InvestigationStarterDep = Annotated[InvestigationStarterService, Depends(get_investigation_starter)] class StartInvestigationRequest(BaseModel): - """Request body for starting an investigation.""" + """Start a run from a brief (the UI) or an alert (SDK, CLI, notebook). - alert: dict[str, Any] # AnomalyAlert data - datasource_id: UUID | None = None # Optional datasource ID for durable execution + Every run lives in an issue: the one named by issue_id, or one opened for it + (docs/specs/0001_issue_chat.md §7.11). + """ + + brief: InvestigationBrief | None = None + alert: AnomalyAlert | None = None + datasource_id: UUID | None = None # Else the brief's scope, else the tenant's only one + execution_profile: Literal["safe", "standard", "deep"] = "standard" + issue_id: UUID | None = None + + @model_validator(mode="after") + def _one_source(self) -> StartInvestigationRequest: + """Require exactly one of brief and alert.""" + if (self.brief is None) == (self.alert is None): + raise ValueError("Send exactly one of brief and alert") + return self class StartInvestigationResponse(BaseModel): - """Response for starting an investigation.""" + """The started run and the issue it lives in.""" investigation_id: UUID - main_branch_id: UUID + main_branch_id: UUID # The investigation id; the SDK reads it status: str = "queued" + run_id: UUID + issue_id: UUID + issue_number: int class CancelInvestigationResponse(BaseModel): @@ -91,13 +117,27 @@ class BranchStateResponse(BaseModel): class InvestigationStateResponse(BaseModel): - """Full investigation state for API responses.""" + """Full investigation state for API responses. + + The run's details page links back to its issue's thread, so the state carries + the issue, the run's number among its runs, its brief and, for a failed run, + why it failed (docs/specs/0001_issue_chat.md §8.3). + """ investigation_id: UUID status: str main_branch: BranchStateResponse user_branch: BranchStateResponse | None = None root_hash: str | None = None + issue_id: UUID | None = None # None only for an imported snapshot + issue_number: int | None = None + issue_title: str | None = None + run_number: int | None = None + brief: dict[str, Any] | None = None + execution_profile: str | None = None + error: dict[str, Any] | None = None # {code, message, step} for a failed run + # Each hypothesis's id, title, status and reasoning, as far as the run got + hypotheses: list[dict[str, Any]] = Field(default_factory=list) class ChainVerificationResponse(BaseModel): @@ -333,68 +373,23 @@ async def start_investigation( http_request: Request, request: StartInvestigationRequest, auth: WriteScopeDep, - db: AppDbDep, - temporal_client: TemporalClientDep, + starter: InvestigationStarterDep, ) -> StartInvestigationResponse: - """Start a new investigation for an alert. - - Creates a new investigation with Temporal workflow for durable execution. + """Start an investigation, in an issue. - Args: - http_request: The HTTP request for accessing app state. - request: The investigation request containing alert data. - auth: Authentication context from API key/JWT. - db: Application database. - temporal_client: Temporal client for durable execution. - - Returns: - StartInvestigationResponse with investigation and branch IDs. + Without issue_id this opens the issue: titled with the symptom, attributed to the + caller, or to dataing for an API key without a person. The run's card is in the + issue's thread and its outcome is written back there. """ from dataing.entrypoints.api.deps import resolve_datasource_id - # Parse alert from request - alert_data = request.alert - metric_spec_data = alert_data.get("metric_spec", {}) - - metric_spec = MetricSpec( - metric_type=metric_spec_data.get("metric_type", "column"), - expression=metric_spec_data.get("expression", ""), - display_name=metric_spec_data.get("display_name", ""), - columns_referenced=metric_spec_data.get("columns_referenced", []), - source_url=metric_spec_data.get("source_url"), - ) - - # Handle both dataset_id (singular) and dataset_ids (plural) for backward compatibility - dataset_ids = alert_data.get("dataset_ids") - if dataset_ids is None: - # Convert singular to list - dataset_id = alert_data.get("dataset_id", "unknown") - dataset_ids = [dataset_id] if isinstance(dataset_id, str) else dataset_id - - alert = AnomalyAlert( - dataset_ids=dataset_ids, - metric_spec=metric_spec, - anomaly_type=alert_data["anomaly_type"], - expected_value=alert_data["expected_value"], - actual_value=alert_data["actual_value"], - deviation_pct=alert_data["deviation_pct"], - anomaly_date=alert_data["anomaly_date"], - severity=alert_data.get("severity", "medium"), - source_system=alert_data.get("source_system"), - source_alert_id=alert_data.get("source_alert_id"), - source_url=alert_data.get("source_url"), - metadata=alert_data.get("metadata"), - ) - - # Resolve datasource_id (use provided or get default) + scope_datasource = request.brief.scope.datasource_id if request.brief else None try: datasource_id = await resolve_datasource_id( - http_request, auth.tenant_id, explicit_id=request.datasource_id + http_request, auth.tenant_id, explicit_id=request.datasource_id or scope_datasource ) except ValueError as e: - error_msg = str(e) - if error_msg.startswith("ambiguous_datasource:"): - # Parse the ambiguous datasource error for 409 response + if str(e).startswith("ambiguous_datasource:"): raise HTTPException( status_code=409, detail={ @@ -403,62 +398,40 @@ async def start_investigation( "hint": "Specify datasource_id or use %dataing attach", }, ) from e - raise HTTPException( - status_code=400, - detail=error_msg, - ) from e - - investigation_id = uuid4() - # Build rich alert summary with all critical information (matches main branch) - metric_name = alert.metric_spec.display_name - columns = ", ".join(alert.metric_spec.columns_referenced) or "unknown column" - alert_summary = ( - f"{alert.anomaly_type} anomaly on {columns} in {alert.dataset_id}: " - f"expected {alert.expected_value}, actual {alert.actual_value} " - f"({alert.deviation_pct:.1f}% deviation). " - f"Metric: {metric_name}. Date: {alert.anomaly_date}." - ) + raise HTTPException(status_code=400, detail=str(e)) from e + person = auth.user_id is not None try: - # Save investigation to database first (so GET /investigations/{id} works) - # Note: The unified schema stores datasource_id in alert metadata - # Use mode="json" to ensure dates are serialized as ISO strings - alert_dict = alert.model_dump(mode="json") - alert_dict["datasource_id"] = str(datasource_id) - await db.execute( - """ - INSERT INTO investigations (id, tenant_id, alert) - VALUES ($1, $2, $3) - """, - investigation_id, - auth.tenant_id, - json.dumps(alert_dict), - ) - - # Start the Temporal workflow - # Use mode="json" to ensure all values are JSON-serializable for Temporal - await temporal_client.start_investigation( - investigation_id=str(investigation_id), - tenant_id=str(auth.tenant_id), - datasource_id=str(datasource_id), - alert_data=alert.model_dump(mode="json"), - alert_summary=alert_summary, - ) - logger.info( - f"Started Temporal investigation: investigation_id={investigation_id}, " - f"tenant_id={auth.tenant_id}" + started = await starter.start( + tenant_id=auth.tenant_id, + datasource_id=datasource_id, + trigger_type="human" if person else "api", + brief=request.brief, + alert=request.alert, + issue_id=request.issue_id, + actor_user_id=auth.user_id, + trigger_ref=( + {"user_id": str(auth.user_id)} if person else {"api_key_id": str(auth.key_id)} + ), + execution_profile=request.execution_profile, ) - return StartInvestigationResponse( - investigation_id=investigation_id, - main_branch_id=investigation_id, # Temporal uses single workflow ID - status="queued", - ) - except Exception as e: - logger.error(f"Failed to start Temporal investigation: {e}") - raise HTTPException( - status_code=500, - detail=f"Failed to start investigation: {e}", - ) from e + except IssueNotFoundError as e: + raise HTTPException(status_code=404, detail="Issue not found") from e + except DatasetRequiredError as e: + raise HTTPException(status_code=400, detail=str(e)) from e + except InvestigationStartFailed as e: + raise HTTPException(status_code=503, detail=str(e)) from e + except RuntimeError as e: + raise HTTPException(status_code=503, detail=str(e)) from e + + return StartInvestigationResponse( + investigation_id=started.investigation_id, + main_branch_id=started.investigation_id, + status=started.status, + run_id=started.run_id, + issue_id=started.issue_id, + issue_number=started.issue_number, + ) @router.post("/{investigation_id}/cancel", response_model=CancelInvestigationResponse) @@ -503,6 +476,7 @@ async def cancel_investigation( async def get_investigation( investigation_id: TenantInvestigationId, auth: AuthDep, + db: AppDbDep, temporal_client: TemporalClientDep, ) -> InvestigationStateResponse: """Get investigation state from Temporal workflow. @@ -513,6 +487,7 @@ async def get_investigation( Args: investigation_id: UUID of the investigation. auth: Authentication context from API key/JWT. + db: Application database, for the run's issue, number, brief and outcome. temporal_client: Temporal client for durable execution. Returns: @@ -538,14 +513,6 @@ async def get_investigation( ) root_hash = getattr(status.result, "root_hash", None) if status.result else None - - return InvestigationStateResponse( - investigation_id=investigation_id, - status=status.workflow_status, - main_branch=main_branch, - user_branch=None, - root_hash=root_hash, - ) except Exception as e: logger.error(f"Failed to get Temporal investigation: {e}") raise HTTPException( @@ -553,6 +520,76 @@ async def get_investigation( detail=f"Investigation not found: {e}", ) from e + run = await _run_context(db, auth.tenant_id, investigation_id) or {} + outcome = _json(run.get("outcome")) or {} + return InvestigationStateResponse( + investigation_id=investigation_id, + status=status.workflow_status, + main_branch=main_branch, + user_branch=None, + root_hash=root_hash, + issue_id=run.get("issue_id"), + issue_number=run.get("issue_number"), + issue_title=run.get("issue_title"), + run_number=run.get("run_number"), + brief=_json(run.get("brief")), + execution_profile=run.get("execution_profile"), + error=outcome.get("error") if outcome.get("status") == "failed" else None, + hypotheses=_hypotheses(outcome, status), + ) + + +async def _run_context( + db: AppDatabase, tenant_id: UUID, investigation_id: UUID +) -> dict[str, Any] | None: + """Return a run's outcome, issue, number among the issue's runs and brief.""" + return await db.fetch_one( + """ + SELECT i.outcome, r.issue_id, r.brief, r.execution_profile, + s.number AS issue_number, s.title AS issue_title, + (SELECT COUNT(*)::int FROM issue_investigation_runs earlier + WHERE earlier.issue_id = r.issue_id + AND (earlier.created_at, earlier.id) <= (r.created_at, r.id)) AS run_number + FROM investigations i + LEFT JOIN issue_investigation_runs r ON r.investigation_id = i.id + LEFT JOIN issues s ON s.id = r.issue_id + WHERE i.id = $1 AND i.tenant_id = $2 + """, + investigation_id, + tenant_id, + ) + + +def _json(value: Any) -> dict[str, Any] | None: + """Decode a JSONB column, which comes back as text without a codec.""" + if isinstance(value, str): + decoded: dict[str, Any] = json.loads(value) + return decoded + return value if isinstance(value, dict) else None + + +def _hypotheses(outcome: dict[str, Any], status: Any) -> list[dict[str, Any]]: + """Return each hypothesis's id, title, status and reasoning. + + A finished run's outcome has how each ended; a running one's live status has + where each is. The run's own hypotheses add the reasoning when the result has it. + """ + listed = outcome.get("hypotheses") or status.hypotheses or [] + reasoning = { + str(h.get("id")): h.get("reasoning") + for h in (status.result.hypotheses if status.result else []) + if isinstance(h, dict) + } + return [ + { + "id": str(h.get("id")), + "title": h.get("title", ""), + "status": h.get("status", "pending"), + "reasoning": reasoning.get(str(h.get("id"))), + } + for h in listed + ] + @router.get("/{investigation_id}/verify", response_model=ChainVerificationResponse) async def verify_investigation( diff --git a/python-packages/dataing/src/dataing/entrypoints/api/routes/issue_threads.py b/python-packages/dataing/src/dataing/entrypoints/api/routes/issue_threads.py index 4cd93528..99c8fcc6 100644 --- a/python-packages/dataing/src/dataing/entrypoints/api/routes/issue_threads.py +++ b/python-packages/dataing/src/dataing/entrypoints/api/routes/issue_threads.py @@ -26,10 +26,11 @@ from dataing.adapters.db.app_db import AppDatabase from dataing.adapters.db.issue_threads import IssueThreadRepository +from dataing.adapters.db.issues import record_issue_event from dataing.core.json_utils import to_json_string from dataing.entrypoints.api.deps import get_app_db from dataing.entrypoints.api.middleware.auth import ApiKeyContext, require_scope, verify_api_key -from dataing.entrypoints.api.routes.issues import _record_issue_event, _verify_issue_access +from dataing.entrypoints.api.routes.issues import _verify_issue_access logger = logging.getLogger(__name__) @@ -340,7 +341,7 @@ async def post_message( reply_to_id=body.reply_to_id, ) if thread["kind"] == "shared": - await _record_issue_event( + await record_issue_event( db, issue_id, "comment_added", user_id, {"message_id": str(row["id"])} ) await db.execute("UPDATE issues SET updated_at = NOW() WHERE id = $1", issue_id) diff --git a/python-packages/dataing/src/dataing/entrypoints/api/routes/issues.py b/python-packages/dataing/src/dataing/entrypoints/api/routes/issues.py index c590bd49..27a001a1 100644 --- a/python-packages/dataing/src/dataing/entrypoints/api/routes/issues.py +++ b/python-packages/dataing/src/dataing/entrypoints/api/routes/issues.py @@ -20,10 +20,13 @@ from sse_starlette.sse import EventSourceResponse from dataing.adapters.db.app_db import AppDatabase -from dataing.adapters.db.issue_threads import IssueThreadRepository -from dataing.agents.prompts.brief import BRIEF_METADATA_KEY -from dataing.core.domain_types import AnomalyAlert, MetricSpec -from dataing.core.investigation.brief import InvestigationBrief, brief_to_prompt +from dataing.adapters.db.issues import ( + ISSUE_COLUMNS, + list_runs, + open_issue, + record_issue_event, +) +from dataing.core.investigation.brief import InvestigationBrief from dataing.core.json_utils import to_json_safe, to_json_string from dataing.entrypoints.api.deps import ( get_app_db, @@ -32,7 +35,12 @@ ) from dataing.entrypoints.api.middleware.auth import ApiKeyContext, require_scope, verify_api_key from dataing.models.issue import IssueStatus -from dataing.services.investigation import InvestigationStarterService +from dataing.services.investigation import ( + DatasetRequiredError, + InvestigationStarterService, + InvestigationStartFailed, + IssueNotFoundError, +) logger = logging.getLogger(__name__) @@ -289,11 +297,6 @@ class IssueListResponse(BaseModel): # Helper Functions # ============================================================================ -ISSUE_COLUMNS = """id, number, title, description, status, priority, severity, - dataset_id, due_at, assignee_user_id, acknowledged_by, created_by_user_id, - author_type, source_provider, source_external_id, source_external_url, - resolution_note, context, created_at, updated_at, closed_at""" - def _encode_cursor(created_at: datetime, issue_id: UUID) -> str: """Encode pagination cursor.""" @@ -426,35 +429,6 @@ async def _issue_response(db: AppDatabase, row: dict[str, Any]) -> IssueResponse return _build_issue_response(row, labels, row["id"] in synthesized) -THREAD_SILENT_EVENTS = frozenset({"comment_added", "investigation_spawned"}) - - -async def _record_issue_event( - db: AppDatabase, - issue_id: UUID, - event_type: str, - actor_user_id: UUID | None, - payload: dict[str, Any] | None = None, -) -> None: - """Record an issue event and show it in the issue's shared thread. - - A new comment and a new investigation are already messages in the thread (a - comment, an investigation card), so those events are only in the event log. - """ - await db.execute( - """ - INSERT INTO issue_events (issue_id, event_type, actor_user_id, payload) - VALUES ($1, $2, $3, $4) - """, - issue_id, - event_type, - actor_user_id, - to_json_string(payload or {}), - ) - if event_type not in THREAD_SILENT_EVENTS: - await IssueThreadRepository(db).append_event(issue_id, event_type, actor_user_id, payload) - - async def _record_confirmed_cause( db: AppDatabase, issue_id: UUID, actor_user_id: UUID | None ) -> None: @@ -469,7 +443,7 @@ async def _record_confirmed_cause( ) if confirmed is None: return - await _record_issue_event( + await record_issue_event( db, issue_id, "resolved_with_cause", @@ -655,54 +629,18 @@ async def create_issue( Issues are created in OPEN status. Number is auto-assigned per-tenant. """ - # Get next issue number - number_row = await db.fetch_one( - "SELECT next_issue_number($1) as number", - auth.tenant_id, - ) - number = number_row["number"] if number_row else 1 - - # Insert issue - row = await db.execute_returning( - f""" - INSERT INTO issues ( - tenant_id, number, title, description, status, priority, severity, - dataset_id, created_by_user_id, author_type, context - ) - VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11) - RETURNING {ISSUE_COLUMNS} - """, - auth.tenant_id, - number, - body.title, - body.description, - IssueStatus.OPEN.value, - body.priority, - body.severity, - body.dataset_id, - auth.user_id, - "human", - to_json_string(body.context), - ) - - if not row: - raise HTTPException(status_code=500, detail="Failed to create issue") - - issue_id = row["id"] - - # Set labels - if body.labels: - await _set_issue_labels(db, issue_id, body.labels) - - # Record creation event - await _record_issue_event( + row = await open_issue( db, - issue_id, - "created", - auth.user_id, - {"title": body.title}, + tenant_id=auth.tenant_id, + title=body.title, + description=body.description, + priority=body.priority, + severity=body.severity, + dataset_id=body.dataset_id, + labels=body.labels, + context=body.context, + created_by=auth.user_id, ) - return await _issue_response(db, row) @@ -807,7 +745,7 @@ async def update_issue( for field, value in changes.items(): old = _decode_json_object(current[field]) if field == "context" else current[field] event_type, payload = _field_change_event(field, old, value) - await _record_issue_event(db, issue_id, event_type, auth.user_id, payload) + await record_issue_event(db, issue_id, event_type, auth.user_id, payload) if changes.get("status") == IssueStatus.RESOLVED.value: await _record_confirmed_cause(db, issue_id, auth.user_id) @@ -816,9 +754,9 @@ async def update_issue( new_labels = sorted(set(body.labels or [])) await _set_issue_labels(db, issue_id, new_labels) for label in sorted(set(new_labels) - set(old_labels)): - await _record_issue_event(db, issue_id, "label_added", auth.user_id, {"label": label}) + await record_issue_event(db, issue_id, "label_added", auth.user_id, {"label": label}) for label in sorted(set(old_labels) - set(new_labels)): - await _record_issue_event(db, issue_id, "label_removed", auth.user_id, {"label": label}) + await record_issue_event(db, issue_id, "label_removed", auth.user_id, {"label": label}) return await _issue_response(db, row) @@ -1017,14 +955,9 @@ class InvestigationRunResponse(BaseModel): outcome_note: str | None = None outcome_reviewed_by: UUID | None = None outcome_reviewed_at: datetime | None = None - - -RUN_COLUMNS = """ - id, issue_id, investigation_id, trigger_type, brief, source_thread_id, parent_run_id, - execution_profile, approval_status, confidence, root_cause_tag, synthesis_summary, - created_at, completed_at, outcome_verdict, outcome_note, outcome_reviewed_by, - outcome_reviewed_at -""" + number: int # The run's position among the issue's runs, by start time + status: str # running, completed or failed, from the investigation's outcome + error: str | None = None # Why a failed run failed def _run_response(row: dict[str, Any]) -> InvestigationRunResponse: @@ -1059,16 +992,7 @@ async def list_investigation_runs( """List investigation runs for an issue.""" await _verify_issue_access(db, issue_id, auth.tenant_id) - rows = await db.fetch_all( - f""" - SELECT {RUN_COLUMNS} - FROM issue_investigation_runs - WHERE issue_id = $1 - ORDER BY created_at DESC - """, - issue_id, - ) - items = [_run_response(row) for row in rows] + items = [_run_response(row) for row in await list_runs(db, issue_id)] return InvestigationRunListResponse(items=items, total=len(items)) @@ -1095,31 +1019,14 @@ async def spawn_investigation( Requires user identity (JWT auth or user-scoped API key). """ - issue = await db.fetch_one( - """ - SELECT id, tenant_id, dataset_id, title, severity, created_at - FROM issues - WHERE id = $1 AND tenant_id = $2 - """, - issue_id, - auth.tenant_id, - ) - if not issue: - raise HTTPException(status_code=404, detail="Issue not found") if auth.user_id is None: raise HTTPException( status_code=403, detail="User identity required to spawn investigations", ) + await _verify_issue_access(db, issue_id, auth.tenant_id) brief = body.brief - dataset_id = body.dataset_id or issue["dataset_id"] or next(iter(brief.scope.tables), None) - if not dataset_id: - raise HTTPException( - status_code=400, - detail="dataset_id required - not set on issue, request or brief scope", - ) - if body.parent_run_id is not None: parent = await db.fetch_one( "SELECT id FROM issue_investigation_runs WHERE id = $1 AND issue_id = $2", @@ -1148,116 +1055,30 @@ async def spawn_investigation( ) from e raise HTTPException(status_code=400, detail=error_msg) from e - approval_status = "approved" if body.execution_profile == "deep" else None - brief = brief.model_copy( - update={"scope": brief.scope.model_copy(update={"datasource_id": datasource_id})} - ) - brief_json = brief.model_dump(mode="json", by_alias=True) - - # The issue's dataset comes first; the brief's other tables become reference tables - dataset_ids = [dataset_id] + [t for t in brief.scope.tables if t != dataset_id] - alert = AnomalyAlert( - dataset_ids=dataset_ids, - metric_spec=MetricSpec( - metric_type="description", - expression=brief.symptom, - display_name=issue["title"], - ), - anomaly_type="custom", - expected_value=0.0, - actual_value=0.0, - deviation_pct=0.0, - anomaly_date=issue["created_at"].date().isoformat(), - severity=issue["severity"] or "medium", - # Every prompt that renders the alert adds this as its "Team brief" section - metadata={BRIEF_METADATA_KEY: brief_to_prompt(brief)}, - ) - alert_data = { - **alert.model_dump(mode="json"), - "issue_id": str(issue_id), - "brief": brief_json, - } - alert_summary = ( - f"Investigation spawned from issue: {issue.get('title', 'Untitled')}. " - f"Dataset: {dataset_id}. Symptom: {brief.symptom}" - ) - try: - result = await investigation_starter.start_investigation( + started = await investigation_starter.start( tenant_id=auth.tenant_id, datasource_id=datasource_id, - alert_data=alert_data, - alert_summary=alert_summary, - created_by=auth.user_id, + trigger_type="human", + brief=brief, + issue_id=issue_id, + dataset_id=body.dataset_id, + actor_user_id=auth.user_id, + trigger_ref={"user_id": str(auth.user_id)}, + execution_profile=body.execution_profile, + source_thread_id=body.source_thread_id, + parent_run_id=body.parent_run_id, ) - investigation_id = result.investigation_id - except RuntimeError as e: + except IssueNotFoundError as e: + raise HTTPException(status_code=404, detail="Issue not found") from e + except DatasetRequiredError as e: + raise HTTPException( + status_code=400, + detail="dataset_id required - not set on issue, request or brief scope", + ) from e + except (InvestigationStartFailed, RuntimeError) as e: raise HTTPException(status_code=503, detail=str(e)) from e - except Exception as e: - logger.error(f"Failed to start investigation: {e}") - raise HTTPException(status_code=500, detail=f"Failed to start investigation: {e}") from e - - trigger_ref = {"user_id": str(auth.user_id), "dataset_id": dataset_id} - row = await db.execute_returning( - f""" - INSERT INTO issue_investigation_runs ( - issue_id, investigation_id, trigger_type, trigger_ref, brief, - source_thread_id, parent_run_id, execution_profile, approval_status - ) - VALUES ($1, $2, 'human', $3, $4, $5, $6, $7, $8) - RETURNING {RUN_COLUMNS} - """, - issue_id, - investigation_id, - to_json_string(trigger_ref), - to_json_string(brief_json), - body.source_thread_id, - body.parent_run_id, - body.execution_profile, - approval_status, - ) - if not row: - raise HTTPException(status_code=500, detail="Failed to create investigation run") - - await _record_issue_event( - db, - issue_id, - "investigation_spawned", - auth.user_id, - { - "investigation_id": str(investigation_id), - "run_id": str(row["id"]), - "symptom": brief.symptom, - "execution_profile": body.execution_profile, - }, - ) - - # The run's card in the shared thread (from a scratch chat too) - threads = IssueThreadRepository(db) - shared = await threads.ensure_shared_thread(issue_id) - started_from = ( - " from a scratch chat" - if body.source_thread_id and body.source_thread_id != shared["id"] - else "" - ) - await threads.append_message( - shared["id"], - author_kind="user", - kind="investigation", - author_user_id=auth.user_id, - body_md=f"Started an investigation{started_from}: {brief.symptom}", - payload={ - "investigation_id": str(investigation_id), - "run_id": str(row["id"]), - "execution_profile": body.execution_profile, - "brief": brief_json, - "source_thread_id": str(body.source_thread_id) if body.source_thread_id else None, - "parent_run_id": str(body.parent_run_id) if body.parent_run_id else None, - }, - ) - - await db.execute("UPDATE issues SET updated_at = NOW() WHERE id = $1", issue_id) - return _run_response(row) + return _run_response(started.run) # ============================================================================ diff --git a/python-packages/dataing/src/dataing/services/investigation.py b/python-packages/dataing/src/dataing/services/investigation.py index 4df6e68a..ff29551f 100644 --- a/python-packages/dataing/src/dataing/services/investigation.py +++ b/python-packages/dataing/src/dataing/services/investigation.py @@ -1,120 +1,338 @@ -"""Investigation starter service for Temporal-based investigations. +"""The one way to start an investigation (docs/specs/0001_issue_chat.md §7.11). -This module provides a centralized service for starting investigations, -ensuring consistent behavior across all entry points (API routes, integrations, etc.). +Every run belongs to an issue. The starter resolves the issue it was given, or opens +one, then writes the run: the investigations row, the issue's run row and the card in +the issue's shared thread. Last, it starts the workflow with the issue linked, so the +outcome is always written back. The issue page, POST /investigations (UI, SDK, CLI, +notebook), both webhooks and the EE rule action all start runs here. """ from __future__ import annotations -import json from dataclasses import dataclass from typing import TYPE_CHECKING, Any from uuid import UUID, uuid4 import structlog +from dataing.adapters.db.issue_threads import IssueThreadRepository +from dataing.adapters.db.issues import get_run, open_issue, record_issue_event +from dataing.agents.prompts.brief import BRIEF_METADATA_KEY +from dataing.core.domain_types import AnomalyAlert, MetricSpec +from dataing.core.investigation.brief import ( + InvestigationBrief, + brief_from_alert, + brief_to_prompt, +) +from dataing.core.json_utils import to_json_string +from dataing.temporal.activities.publish_outcome import publish_outcome + if TYPE_CHECKING: from dataing.adapters.db.app_db import AppDatabase from dataing.temporal.client import TemporalInvestigationClient logger = structlog.get_logger() +TRIGGER_TYPES = frozenset({"human", "api", "webhook", "rule"}) +MAX_TITLE = 500 + + +class IssueNotFoundError(LookupError): + """The issue to start on doesn't exist in the tenant.""" + + +class DatasetRequiredError(ValueError): + """Neither the request, the issue, the brief nor the alert names a table.""" + + +class InvestigationStartFailed(RuntimeError): + """The run was written but its workflow didn't start; the run shows why.""" + @dataclass -class StartInvestigationResult: - """Result from starting an investigation.""" +class StartedInvestigation: + """A run that started, and the issue it lives in.""" investigation_id: UUID - status: str + run_id: UUID + issue_id: UUID + issue_number: int + run: dict[str, Any] + status: str = "queued" class InvestigationStarterService: - """Service for starting Temporal investigations. - - Centralizes the logic for creating investigation records and starting - Temporal workflows, ensuring DRY across all entry points. - """ + """Starts investigations, each in an issue, for every entry point.""" def __init__( self, db: AppDatabase, temporal_client: TemporalInvestigationClient | None = None, ) -> None: - """Initialize the investigation service.""" + """Initialize the starter.""" self.db = db self.temporal_client = temporal_client - async def start_investigation( + async def start( self, *, tenant_id: UUID, datasource_id: UUID, - alert_data: dict[str, Any], - alert_summary: str, - created_by: UUID | None = None, - investigation_id: UUID | None = None, - ) -> StartInvestigationResult: - """Start a new investigation with Temporal workflow. - - This method: - 1. Creates the investigation record in the database - 2. Starts the Temporal workflow for processing + trigger_type: str, + brief: InvestigationBrief | None = None, + alert: AnomalyAlert | None = None, + issue_id: UUID | None = None, + dataset_id: str | None = None, + actor_user_id: UUID | None = None, + trigger_ref: dict[str, Any] | None = None, + execution_profile: str = "standard", + source_thread_id: UUID | None = None, + parent_run_id: UUID | None = None, + source_provider: str | None = None, + ) -> StartedInvestigation: + """Start a run in an issue: the given one, or one opened for it. Args: - tenant_id: The tenant ID. - datasource_id: The datasource to investigate. - alert_data: Alert data containing investigation context. - alert_summary: Human-readable summary of the alert. - created_by: Optional user ID who created the investigation. - investigation_id: Optional pre-generated investigation ID. - - Returns: - StartInvestigationResult with the investigation ID and status. + tenant_id: The tenant. + datasource_id: The datasource the run queries, already resolved. + trigger_type: human, api, webhook or rule. + brief: What the run starts from. Built from the alert when not given. + alert: The alert that triggered it (SDK, webhooks, rules). Built from the + brief when not given. + issue_id: The issue to run in; without one, an issue is opened. + dataset_id: The table to investigate, if the issue has none. + actor_user_id: The person who started it; None for API keys and webhooks. + trigger_ref: What triggered it, recorded on the run row. + execution_profile: safe, standard or deep. + source_thread_id: The thread the brief was drafted in. + parent_run_id: The run this one continues. + source_provider: For an issue this opens, the integration it came from. Raises: - RuntimeError: If Temporal client is not configured. - Exception: If database insert or workflow start fails. + RuntimeError: Temporal isn't configured. + IssueNotFoundError: issue_id isn't an issue of the tenant. + DatasetRequiredError: Nothing names a table to investigate. + InvestigationStartFailed: The workflow didn't start; the run shows why. """ if self.temporal_client is None: raise RuntimeError("Temporal client not configured") + if trigger_type not in TRIGGER_TYPES: + raise ValueError(f"Unknown trigger type {trigger_type!r}") + if brief is None: + if alert is None: + raise ValueError("A brief or an alert is required") + brief = brief_from_alert(alert, datasource_id) + brief = brief.model_copy( + update={"scope": brief.scope.model_copy(update={"datasource_id": datasource_id})} + ) - # Generate ID if not provided - if investigation_id is None: - investigation_id = uuid4() - - # Ensure datasource_id is in alert_data - alert_data_with_ds = {**alert_data, "datasource_id": str(datasource_id)} + existing = await self._issue(tenant_id, issue_id) if issue_id else None + dataset = ( + dataset_id + or (existing["dataset_id"] if existing else None) + or next(iter(brief.scope.tables), None) + or (alert.dataset_ids[0] if alert and alert.dataset_ids else None) + ) + if not dataset: + raise DatasetRequiredError( + "Name a table to investigate: the issue, the brief's scope tables and the " + "alert have none" + ) + issue = existing or await open_issue( + self.db, + tenant_id=tenant_id, + title=_title(brief.symptom), + severity=alert.severity if alert else None, + dataset_id=dataset, + created_by=actor_user_id, + author_type="human" if actor_user_id else "integration", + source_provider=source_provider, + ) - # Create database record + brief_json = brief.model_dump(mode="json", by_alias=True) + if alert is None: + alert = _alert_from_brief(brief, dataset=dataset, issue=issue) + alert_data = { + **alert.model_dump(mode="json"), + "datasource_id": str(datasource_id), + "issue_id": str(issue["id"]), + "brief": brief_json, + } + investigation_id = uuid4() await self.db.execute( + "INSERT INTO investigations (id, tenant_id, alert, created_by) VALUES ($1, $2, $3, $4)", + investigation_id, + tenant_id, + to_json_string(alert_data), + actor_user_id, + ) + run_row = await self.db.execute_returning( """ - INSERT INTO investigations (id, tenant_id, alert, created_by) - VALUES ($1, $2, $3, $4) + INSERT INTO issue_investigation_runs ( + issue_id, investigation_id, trigger_type, trigger_ref, brief, + source_thread_id, parent_run_id, execution_profile, approval_status + ) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9) + RETURNING id """, + issue["id"], investigation_id, - tenant_id, - json.dumps(alert_data_with_ds), - created_by, + trigger_type, + to_json_string(trigger_ref or {}), + to_json_string(brief_json), + source_thread_id, + parent_run_id, + execution_profile, + "approved" if execution_profile == "deep" else None, ) - - # Start Temporal workflow - await self.temporal_client.start_investigation( - investigation_id=str(investigation_id), - tenant_id=str(tenant_id), - datasource_id=str(datasource_id), - alert_data=alert_data_with_ds, - alert_summary=alert_summary, + if run_row is None: + raise RuntimeError("Failed to create the investigation run") + run_id: UUID = run_row["id"] + await self._announce( + issue_id=issue["id"], + investigation_id=investigation_id, + run_id=run_id, + brief=brief, + brief_json=brief_json, + actor_user_id=actor_user_id, + trigger_type=trigger_type, + execution_profile=execution_profile, + source_thread_id=source_thread_id, + parent_run_id=parent_run_id, ) + try: + await self.temporal_client.start_investigation( + investigation_id=str(investigation_id), + tenant_id=str(tenant_id), + datasource_id=str(datasource_id), + alert_data=alert_data, + alert_summary=( + f"Investigation spawned from issue: {issue['title']}. " + f"Dataset: {dataset}. Symptom: {brief.symptom}" + ), + ) + except Exception as e: + message = f"Couldn't start the investigation: {e}" + logger.error("investigation_start_failed", error=str(e)) + await publish_outcome( + self.db, + { + "investigation_id": str(investigation_id), + "tenant_id": str(tenant_id), + "issue_id": str(issue["id"]), + "failure": {"code": "start_failed", "message": message, "step": "start"}, + }, + ) + raise InvestigationStartFailed(message) from e + logger.info( "investigation_started", investigation_id=str(investigation_id), - tenant_id=str(tenant_id), - datasource_id=str(datasource_id), - created_by=str(created_by) if created_by else None, + issue_id=str(issue["id"]), + trigger_type=trigger_type, ) - - return StartInvestigationResult( + run = await get_run(self.db, run_id) + assert run is not None + return StartedInvestigation( investigation_id=investigation_id, - status="queued", + run_id=run_id, + issue_id=issue["id"], + issue_number=issue["number"], + run=run, ) + + async def _issue(self, tenant_id: UUID, issue_id: UUID) -> dict[str, Any]: + issue = await self.db.fetch_one( + """ + SELECT id, number, title, severity, dataset_id, created_at + FROM issues WHERE id = $1 AND tenant_id = $2 + """, + issue_id, + tenant_id, + ) + if issue is None: + raise IssueNotFoundError(f"Issue {issue_id} not found") + return issue + + async def _announce( + self, + *, + issue_id: UUID, + investigation_id: UUID, + run_id: UUID, + brief: InvestigationBrief, + brief_json: dict[str, Any], + actor_user_id: UUID | None, + trigger_type: str, + execution_profile: str, + source_thread_id: UUID | None, + parent_run_id: UUID | None, + ) -> None: + """Log the run on the issue and put its card in the shared thread.""" + await record_issue_event( + self.db, + issue_id, + "investigation_spawned", + actor_user_id, + { + "investigation_id": str(investigation_id), + "run_id": str(run_id), + "symptom": brief.symptom, + "execution_profile": execution_profile, + "trigger_type": trigger_type, + }, + ) + threads = IssueThreadRepository(self.db) + shared = await threads.ensure_shared_thread(issue_id) + started_from = ( + " from a scratch chat" if source_thread_id and source_thread_id != shared["id"] else "" + ) + await threads.append_message( + shared["id"], + # A run no person started (API key, webhook, rule) is dataing's + author_kind="user" if actor_user_id else "system", + kind="investigation", + author_user_id=actor_user_id, + body_md=f"Started an investigation{started_from}: {brief.symptom}", + payload={ + "investigation_id": str(investigation_id), + "run_id": str(run_id), + "execution_profile": execution_profile, + "trigger_type": trigger_type, + "brief": brief_json, + "source_thread_id": str(source_thread_id) if source_thread_id else None, + "parent_run_id": str(parent_run_id) if parent_run_id else None, + }, + ) + await self.db.execute("UPDATE issues SET updated_at = NOW() WHERE id = $1", issue_id) + + +def _title(symptom: str) -> str: + """Return the first line of a symptom as an issue title.""" + first = symptom.strip().splitlines()[0] if symptom.strip() else "Investigation" + return first if len(first) <= MAX_TITLE else first[: MAX_TITLE - 1] + "…" + + +def _alert_from_brief( + brief: InvestigationBrief, *, dataset: str, issue: dict[str, Any] +) -> AnomalyAlert: + """Describe a brief as the alert its run starts from. + + The symptom is the metric the agents investigate. The issue's dataset comes first + and the brief's other tables become reference tables; every prompt that renders + the alert adds the brief as its "Team brief" section. + """ + return AnomalyAlert( + dataset_ids=[dataset] + [t for t in brief.scope.tables if t != dataset], + metric_spec=MetricSpec( + metric_type="description", expression=brief.symptom, display_name=issue["title"] + ), + anomaly_type="custom", + expected_value=0.0, + actual_value=0.0, + deviation_pct=0.0, + anomaly_date=issue["created_at"].date().isoformat(), + severity=issue["severity"] or "medium", + metadata={BRIEF_METADATA_KEY: brief_to_prompt(brief)}, + ) diff --git a/python-packages/dataing/src/dataing/temporal/activities/publish_outcome.py b/python-packages/dataing/src/dataing/temporal/activities/publish_outcome.py index 8d80f24a..7dd841ac 100644 --- a/python-packages/dataing/src/dataing/temporal/activities/publish_outcome.py +++ b/python-packages/dataing/src/dataing/temporal/activities/publish_outcome.py @@ -49,93 +49,100 @@ def make_publish_investigation_outcome_activity(app_db: AppDatabase) -> Any: @activity.defn(name="publish_investigation_outcome") async def publish_investigation_outcome(payload: dict[str, Any]) -> dict[str, Any]: """Write the outcome to the investigation, its issue run and the issue thread.""" - investigation_id = UUID(str(payload["investigation_id"])) - tenant_id = UUID(str(payload["tenant_id"])) - synthesis: dict[str, Any] = payload.get("synthesis") or {} - hypotheses: list[dict[str, Any]] = payload.get("hypotheses") or [] - failure: dict[str, Any] | None = payload.get("failure") - outcome: dict[str, Any] - if failure: - outcome = {"status": "failed", "error": failure, "hypotheses": hypotheses} - else: - outcome = { - "status": "completed", - "root_cause": synthesis.get("root_cause"), - "confidence": synthesis.get("confidence"), - "recommendations": synthesis.get("recommendations") or [], - "supporting_evidence": synthesis.get("supporting_evidence") or [], - "hypotheses": hypotheses, - "counter_analysis": payload.get("counter_analysis"), - } - await app_db.execute( - "UPDATE investigations SET outcome = $3 WHERE id = $1 AND tenant_id = $2", - investigation_id, - tenant_id, - to_json_string(outcome), - ) + return await publish_outcome(app_db, payload) - issue_id_raw = payload.get("issue_id") - if not issue_id_raw: - return {"published": True, "thread_message": False} - issue_id = UUID(str(issue_id_raw)) + return publish_investigation_outcome - run = await app_db.execute_returning( - """ - UPDATE issue_investigation_runs - SET synthesis_summary = $3, confidence = $4, - completed_at = COALESCE(completed_at, NOW()) - WHERE investigation_id = $1 AND issue_id = $2 - RETURNING id - """, - investigation_id, - issue_id, - synthesis.get("root_cause"), - synthesis.get("confidence"), - ) - threads = IssueThreadRepository(app_db) - thread = await threads.ensure_shared_thread(issue_id) - existing = await app_db.fetch_one( +async def publish_outcome(app_db: AppDatabase, payload: dict[str, Any]) -> dict[str, Any]: + """Write a finished or failed run's outcome to the investigation, run and thread. + + The workflow publishes through the activity; the starter calls this directly to + record a run whose workflow never started. + """ + investigation_id = UUID(str(payload["investigation_id"])) + tenant_id = UUID(str(payload["tenant_id"])) + synthesis: dict[str, Any] = payload.get("synthesis") or {} + hypotheses: list[dict[str, Any]] = payload.get("hypotheses") or [] + failure: dict[str, Any] | None = payload.get("failure") + outcome: dict[str, Any] + if failure: + outcome = {"status": "failed", "error": failure, "hypotheses": hypotheses} + else: + outcome = { + "status": "completed", + "root_cause": synthesis.get("root_cause"), + "confidence": synthesis.get("confidence"), + "recommendations": synthesis.get("recommendations") or [], + "supporting_evidence": synthesis.get("supporting_evidence") or [], + "hypotheses": hypotheses, + "counter_analysis": payload.get("counter_analysis"), + } + await app_db.execute( + "UPDATE investigations SET outcome = $3 WHERE id = $1 AND tenant_id = $2", + investigation_id, + tenant_id, + to_json_string(outcome), + ) + + issue_id_raw = payload.get("issue_id") + if not issue_id_raw: + return {"published": True, "thread_message": False} + issue_id = UUID(str(issue_id_raw)) + + run = await app_db.execute_returning( + """ + UPDATE issue_investigation_runs + SET synthesis_summary = $3, confidence = $4, + completed_at = COALESCE(completed_at, NOW()) + WHERE investigation_id = $1 AND issue_id = $2 + RETURNING id + """, + investigation_id, + issue_id, + synthesis.get("root_cause"), + synthesis.get("confidence"), + ) + + threads = IssueThreadRepository(app_db) + thread = await threads.ensure_shared_thread(issue_id) + existing = await app_db.fetch_one( + """ + SELECT id FROM issue_thread_messages + WHERE thread_id = $1 AND kind = 'investigation' + AND payload->>'outcome_for' = $2 + """, + thread["id"], + str(investigation_id), + ) + if existing is None: + await threads.append_message( + thread["id"], + author_kind="agent", + kind="investigation", + body_md=( + failure_markdown(failure) if failure else outcome_markdown(synthesis, hypotheses) + ), + payload={ + "phase": "outcome", + "outcome_for": str(investigation_id), + "investigation_id": str(investigation_id), + "run_id": str(run["id"]) if run else None, + "outcome": outcome, + }, + ) + event_type, event = ( + ("investigation_failed", {"code": failure.get("code")}) + if failure + else ("investigation_completed", {"confidence": synthesis.get("confidence")}) + ) + await app_db.execute( """ - SELECT id FROM issue_thread_messages - WHERE thread_id = $1 AND kind = 'investigation' - AND payload->>'outcome_for' = $2 + INSERT INTO issue_events (issue_id, event_type, payload) + VALUES ($1, $2, $3) """, - thread["id"], - str(investigation_id), + issue_id, + event_type, + to_json_string({"investigation_id": str(investigation_id), **event}), ) - if existing is None: - await threads.append_message( - thread["id"], - author_kind="agent", - kind="investigation", - body_md=( - failure_markdown(failure) - if failure - else outcome_markdown(synthesis, hypotheses) - ), - payload={ - "phase": "outcome", - "outcome_for": str(investigation_id), - "investigation_id": str(investigation_id), - "run_id": str(run["id"]) if run else None, - "outcome": outcome, - }, - ) - event_type, event = ( - ("investigation_failed", {"code": failure.get("code")}) - if failure - else ("investigation_completed", {"confidence": synthesis.get("confidence")}) - ) - await app_db.execute( - """ - INSERT INTO issue_events (issue_id, event_type, payload) - VALUES ($1, $2, $3) - """, - issue_id, - event_type, - to_json_string({"investigation_id": str(investigation_id), **event}), - ) - return {"published": True, "thread_message": existing is None} - - return publish_investigation_outcome + return {"published": True, "thread_message": existing is None} diff --git a/python-packages/dataing/tests/integration/api/test_start_investigation.py b/python-packages/dataing/tests/integration/api/test_start_investigation.py new file mode 100644 index 00000000..caabe4bc --- /dev/null +++ b/python-packages/dataing/tests/integration/api/test_start_investigation.py @@ -0,0 +1,401 @@ +"""POST /investigations: every run lives in an issue (docs/specs/0001_issue_chat.md §7.11).""" + +from __future__ import annotations + +import json +from typing import Any +from unittest.mock import AsyncMock +from uuid import UUID, uuid4 + +import httpx +import pytest +from fastapi import FastAPI + +from dataing.adapters.db.app_db import AppDatabase +from dataing.adapters.db.issue_threads import IssueThreadRepository +from dataing.adapters.db.issues import open_issue +from dataing.entrypoints.api.middleware.auth import ApiKeyContext, verify_api_key +from dataing.entrypoints.api.routes.investigations import router + +pytestmark = pytest.mark.integration + +SYMPTOM = "Completed orders dropped about 30% on 2026-09-14" +SDK_ALERT: dict[str, Any] = { + "dataset_ids": ["public.orders"], + "metric_spec": { + "metric_type": "column", + "expression": "customer_id", + "display_name": "Null rate of customer_id", + "columns_referenced": ["customer_id"], + }, + "anomaly_type": "null_rate", + "expected_value": 0.01, + "actual_value": 0.4, + "deviation_pct": 3900.0, + "anomaly_date": "2026-09-14", + "severity": "high", +} + + +async def _tenant(db: AppDatabase) -> UUID: + tenant_id = uuid4() + await db.execute( + "INSERT INTO tenants (id, name, slug) VALUES ($1, 't', $2)", tenant_id, f"t-{tenant_id.hex}" + ) + return tenant_id + + +async def _user(db: AppDatabase) -> UUID: + user_id = uuid4() + await db.execute( + "INSERT INTO users (id, email) VALUES ($1, $2)", user_id, f"{user_id.hex[:12]}@example.com" + ) + return user_id + + +async def _datasource(db: AppDatabase, tenant_id: UUID) -> UUID: + datasource_id = uuid4() + await db.execute( + """INSERT INTO data_sources (id, tenant_id, name, type, connection_config_encrypted) + VALUES ($1, $2, $3, 'postgresql', 'unused')""", + datasource_id, + tenant_id, + f"warehouse-{datasource_id.hex[:8]}", + ) + return datasource_id + + +async def _post( + db: AppDatabase, + tenant_id: UUID, + body: dict[str, Any], + *, + user_id: UUID | None, + temporal: AsyncMock | None = None, +) -> tuple[httpx.Response, AsyncMock]: + """POST /investigations with Temporal replaced by a mock that records the start.""" + temporal = temporal or AsyncMock() + app = FastAPI() + app.include_router(router, prefix="/api/v1") + app.state.app_db = db + app.state.temporal_client = temporal + app.dependency_overrides[verify_api_key] = lambda: ApiKeyContext( + key_id=uuid4(), + tenant_id=tenant_id, + tenant_slug="test", + tenant_name="Test Tenant", + user_id=user_id, + scopes=["read", "write"], + ) + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient(transport=transport, base_url="http://test") as client: + response = await client.post("/api/v1/investigations", json=body) + return response, temporal + + +async def _thread(db: AppDatabase, issue_id: UUID) -> list[dict[str, Any]]: + threads = IssueThreadRepository(db) + thread = await threads.ensure_shared_thread(issue_id) + return await threads.list_messages(thread["id"]) + + +async def _alert(db: AppDatabase, investigation_id: str) -> dict[str, Any]: + row = await db.fetch_one( + "SELECT alert FROM investigations WHERE id = $1", UUID(investigation_id) + ) + assert row is not None + alert: dict[str, Any] = json.loads(row["alert"]) + return alert + + +async def test_a_brief_opens_an_issue_and_starts_its_run(migrated_db: AppDatabase) -> None: + """Investigate… from any page: one call opens the issue and starts the run in its thread.""" + tenant_id = await _tenant(migrated_db) + user_id = await _user(migrated_db) + datasource_id = await _datasource(migrated_db, tenant_id) + + response, temporal = await _post( + migrated_db, + tenant_id, + { + "brief": {"symptom": SYMPTOM, "scope": {"tables": ["public.orders"]}}, + "datasource_id": str(datasource_id), + }, + user_id=user_id, + ) + + assert response.status_code == 200, response.text + started = response.json() + assert (started["issue_number"], started["status"]) == (1, "queued") + assert started["main_branch_id"] == started["investigation_id"] + issue_id = UUID(started["issue_id"]) + issue = await migrated_db.fetch_one("SELECT * FROM issues WHERE id = $1", issue_id) + assert issue is not None + assert (issue["title"], issue["dataset_id"], issue["created_by_user_id"]) == ( + SYMPTOM, + "public.orders", + user_id, + ) + run = await migrated_db.fetch_one( + "SELECT * FROM issue_investigation_runs WHERE id = $1", UUID(started["run_id"]) + ) + assert run is not None + assert (run["issue_id"], str(run["investigation_id"]), run["trigger_type"]) == ( + issue_id, + started["investigation_id"], + "human", + ) + opening, card = await _thread(migrated_db, issue_id) + assert opening["payload"]["event_type"] == "created" + assert (card["kind"], card["author_kind"], card["author_user_id"]) == ( + "investigation", + "user", + user_id, + ) + assert card["payload"]["run_id"] == started["run_id"] + stored = await _alert(migrated_db, started["investigation_id"]) + assert stored["issue_id"] == str(issue_id) + assert temporal.start_investigation.await_args.kwargs["alert_data"] == stored + + +async def test_an_sdk_alert_opens_an_issue_attributed_to_dataing( + migrated_db: AppDatabase, +) -> None: + """A run from an API key without a person opens an issue by dataing, with a brief.""" + tenant_id = await _tenant(migrated_db) + datasource_id = await _datasource(migrated_db, tenant_id) + + response, _ = await _post( + migrated_db, + tenant_id, + {"alert": SDK_ALERT, "datasource_id": str(datasource_id)}, + user_id=None, + ) + + assert response.status_code == 200, response.text + started = response.json() + symptom = ( + "Null rate of customer_id on public.orders: expected 0.01, got 0.4 (+3900.0%) on 2026-09-14" + ) + issue = await migrated_db.fetch_one( + "SELECT * FROM issues WHERE id = $1", UUID(started["issue_id"]) + ) + assert issue is not None + assert (issue["title"], issue["severity"], issue["created_by_user_id"]) == ( + symptom, + "high", + None, + ) + assert issue["author_type"] == "integration" + run = await migrated_db.fetch_one( + "SELECT trigger_type, brief FROM issue_investigation_runs WHERE id = $1", + UUID(started["run_id"]), + ) + assert run is not None + assert run["trigger_type"] == "api" + brief = json.loads(run["brief"]) + assert brief["symptom"] == symptom + assert brief["scope"]["tables"] == ["public.orders"] + assert brief["scope"]["time_window"] == { + "from": "2026-09-13T00:00:00Z", + "to": "2026-09-15T00:00:00Z", + } + _, card = await _thread(migrated_db, UUID(started["issue_id"])) + assert (card["author_kind"], card["author_user_id"]) == ("system", None) + stored = await _alert(migrated_db, started["investigation_id"]) + assert stored["anomaly_type"] == "null_rate" + assert stored["actual_value"] == 0.4 + + +async def test_an_issue_id_adds_the_run_to_that_issue(migrated_db: AppDatabase) -> None: + """Starting on an existing issue opens nothing new.""" + tenant_id = await _tenant(migrated_db) + user_id = await _user(migrated_db) + datasource_id = await _datasource(migrated_db, tenant_id) + issue = await open_issue( + migrated_db, tenant_id=tenant_id, title="Orders dropped", dataset_id="public.orders" + ) + + response, _ = await _post( + migrated_db, + tenant_id, + {"alert": SDK_ALERT, "issue_id": str(issue["id"]), "datasource_id": str(datasource_id)}, + user_id=user_id, + ) + + assert response.status_code == 200, response.text + assert response.json()["issue_id"] == str(issue["id"]) + count = await migrated_db.fetch_one( + "SELECT COUNT(*)::int AS n FROM issues WHERE tenant_id = $1", tenant_id + ) + assert count is not None + assert count["n"] == 1 + + +async def test_an_unknown_issue_is_not_found(migrated_db: AppDatabase) -> None: + """An issue_id from another tenant, or none at all, is a 404.""" + tenant_id = await _tenant(migrated_db) + datasource_id = await _datasource(migrated_db, tenant_id) + + response, temporal = await _post( + migrated_db, + tenant_id, + { + "brief": {"symptom": SYMPTOM}, + "issue_id": str(uuid4()), + "datasource_id": str(datasource_id), + }, + user_id=None, + ) + + assert response.status_code == 404 + temporal.start_investigation.assert_not_awaited() + + +@pytest.mark.parametrize( + "body", + [ + pytest.param({}, id="neither"), + pytest.param({"brief": {"symptom": SYMPTOM}, "alert": SDK_ALERT}, id="both"), + pytest.param( + {"alert": {k: v for k, v in SDK_ALERT.items() if k != "anomaly_type"}}, + id="malformed-alert", + ), + ], +) +async def test_the_request_needs_exactly_one_valid_brief_or_alert( + migrated_db: AppDatabase, body: dict[str, Any] +) -> None: + """A bad request is a 422, not a 500.""" + tenant_id = await _tenant(migrated_db) + + response, _ = await _post(migrated_db, tenant_id, body, user_id=None) + + assert response.status_code == 422 + + +async def test_a_brief_without_any_table_is_rejected(migrated_db: AppDatabase) -> None: + """An investigation needs a table to look at.""" + tenant_id = await _tenant(migrated_db) + datasource_id = await _datasource(migrated_db, tenant_id) + + response, _ = await _post( + migrated_db, + tenant_id, + {"brief": {"symptom": SYMPTOM}, "datasource_id": str(datasource_id)}, + user_id=None, + ) + + assert response.status_code == 400 + assert "table" in response.json()["detail"] + + +async def test_a_run_that_cannot_start_shows_why_on_its_card(migrated_db: AppDatabase) -> None: + """If Temporal refuses the start, the run is failed with the reason, and the API says 503.""" + tenant_id = await _tenant(migrated_db) + user_id = await _user(migrated_db) + datasource_id = await _datasource(migrated_db, tenant_id) + temporal = AsyncMock() + temporal.start_investigation.side_effect = RuntimeError("Temporal is unreachable") + + response, _ = await _post( + migrated_db, + tenant_id, + { + "brief": {"symptom": SYMPTOM, "scope": {"tables": ["public.orders"]}}, + "datasource_id": str(datasource_id), + }, + user_id=user_id, + temporal=temporal, + ) + + assert response.status_code == 503 + run = await migrated_db.fetch_one( + """ + SELECT r.issue_id, i.outcome FROM issue_investigation_runs r + JOIN investigations i ON i.id = r.investigation_id + JOIN issues s ON s.id = r.issue_id + WHERE s.tenant_id = $1 + """, + tenant_id, + ) + assert run is not None + outcome = json.loads(run["outcome"]) + assert outcome["status"] == "failed" + assert outcome["error"]["message"] == ( + "Couldn't start the investigation: Temporal is unreachable" + ) + messages = await _thread(migrated_db, run["issue_id"]) + kinds = [(m["kind"], m["payload"].get("phase")) for m in messages] + assert ("investigation", "outcome") in kinds + + +async def test_a_run_links_back_to_its_issue_with_its_number_and_failure( + migrated_db: AppDatabase, +) -> None: + """The run's details page gets the issue, the run number, the brief and why it failed.""" + from dataing.temporal.activities.publish_outcome import publish_outcome + from dataing.temporal.client import InvestigationStatus + + tenant_id = await _tenant(migrated_db) + user_id = await _user(migrated_db) + datasource_id = await _datasource(migrated_db, tenant_id) + body = { + "brief": {"symptom": SYMPTOM, "scope": {"tables": ["public.orders"]}}, + "datasource_id": str(datasource_id), + } + first, temporal = await _post(migrated_db, tenant_id, body, user_id=user_id) + issue_id = first.json()["issue_id"] + second, _ = await _post( + migrated_db, tenant_id, {**body, "issue_id": issue_id}, user_id=user_id, temporal=temporal + ) + started = second.json() + failure = { + "code": "invalid_key", + "message": "Anthropic rejected the API key (401).", + "step": "synthesize", + } + await publish_outcome( + migrated_db, + { + "investigation_id": started["investigation_id"], + "tenant_id": str(tenant_id), + "issue_id": issue_id, + "failure": failure, + "hypotheses": [{"id": "h1", "title": "late loads", "status": "untested"}], + }, + ) + temporal.get_status.return_value = InvestigationStatus( + workflow_id=started["investigation_id"], run_id=None, workflow_status="failed" + ) + + app = FastAPI() + app.include_router(router, prefix="/api/v1") + app.state.app_db = migrated_db + app.state.temporal_client = temporal + app.dependency_overrides[verify_api_key] = lambda: ApiKeyContext( + key_id=uuid4(), + tenant_id=tenant_id, + tenant_slug="test", + tenant_name="Test Tenant", + user_id=user_id, + scopes=["read"], + ) + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient(transport=transport, base_url="http://test") as client: + response = await client.get(f"/api/v1/investigations/{started['investigation_id']}") + + assert response.status_code == 200, response.text + state = response.json() + assert (state["issue_id"], state["issue_number"], state["issue_title"]) == ( + issue_id, + 1, + SYMPTOM, + ) + assert (state["run_number"], state["execution_profile"]) == (2, "standard") + assert state["brief"]["symptom"] == SYMPTOM + assert state["status"] == "failed" + assert state["error"] == failure + assert state["hypotheses"] == [ + {"id": "h1", "title": "late loads", "status": "untested", "reasoning": None} + ] diff --git a/python-packages/dataing/tests/integration/test_open_issue.py b/python-packages/dataing/tests/integration/test_open_issue.py new file mode 100644 index 00000000..3cca0219 --- /dev/null +++ b/python-packages/dataing/tests/integration/test_open_issue.py @@ -0,0 +1,112 @@ +"""open_issue() is the one way to insert an issue (docs/specs/0001_issue_chat.md §7.11).""" + +from __future__ import annotations + +import json +from uuid import UUID, uuid4 + +import pytest + +from dataing.adapters.db.app_db import AppDatabase +from dataing.adapters.db.issue_threads import IssueThreadRepository +from dataing.adapters.db.issues import open_issue + +pytestmark = pytest.mark.integration + + +async def _tenant(db: AppDatabase) -> UUID: + tenant_id = uuid4() + await db.execute( + "INSERT INTO tenants (id, name, slug) VALUES ($1, 't', $2)", tenant_id, f"t-{tenant_id.hex}" + ) + return tenant_id + + +async def _user(db: AppDatabase) -> UUID: + user_id = uuid4() + await db.execute( + "INSERT INTO users (id, email) VALUES ($1, $2)", user_id, f"{user_id.hex[:12]}@example.com" + ) + return user_id + + +async def _thread_messages(db: AppDatabase, issue_id: UUID) -> list[dict[str, object]]: + threads = IssueThreadRepository(db) + thread = await threads.ensure_shared_thread(issue_id) + return await threads.list_messages(thread["id"]) + + +async def test_a_person_opens_an_issue_and_its_thread_starts_with_it( + migrated_db: AppDatabase, +) -> None: + """The issue gets the next number and labels; the thread's first entry is the opening.""" + tenant_id = await _tenant(migrated_db) + user_id = await _user(migrated_db) + + first = await open_issue( + migrated_db, tenant_id=tenant_id, title="Orders dropped", created_by=user_id + ) + second = await open_issue( + migrated_db, + tenant_id=tenant_id, + title="Nulls in customer_id", + severity="high", + dataset_id="public.orders", + labels=["orders", "nulls"], + created_by=user_id, + ) + + assert (first["number"], second["number"]) == (1, 2) + assert (second["status"], second["author_type"], second["created_by_user_id"]) == ( + "open", + "human", + user_id, + ) + labels = await migrated_db.fetch_all( + "SELECT label FROM issue_labels WHERE issue_id = $1 ORDER BY label", second["id"] + ) + assert [row["label"] for row in labels] == ["nulls", "orders"] + (opening,) = await _thread_messages(migrated_db, second["id"]) + assert opening["kind"] == "event" + assert opening["author_user_id"] == user_id + assert opening["payload"] == {"event_type": "created", "title": "Nulls in customer_id"} + event = await migrated_db.fetch_one( + "SELECT actor_user_id FROM issue_events WHERE issue_id = $1 AND event_type = 'created'", + second["id"], + ) + assert event is not None + assert event["actor_user_id"] == user_id + + +async def test_an_integration_opens_an_issue_attributed_to_its_source( + migrated_db: AppDatabase, +) -> None: + """A webhook's issue has no person behind it; its opening names the source.""" + tenant_id = await _tenant(migrated_db) + + issue = await open_issue( + migrated_db, + tenant_id=tenant_id, + title="Freshness check failed", + author_type="integration", + source_provider="monte_carlo", + source_external_id="mc_incident_42", + source_external_url="https://getmontecarlo.com/incidents/42", + event_payload={"source": "webhook"}, + ) + + assert (issue["author_type"], issue["source_external_id"]) == ("integration", "mc_incident_42") + (opening,) = await _thread_messages(migrated_db, issue["id"]) + assert opening["author_user_id"] is None + assert opening["payload"] == { + "event_type": "created", + "title": "Freshness check failed", + "source_provider": "monte_carlo", + "source": "webhook", + } + event = await migrated_db.fetch_one( + "SELECT payload FROM issue_events WHERE issue_id = $1 AND event_type = 'created'", + issue["id"], + ) + assert event is not None + assert json.loads(event["payload"])["source_provider"] == "monte_carlo" diff --git a/python-packages/dataing/tests/unit/agents/test_chat_agent.py b/python-packages/dataing/tests/unit/agents/test_chat_agent.py index 9c31d1f2..63b20e00 100644 --- a/python-packages/dataing/tests/unit/agents/test_chat_agent.py +++ b/python-packages/dataing/tests/unit/agents/test_chat_agent.py @@ -152,6 +152,9 @@ async def test_query_tool_call_is_recorded_and_snapshotted(self) -> None: assert call.status == "ok" assert call.query_result_id is not None assert "1 row" in call.summary + # Totals for the collapsed line: "Ran 1 query · 12 ms · 1 row" + assert (call.duration_ms, call.row_count) == (12, 1) + assert call.to_payload()["duration_ms"] == 12 assert result.text == "Only us has rows." async def test_missing_credentials_is_a_structured_tool_result(self) -> None: diff --git a/python-packages/dataing/tests/unit/api/test_integrations_routes.py b/python-packages/dataing/tests/unit/api/test_integrations_routes.py index c2ba9eb8..e80f7667 100644 --- a/python-packages/dataing/tests/unit/api/test_integrations_routes.py +++ b/python-packages/dataing/tests/unit/api/test_integrations_routes.py @@ -370,10 +370,15 @@ async def test_auto_action_starts_investigation_when_temporal_available( mock_temporal.start_investigation = AsyncMock() mock_request.app.state.temporal_client = mock_temporal + issue_id = uuid4() + investigation_id = uuid4() with ( patch("dataing.entrypoints.api.routes.integrations.TeamPolicyRepository") as MockRepo, patch("dataing.entrypoints.api.routes.integrations.PolicyService") as MockPolicyService, patch("dataing.entrypoints.api.deps.resolve_datasource_id") as mock_resolve_ds, + patch( + "dataing.entrypoints.api.routes.integrations.InvestigationStarterService" + ) as MockStarter, ): mock_repo = MockRepo.return_value mock_repo.get_default_team_for_tenant = AsyncMock(return_value=team_id) @@ -390,18 +395,24 @@ async def test_auto_action_starts_investigation_when_temporal_available( mock_resolve_ds.return_value = uuid4() mock_db.execute = AsyncMock() + MockStarter.return_value.start = AsyncMock( + return_value=MagicMock(investigation_id=investigation_id) + ) action, inv_id = await _evaluate_and_apply_policy( request=mock_request, db=mock_db, auth=mock_auth, - issue_id=uuid4(), + issue_id=issue_id, payload=sample_payload, ) assert action == "auto" - assert inv_id is not None - mock_temporal.start_investigation.assert_called_once() + assert inv_id == investigation_id + # The run is started in the issue the webhook opened, like every run + start = MockStarter.return_value.start.await_args.kwargs + assert (start["issue_id"], start["trigger_type"]) == (issue_id, "webhook") + assert start["alert"].metric_spec.expression == "Test Issue" @pytest.mark.parametrize( "resolve_error", diff --git a/python-packages/dataing/tests/unit/api/test_issues_routes.py b/python-packages/dataing/tests/unit/api/test_issues_routes.py index 23b84efe..66fecd77 100644 --- a/python-packages/dataing/tests/unit/api/test_issues_routes.py +++ b/python-packages/dataing/tests/unit/api/test_issues_routes.py @@ -393,7 +393,10 @@ def test_investigation_run_response_fields(self) -> None: synthesis_summary=None, created_at=datetime.now(UTC), completed_at=None, + number=1, + status="running", ) + assert (data.number, data.status, data.error) == (1, "running", None) assert data.trigger_type == "human" assert data.execution_profile == "standard" assert data.approval_status is None diff --git a/python-packages/dataing/tests/unit/entrypoints/api/routes/test_investigations.py b/python-packages/dataing/tests/unit/entrypoints/api/routes/test_investigations.py index 902fec95..60375d08 100644 --- a/python-packages/dataing/tests/unit/entrypoints/api/routes/test_investigations.py +++ b/python-packages/dataing/tests/unit/entrypoints/api/routes/test_investigations.py @@ -78,8 +78,33 @@ def test_request_model_validation(self, sample_alert: dict[str, Any]) -> None: ) request = StartInvestigationRequest(alert=sample_alert) - assert request.alert["dataset_ids"] == ["analytics.events"] - assert request.alert["anomaly_type"] == "null_rate" + assert request.alert is not None + assert request.alert.dataset_ids == ["analytics.events"] + assert request.alert.anomaly_type == "null_rate" + assert (request.execution_profile, request.issue_id) == ("standard", None) + + @pytest.mark.parametrize( + "body", + [ + pytest.param({}, id="neither"), + pytest.param({"brief": {"symptom": "Orders dropped"}, "alert": None}, id="brief-only"), + ], + ) + def test_request_needs_exactly_one_of_brief_and_alert( + self, body: dict[str, Any], sample_alert: dict[str, Any] + ) -> None: + """A run starts from a brief or from an alert, never both or neither.""" + from pydantic import ValidationError + + from dataing.entrypoints.api.routes.investigations import StartInvestigationRequest + + if "brief" in body: + assert StartInvestigationRequest.model_validate(body).brief is not None + with pytest.raises(ValidationError): + StartInvestigationRequest.model_validate({**body, "alert": sample_alert}) + else: + with pytest.raises(ValidationError): + StartInvestigationRequest.model_validate(body) def test_response_model(self) -> None: """Test response model structure.""" @@ -90,13 +115,22 @@ def test_response_model(self) -> None: investigation_id = uuid.uuid4() branch_id = uuid.uuid4() + issue_id = uuid.uuid4() response = StartInvestigationResponse( investigation_id=investigation_id, main_branch_id=branch_id, + run_id=uuid.uuid4(), + issue_id=issue_id, + issue_number=42, ) assert response.investigation_id == investigation_id assert response.main_branch_id == branch_id + assert (response.issue_id, response.issue_number, response.status) == ( + issue_id, + 42, + "queued", + ) class TestGetInvestigationRoute: @@ -338,25 +372,15 @@ async def test_start_investigation_parses_alert( request = StartInvestigationRequest(alert=sample_alert) - # Verify the request parsing works - alert = AnomalyAlert( - dataset_ids=request.alert["dataset_ids"], - metric_spec=MetricSpec( - metric_type=request.alert["metric_spec"]["metric_type"], - expression=request.alert["metric_spec"]["expression"], - display_name=request.alert["metric_spec"]["display_name"], - columns_referenced=request.alert["metric_spec"].get("columns_referenced", []), - ), - anomaly_type=request.alert["anomaly_type"], - expected_value=request.alert["expected_value"], - actual_value=request.alert["actual_value"], - deviation_pct=request.alert["deviation_pct"], - anomaly_date=request.alert["anomaly_date"], - severity=request.alert["severity"], - ) - + alert = request.alert + assert isinstance(alert, AnomalyAlert) assert alert.dataset_id == "analytics.events" - assert alert.metric_spec.display_name == "NULL rate" + assert alert.metric_spec == MetricSpec( + metric_type="column", + expression="user_id", + display_name="NULL rate", + columns_referenced=["user_id"], + ) @pytest.mark.asyncio async def test_get_investigation_returns_state( diff --git a/python-packages/dataing/tests/unit/entrypoints/api/routes/test_issue_sidebar.py b/python-packages/dataing/tests/unit/entrypoints/api/routes/test_issue_sidebar.py index 4295665c..859e6852 100644 --- a/python-packages/dataing/tests/unit/entrypoints/api/routes/test_issue_sidebar.py +++ b/python-packages/dataing/tests/unit/entrypoints/api/routes/test_issue_sidebar.py @@ -19,10 +19,10 @@ from fastapi import FastAPI from fastapi.testclient import TestClient +from dataing.adapters.db import issues as issues_adapter from dataing.core.auth.jwt import create_access_token from dataing.core.auth.types import OrgRole from dataing.entrypoints.api.deps import get_app_db -from dataing.entrypoints.api.routes import issues as issues_module from dataing.entrypoints.api.routes.issues import router, transition_options TENANT_ID = uuid.uuid4() @@ -162,7 +162,7 @@ async def append_event(self, *args: Any, **kwargs: Any) -> dict[str, Any]: @pytest.fixture def db(monkeypatch: pytest.MonkeyPatch) -> FakeIssueDb: """Return the fake database, with thread event copies disabled.""" - monkeypatch.setattr(issues_module, "IssueThreadRepository", FakeThreadRepository) + monkeypatch.setattr(issues_adapter, "IssueThreadRepository", FakeThreadRepository) return FakeIssueDb() diff --git a/python-packages/dataing/tests/unit/entrypoints/api/routes/test_issue_threads.py b/python-packages/dataing/tests/unit/entrypoints/api/routes/test_issue_threads.py index 590407d8..df6f55ca 100644 --- a/python-packages/dataing/tests/unit/entrypoints/api/routes/test_issue_threads.py +++ b/python-packages/dataing/tests/unit/entrypoints/api/routes/test_issue_threads.py @@ -599,6 +599,12 @@ class TestEventDescriptions: {"root_cause": "app_v2 writes COMPLETE"}, "Resolved with confirmed cause: app_v2 writes COMPLETE", ), + ("created", {"title": "Orders dropped"}, "Issue opened"), + ( + "created", + {"title": "Orders dropped", "source_provider": "monte_carlo"}, + "Issue opened from monte_carlo", + ), ], ) def test_describe_event(self, event_type: str, payload: dict[str, Any], text: str) -> None: From 1eb1564762d96e946811910e7620fef07e54ce66 Mon Sep 17 00:00:00 2001 From: bordumb Date: Mon, 28 Sep 2026 21:08:57 +0100 Subject: [PATCH 04/18] feat(api): check the Anthropic key at startup and say what to fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nothing checked the key before, so a rejected key first showed up as a run that did nothing, and a chat reply or brief draft that failed showed the raw ModelHTTPError text. Now (spec 0001 §7.12): - at startup the API calls GET /v1/models/{id} for LLM_MODEL and CHAT_AGENT_MODEL (no tokens). It runs in the background with a 10 s timeout and no retries, so startup never waits on it - GET /api/v1/system/llm returns {state, message, models, checked_at} for the app's banner. A transient problem is checked again once it is over a minute old - chat turns and brief drafts raise the classified LLMRejected or LLMUnavailable error. The reply shows what to fix, and a rejected key isn't retried Checked against the real API with a bogus key: invalid_key, "Anthropic rejected the API key (401). Set a valid ANTHROPIC_API_KEY and restart the API and the worker." Co-Authored-By: Claude Opus 5.5 Signed-off-by: Claude --- .flow/tasks/fn-70.20.json | 8 +- .flow/tasks/fn-70.21.json | 17 +- .flow/tasks/fn-70.21.md | 7 +- .../src/dataing/entrypoints/api/deps.py | 13 +- .../entrypoints/api/routes/__init__.py | 2 + .../dataing/entrypoints/api/routes/system.py | 41 ++++ .../src/dataing/services/llm_status.py | 132 +++++++++++++ .../dataing/temporal/activities/agent_turn.py | 29 ++- .../temporal/workflows/issue_thread.py | 19 +- .../integration/test_agent_turn_activity.py | 63 ++++++ .../entrypoints/api/routes/test_system.py | 58 ++++++ .../tests/unit/services/test_llm_status.py | 179 ++++++++++++++++++ .../temporal/test_issue_thread_workflow.py | 38 ++++ 13 files changed, 587 insertions(+), 19 deletions(-) create mode 100644 python-packages/dataing/src/dataing/entrypoints/api/routes/system.py create mode 100644 python-packages/dataing/src/dataing/services/llm_status.py create mode 100644 python-packages/dataing/tests/unit/entrypoints/api/routes/test_system.py create mode 100644 python-packages/dataing/tests/unit/services/test_llm_status.py diff --git a/.flow/tasks/fn-70.20.json b/.flow/tasks/fn-70.20.json index 897d72d3..09fc130c 100644 --- a/.flow/tasks/fn-70.20.json +++ b/.flow/tasks/fn-70.20.json @@ -1,7 +1,7 @@ { - "assignee": null, + "assignee": "bordumbb@gmail.com", "claim_note": "", - "claimed_at": null, + "claimed_at": "2026-09-28T19:34:45.513313Z", "created_at": "2026-09-28T17:51:59.475477Z", "depends_on": [ "fn-70.19" @@ -10,7 +10,7 @@ "id": "fn-70.20", "priority": null, "spec_path": ".flow/tasks/fn-70.20.md", - "status": "todo", + "status": "in_progress", "title": "M5: LLM key check at startup, GET /system/llm, readable chat errors", - "updated_at": "2026-09-28T17:51:59.475730Z" + "updated_at": "2026-09-28T19:34:45.513545Z" } diff --git a/.flow/tasks/fn-70.21.json b/.flow/tasks/fn-70.21.json index b49294a0..1803c1fd 100644 --- a/.flow/tasks/fn-70.21.json +++ b/.flow/tasks/fn-70.21.json @@ -7,10 +7,23 @@ "fn-70.18" ], "epic": "fn-70", + "evidence": { + "commits": [ + "d7522281" + ], + "prs": [], + "tests": [ + "tests/integration/test_open_issue.py", + "tests/integration/api/test_start_investigation.py", + "tests/integration/api/test_issue_investigation_runs.py", + "dataing-ee tests/integration/core/automation/test_spawn_investigation.py", + "CE unit 3173, EE unit 714, CE integration 143, EE integration 16, SDK+CLI 272" + ] + }, "id": "fn-70.21", "priority": null, "spec_path": ".flow/tasks/fn-70.21.md", - "status": "in_progress", + "status": "done", "title": "M5: one starter and open_issue() for every start path", - "updated_at": "2026-09-28T18:22:06.382808Z" + "updated_at": "2026-09-28T19:34:45.173067Z" } diff --git a/.flow/tasks/fn-70.21.md b/.flow/tasks/fn-70.21.md index 5c8b468b..32441cb3 100644 --- a/.flow/tasks/fn-70.21.md +++ b/.flow/tasks/fn-70.21.md @@ -15,9 +15,8 @@ TBD ## Done summary -TBD - +open_issue() is the only issue insert (API, CE + EE webhooks, starter) and posts the thread's opening event. InvestigationStarterService.start() resolves/opens the issue, builds the missing brief/alert, writes investigation + run row + card, starts the workflow with alert.issue_id, and records a failed start on the card. POST /investigations takes brief|alert (+issue_id, profile) and returns issue_id/issue_number/run_id; spawn route, webhooks and EE rule action use the starter. Runs report number/status/error; GET /investigations/{id} adds issue, run number, brief, error, hypotheses. Tool calls record duration_ms/row_count. SDK Investigation gains issue fields; CLI links the issue. ## Evidence -- Commits: -- Tests: +- Commits: d7522281 +- Tests: tests/integration/test_open_issue.py, tests/integration/api/test_start_investigation.py, tests/integration/api/test_issue_investigation_runs.py, dataing-ee tests/integration/core/automation/test_spawn_investigation.py, CE unit 3173, EE unit 714, CE integration 143, EE integration 16, SDK+CLI 272 - PRs: diff --git a/python-packages/dataing/src/dataing/entrypoints/api/deps.py b/python-packages/dataing/src/dataing/entrypoints/api/deps.py index 74578f43..fc5ac459 100644 --- a/python-packages/dataing/src/dataing/entrypoints/api/deps.py +++ b/python-packages/dataing/src/dataing/entrypoints/api/deps.py @@ -32,6 +32,7 @@ from dataing.core.investigation.collaboration import CollaborationService from dataing.core.investigation.service import InvestigationService from dataing.core.json_utils import to_json_string +from dataing.services.llm_status import LLMStatusChecker from dataing.services.usage import UsageTracker if TYPE_CHECKING: @@ -51,6 +52,15 @@ async def lifespan(app: FastAPI) -> AsyncIterator[None]: - LLM client initialization - Orchestrator configuration """ + # Check the Anthropic key and models in the background; the app's banner reads + # the result (docs/specs/0001_issue_chat.md §7.12) + llm_status = LLMStatusChecker( + api_key=settings.anthropic_api_key, + models=[settings.llm_model, settings.chat_agent_model], + ) + llm_status.start() + app.state.llm_status = llm_status + # Setup application database app_db = AppDatabase(settings.app_database_url) await app_db.connect() @@ -210,7 +220,8 @@ async def lifespan(app: FastAPI) -> AsyncIterator[None]: yield - # Teardown - close all cached adapters + # Teardown - stop the key check if it is still running, close all cached adapters + await llm_status.aclose() for cache_key, adapter in app.state.adapter_cache.items(): try: await adapter.disconnect() diff --git a/python-packages/dataing/src/dataing/entrypoints/api/routes/__init__.py b/python-packages/dataing/src/dataing/entrypoints/api/routes/__init__.py index 426e1a23..88fad126 100644 --- a/python-packages/dataing/src/dataing/entrypoints/api/routes/__init__.py +++ b/python-packages/dataing/src/dataing/entrypoints/api/routes/__init__.py @@ -46,6 +46,7 @@ from dataing.entrypoints.api.routes.repo_mappings import router as repo_mappings_router from dataing.entrypoints.api.routes.schema_comments import router as schema_comments_router from dataing.entrypoints.api.routes.sla_policies import router as sla_policies_router +from dataing.entrypoints.api.routes.system import router as system_router from dataing.entrypoints.api.routes.tags import ( investigation_tags_router, ) @@ -67,6 +68,7 @@ api_router.include_router(investigation_steers_router) # Steer a running investigation api_router.include_router(issues_router) # Issues CRUD API api_router.include_router(issue_threads_router) # Issue hub threads and messages +api_router.include_router(system_router) # Key and model check for the app banner api_router.include_router(datasources_router) api_router.include_router(datasources_v2_router, prefix="/v2") # New unified adapter API api_router.include_router(credentials_router) # User datasource credentials diff --git a/python-packages/dataing/src/dataing/entrypoints/api/routes/system.py b/python-packages/dataing/src/dataing/entrypoints/api/routes/system.py new file mode 100644 index 00000000..bc573f09 --- /dev/null +++ b/python-packages/dataing/src/dataing/entrypoints/api/routes/system.py @@ -0,0 +1,41 @@ +"""System status for the app's banner (docs/specs/0001_issue_chat.md §7.12).""" + +from __future__ import annotations + +from datetime import datetime +from typing import Annotated + +from fastapi import APIRouter, Depends, Request +from pydantic import BaseModel + +from dataing.entrypoints.api.middleware.auth import ApiKeyContext, verify_api_key +from dataing.services.llm_status import LLMStatusChecker + +router = APIRouter(prefix="/system", tags=["system"]) + +AuthDep = Annotated[ApiKeyContext, Depends(verify_api_key)] + + +class LLMStatusResponse(BaseModel): + """Whether the Anthropic key and models work.""" + + state: str # ok, checking, or a problem code such as invalid_key or unknown_model + message: str # What is wrong and what to fix, or that the key works + models: list[str] + checked_at: datetime | None = None + + +@router.get("/llm", response_model=LLMStatusResponse) +async def get_llm_status(request: Request, auth: AuthDep) -> LLMStatusResponse: + """Return the startup check of the Anthropic key and models. + + Every page shows a banner while this reports a problem. A transient problem + (Anthropic unreachable, overloaded) is checked again when it is over a minute old. + """ + checker: LLMStatusChecker | None = getattr(request.app.state, "llm_status", None) + if checker is None: + return LLMStatusResponse( + state="checking", message="The API key hasn't been checked yet.", models=[] + ) + status = await checker.current() + return LLMStatusResponse(**status.to_dict()) diff --git a/python-packages/dataing/src/dataing/services/llm_status.py b/python-packages/dataing/src/dataing/services/llm_status.py new file mode 100644 index 00000000..e04c73fd --- /dev/null +++ b/python-packages/dataing/src/dataing/services/llm_status.py @@ -0,0 +1,132 @@ +"""Whether the Anthropic key and models work (docs/specs/0001_issue_chat.md §7.12). + +At startup the API asks Anthropic for each configured model (GET /v1/models/{id}, +which costs no tokens). Every page shows a banner while the answer is a problem, so +a rejected key is visible before anyone starts a run. The check runs in the +background with a short timeout and no retries, so a slow network never delays +startup. +""" + +from __future__ import annotations + +import asyncio +import contextlib +from collections.abc import Callable, Sequence +from dataclasses import dataclass +from datetime import UTC, datetime, timedelta +from typing import Any + +import anthropic +import structlog + +from dataing.agents.errors import classify_llm_error + +logger = structlog.get_logger() + +CHECK_TIMEOUT_SECONDS = 10.0 +# Problems that may pass on their own; the others need a new key or model and a restart +TRANSIENT_STATES = frozenset({"unreachable", "rate_limited", "overloaded", "server_error"}) +RECHECK_AFTER = timedelta(seconds=60) + + +@dataclass +class LLMStatus: + """The result of the last check.""" + + state: str + message: str + models: list[str] + checked_at: datetime | None = None + + def to_dict(self) -> dict[str, Any]: + """Return the status as the API returns it.""" + return { + "state": self.state, + "message": self.message, + "models": self.models, + "checked_at": self.checked_at, + } + + +def _anthropic_client(api_key: str) -> Any: + return anthropic.AsyncAnthropic(api_key=api_key, max_retries=0, timeout=CHECK_TIMEOUT_SECONDS) + + +class LLMStatusChecker: + """Checks, once at startup, that the key works for every configured model.""" + + def __init__( + self, + api_key: str, + models: Sequence[str], + client_factory: Callable[[str], Any] = _anthropic_client, + clock: Callable[[], datetime] = lambda: datetime.now(UTC), + ) -> None: + """Initialize with the key and the models the API and worker use.""" + self._api_key = api_key + self._models = list(dict.fromkeys(model for model in models if model)) + self._client_factory = client_factory + self._clock = clock + self._task: asyncio.Task[LLMStatus] | None = None + self._status = LLMStatus( + state="checking", + message="Checking the Anthropic API key…", + models=self._models, + ) + + @property + def status(self) -> LLMStatus: + """Return the last result, without checking again.""" + return self._status + + def start(self) -> None: + """Check in the background.""" + self._task = asyncio.create_task(self.check()) + + async def wait(self) -> LLMStatus: + """Wait for a started check to finish and return its result.""" + if self._task is not None: + with contextlib.suppress(Exception): + await self._task + return self._status + + async def aclose(self) -> None: + """Stop a check that is still running.""" + if self._task is not None and not self._task.done(): + self._task.cancel() + with contextlib.suppress(asyncio.CancelledError, Exception): + await self._task + + async def current(self) -> LLMStatus: + """Return the status, checking again when a transient problem has gone stale.""" + checked_at = self._status.checked_at + stale = checked_at is None or self._clock() - checked_at >= RECHECK_AFTER + running = self._task is not None and not self._task.done() + if self._status.state in TRANSIENT_STATES and stale and not running: + return await self.check() + return self._status + + async def check(self) -> LLMStatus: + """Ask Anthropic for every model; stop at the first problem.""" + if not self._api_key: + return self._set( + "missing_key", + "ANTHROPIC_API_KEY isn't set. Set it and restart the API and the worker.", + ) + client = self._client_factory(self._api_key) + for model in self._models: + try: + await client.models.retrieve(model) + except Exception as e: + failure = classify_llm_error(e, model=model) + if failure is None: + return self._set("unreachable", f"Couldn't check the Anthropic API key: {e}") + logger.warning("llm_check_failed", state=failure.code, model=model) + return self._set(failure.code, failure.message) + return self._set("ok", f"Anthropic accepted the API key for {', '.join(self._models)}.") + + def _set(self, state: str, message: str) -> LLMStatus: + self._status = LLMStatus( + state=state, message=message, models=self._models, checked_at=self._clock() + ) + return self._status diff --git a/python-packages/dataing/src/dataing/temporal/activities/agent_turn.py b/python-packages/dataing/src/dataing/temporal/activities/agent_turn.py index 63bad567..8fecb7b4 100644 --- a/python-packages/dataing/src/dataing/temporal/activities/agent_turn.py +++ b/python-packages/dataing/src/dataing/temporal/activities/agent_turn.py @@ -44,6 +44,7 @@ draft_brief, run_turn, ) +from dataing.agents.errors import classify_llm_error from dataing.core.agent_query import AgentQueryService from dataing.core.investigation.brief import BriefDraft, brief_from_draft, brief_to_markdown from dataing.core.issue_chat import ( @@ -51,6 +52,7 @@ ThreadChatServices, resolve_chat_datasource, ) +from dataing.temporal.errors import llm_activity_error logger = logging.getLogger(__name__) @@ -202,6 +204,8 @@ def on_text(delta: str) -> None: status="cancelled", ) raise + except Exception as e: + raise _readable(e) from e await threads.update_message( reply_id, @@ -242,6 +246,16 @@ async def mark_turn_failed(request: dict[str, Any]) -> None: return mark_turn_failed +def _readable(error: Exception) -> Exception: + """Return the error to fail a turn with: a model failure says what to fix. + + A failure no retry can fix is LLMRejected, so the turn isn't retried + (docs/specs/0001_issue_chat.md §7.12); anything else is raised as it is. + """ + failure = classify_llm_error(error) + return llm_activity_error(failure) if failure is not None else error + + async def _with_heartbeats(work: Awaitable[T]) -> T: """Await work while heartbeating, so long model calls don't time out.""" task = asyncio.ensure_future(work) @@ -382,13 +396,16 @@ async def run_brief_draft(request: dict[str, Any]) -> dict[str, Any]: overview = await services.issue_context() sources = await _brief_sources(app_db, threads, issue_id, thread_id, message["seq"]) - draft, usage = await _with_heartbeats( - draft_brief( - agent_factory(), - build_history(sources.history, max_messages=len(sources.history)), - instructions=build_instructions(overview), + try: + draft, usage = await _with_heartbeats( + draft_brief( + agent_factory(), + build_history(sources.history, max_messages=len(sources.history)), + instructions=build_instructions(overview), + ) ) - ) + except Exception as e: + raise _readable(e) from e brief = brief_from_draft( draft, seq_to_message=sources.seq_to_message, diff --git a/python-packages/dataing/src/dataing/temporal/workflows/issue_thread.py b/python-packages/dataing/src/dataing/temporal/workflows/issue_thread.py index 684c5de3..de8fe016 100644 --- a/python-packages/dataing/src/dataing/temporal/workflows/issue_thread.py +++ b/python-packages/dataing/src/dataing/temporal/workflows/issue_thread.py @@ -22,7 +22,10 @@ from temporalio import workflow from temporalio.common import RetryPolicy -from temporalio.exceptions import ActivityError +from temporalio.exceptions import ActivityError, ApplicationError + +with workflow.unsafe.imports_passed_through(): + from dataing.temporal.errors import LLM_REJECTED, LLM_UNAVAILABLE TURN_TIMEOUT = timedelta(minutes=5) TURN_HEARTBEAT_TIMEOUT = timedelta(seconds=30) @@ -138,9 +141,21 @@ async def _run_turn(self, request: dict[str, Any]) -> None: workflow.logger.warning(f"Turn failed: {self._current}: {e}") await workflow.execute_activity( "mark_turn_failed", - {**request, "error": str(e.cause or e)}, + {**request, "error": _failure_text(e)}, start_to_close_timeout=timedelta(seconds=30), ) finally: self._current = None self._current_turn = None + + +def _failure_text(error: ActivityError) -> str: + """Return why a turn failed, as the person reading the thread needs it. + + A model failure's message says what to fix (docs/specs/0001_issue_chat.md §7.12); + anything else keeps its error type. + """ + cause = error.cause + if isinstance(cause, ApplicationError) and cause.type in (LLM_REJECTED, LLM_UNAVAILABLE): + return cause.message + return str(cause or error) diff --git a/python-packages/dataing/tests/integration/test_agent_turn_activity.py b/python-packages/dataing/tests/integration/test_agent_turn_activity.py index f894a952..e29a457f 100644 --- a/python-packages/dataing/tests/integration/test_agent_turn_activity.py +++ b/python-packages/dataing/tests/integration/test_agent_turn_activity.py @@ -389,3 +389,66 @@ def respond(messages: list[ModelMessage], info: AgentInfo) -> ModelResponse: assert scratch_claim["message_id"] == str(scratch_note["id"]) assert scratch_claim["query_result_id"] == str(scratch_result) assert brief["ruled_out"][0]["message_id"] == str(scratch_reply["id"]) + + +INVALID_KEY = ( + "Anthropic rejected the API key (401). Set a valid ANTHROPIC_API_KEY and " + "restart the API and the worker." +) + + +async def test_a_rejected_key_fails_the_turn_with_what_to_fix(migrated_db: AppDatabase) -> None: + """The reply shows what to fix, not the raw ModelHTTPError, and isn't retried.""" + from pydantic_ai.exceptions import ModelHTTPError + from temporalio.exceptions import ApplicationError + + from dataing.agents.chat import build_chat_agent + + async def reject(messages: list[ModelMessage], info: AgentInfo) -> Any: + raise ModelHTTPError(status_code=401, model_name="claude-opus-5-5", body=None) + yield # an async generator, as stream functions are + + request = await _thread_with_question(migrated_db) + activity_fn = make_run_agent_turn_activity( + migrated_db, lambda: build_chat_agent(FunctionModel(stream_function=reject)), FakeQueries() + ) + + with pytest.raises(ApplicationError) as caught: + await ActivityEnvironment().run(activity_fn, request) + + assert (caught.value.type, caught.value.non_retryable) == ("LLMRejected", True) + assert caught.value.message == INVALID_KEY + + +async def test_a_rejected_key_fails_the_brief_draft_with_what_to_fix( + migrated_db: AppDatabase, +) -> None: + """Drafting a brief fails the same way: the reason, once.""" + from pydantic_ai.exceptions import ModelHTTPError + from pydantic_ai.messages import ModelResponse + from temporalio.exceptions import ApplicationError + + from dataing.agents.chat import build_brief_agent + from dataing.temporal.activities.agent_turn import make_run_brief_draft_activity + + def reject(messages: list[ModelMessage], info: AgentInfo) -> ModelResponse: + raise ModelHTTPError(status_code=401, model_name="claude-opus-5-5", body=None) + + request = await _thread_with_question(migrated_db) + brief_msg = await IssueThreadRepository(migrated_db).append_message( + uuid.UUID(request["thread_id"]), + author_kind="agent", + kind="brief", + requested_by_user_id=uuid.UUID(request["requested_by"]), + status="queued", + ) + activity_fn = make_run_brief_draft_activity( + migrated_db, lambda: build_brief_agent(FunctionModel(reject)), FakeQueries() + ) + + with pytest.raises(ApplicationError) as caught: + await ActivityEnvironment().run( + activity_fn, {**request, "message_id": str(brief_msg["id"]), "kind": "draft_brief"} + ) + + assert (caught.value.type, caught.value.message) == ("LLMRejected", INVALID_KEY) diff --git a/python-packages/dataing/tests/unit/entrypoints/api/routes/test_system.py b/python-packages/dataing/tests/unit/entrypoints/api/routes/test_system.py new file mode 100644 index 00000000..2fd2b224 --- /dev/null +++ b/python-packages/dataing/tests/unit/entrypoints/api/routes/test_system.py @@ -0,0 +1,58 @@ +"""GET /system/llm: whether the Anthropic key and models work (spec 0001 §7.12).""" + +from __future__ import annotations + +from uuid import uuid4 + +from fastapi import FastAPI +from fastapi.testclient import TestClient + +from dataing.entrypoints.api.middleware.auth import ApiKeyContext, verify_api_key +from dataing.entrypoints.api.routes.system import router +from dataing.services.llm_status import LLMStatusChecker + + +def _client(checker: LLMStatusChecker | None, *, signed_in: bool = True) -> TestClient: + app = FastAPI() + app.include_router(router, prefix="/api/v1") + if checker is not None: + app.state.llm_status = checker + if signed_in: + app.dependency_overrides[verify_api_key] = lambda: ApiKeyContext( + key_id=uuid4(), + tenant_id=uuid4(), + tenant_slug="t", + tenant_name="T", + user_id=uuid4(), + scopes=["read"], + ) + return TestClient(app) + + +async def test_any_signed_in_person_sees_the_last_check() -> None: + """Viewers see the banner too: a broken key affects everyone.""" + checker = LLMStatusChecker(api_key="", models=["claude-opus-5-5"]) + await checker.check() + + response = _client(checker).get("/api/v1/system/llm") + + assert response.status_code == 200 + body = response.json() + assert (body["state"], body["models"]) == ("missing_key", ["claude-opus-5-5"]) + assert body["message"].startswith("ANTHROPIC_API_KEY isn't set") + assert body["checked_at"] is not None + + +def test_before_any_check_the_state_is_checking() -> None: + """An app without a checker (tests, scripts) reports nothing wrong yet.""" + response = _client(None).get("/api/v1/system/llm") + + assert response.status_code == 200 + assert response.json()["state"] == "checking" + + +def test_it_needs_a_signed_in_caller() -> None: + """The status names the configured models, so it isn't public.""" + response = _client(None, signed_in=False).get("/api/v1/system/llm") + + assert response.status_code == 401 diff --git a/python-packages/dataing/tests/unit/services/test_llm_status.py b/python-packages/dataing/tests/unit/services/test_llm_status.py new file mode 100644 index 00000000..452ed870 --- /dev/null +++ b/python-packages/dataing/tests/unit/services/test_llm_status.py @@ -0,0 +1,179 @@ +"""The API checks the Anthropic key and models at startup (docs/specs/0001_issue_chat.md §7.12). + +The Anthropic client is faked: each model id either answers or raises the error +the real SDK raises for it. +""" + +from __future__ import annotations + +from datetime import UTC, datetime, timedelta +from typing import Any + +import anthropic +import httpx2 +import pytest + +from dataing.services.llm_status import LLMStatusChecker + +REQUEST = httpx2.Request("GET", "https://api.anthropic.com/v1/models/x") +INVESTIGATION_MODEL = "claude-sonnet-4-20250514" +CHAT_MODEL = "claude-opus-5-5" + + +def _status_error(status: int) -> anthropic.APIStatusError: + response = httpx2.Response(status, request=REQUEST, json={}) + classes: dict[int, type[anthropic.APIStatusError]] = { + 401: anthropic.AuthenticationError, + 403: anthropic.PermissionDeniedError, + 404: anthropic.NotFoundError, + } + return classes[status](f"Error code: {status}", response=response, body=None) + + +class FakeAnthropic: + """Answers models.retrieve, raising the scripted error for a model id.""" + + def __init__(self, errors: dict[str, Exception]) -> None: + self.errors = errors + self.retrieved: list[str] = [] + self.models = self + + async def retrieve(self, model_id: str, **_: Any) -> dict[str, str]: + self.retrieved.append(model_id) + if model_id in self.errors: + raise self.errors[model_id] + return {"id": model_id} + + +class Clock: + """A clock tests can move.""" + + def __init__(self) -> None: + self.now = datetime(2026, 9, 28, 12, 0, tzinfo=UTC) + + def __call__(self) -> datetime: + return self.now + + +def _checker( + client: FakeAnthropic, *, api_key: str = "sk-test", clock: Clock | None = None +) -> LLMStatusChecker: + return LLMStatusChecker( + api_key=api_key, + models=[INVESTIGATION_MODEL, CHAT_MODEL, INVESTIGATION_MODEL], + client_factory=lambda key: client, + clock=clock or Clock(), + ) + + +def test_the_status_is_checking_until_the_first_check() -> None: + """Before the background check finishes there is nothing to report.""" + status = _checker(FakeAnthropic({})).status + + assert (status.state, status.checked_at) == ("checking", None) + assert status.models == [INVESTIGATION_MODEL, CHAT_MODEL] + + +async def test_a_working_key_is_ok_for_every_model() -> None: + """Each configured model is retrieved once.""" + client = FakeAnthropic({}) + + status = await _checker(client).check() + + assert status.state == "ok" + assert client.retrieved == [INVESTIGATION_MODEL, CHAT_MODEL] + assert status.checked_at is not None + + +async def test_an_empty_key_is_reported_without_a_request() -> None: + """With no key there is nothing to ask Anthropic.""" + + def no_client(key: str) -> FakeAnthropic: + raise AssertionError("no request without a key") + + checker = LLMStatusChecker( + api_key="", models=[INVESTIGATION_MODEL], client_factory=no_client, clock=Clock() + ) + + status = await checker.check() + + assert status.state == "missing_key" + assert status.message == ( + "ANTHROPIC_API_KEY isn't set. Set it and restart the API and the worker." + ) + + +async def test_a_rejected_key_says_what_to_fix() -> None: + """A 401 stops the check: every model would fail the same way.""" + client = FakeAnthropic({INVESTIGATION_MODEL: _status_error(401)}) + + status = await _checker(client).check() + + assert status.state == "invalid_key" + assert status.message == ( + "Anthropic rejected the API key (401). Set a valid ANTHROPIC_API_KEY and " + "restart the API and the worker." + ) + assert client.retrieved == [INVESTIGATION_MODEL] + + +@pytest.mark.parametrize( + ("status_code", "state", "message"), + [ + ( + 404, + "unknown_model", + f"Anthropic doesn't know the model {CHAT_MODEL} (404). " + "Check LLM_MODEL and CHAT_AGENT_MODEL.", + ), + (403, "forbidden", f"The API key isn't allowed to use {CHAT_MODEL} (403)."), + ], +) +async def test_a_model_the_key_cannot_use_is_named( + status_code: int, state: str, message: str +) -> None: + """The message names the model that failed.""" + client = FakeAnthropic({CHAT_MODEL: _status_error(status_code)}) + + status = await _checker(client).check() + + assert (status.state, status.message) == (state, message) + + +async def test_an_unreachable_api_is_checked_again_after_a_minute() -> None: + """A network problem may pass, so a stale unreachable result is checked again on read.""" + clock = Clock() + client = FakeAnthropic({INVESTIGATION_MODEL: anthropic.APIConnectionError(request=REQUEST)}) + checker = _checker(client, clock=clock) + assert (await checker.check()).state == "unreachable" + + client.errors.clear() + clock.now += timedelta(seconds=30) + assert (await checker.current()).state == "unreachable" + clock.now += timedelta(seconds=31) + assert (await checker.current()).state == "ok" + + +async def test_a_rejected_key_is_not_checked_again() -> None: + """The key comes from the environment: only a restart can change it.""" + clock = Clock() + client = FakeAnthropic({INVESTIGATION_MODEL: _status_error(401)}) + checker = _checker(client, clock=clock) + await checker.check() + + clock.now += timedelta(hours=1) + await checker.current() + + assert client.retrieved == [INVESTIGATION_MODEL] + + +async def test_a_started_check_runs_in_the_background() -> None: + """start() never blocks the caller; close() stops a check still running.""" + client = FakeAnthropic({}) + checker = _checker(client) + + checker.start() + await checker.wait() + await checker.aclose() + + assert checker.status.state == "ok" diff --git a/python-packages/dataing/tests/unit/temporal/test_issue_thread_workflow.py b/python-packages/dataing/tests/unit/temporal/test_issue_thread_workflow.py index a605d106..d1a398f0 100644 --- a/python-packages/dataing/tests/unit/temporal/test_issue_thread_workflow.py +++ b/python-packages/dataing/tests/unit/temporal/test_issue_thread_workflow.py @@ -37,6 +37,10 @@ def __init__(self) -> None: self.release = asyncio.Event() self.block: set[str] = set() self.fail: set[str] = set() + # Message ids whose turn raises this error; errors mark_turn_failed recorded + self.raises: dict[str, BaseException] = {} + self.errors: dict[str, str] = {} + self.attempts: dict[str, int] = {} def activities(self) -> list[Any]: """Return fake run_agent_turn and mark_turn_failed activities.""" @@ -44,6 +48,9 @@ def activities(self) -> list[Any]: @activity.defn(name="run_agent_turn") async def run_agent_turn(request: dict[str, Any]) -> dict[str, Any]: message_id = request["message_id"] + self.attempts[message_id] = self.attempts.get(message_id, 0) + 1 + if message_id in self.raises: + raise self.raises[message_id] if message_id in self.fail: raise RuntimeError("model unavailable") if message_id in self.block: @@ -61,6 +68,7 @@ async def run_brief_draft(request: dict[str, Any]) -> dict[str, Any]: @activity.defn(name="mark_turn_failed") async def mark_turn_failed(request: dict[str, Any]) -> None: self.failed.append(request["message_id"]) + self.errors[request["message_id"]] = request["error"] return [run_agent_turn, run_brief_draft, mark_turn_failed] @@ -218,6 +226,36 @@ async def test_failed_turn_is_marked_and_the_queue_moves_on(env: WorkflowEnviron assert turns.failed == ["a"] +async def test_a_rejected_key_fails_the_turn_once_with_what_to_fix( + env: WorkflowEnvironment, +) -> None: + """A model error no retry can fix isn't retried; the reply says what to fix.""" + from fixtures.llm_errors import anthropic_error + + from dataing.agents.errors import classify_llm_error + from dataing.temporal.errors import llm_activity_error + + failure = classify_llm_error(anthropic_error(401)) + assert failure is not None + turns = Turns() + thread_id = str(uuid.uuid4()) + turns.raises["a"] = llm_activity_error(failure) + async with Worker( + env.client, + workflow_runner=workflow_runner(), + task_queue=TASK_QUEUE, + workflows=[IssueThreadWorkflow], + activities=turns.activities(), + ): + with env.auto_time_skipping_disabled(): + await _enqueue(env.client, thread_id, "a") + await _wait_for(lambda: turns.failed == ["a"], timeout=30) + + assert turns.attempts["a"] == 1 + # The message, not "LLMRejected: ..." or the raw ModelHTTPError text + assert turns.errors["a"] == failure.message + + async def test_brief_requests_run_the_draft_activity(env: WorkflowEnvironment) -> None: """draft_brief requests go to run_brief_draft, in queue order with answers.""" turns = Turns() From 50abdaf3bf222560688ed953e88210ea45479944 Mon Sep 17 00:00:00 2001 From: bordumb Date: Mon, 28 Sep 2026 21:18:40 +0100 Subject: [PATCH 05/18] docs(api): record the M5 API shapes in openapi.json Copies only the changed operations from the app's schema: - POST /investigations (brief or alert, issue link in the response) - GET /investigations/{id} (issue, run number, brief, error, hypotheses) - the issue's investigation runs and outcome review (number, status, error) - the new GET /system/llm Also adds the schemas these reference. The rest of the committed spec is untouched. The TypeScript client is not regenerated: orval 8 (#221) would rewrite all 293 generated files (+31k lines) and turn GET hooks into mutations. The frontend reaches these routes through its hand-written wrappers. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Claude --- .flow/tasks/fn-70.20.json | 17 +- .flow/tasks/fn-70.20.md | 7 +- python-packages/dataing/openapi.json | 415 ++++++++++++++++++++++++++- 3 files changed, 420 insertions(+), 19 deletions(-) diff --git a/.flow/tasks/fn-70.20.json b/.flow/tasks/fn-70.20.json index 09fc130c..28b8cc8d 100644 --- a/.flow/tasks/fn-70.20.json +++ b/.flow/tasks/fn-70.20.json @@ -7,10 +7,23 @@ "fn-70.19" ], "epic": "fn-70", + "evidence": { + "commits": [ + "784d88d6" + ], + "prs": [], + "tests": [ + "tests/unit/services/test_llm_status.py", + "tests/unit/entrypoints/api/routes/test_system.py", + "tests/unit/temporal/test_issue_thread_workflow.py", + "tests/integration/test_agent_turn_activity.py", + "CE unit 3186, CE integration 145, EE unit 714" + ] + }, "id": "fn-70.20", "priority": null, "spec_path": ".flow/tasks/fn-70.20.md", - "status": "in_progress", + "status": "done", "title": "M5: LLM key check at startup, GET /system/llm, readable chat errors", - "updated_at": "2026-09-28T19:34:45.513545Z" + "updated_at": "2026-09-28T20:10:32.733100Z" } diff --git a/.flow/tasks/fn-70.20.md b/.flow/tasks/fn-70.20.md index 919947d6..6ca918b6 100644 --- a/.flow/tasks/fn-70.20.md +++ b/.flow/tasks/fn-70.20.md @@ -11,9 +11,8 @@ TBD ## Done summary -TBD - +LLMStatusChecker checks GET /v1/models/{id} for LLM_MODEL and CHAT_AGENT_MODEL in the background at API startup (10 s timeout, no SDK retries; empty key reported without a request). GET /api/v1/system/llm returns {state, message, models, checked_at}; transient problems re-check after 60 s. Chat turns and brief drafts raise the classified LLMRejected/LLMUnavailable error, so replies show what to fix and rejected keys aren't retried. Verified live: a bogus key → invalid_key. ## Evidence -- Commits: -- Tests: +- Commits: 784d88d6 +- Tests: tests/unit/services/test_llm_status.py, tests/unit/entrypoints/api/routes/test_system.py, tests/unit/temporal/test_issue_thread_workflow.py, tests/integration/test_agent_turn_activity.py, CE unit 3186, CE integration 145, EE unit 714 - PRs: diff --git a/python-packages/dataing/openapi.json b/python-packages/dataing/openapi.json index 80e6df06..6de8cc58 100644 --- a/python-packages/dataing/openapi.json +++ b/python-packages/dataing/openapi.json @@ -471,7 +471,7 @@ "investigations" ], "summary": "Start Investigation", - "description": "Start a new investigation for an alert.\n\nCreates a new investigation with Temporal workflow for durable execution.\n\nArgs:\n http_request: The HTTP request for accessing app state.\n request: The investigation request containing alert data.\n auth: Authentication context from API key/JWT.\n db: Application database.\n temporal_client: Temporal client for durable execution.\n\nReturns:\n StartInvestigationResponse with investigation and branch IDs.", + "description": "Start an investigation, in an issue.\n\nWithout issue_id this opens the issue: titled with the symptom, attributed to the\ncaller, or to dataing for an API key without a person. The run's card is in the\nissue's thread and its outcome is written back there.", "operationId": "start_investigation_api_v1_investigations_post", "requestBody": { "content": { @@ -573,7 +573,7 @@ "investigations" ], "summary": "Get Investigation", - "description": "Get investigation state from Temporal workflow.\n\nReturns the current state of the investigation including progress\nand any available results.\n\nArgs:\n investigation_id: UUID of the investigation.\n auth: Authentication context from API key/JWT.\n temporal_client: Temporal client for durable execution.\n\nReturns:\n InvestigationStateResponse with main branch state.\n\nRaises:\n HTTPException: If investigation not found.", + "description": "Get investigation state from Temporal workflow.\n\nReturns the current state of the investigation including progress\nand any available results.\n\nArgs:\n investigation_id: UUID of the investigation.\n auth: Authentication context from API key/JWT.\n db: Application database, for the run's issue, number, brief and outcome.\n temporal_client: Temporal client for durable execution.\n\nReturns:\n InvestigationStateResponse with main branch state.\n\nRaises:\n HTTPException: If investigation not found.", "operationId": "get_investigation_api_v1_investigations__investigation_id__get", "security": [ { @@ -9241,6 +9241,36 @@ } } } + }, + "/api/v1/system/llm": { + "get": { + "tags": [ + "system" + ], + "summary": "Get Llm Status", + "description": "Return the startup check of the Anthropic key and models.\n\nEvery page shows a banner while this reports a problem. A transient problem\n(Anthropic unreachable, overloaded) is checked again when it is over a minute old.", + "operationId": "get_llm_status_api_v1_system_llm_get", + "responses": { + "200": { + "description": "Successful Response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/LLMStatusResponse" + } + } + } + } + }, + "security": [ + { + "APIKeyHeader": [] + }, + { + "HTTPBearer": [] + } + ] + } } }, "components": { @@ -11975,6 +12005,25 @@ } ], "title": "Outcome Reviewed At" + }, + "number": { + "type": "integer", + "title": "Number" + }, + "status": { + "type": "string", + "title": "Status" + }, + "error": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "title": "Error" } }, "type": "object", @@ -11992,7 +12041,9 @@ "root_cause_tag", "synthesis_summary", "created_at", - "completed_at" + "completed_at", + "number", + "status" ], "title": "InvestigationRunResponse", "description": "Response for an investigation run." @@ -12031,6 +12082,94 @@ } ], "title": "Root Hash" + }, + "issue_id": { + "anyOf": [ + { + "type": "string", + "format": "uuid" + }, + { + "type": "null" + } + ], + "title": "Issue Id" + }, + "issue_number": { + "anyOf": [ + { + "type": "integer" + }, + { + "type": "null" + } + ], + "title": "Issue Number" + }, + "issue_title": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "title": "Issue Title" + }, + "run_number": { + "anyOf": [ + { + "type": "integer" + }, + { + "type": "null" + } + ], + "title": "Run Number" + }, + "brief": { + "anyOf": [ + { + "additionalProperties": true, + "type": "object" + }, + { + "type": "null" + } + ], + "title": "Brief" + }, + "execution_profile": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "title": "Execution Profile" + }, + "error": { + "anyOf": [ + { + "additionalProperties": true, + "type": "object" + }, + { + "type": "null" + } + ], + "title": "Error" + }, + "hypotheses": { + "items": { + "additionalProperties": true, + "type": "object" + }, + "type": "array", + "title": "Hypotheses" } }, "type": "object", @@ -12040,7 +12179,7 @@ "main_branch" ], "title": "InvestigationStateResponse", - "description": "Full investigation state for API responses." + "description": "Full investigation state for API responses.\n\nThe run's details page links back to its issue's thread, so the state carries\nthe issue, the run's number among its runs, its brief and, for a failed run,\nwhy it failed (docs/specs/0001_issue_chat.md \u00a78.3)." }, "InvestigationSummary": { "properties": { @@ -15140,10 +15279,25 @@ }, "StartInvestigationRequest": { "properties": { + "brief": { + "anyOf": [ + { + "$ref": "#/components/schemas/InvestigationBrief" + }, + { + "type": "null" + } + ] + }, "alert": { - "additionalProperties": true, - "type": "object", - "title": "Alert" + "anyOf": [ + { + "$ref": "#/components/schemas/AnomalyAlert" + }, + { + "type": "null" + } + ] }, "datasource_id": { "anyOf": [ @@ -15156,14 +15310,33 @@ } ], "title": "Datasource Id" + }, + "execution_profile": { + "type": "string", + "enum": [ + "safe", + "standard", + "deep" + ], + "title": "Execution Profile", + "default": "standard" + }, + "issue_id": { + "anyOf": [ + { + "type": "string", + "format": "uuid" + }, + { + "type": "null" + } + ], + "title": "Issue Id" } }, "type": "object", - "required": [ - "alert" - ], "title": "StartInvestigationRequest", - "description": "Request body for starting an investigation." + "description": "Start a run from a brief (the UI) or an alert (SDK, CLI, notebook).\n\nEvery run lives in an issue: the one named by issue_id, or one opened for it\n(docs/specs/0001_issue_chat.md \u00a77.11)." }, "StartInvestigationResponse": { "properties": { @@ -15181,15 +15354,32 @@ "type": "string", "title": "Status", "default": "queued" + }, + "run_id": { + "type": "string", + "format": "uuid", + "title": "Run Id" + }, + "issue_id": { + "type": "string", + "format": "uuid", + "title": "Issue Id" + }, + "issue_number": { + "type": "integer", + "title": "Issue Number" } }, "type": "object", "required": [ "investigation_id", - "main_branch_id" + "main_branch_id", + "run_id", + "issue_id", + "issue_number" ], "title": "StartInvestigationResponse", - "description": "Response for starting an investigation." + "description": "The started run and the issue it lives in." }, "StatsRequest": { "properties": { @@ -17002,6 +17192,205 @@ ], "title": "WeeklyUsageResponse", "description": "Weekly usage statistics response." + }, + "AnomalyAlert": { + "properties": { + "dataset_ids": { + "items": { + "type": "string" + }, + "type": "array", + "title": "Dataset Ids" + }, + "metric_spec": { + "$ref": "#/components/schemas/MetricSpec" + }, + "anomaly_type": { + "type": "string", + "title": "Anomaly Type" + }, + "expected_value": { + "type": "number", + "title": "Expected Value" + }, + "actual_value": { + "type": "number", + "title": "Actual Value" + }, + "deviation_pct": { + "type": "number", + "title": "Deviation Pct" + }, + "anomaly_date": { + "type": "string", + "title": "Anomaly Date" + }, + "severity": { + "type": "string", + "title": "Severity" + }, + "source_system": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "title": "Source System" + }, + "source_alert_id": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "title": "Source Alert Id" + }, + "source_url": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "title": "Source Url" + }, + "metadata": { + "anyOf": [ + { + "additionalProperties": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "integer" + }, + { + "type": "number" + }, + { + "type": "boolean" + } + ] + }, + "type": "object" + }, + { + "type": "null" + } + ], + "title": "Metadata" + } + }, + "type": "object", + "required": [ + "dataset_ids", + "metric_spec", + "anomaly_type", + "expected_value", + "actual_value", + "deviation_pct", + "anomaly_date", + "severity" + ], + "title": "AnomalyAlert", + "description": "Input: The anomaly that triggered the investigation.\n\nThis system performs ROOT CAUSE ANALYSIS, not anomaly detection.\nThe upstream anomaly detector provides structured metric specification.\n\nAttributes:\n dataset_ids: The affected tables in \"schema.table_name\" format.\n First table is the primary target; additional tables are reference context.\n metric_spec: Structured specification of what metric is anomalous.\n anomaly_type: What kind of anomaly (null_rate, row_count, freshness, custom).\n expected_value: The expected metric value based on historical data.\n actual_value: The actual observed metric value.\n deviation_pct: Percentage deviation from expected.\n anomaly_date: Date of the anomaly in \"YYYY-MM-DD\" format.\n severity: Alert severity level.\n source_system: Origin system (monte_carlo, great_expectations, dbt, etc.).\n source_alert_id: ID for linking back to source system.\n source_url: Deep link to alert in source system.\n metadata: Optional additional context." + }, + "LLMStatusResponse": { + "properties": { + "state": { + "type": "string", + "title": "State" + }, + "message": { + "type": "string", + "title": "Message" + }, + "models": { + "items": { + "type": "string" + }, + "type": "array", + "title": "Models" + }, + "checked_at": { + "anyOf": [ + { + "type": "string", + "format": "date-time" + }, + { + "type": "null" + } + ], + "title": "Checked At" + } + }, + "type": "object", + "required": [ + "state", + "message", + "models" + ], + "title": "LLMStatusResponse", + "description": "Whether the Anthropic key and models work." + }, + "MetricSpec": { + "properties": { + "metric_type": { + "type": "string", + "enum": [ + "column", + "sql_expression", + "dbt_metric", + "description" + ], + "title": "Metric Type" + }, + "expression": { + "type": "string", + "title": "Expression" + }, + "display_name": { + "type": "string", + "title": "Display Name" + }, + "columns_referenced": { + "items": { + "type": "string" + }, + "type": "array", + "title": "Columns Referenced", + "default": [] + }, + "source_url": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "title": "Source Url" + } + }, + "type": "object", + "required": [ + "metric_type", + "expression", + "display_name" + ], + "title": "MetricSpec", + "description": "Specification of what metric is anomalous.\n\nProvides structure for LLM prompt generation while remaining flexible\nenough to accept input from various anomaly detection systems.\n\nAttributes:\n metric_type: How to interpret the expression field.\n expression: The metric definition (column name, SQL, metric ref, or description).\n display_name: Human-readable name for logs and UI.\n columns_referenced: Columns involved in this metric (for schema filtering).\n source_url: Link to metric definition in source system." } }, "securitySchemes": { From 3a71fb9b82ed44046118bb016cadafbd3e6fd9d4 Mon Sep 17 00:00:00 2001 From: bordumb Date: Mon, 28 Sep 2026 19:15:22 +0100 Subject: [PATCH 06/18] =?UTF-8?q?feat(frontend):=20Investigate=E2=80=A6=20?= =?UTF-8?q?starts=20a=20run=20from=20every=20page=20(fn-70.22)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every start button now opens the brief editor in new mode where the person is, instead of the /investigations/new page (spec 0001 D13, §8.2): the sidebar quick action, the dashboard header and its empty recent-investigations card, the investigations list header and empty state, and "Investigate this dataset" in the dataset page header, which is shown whether or not the dataset has runs and pre-fills the table's native path and datasource. New mode reuses BriefFormView, titled "Start an investigation": - symptom and at least one scope table are required - the datasource select lists the tenant's datasources, preselects the only one and requires a choice when there are several; it has no "The issue's datasource" option - findings, ruled out and leads start empty - Start posts the brief to POST /api/v1/investigations and lands on /issues/{issue_id}; a 409 ambiguous_datasource asks the person to pick a datasource, and other errors toast the server's message The API client now throws ApiError, which keeps the status and the detail's error code. BriefFormView takes a mode and an onSubmit; the hand-off submit moved into HandoffForm unchanged. Removed: the investigations/new route, NewInvestigation.tsx, the alert-based useStartInvestigation and the dead components/Layout.tsx. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Claude --- frontend/app/src/App.tsx | 14 - frontend/app/src/components/Layout.tsx | 34 -- .../components/layout/app-sidebar.test.tsx | 30 +- .../app/src/components/layout/app-sidebar.tsx | 20 +- .../dashboard/dashboard-page.test.tsx | 36 +- .../src/features/dashboard/dashboard-page.tsx | 15 +- .../dashboard/recent-investigations.tsx | 12 +- .../datasets/dataset-detail-page.test.tsx | 101 +++- .../features/datasets/dataset-detail-page.tsx | 21 +- .../investigation/InvestigationList.test.tsx | 31 +- .../investigation/InvestigationList.tsx | 22 +- .../investigation/NewInvestigation.tsx | 479 ------------------ .../src/features/issues/brief/BriefEditor.tsx | 217 ++++++-- .../issues/brief/StartInvestigation.test.tsx | 305 +++++++++++ .../issues/brief/StartInvestigation.tsx | 178 +++++++ .../src/features/issues/brief/brief-form.ts | 19 +- frontend/app/src/lib/api/client.ts | 26 +- frontend/app/src/lib/api/investigations.ts | 47 +- 18 files changed, 905 insertions(+), 702 deletions(-) delete mode 100644 frontend/app/src/components/Layout.tsx delete mode 100644 frontend/app/src/features/investigation/NewInvestigation.tsx create mode 100644 frontend/app/src/features/issues/brief/StartInvestigation.test.tsx create mode 100644 frontend/app/src/features/issues/brief/StartInvestigation.tsx diff --git a/frontend/app/src/App.tsx b/frontend/app/src/App.tsx index aa55546b..075c9541 100644 --- a/frontend/app/src/App.tsx +++ b/frontend/app/src/App.tsx @@ -26,7 +26,6 @@ import { import { DashboardPage } from "@/features/dashboard/dashboard-page"; import { InvestigationList } from "@/features/investigation/InvestigationList"; import { InvestigationDetail } from "@/features/investigation/InvestigationDetail"; -import { NewInvestigation } from "@/features/investigation/NewInvestigation"; import { DataSourcePage } from "@/features/datasources/datasource-page"; import { DatasetListPage, DatasetDetailPage } from "@/features/datasets"; import { SettingsPage } from "@/features/settings/settings-page"; @@ -145,19 +144,6 @@ function AppWithEntitlements() { } /> - - - - - - } - /> -
-
- - - dataing - - -
-
-
- -
- - ); -} diff --git a/frontend/app/src/components/layout/app-sidebar.test.tsx b/frontend/app/src/components/layout/app-sidebar.test.tsx index 45d03fbc..73ded715 100644 --- a/frontend/app/src/components/layout/app-sidebar.test.tsx +++ b/frontend/app/src/components/layout/app-sidebar.test.tsx @@ -1,8 +1,10 @@ -import { afterEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeAll, describe, expect, it, vi } from "vitest"; import { screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; import { SidebarProvider } from "@/components/ui/sidebar"; import type { OrgRole } from "@/lib/auth/types"; +import { stubApi, stubRadixDom } from "@/test/api"; import { renderAsRole } from "@/test/auth"; import { AppSidebar } from "./app-sidebar"; @@ -24,20 +26,36 @@ function renderSidebar(role: OrgRole) { ); } -afterEach(() => localStorage.clear()); +beforeAll(() => stubRadixDom()); + +afterEach(() => { + vi.unstubAllGlobals(); + localStorage.clear(); +}); describe("AppSidebar", () => { - it("does not offer viewers the new investigation shortcut", async () => { + it("does not offer viewers the Investigate… shortcut", async () => { renderSidebar("viewer"); expect(await screen.findByText("Platform")).toBeInTheDocument(); - expect(screen.queryByText("New Investigation")).not.toBeInTheDocument(); + expect(screen.queryByText("Investigate…")).not.toBeInTheDocument(); }); - it("offers members the new investigation shortcut", async () => { + it("opens the brief editor from the Investigate… shortcut", async () => { + stubApi({ "GET /api/v1/datasources": { body: { items: [], total: 0 } } }); renderSidebar("member"); - expect(await screen.findByText("New Investigation")).toBeInTheDocument(); + await userEvent.click( + await screen.findByRole("button", { name: "Investigate…" }), + ); + + expect( + await screen.findByRole("dialog", { name: "Start an investigation" }), + ).toBeInTheDocument(); + // No page links to the removed /investigations/new. + expect( + document.querySelector('a[href="/investigations/new"]'), + ).not.toBeInTheDocument(); }); it.each(["viewer", "member"] as const)( diff --git a/frontend/app/src/components/layout/app-sidebar.tsx b/frontend/app/src/components/layout/app-sidebar.tsx index 6a494e79..eeae00f7 100644 --- a/frontend/app/src/components/layout/app-sidebar.tsx +++ b/frontend/app/src/components/layout/app-sidebar.tsx @@ -1,3 +1,4 @@ +import { useState } from "react"; import { Link, useLocation } from "react-router-dom"; import { Search, @@ -35,6 +36,7 @@ import { } from "@/components/ui/dropdown-menu"; import { Avatar, AvatarFallback } from "@/components/ui/avatar"; import { Badge } from "@/components/ui/Badge"; +import { StartInvestigationDialog } from "@/features/issues/brief/StartInvestigation"; import { useJwtAuth } from "@/lib/auth/jwt-context"; import { useRole } from "@/lib/auth"; import { useNotifications } from "@/lib/notifications"; @@ -80,6 +82,7 @@ export function AppSidebar() { const { logout, org } = useJwtAuth(); const { isAdmin, isMember } = useRole(); const { unreadCount } = useNotifications(); + const [startOpen, setStartOpen] = useState(false); // Build settings nav items based on role // Admin link only visible to admin/owner roles @@ -133,19 +136,24 @@ export function AppSidebar() { - {/* Quick Action */} + {/* Quick action: the brief editor in new mode, over this page */} {isMember && ( - - - - New Investigation - + setStartOpen(true)} + > + + Investigate… + )} diff --git a/frontend/app/src/features/dashboard/dashboard-page.test.tsx b/frontend/app/src/features/dashboard/dashboard-page.test.tsx index 32205a95..b7baffc7 100644 --- a/frontend/app/src/features/dashboard/dashboard-page.test.tsx +++ b/frontend/app/src/features/dashboard/dashboard-page.test.tsx @@ -1,6 +1,8 @@ -import { afterEach, describe, expect, it, vi } from "vitest"; -import { screen } from "@testing-library/react"; +import { afterEach, beforeAll, describe, expect, it, vi } from "vitest"; +import { screen, within } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { stubApi, stubRadixDom } from "@/test/api"; import { renderAsRole } from "@/test/auth"; import { DashboardPage } from "./dashboard-page"; @@ -18,7 +20,12 @@ vi.mock("@/lib/api/investigations", async (importOriginal) => ({ useInvestigations: () => ({ data: [], isLoading: false, error: null }), })); -afterEach(() => localStorage.clear()); +beforeAll(() => stubRadixDom()); + +afterEach(() => { + vi.unstubAllGlobals(); + localStorage.clear(); +}); describe("DashboardPage", () => { it("does not offer viewers a way to start an investigation", async () => { @@ -27,14 +34,27 @@ describe("DashboardPage", () => { expect( await screen.findByText("No investigations yet"), ).toBeInTheDocument(); - expect(screen.queryByText("New Investigation")).not.toBeInTheDocument(); - expect(screen.queryByText("Create Investigation")).not.toBeInTheDocument(); + expect(screen.queryByText("Investigate…")).not.toBeInTheDocument(); }); - it("offers members a way to start an investigation", async () => { + it.each([ + ["the header", 0], + ["the empty recent-investigations card", 1], + ])("opens the brief editor from %s", async (_where, index) => { + stubApi({ "GET /api/v1/datasources": { body: { items: [], total: 0 } } }); renderAsRole(, "member"); - expect(await screen.findByText("New Investigation")).toBeInTheDocument(); - expect(screen.getByText("Create Investigation")).toBeInTheDocument(); + const buttons = await screen.findAllByRole("button", { + name: "Investigate…", + }); + expect(buttons).toHaveLength(2); + await userEvent.click(buttons[index]); + + const dialog = await screen.findByRole("dialog", { + name: "Start an investigation", + }); + // The dashboard knows nothing about the problem yet. + expect(within(dialog).getByLabelText("Symptom")).toHaveValue(""); + expect(within(dialog).getByLabelText("Scope: tables")).toHaveValue(""); }); }); diff --git a/frontend/app/src/features/dashboard/dashboard-page.tsx b/frontend/app/src/features/dashboard/dashboard-page.tsx index 0661519d..86151cbf 100644 --- a/frontend/app/src/features/dashboard/dashboard-page.tsx +++ b/frontend/app/src/features/dashboard/dashboard-page.tsx @@ -1,9 +1,7 @@ import { useQuery } from "@tanstack/react-query"; -import { Link } from "react-router-dom"; -import { Plus } from "lucide-react"; import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/Card"; -import { Button } from "@/components/ui/Button"; +import { InvestigateButton } from "@/features/issues/brief/StartInvestigation"; import { fetchDashboardStats } from "@/lib/api/dashboard"; import { DashboardStatsCards } from "./dashboard-stats"; import { RecentInvestigations } from "./recent-investigations"; @@ -25,16 +23,7 @@ export function DashboardPage() {
- - - New Investigation - - - ) : undefined - } + action={isMember ? : undefined} /> - Create Investigation - - ) : undefined - } + description="Say what looks wrong and the agent investigates it. Each run opens an issue your team can follow." + action={isMember ? : undefined} /> ); } diff --git a/frontend/app/src/features/datasets/dataset-detail-page.test.tsx b/frontend/app/src/features/datasets/dataset-detail-page.test.tsx index bb7c0305..b34c2477 100644 --- a/frontend/app/src/features/datasets/dataset-detail-page.test.tsx +++ b/frontend/app/src/features/datasets/dataset-detail-page.test.tsx @@ -1,12 +1,15 @@ -import { afterEach, describe, expect, it, vi } from "vitest"; -import { screen } from "@testing-library/react"; +import { afterEach, beforeAll, describe, expect, it, vi } from "vitest"; +import { screen, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { Route, Routes } from "react-router-dom"; import type { OrgRole } from "@/lib/auth/types"; +import { stubApi, stubRadixDom } from "@/test/api"; import { renderAsRole } from "@/test/auth"; import { DatasetDetailPage } from "./dataset-detail-page"; +const runs = vi.hoisted(() => ({ current: [] as Record[] })); + vi.mock("@/lib/api/datasets", async (importOriginal) => ({ ...(await importOriginal()), useDataset: () => ({ @@ -14,8 +17,8 @@ vi.mock("@/lib/api/datasets", async (importOriginal) => ({ id: "ds-1", name: "orders", native_path: "analytics.orders", - datasource_id: "src-1", - datasource_name: "warehouse", + datasource_id: "src-2", + datasource_name: "lake", datasource_type: "postgresql", table_type: "table", row_count: 10, @@ -28,7 +31,7 @@ vi.mock("@/lib/api/datasets", async (importOriginal) => ({ refetch: vi.fn(), }), useDatasetInvestigations: () => ({ - data: { investigations: [] }, + data: { investigations: runs.current }, isLoading: false, }), })); @@ -38,7 +41,28 @@ vi.mock("@/lib/api/schema-comments", async (importOriginal) => ({ useSchemaComments: () => ({ data: [] }), })); -async function openInvestigationsTab(role: OrgRole) { +function datasource(id: string, name: string) { + return { + id, + name, + type: "postgres", + category: "database", + is_active: true, + is_default: false, + status: "connected", + created_at: "2026-01-01T00:00:00Z", + }; +} + +function renderPage(role: OrgRole) { + stubApi({ + "GET /api/v1/datasources": { + body: { + items: [datasource("src-1", "warehouse"), datasource("src-2", "lake")], + total: 2, + }, + }, + }); renderAsRole( } /> @@ -46,24 +70,67 @@ async function openInvestigationsTab(role: OrgRole) { role, "/datasets/ds-1", ); - await userEvent.click( - await screen.findByRole("tab", { name: /investigations/i }), - ); - expect(await screen.findByText("No investigations")).toBeInTheDocument(); } -afterEach(() => localStorage.clear()); +beforeAll(() => stubRadixDom()); -describe("DatasetDetailPage investigations tab", () => { +afterEach(() => { + runs.current = []; + vi.unstubAllGlobals(); + localStorage.clear(); +}); + +describe("DatasetDetailPage", () => { it("does not offer viewers a way to start an investigation", async () => { - await openInvestigationsTab("viewer"); + renderPage("viewer"); + + expect( + await screen.findByRole("heading", { name: "analytics.orders" }), + ).toBeInTheDocument(); + expect( + screen.queryByRole("button", { name: "Investigate this dataset" }), + ).not.toBeInTheDocument(); + await userEvent.click(screen.getByRole("tab", { name: /investigations/i })); + expect(await screen.findByText("No investigations")).toBeInTheDocument(); + expect(screen.queryByText(/Investigate/)).not.toBeInTheDocument(); + }); + + it("investigates the dataset, pre-filled with its table and datasource", async () => { + renderPage("member"); + + await userEvent.click( + await screen.findByRole("button", { name: "Investigate this dataset" }), + ); - expect(screen.queryByText("Start Investigation")).not.toBeInTheDocument(); + const dialog = await screen.findByRole("dialog", { + name: "Start an investigation", + }); + expect(within(dialog).getByLabelText("Scope: tables")).toHaveValue( + "analytics.orders", + ); + await within(dialog).findByRole("option", { name: /lake/ }); + expect(within(dialog).getByLabelText("Datasource")).toHaveValue("src-2"); }); - it("offers members a way to start an investigation", async () => { - await openInvestigationsTab("member"); + it("offers it whether or not the dataset has runs", async () => { + runs.current = [ + { + id: "inv-1", + metric_name: "null_rate", + status: "completed", + severity: null, + created_at: "2026-09-14T08:00:00Z", + }, + ]; + renderPage("member"); - expect(screen.getByText("Start Investigation")).toBeInTheDocument(); + expect( + await screen.findByRole("button", { name: "Investigate this dataset" }), + ).toBeInTheDocument(); + await userEvent.click(screen.getByRole("tab", { name: /investigations/i })); + expect(await screen.findByText("null_rate")).toBeInTheDocument(); + expect( + screen.getAllByRole("button", { name: "Investigate this dataset" }), + ).toHaveLength(1); }); }); diff --git a/frontend/app/src/features/datasets/dataset-detail-page.tsx b/frontend/app/src/features/datasets/dataset-detail-page.tsx index fccb2ebf..e79ce2b3 100644 --- a/frontend/app/src/features/datasets/dataset-detail-page.tsx +++ b/frontend/app/src/features/datasets/dataset-detail-page.tsx @@ -29,9 +29,9 @@ import { LoadingSpinner } from "@/components/shared/loading-spinner"; import { EmptyState } from "@/components/shared/empty-state"; import { useDataset, useDatasetInvestigations } from "@/lib/api/datasets"; import { useSchemaComments } from "@/lib/api/schema-comments"; -import { useRole } from "@/lib/auth"; import { formatNumber, formatRelativeTime } from "@/lib/utils"; import { LineagePanel } from "@/features/investigation/components/lineage-panel"; +import { InvestigateButton } from "@/features/issues/brief/StartInvestigation"; import { SchemaCommentIndicator } from "./components/schema-comment-indicator"; import { CommentSlidePanel } from "./components/comment-slide-panel"; import { KnowledgeTab } from "./components/knowledge-tab"; @@ -76,7 +76,6 @@ export function DatasetDetailPage() { } = useDataset(datasetId ?? null); const { data: investigationsResponse, isLoading: investigationsLoading } = useDatasetInvestigations(datasetId ?? null); - const { isMember } = useRole(); const [selectedField, setSelectedField] = useState(null); // Fetch all comments for the dataset once (no fieldName filter) to avoid N+1 queries @@ -170,6 +169,14 @@ export function DatasetDetailPage() { {dataset.datasource_type && ( {dataset.datasource_type} )} +
@@ -332,16 +339,6 @@ export function DatasetDetailPage() { icon={Search} title="No investigations" description="No investigations have been run on this dataset yet." - action={ - isMember ? ( - - - - ) : undefined - } /> ) : (
diff --git a/frontend/app/src/features/investigation/InvestigationList.test.tsx b/frontend/app/src/features/investigation/InvestigationList.test.tsx index 0c47ad4b..76ddfb03 100644 --- a/frontend/app/src/features/investigation/InvestigationList.test.tsx +++ b/frontend/app/src/features/investigation/InvestigationList.test.tsx @@ -1,6 +1,8 @@ -import { afterEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeAll, describe, expect, it, vi } from "vitest"; import { screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { stubApi, stubRadixDom } from "@/test/api"; import { renderAsRole } from "@/test/auth"; import { InvestigationList } from "./InvestigationList"; @@ -14,7 +16,12 @@ vi.mock("@/lib/api/investigations", async (importOriginal) => ({ }), })); -afterEach(() => localStorage.clear()); +beforeAll(() => stubRadixDom()); + +afterEach(() => { + vi.unstubAllGlobals(); + localStorage.clear(); +}); describe("InvestigationList", () => { it("does not offer viewers a way to start an investigation", async () => { @@ -23,18 +30,24 @@ describe("InvestigationList", () => { expect( await screen.findByText("No investigations yet."), ).toBeInTheDocument(); - expect(screen.queryByText("New Investigation")).not.toBeInTheDocument(); - expect( - screen.queryByText("Create your first investigation"), - ).not.toBeInTheDocument(); + expect(screen.queryByText("Investigate…")).not.toBeInTheDocument(); }); - it("offers members a way to start an investigation", async () => { + it.each([ + ["the header", 0], + ["the empty state", 1], + ])("opens the brief editor from %s", async (_where, index) => { + stubApi({ "GET /api/v1/datasources": { body: { items: [], total: 0 } } }); renderAsRole(, "member"); - expect(await screen.findByText("New Investigation")).toBeInTheDocument(); + const buttons = await screen.findAllByRole("button", { + name: "Investigate…", + }); + expect(buttons).toHaveLength(2); + await userEvent.click(buttons[index]); + expect( - screen.getByText("Create your first investigation"), + await screen.findByRole("dialog", { name: "Start an investigation" }), ).toBeInTheDocument(); }); }); diff --git a/frontend/app/src/features/investigation/InvestigationList.tsx b/frontend/app/src/features/investigation/InvestigationList.tsx index 647bf740..1815c24a 100644 --- a/frontend/app/src/features/investigation/InvestigationList.tsx +++ b/frontend/app/src/features/investigation/InvestigationList.tsx @@ -5,11 +5,9 @@ import { } from "@/lib/api/investigations"; import { Card, CardHeader, CardTitle, CardContent } from "@/components/ui/Card"; import { Badge } from "@/components/ui/Badge"; -import { Button } from "@/components/ui/Button"; import { AsyncBoundary } from "@/components/async-boundary"; -import { useRole } from "@/lib/auth"; +import { InvestigateButton } from "@/features/issues/brief/StartInvestigation"; import { formatDate } from "@/lib/utils"; -import { Plus } from "lucide-react"; function getStatusVariant(status: string) { switch (status) { @@ -30,18 +28,12 @@ function InvestigationListContent({ }: { investigations: InvestigationListItem[]; }) { - const { isMember } = useRole(); - if (investigations.length === 0) { return (

No investigations yet.

- {isMember && ( - - - - )} + ); @@ -77,20 +69,12 @@ function InvestigationListContent({ export function InvestigationList() { const query = useInvestigations(); - const { isMember } = useRole(); return (

Investigations

- {isMember && ( - - - - )} +
diff --git a/frontend/app/src/features/investigation/NewInvestigation.tsx b/frontend/app/src/features/investigation/NewInvestigation.tsx deleted file mode 100644 index f7d98c8f..00000000 --- a/frontend/app/src/features/investigation/NewInvestigation.tsx +++ /dev/null @@ -1,479 +0,0 @@ -import { useState, useEffect, useCallback } from "react"; -import { useNavigate, Link } from "react-router-dom"; -import { useStartInvestigation } from "@/lib/api/investigations"; -import { - useDataSources, - useDataSourceSchema, - SchemaTable, -} from "@/lib/api/datasources"; -import { Card, CardHeader, CardTitle, CardContent } from "@/components/ui/Card"; -import { Button } from "@/components/ui/Button"; -import { Input } from "@/components/ui/Input"; -import { - DatePicker, - DatePickerValue, - datePickerValueToString, - stringToDatePickerValue, -} from "@/components/ui/DatePicker"; -import { ArrowLeft, Loader2, Plus, AlertCircle } from "lucide-react"; - -import { SchemaViewer, LineagePanel, DatasetEntry } from "./components"; - -interface Dataset { - id: string; - datasourceId: string; - identifier: string; -} - -interface FormData { - anomaly_type: string; - column_name: string; - display_name: string; - expected_value: string; - actual_value: string; - deviation_pct: string; - severity: string; - description: string; -} - -export function NewInvestigation() { - const navigate = useNavigate(); - const startInvestigation = useStartInvestigation(); - - const [selectedTable, setSelectedTable] = useState(null); - const [datasets, setDatasets] = useState([ - { id: crypto.randomUUID(), datasourceId: "", identifier: "" }, - ]); - const [anomalyDate, setAnomalyDate] = useState(() => - stringToDatePickerValue(new Date().toISOString().split("T")[0]), - ); - const [formData, setFormData] = useState({ - anomaly_type: "null_rate", - column_name: "", - display_name: "", - expected_value: "", - actual_value: "", - deviation_pct: "", - severity: "medium", - description: "", - }); - - const { - data: dataSources, - isLoading: isLoadingDataSources, - error: dataSourcesError, - } = useDataSources(); - const { isLoading: isLoadingSchema } = useDataSourceSchema( - datasets[0]?.datasourceId || null, - ); - - // Auto-select first datasource - useEffect(() => { - if (dataSources && dataSources.length > 0 && !datasets[0].datasourceId) { - setDatasets((prev) => - prev.map((ds, i) => - i === 0 ? { ...ds, datasourceId: dataSources[0].id } : ds, - ), - ); - } - }, [dataSources, datasets]); - - const handleSubmit = async (e: React.FormEvent) => { - e.preventDefault(); - const primaryDataset = datasets[0]; - if (!primaryDataset.identifier.trim()) return; - - const dateStr = datePickerValueToString(anomalyDate); - if (!dateStr) return; - - try { - // Build display name from column and anomaly type if not provided - const displayName = - formData.display_name.trim() || - `${formData.anomaly_type} on ${formData.column_name || primaryDataset.identifier}`; - - // Send all datasets - first is primary, rest are reference context - const datasetIds = datasets - .map((ds) => ds.identifier.trim()) - .filter((id) => id.length > 0); - - const result = await startInvestigation.mutateAsync({ - dataset_ids: datasetIds, - metric_spec: { - metric_type: "column", - expression: formData.column_name || primaryDataset.identifier, - display_name: displayName, - columns_referenced: formData.column_name - ? [formData.column_name] - : [], - }, - anomaly_type: formData.anomaly_type, - expected_value: parseFloat(formData.expected_value), - actual_value: parseFloat(formData.actual_value), - deviation_pct: parseFloat(formData.deviation_pct), - anomaly_date: dateStr, - severity: formData.severity, - }); - navigate(`/investigations/${result.investigation_id}`); - } catch (error) { - console.error("Failed to create investigation:", error); - } - }; - - const handleChange = ( - e: React.ChangeEvent< - HTMLInputElement | HTMLSelectElement | HTMLTextAreaElement - >, - ) => { - setFormData((prev) => ({ ...prev, [e.target.name]: e.target.value })); - }; - - const updateDataset = useCallback( - ( - id: string, - updates: Partial<{ datasourceId: string; identifier: string }>, - ) => { - setDatasets((prev) => - prev.map((ds) => (ds.id === id ? { ...ds, ...updates } : ds)), - ); - if (updates.identifier === "" || updates.datasourceId) { - setSelectedTable(null); - } - }, - [], - ); - - const addDataset = useCallback(() => { - const defaultDsId = dataSources?.[0]?.id || ""; - setDatasets((prev) => [ - ...prev, - { id: crypto.randomUUID(), datasourceId: defaultDsId, identifier: "" }, - ]); - }, [dataSources]); - - const removeDataset = useCallback((id: string) => { - setDatasets((prev) => { - if (prev.length <= 1) return prev; - return prev.filter((ds) => ds.id !== id); - }); - }, []); - - const primaryDataset = datasets[0]; - const hasEmptyDataset = datasets.some((ds) => !ds.identifier.trim()); - const isSubmitDisabled = - startInvestigation.isPending || hasEmptyDataset || !anomalyDate.start; - - if (isLoadingDataSources) { - return ( -
- -
- ); - } - - return ( -
-
- - - -

Start Investigation

-
- - {dataSourcesError && ( -
- -
-

- Failed to load data sources -

-

- {dataSourcesError.message}. Please check your API key and try - again. -

-
- - - -
- )} - -
-
- - - Investigation Details -

- Configure the investigation parameters and target datasets. -

- - -
-
- -
- {datasets.map((dataset, index) => { - const ds = dataSources?.find( - (d) => d.id === dataset.datasourceId, - ); - return ( - - updateDataset(dataset.id, { datasourceId: id }) - } - onIdentifierChange={(val) => - updateDataset(dataset.id, { identifier: val }) - } - onRemove={() => removeDataset(dataset.id)} - canRemove={datasets.length > 1} - disabled={startInvestigation.isPending} - autoFocus={ - index === datasets.length - 1 && !dataset.identifier - } - dataSources={dataSources || []} - onTableSelect={setSelectedTable} - /> - ); - })} -
- -
- - - -
-
- - -
-
- - -

- The specific column affected (optional) -

-
-
- -
-
- - -

- Human-readable label (auto-generated if empty) -

-
-
- - -
-
- -
-
- - -
-
- - -
-
- - -
-
- -
- -