From 2a9dad3c4aa28a0dfd962daccde6c6eaa791de25 Mon Sep 17 00:00:00 2001 From: Lucy Macartney Date: Mon, 7 Sep 2026 16:11:43 -0500 Subject: [PATCH 01/20] Treat an empty error type as missing in failure signatures Step error type coalesces to the run's with `||`, and `"" || x` returns `""` since empty string is truthy in Elixir. A step reporting an empty error type produced its own signature instead of falling through, splitting one failure into two rows with identical labels and split counts. The run's error type can be empty the same way, and is read straight into the signature, so it splits the run-level rows too. Normalise "" to nil on both sides in to_signature/2. --- lib/lightning/workflows/stats.ex | 11 +++++++- test/lightning/workflows/stats_test.exs | 35 +++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/lib/lightning/workflows/stats.ex b/lib/lightning/workflows/stats.ex index cfa204078c..f6243272f3 100644 --- a/lib/lightning/workflows/stats.ex +++ b/lib/lightning/workflows/stats.ex @@ -280,12 +280,21 @@ defmodule Lightning.Workflows.Stats do %{ count: row.count, exit_reason: row.exit_reason || @state_reasons[row.run_state], - error_type: row.error_type || row.run_error_type, + error_type: + blank_to_nil(row.error_type) || blank_to_nil(row.run_error_type), step_name: step_name, adaptor: adaptor } end + # `"" || x` returns `""` — empty string is truthy in Elixir — so a step that + # reported an empty error type would coalesce to `""` instead of falling + # through to the run's, splitting one signature into two identical-looking + # rows. The run's error type can be empty the same way, and would land in + # the signature unread, so both sides are normalised: empty is absent. + defp blank_to_nil(""), do: nil + defp blank_to_nil(other), do: other + # Two groups can collapse into one signature — a crashed run and a failed run # whose steps both reported `fail`, say — so the fold happens after the # coalesce, not in the `group_by`. diff --git a/test/lightning/workflows/stats_test.exs b/test/lightning/workflows/stats_test.exs index ccfa11711e..92fb554a04 100644 --- a/test/lightning/workflows/stats_test.exs +++ b/test/lightning/workflows/stats_test.exs @@ -332,6 +332,41 @@ defmodule Lightning.Workflows.StatsTest do } end + # `"" || x` returns `""` in Elixir, so an empty error type would otherwise + # coalesce to itself instead of falling through — splitting one signature + # into two rows that render identically and each half the count. + test "treats an empty error type the same as a missing one", %{ + workflow: workflow, + trigger: trigger + } do + job = hd(workflow.jobs) + + failed_run(workflow, trigger, [], [ + step(job, exit_reason: "fail", error_type: "") + ]) + + failed_run(workflow, trigger, [], [ + step(job, exit_reason: "fail", error_type: nil) + ]) + + assert %{signatures: [%{count: 2, error_type: nil}]} = + Stats.failure_signatures(workflow) + end + + # And on the run's own error type, which the step falls through to: it is + # read straight into the signature, so an empty one splits the rows there + # instead. + test "treats an empty error type on the run the same as a missing one", %{ + workflow: workflow, + trigger: trigger + } do + failed_run(workflow, trigger, state: :crashed, error_type: "") + failed_run(workflow, trigger, state: :crashed, error_type: nil) + + assert %{signatures: [%{count: 2, exit_reason: "crash", error_type: nil}]} = + Stats.failure_signatures(workflow) + end + test "groups matching work orders and sorts the heaviest first", %{ workflow: workflow, trigger: trigger From 483c9c7d5767d440f756dd2c5feed92820c397b2 Mon Sep 17 00:00:00 2001 From: Lucy Macartney Date: Tue, 8 Sep 2026 12:41:27 -0500 Subject: [PATCH 02/20] One shared signature query, rows keyed by job_id Extracts the failing-step predicate, the two exit_reason/error_type coalesces, and the latest-run tiebreak from Stats into Lightning.Invocation.Query, alongside the failure-state list it already needs to share with the upcoming history filter. Stats composes them instead of inlining them; latest_runs/2 keeps its DISTINCT ON. Triage rows now merge on job_id rather than on every display field, so a job renamed (or adaptor-bumped) mid-window is one row, not two. The label is taken from the group's most recent failing snapshot, using the snapshot's lock_version as the tiebreak since the grouped rows carry no timestamp to compare by. job_id joins the signature map and the health JSON. --- CHANGELOG.md | 30 +++++---- lib/lightning/invocation/query.ex | 69 +++++++++++++++++++ lib/lightning/workflows/stats.ex | 90 +++++++++++++++---------- test/lightning/workflows/stats_test.exs | 38 +++++++++++ 4 files changed, 179 insertions(+), 48 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dfbfe35740..54564db0a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,10 +17,16 @@ and this project adheres to ### Changed -- Runs on Erlang/OTP 28 and Elixir 1.18.4. OTP 27 only finishes normalising - the first character of a string, which breaks names in many languages. - Lightning does not normalise anything today, but #4577 adds it on every name, - so the runtime moves first. +- Runs on Erlang/OTP 28 and Elixir 1.18.4. OTP 27 only finishes normalising the + first character of a string, which breaks names in many languages. Lightning + does not normalise anything today, but #4577 adds it on every name, so the + runtime moves first. +- The workflow health page's triage table now merges two rows that describe the + same failure into one, correcting a split count on a shipped page: a step + reporting an empty error type instead of none, and a job renamed or + adaptor-bumped mid-window, both used to draw two identical-looking rows with + the count divided between them. Merged rows are labelled from the most recent + snapshot to fail. ### Added @@ -73,17 +79,17 @@ and this project adheres to - AI assistant code blocks share the surface the workflow diffs use, so a reply and the diff below it no longer read as two different products. [#5118](https://github.com/OpenFn/lightning/issues/5118) -- A global assistant reply whose changes could not be applied now says so on - the reply itself, beside the diffs that did not land, and offers to try - again. It used to fall back to a raw YAML panel. +- A global assistant reply whose changes could not be applied now says so on the + reply itself, beside the diffs that did not land, and offers to try again. It + used to fall back to a raw YAML panel. [#5118](https://github.com/OpenFn/lightning/issues/5118) -- A failed apply is now remembered, so reloading no longer turns it back into - a success. The reply kept its diff blocks and offered to undo changes that - had never landed. A retry that works clears the record. +- A failed apply is now remembered, so reloading no longer turns it back into a + success. The reply kept its diff blocks and offered to undo changes that had + never landed. A retry that works clears the record. [#5118](https://github.com/OpenFn/lightning/issues/5118) - Editing an open step with the global assistant no longer puts a diff in the - code editor. The change is already applied, so the diff read as a proposal - to accept or reject when the only control was a close button, and reloading + code editor. The change is already applied, so the diff read as a proposal to + accept or reject when the only control was a close button, and reloading revealed the change had been written all along. [#5118](https://github.com/OpenFn/lightning/issues/5118) diff --git a/lib/lightning/invocation/query.ex b/lib/lightning/invocation/query.ex index 710ec3591f..5fd7c28e9a 100644 --- a/lib/lightning/invocation/query.ex +++ b/lib/lightning/invocation/query.ex @@ -40,6 +40,75 @@ defmodule Lightning.Invocation.Query do ) end + @doc """ + Work order states the workflow health page's triage table treats as a + failure. + + Narrower than `WorkOrder.failure_states/0`: `:cancelled` is final but not a + failure — someone stopped it on purpose, their own outcome rather than the + red wedge — so it is excluded here rather than in the schema's own list. + Shared so the history filter narrows on the same set `Stats` does. + """ + @spec failure_states() :: [atom()] + def failure_states, do: WorkOrder.failure_states() -- [:cancelled] + + @doc """ + Appends `exit_reason != "success"` to a query of `Step`. + + A step that never finished carries a `nil` `exit_reason`, and `NULL != + 'success'` is false in SQL, so it is excluded here too — the same answer + the janitor's own race resolves to. Do not "fix" this to `IS DISTINCT + FROM`; that changes which side of the race a lost step lands on. + """ + @spec where_step_failed(Ecto.Queryable.t()) :: Ecto.Queryable.t() + def where_step_failed(query) do + from(s in query, where: s.exit_reason != "success") + end + + @doc """ + Appends the tiebreak for "the run that speaks for a work order" to a query + with a `:run` binding: most recently finished first, ties broken by id. + + A one-liner, but shared so the health page's bulk `DISTINCT ON` and the + history filter's correlated per-work-order lookup can't drift apart on + what "latest" means. + """ + @spec order_by_run_recency(Ecto.Queryable.t()) :: Ecto.Queryable.t() + def order_by_run_recency(query) do + from([run: r] in query, + order_by: [desc_nulls_last: r.finished_at, desc: r.id] + ) + end + + @doc """ + The reason a failure is reported under, coalescing a step's own + `exit_reason` with what the run's terminal state implies for one that + never reported. + + `mark_steps_lost/1` stamps a step's `exit_reason` and nothing else, so a + crashed run with no step at all has to fall back to `Run.state_reasons/0` + — the worker's own words for each terminal state, kept in one place so the + map is not typed out twice. + """ + @spec exit_reason(String.t() | nil, atom() | nil) :: String.t() | nil + def exit_reason(step_exit_reason, run_state) do + step_exit_reason || Map.get(Run.state_reasons(), run_state) + end + + @doc """ + The error type a failure is reported under, coalescing a step's own + `error_type` with the run's — treating an empty string as missing on both, + since `"" || x` returns `""` (empty string is truthy in Elixir) and would + otherwise split one failure into two identical-looking rows. + """ + @spec error_type(String.t() | nil, String.t() | nil) :: String.t() | nil + def error_type(step_error_type, run_error_type) do + blank_to_nil(step_error_type) || blank_to_nil(run_error_type) + end + + defp blank_to_nil(""), do: nil + defp blank_to_nil(other), do: other + @doc """ Runs for a specific project, or all runs available to the requesting user """ diff --git a/lib/lightning/workflows/stats.ex b/lib/lightning/workflows/stats.ex index f6243272f3..c79b24b23f 100644 --- a/lib/lightning/workflows/stats.ex +++ b/lib/lightning/workflows/stats.ex @@ -17,6 +17,7 @@ defmodule Lightning.Workflows.Stats do """ import Ecto.Query + alias Lightning.Invocation.Query alias Lightning.Invocation.Step alias Lightning.Repo alias Lightning.Run @@ -34,14 +35,8 @@ defmodule Lightning.Workflows.Stats do @final_states WorkOrder.final_states() @zero_counts Map.new(@final_states, &{&1, 0}) - # `:cancelled` is final but not a failure — someone stopped it on purpose. Own - # outcome, not the red wedge, which is why this narrows the schema's list - # rather than changing it. - @failure_states WorkOrder.failure_states() -- [:cancelled] - - # The signature grammar is written in the worker's words, so a - # run-level failure has to be mapped back out of its state. - @state_reasons Run.state_reasons() + # Shared with the history filter, which needs the same set `Stats` narrows. + @failure_states Query.failure_states() @doc """ Work order counts by final state over the last `days_back` days. @@ -147,9 +142,7 @@ defmodule Lightning.Workflows.Stats do from(s in Step, join: rs in RunStep, on: rs.step_id == s.id, - where: - rs.run_id == parent_as(:latest_run).run_id and - s.exit_reason != "success", + where: rs.run_id == parent_as(:latest_run).run_id, select: %{ exit_reason: s.exit_reason, error_type: s.error_type, @@ -157,6 +150,7 @@ defmodule Lightning.Workflows.Stats do job_id: s.job_id } ) + |> Query.where_step_failed() from(lr in subquery(latest_runs(workflow_id, since)), as: :latest_run, @@ -181,12 +175,13 @@ defmodule Lightning.Workflows.Stats do from(wo in WorkOrder, # left_join, not join: a rejected work order has no run and still counts. left_join: r in Run, + as: :run, on: r.work_order_id == wo.id, where: wo.workflow_id == ^workflow_id and wo.last_activity > ^since and wo.state in ^@failure_states, distinct: wo.id, - order_by: [asc: wo.id, desc_nulls_last: r.finished_at, desc: r.id], + order_by: [asc: wo.id], select: %{ work_order_id: wo.id, work_order_state: wo.state, @@ -195,6 +190,7 @@ defmodule Lightning.Workflows.Stats do run_error_type: r.error_type } ) + |> Query.order_by_run_recency() end # The job's name and adaptor come off the run's own snapshot, not the live @@ -240,8 +236,12 @@ defmodule Lightning.Workflows.Stats do end # Keyed by snapshot as well as job, because the same job id carries a - # different name in every snapshot that renamed it. Only `name` and `adaptor` - # are read out, so the job bodies alongside them never cross the wire. + # different name in every snapshot that renamed it. `lock_version` rides + # along for `merge_counts/1` to pick a label with, since a snapshot's own id + # carries no timestamp to compare rows by — it is unique and monotonic per + # workflow, so the highest one read is the most recent. Only these three + # fields are read out, so the job bodies alongside them never cross the + # wire. defp snapshot_jobs([]), do: %{} defp snapshot_jobs(snapshot_ids) do @@ -250,7 +250,8 @@ defmodule Lightning.Workflows.Stats do cross_lateral_join: j in fragment("jsonb_array_elements(?)", s.jobs), select: {{s.id, fragment("? ->> ?", j, "id")}, - {fragment("? ->> ?", j, "name"), fragment("? ->> ?", j, "adaptor")}} + {fragment("? ->> ?", j, "name"), fragment("? ->> ?", j, "adaptor"), + s.lock_version}} ) |> Repo.all() |> Map.new() @@ -258,12 +259,14 @@ defmodule Lightning.Workflows.Stats do # A rejected work order never got a run, so there is no signature to read. # `:rejected` has one origin — the run limit refusing a webhook payload — so - # the label names it outright. + # the label names it outright. `job_id: nil` gives it the same shape as + # every other row, for the JSON and the TS type on the other end of it. defp to_signature(%{work_order_state: :rejected} = row, _jobs) do %{ count: row.count, exit_reason: "rejected", error_type: "RunLimitExceeded", + job_id: nil, step_name: nil, adaptor: nil } @@ -273,37 +276,52 @@ defmodule Lightning.Workflows.Stats do # reported — a lost or reaped run — carries only the reason, and the rest # comes off the run. `mark_steps_lost/1` is why: it stamps `exit_reason` and # leaves `error_type` alone. + # + # `job_id` is the key `merge_counts/1` folds rows on; `step_name` and + # `adaptor` are labels, resolved off the snapshot and carried with + # `lock_version` so the fold can pick the newest one and then drop it. defp to_signature(row, jobs) do - {step_name, adaptor} = - Map.get(jobs, {row.snapshot_id, row.job_id}, {nil, nil}) + {step_name, adaptor, lock_version} = + Map.get(jobs, {row.snapshot_id, row.job_id}, {nil, nil, nil}) %{ count: row.count, - exit_reason: row.exit_reason || @state_reasons[row.run_state], - error_type: - blank_to_nil(row.error_type) || blank_to_nil(row.run_error_type), + exit_reason: Query.exit_reason(row.exit_reason, row.run_state), + error_type: Query.error_type(row.error_type, row.run_error_type), + job_id: row.job_id, step_name: step_name, - adaptor: adaptor + adaptor: adaptor, + lock_version: lock_version } end - # `"" || x` returns `""` — empty string is truthy in Elixir — so a step that - # reported an empty error type would coalesce to `""` instead of falling - # through to the run's, splitting one signature into two identical-looking - # rows. The run's error type can be empty the same way, and would land in - # the signature unread, so both sides are normalised: empty is absent. - defp blank_to_nil(""), do: nil - defp blank_to_nil(other), do: other - # Two groups can collapse into one signature — a crashed run and a failed run - # whose steps both reported `fail`, say — so the fold happens after the - # coalesce, not in the `group_by`. + # whose steps both reported `fail`, say, or the same job renamed mid-window — + # so the fold happens after the coalesce, not in the `group_by`. Grouping key + # is the triple that actually identifies a failure, not the whole map: the + # label (`step_name`, `adaptor`) is expected to differ between rows a rename + # merges, and `lock_version` never repeats. defp merge_counts(signatures) do signatures - |> Enum.group_by(&Map.delete(&1, :count), & &1.count) - |> Enum.map(fn {signature, counts} -> - Map.put(signature, :count, Enum.sum(counts)) - end) + |> Enum.group_by(&{&1.exit_reason, &1.error_type, &1.job_id}) + |> Enum.map(fn {_key, rows} -> merge_group(rows) end) |> Enum.sort_by(&{-&1.count, &1.step_name}, :asc) end + + # The label comes from the group's most recent failing snapshot: the row + # with the highest `lock_version`. A row with no snapshot at all (a rejected + # work order, or a run that never reached a step) carries no `lock_version`, + # which sorts lowest and never wins over a labelled one. + defp merge_group(rows) do + label = Enum.max_by(rows, &(&1[:lock_version] || -1)) + + %{ + count: Enum.sum_by(rows, & &1.count), + exit_reason: label.exit_reason, + error_type: label.error_type, + job_id: label.job_id, + step_name: label.step_name, + adaptor: label.adaptor + } + end end diff --git a/test/lightning/workflows/stats_test.exs b/test/lightning/workflows/stats_test.exs index 92fb554a04..3451a9a432 100644 --- a/test/lightning/workflows/stats_test.exs +++ b/test/lightning/workflows/stats_test.exs @@ -245,6 +245,7 @@ defmodule Lightning.Workflows.StatsTest do count: 1, exit_reason: "fail", error_type: "RuntimeError", + job_id: job.id, step_name: job.name, adaptor: job.adaptor } @@ -276,6 +277,41 @@ defmodule Lightning.Workflows.StatsTest do assert signature.adaptor == job.adaptor end + # The row is keyed by `job_id`, not by the resolved name — that is what + # keeps a job renamed mid-window as one row instead of splitting into a + # before-rename row and an after-rename row. The label comes off the + # newer of the two snapshots, since that is the name the job actually has + # by the time anyone triages it. + test "merges a job renamed mid-window into one row, labelled with the newer name", + %{workflow: workflow, trigger: trigger} do + job = hd(workflow.jobs) + job_id = job.id + + failed_run(workflow, trigger, [], [ + step(job, exit_reason: "fail", error_type: "RuntimeError") + ]) + + job + |> Ecto.Changeset.change(name: "Renamed") + |> Repo.update!() + + # `current_snapshot/1` only creates a new snapshot when none exists for + # the workflow's current `lock_version`, so the rename alone would still + # resolve against the snapshot already made above. Bumping it forces a + # second snapshot — the newer one the label tiebreak has to pick. + workflow + |> Ecto.Changeset.change(lock_version: workflow.lock_version + 1) + |> Repo.update!() + + failed_run(workflow, trigger, [], [ + step(job, exit_reason: "fail", error_type: "RuntimeError") + ]) + + assert %{signatures: [signature]} = Stats.failure_signatures(workflow) + + assert %{count: 2, job_id: ^job_id, step_name: "Renamed"} = signature + end + # `mark_steps_lost/1` stamps the step's exit_reason and nothing else, so # the error type has to come off the run or the signature reads `lost:`. test "falls back to the run's error type when the step never reported one", @@ -307,6 +343,7 @@ defmodule Lightning.Workflows.StatsTest do count: 1, exit_reason: "crash", error_type: "CompileError", + job_id: nil, step_name: nil, adaptor: nil } @@ -327,6 +364,7 @@ defmodule Lightning.Workflows.StatsTest do count: 1, exit_reason: "rejected", error_type: "RunLimitExceeded", + job_id: nil, step_name: nil, adaptor: nil } From 8eefdf3cba596e16f64801725c9a0a6e38e3dcd0 Mon Sep 17 00:00:00 2001 From: Lucy Macartney Date: Tue, 8 Sep 2026 14:38:16 -0500 Subject: [PATCH 03/20] History filter on the failure signature Adds exit_reason, error_type, and job_id to SearchParams and the LiveView filter types, and filter_by_signature/2 to scope any work order query to exactly the work orders behind one triage row. A present job_id reads as a step-level row, correlated against the latest run; an absent one reads as a run-level failure (lost, crashed, no step), matched against the inverted Run.state_reasons/0 map and failing closed on an unrecognised reason rather than dropping the filter, which would widen a bulk retry. Carries wo.state in failure_states() itself: a successful work order can hold a failing step under an on_job_failure handler that ran fine, so without this the filter would match work orders the triage row never counted. --- lib/lightning/invocation.ex | 108 +++++++++ lib/lightning/workorders/search_params.ex | 18 +- lib/lightning_web/live/run_live/index.ex | 5 +- test/lightning/invocation_test.exs | 282 ++++++++++++++++++++++ 4 files changed, 410 insertions(+), 3 deletions(-) diff --git a/lib/lightning/invocation.ex b/lib/lightning/invocation.ex index d550a80b10..d479d22cb5 100644 --- a/lib/lightning/invocation.ex +++ b/lib/lightning/invocation.ex @@ -15,6 +15,7 @@ defmodule Lightning.Invocation do alias Lightning.Projects.Project alias Lightning.Repo alias Lightning.Run + alias Lightning.RunStep alias Lightning.Workflows.Edge alias Lightning.Workflows.Job alias Lightning.Workflows.Trigger @@ -622,6 +623,7 @@ defmodule Lightning.Invocation do |> filter_by_wo_date_before(search_params.wo_date_before) |> filter_by_date_after(search_params.date_after) |> filter_by_date_before(search_params.date_before) + |> filter_by_signature(search_params) |> filter_by_body_or_log_or_id( search_params.search_fields, search_params.search_term @@ -717,6 +719,112 @@ defmodule Lightning.Invocation do ) end + # The inverse of `Run.state_reasons/0`, for reading a run-level signature's + # `exit_reason` back into the state it came from. `"rejected"` is not a + # value in that map — it is `to_signature/2`'s own literal for a work order + # that never got a run — so it naturally misses here and falls through to + # `filter_by_signature/2`'s fail-closed branch, exactly like any other + # exit_reason no run can actually be in. + @reason_states Map.new(Run.state_reasons(), fn {state, reason} -> + {reason, state} + end) + + # A triage row's "View" button, scoped to exactly the work orders it + # counted. `exit_reason` switches the filter on; a present `job_id` reads + # as the step-level row, an absent one as the run-level row (no failing + # step) — safe because `steps.job_id` is `NOT NULL`. + # + # Carries `wo.state in failure_states()` itself: a *successful* work order + # can still hold a `fail` step in its latest run (an `on_job_failure` + # handler that ran fine), so without this a signature filter would match + # work orders the triage row never counted, and bulk retry would follow. + defp filter_by_signature(query, %SearchParams{exit_reason: nil}), do: query + + defp filter_by_signature(query, %SearchParams{ + exit_reason: exit_reason, + error_type: error_type, + job_id: job_id + }) + when is_binary(job_id) do + step_match = + from(s in failing_steps_of_latest_run(), + where: s.job_id == ^job_id and s.exit_reason == ^exit_reason, + where: + fragment( + "coalesce(nullif(?, ''), nullif(?, '')) IS NOT DISTINCT FROM ?", + s.error_type, + parent_as(:latest_run).error_type, + type(^error_type, :string) + ) + ) + + from([workorder: wo] in query, + where: wo.state in ^Query.failure_states(), + where: + exists( + from(r in subquery(latest_run_for_workorder()), + as: :latest_run, + where: exists(subquery(step_match)) + ) + ) + ) + end + + defp filter_by_signature(query, %SearchParams{ + exit_reason: exit_reason, + error_type: error_type, + job_id: nil + }) do + case Map.fetch(@reason_states, exit_reason) do + {:ok, state} -> + from([workorder: wo] in query, + where: wo.state in ^Query.failure_states(), + where: + exists( + from(r in subquery(latest_run_for_workorder()), + as: :latest_run, + where: r.state == ^state, + where: + fragment( + "? IS NOT DISTINCT FROM ?", + r.error_type, + type(^error_type, :string) + ), + where: not exists(subquery(failing_steps_of_latest_run())) + ) + ) + ) + + # An `exit_reason` no run can actually be in — fail closed rather than + # drop the filter, which would widen a bulk retry to every failure. + :error -> + from([workorder: wo] in query, where: false) + end + end + + # The run that speaks for a work order: most recently finished first, ties + # broken by id. Correlated on the outer query's `:workorder` binding. + defp latest_run_for_workorder do + from(r in Run, + as: :run, + where: r.work_order_id == parent_as(:workorder).id + ) + |> Query.order_by_run_recency() + |> limit(1) + end + + # Every step of the `:latest_run` binding's run that did not succeed. + # Correlated on `:latest_run`, so it only makes sense nested inside a query + # that introduces that binding. + defp failing_steps_of_latest_run do + from(s in Step, + join: rs in RunStep, + on: rs.step_id == s.id, + where: rs.run_id == parent_as(:latest_run).id + ) + |> Query.where_step_failed() + end + defp filter_by_body_or_log_or_id(query, _search_fields, nil), do: query defp filter_by_body_or_log_or_id(query, search_fields, search_term) do diff --git a/lib/lightning/workorders/search_params.ex b/lib/lightning/workorders/search_params.ex index e68f58f8f1..9ccb1778ab 100644 --- a/lib/lightning/workorders/search_params.ex +++ b/lib/lightning/workorders/search_params.ex @@ -17,7 +17,10 @@ defmodule Lightning.WorkOrders.SearchParams do :wo_date_after, :wo_date_before, :sort_by, - :sort_direction + :sort_direction, + :exit_reason, + :error_type, + :job_id ] @derive {Jason.Encoder, only: @fields} @@ -51,7 +54,10 @@ defmodule Lightning.WorkOrders.SearchParams do wo_date_after: DateTime.t(), wo_date_before: DateTime.t(), sort_by: String.t(), - sort_direction: String.t() + sort_direction: String.t(), + exit_reason: String.t(), + error_type: String.t(), + job_id: Ecto.UUID.t() } @primary_key false @@ -72,6 +78,14 @@ defmodule Lightning.WorkOrders.SearchParams do field(:wo_date_before, :utc_datetime_usec) field(:sort_by, :string) field(:sort_direction, :string) + + # The failure signature the workflow health page's triage row draws its + # "View" button from. `exit_reason` switches the filter on; a present + # `job_id` is a step-level row, an absent one a run-level row. See + # `Lightning.Invocation.filter_by_signature/2`. + field(:exit_reason, :string) + field(:error_type, :string) + field(:job_id, :binary_id) end # Raises on invalid input. A malformed filter is only reachable by hand-editing diff --git a/lib/lightning_web/live/run_live/index.ex b/lib/lightning_web/live/run_live/index.ex index b67e2f9727..4f64164840 100644 --- a/lib/lightning_web/live/run_live/index.ex +++ b/lib/lightning_web/live/run_live/index.ex @@ -46,7 +46,10 @@ defmodule LightningWeb.RunLive.Index do lost: :boolean, rejected: :boolean, sort_by: :string, - sort_direction: :string + sort_direction: :string, + exit_reason: :string, + error_type: :string, + job_id: :string } @empty_page %{ diff --git a/test/lightning/invocation_test.exs b/test/lightning/invocation_test.exs index 873a637aec..e839c2b590 100644 --- a/test/lightning/invocation_test.exs +++ b/test/lightning/invocation_test.exs @@ -1030,6 +1030,288 @@ defmodule Lightning.InvocationTest do end end + # The triage row's "View" button: `filter_by_signature/2` inside + # `search_workorders_query/2`. Exercised through `search_workorders_for_export_query/2` + # since it applies no destructive-action side filter of its own. + describe "filter_by_signature/2" do + defp signature_params(exit_reason, error_type, job_id) do + SearchParams.new(%{ + "status" => SearchParams.status_list(), + "exit_reason" => exit_reason, + "error_type" => error_type, + "job_id" => job_id + }) + end + + defp signature_matches(project, exit_reason, error_type, job_id \\ nil) do + project + |> Invocation.search_workorders_for_export_query( + signature_params(exit_reason, error_type, job_id) + ) + |> Repo.all() + |> Enum.map(& &1.id) + |> MapSet.new() + end + + test "matches exactly the work orders behind a step-level signature, and none of a rejected or lost row" do + project = insert(:project) + + %{workflow: workflow, trigger: trigger, job: job} = + build_workflow(project: project) + + other_job = insert(:job, workflow: workflow) + + crashed_wo = + insert(:workorder, + workflow: workflow, + trigger: trigger, + dataclip: insert(:dataclip), + state: :crashed + ) + + insert(:run, + work_order: crashed_wo, + starting_trigger: trigger, + dataclip: insert(:dataclip), + state: :crashed, + steps: [ + build(:step, job: job, exit_reason: "fail", error_type: "RuntimeError") + ] + ) + + failed_wo = + insert(:workorder, + workflow: workflow, + trigger: trigger, + dataclip: insert(:dataclip), + state: :failed + ) + + insert(:run, + work_order: failed_wo, + starting_trigger: trigger, + dataclip: insert(:dataclip), + state: :failed, + steps: [ + build(:step, job: job, exit_reason: "fail", error_type: "RuntimeError") + ] + ) + + # Same reason and error type, different job — must not match. + other_job_wo = + insert(:workorder, + workflow: workflow, + trigger: trigger, + dataclip: insert(:dataclip), + state: :failed + ) + + insert(:run, + work_order: other_job_wo, + starting_trigger: trigger, + dataclip: insert(:dataclip), + state: :failed, + steps: [ + build(:step, + job: other_job, + exit_reason: "fail", + error_type: "RuntimeError" + ) + ] + ) + + _rejected_wo = + insert(:workorder, + workflow: workflow, + trigger: trigger, + dataclip: insert(:dataclip), + state: :rejected + ) + + lost_wo = + insert(:workorder, + workflow: workflow, + trigger: trigger, + dataclip: insert(:dataclip), + state: :lost + ) + + insert(:run, + work_order: lost_wo, + starting_trigger: trigger, + dataclip: insert(:dataclip), + state: :lost + ) + + assert signature_matches(project, "fail", "RuntimeError", job.id) == + MapSet.new([crashed_wo.id, failed_wo.id]) + + # `"rejected"` is `to_signature/2`'s own literal, not a worker reason — + # not present in `Run.state_reasons/0`, so the run-level branch fails + # closed rather than matching the rejected work order. + assert signature_matches(project, "rejected", "RunLimitExceeded") == + MapSet.new() + + # The run-level branch, for a work order whose latest run never + # reached a step. + assert signature_matches(project, "lost", nil) == MapSet.new([lost_wo.id]) + end + + test "with all three fields nil is a no-op" do + project = insert(:project) + + %{workflow: workflow, trigger: trigger} = build_workflow(project: project) + + wo = + insert(:workorder, + workflow: workflow, + trigger: trigger, + dataclip: insert(:dataclip), + state: :failed + ) + + insert(:run, + work_order: wo, + starting_trigger: trigger, + dataclip: insert(:dataclip), + state: :failed + ) + + params = SearchParams.new(%{"status" => SearchParams.status_list()}) + assert params.exit_reason == nil + assert params.error_type == nil + assert params.job_id == nil + + without_signature = + project + |> Invocation.search_workorders_for_export_query(params) + |> Repo.all() + |> Enum.map(& &1.id) + + with_explicit_nils = + SearchParams.new(%{ + "status" => SearchParams.status_list(), + "exit_reason" => nil, + "error_type" => nil, + "job_id" => nil + }) + + with_signature = + project + |> Invocation.search_workorders_for_export_query(with_explicit_nils) + |> Repo.all() + |> Enum.map(& &1.id) + + assert without_signature == [wo.id] + assert with_signature == without_signature + end + + test "search_workorders_for_retry/2 scopes a bulk retry to the signature" do + project = insert(:project) + + %{workflow: workflow, trigger: trigger, job: job} = + build_workflow(project: project) + + matching_wo = + insert(:workorder, + workflow: workflow, + trigger: trigger, + dataclip: insert(:dataclip), + state: :failed + ) + + insert(:run, + work_order: matching_wo, + starting_trigger: trigger, + dataclip: insert(:dataclip), + state: :failed, + steps: [ + build(:step, job: job, exit_reason: "fail", error_type: "RuntimeError") + ] + ) + + other_wo = + insert(:workorder, + workflow: workflow, + trigger: trigger, + dataclip: insert(:dataclip), + state: :failed + ) + + insert(:run, + work_order: other_wo, + starting_trigger: trigger, + dataclip: insert(:dataclip), + state: :failed, + steps: [ + build(:step, job: job, exit_reason: "fail", error_type: "CompileError") + ] + ) + + found = + Invocation.search_workorders_for_retry( + project, + signature_params("fail", "RuntimeError", job.id) + ) + + assert [matching_wo.id] == Enum.map(found, & &1.id) + end + + test "a nil error_type filter matches a step whose own error_type is the empty string" do + project = insert(:project) + + %{workflow: workflow, trigger: trigger, job: job} = + build_workflow(project: project) + + wo = + insert(:workorder, + workflow: workflow, + trigger: trigger, + dataclip: insert(:dataclip), + state: :failed + ) + + insert(:run, + work_order: wo, + starting_trigger: trigger, + dataclip: insert(:dataclip), + state: :failed, + steps: [build(:step, job: job, exit_reason: "fail", error_type: "")] + ) + + assert signature_matches(project, "fail", nil, job.id) == + MapSet.new([wo.id]) + end + + test "a successful work order is not matched, even when its latest run holds a failing step" do + project = insert(:project) + + %{workflow: workflow, trigger: trigger, job: job} = + build_workflow(project: project) + + success_wo = + insert(:workorder, + workflow: workflow, + trigger: trigger, + dataclip: insert(:dataclip), + state: :success + ) + + insert(:run, + work_order: success_wo, + starting_trigger: trigger, + dataclip: insert(:dataclip), + state: :success, + steps: [ + build(:step, job: job, exit_reason: "fail", error_type: "RuntimeError") + ] + ) + + assert signature_matches(project, "fail", "RuntimeError", job.id) == + MapSet.new() + end + end + describe "search_workorders/1" do test "returns workorders ordered inserted at desc, with nulls first" do project = insert(:project) From 6a691905a64875879e19ca8015c3983b31ff0b79 Mon Sep 17 00:00:00 2001 From: Lucy Macartney Date: Tue, 8 Sep 2026 14:43:30 -0500 Subject: [PATCH 04/20] Filter chip for the failure signature Adds a chip to the history page's filter bar, alongside the existing work order ID chip, that renders when exit_reason is set and clears all three signature keys at once. search_workorders_query/2 is shared with bulk retry, bulk cancel and CSV export, so a signature filter with no visible indicator would be a way to silently act on the wrong group of work orders. --- .../live/run_live/index.html.heex | 20 +++++++ .../live/run_live/index_test.exs | 53 +++++++++++++++++++ 2 files changed, 73 insertions(+) diff --git a/lib/lightning_web/live/run_live/index.html.heex b/lib/lightning_web/live/run_live/index.html.heex index 7318cd0b05..030508d371 100644 --- a/lib/lightning_web/live/run_live/index.html.heex +++ b/lib/lightning_web/live/run_live/index.html.heex @@ -256,6 +256,26 @@ Work order: {display_short_uuid(workorder_id)} <% end %> + + <%!-- Failure signature chip (only when exit_reason filter is active) --%> + <% exit_reason = get_change(@filters_changeset, :exit_reason) %> + <%= if exit_reason do %> + <% error_type = get_change(@filters_changeset, :error_type) + job_id = get_change(@filters_changeset, :job_id) %> + <.filter_chip + id="signature-filter-chip" + active={true} + clear_fields={[ + {:exit_reason, nil}, + {:error_type, nil}, + {:job_id, nil} + ]} + > + {exit_reason}{if error_type, + do: ":#{error_type}"}{if job_id, + do: " @ #{display_short_uuid(job_id)}"} + + <% end %> diff --git a/test/lightning_web/live/run_live/index_test.exs b/test/lightning_web/live/run_live/index_test.exs index 886ffe9a4e..711b79ba3e 100644 --- a/test/lightning_web/live/run_live/index_test.exs +++ b/test/lightning_web/live/run_live/index_test.exs @@ -971,6 +971,59 @@ defmodule LightningWeb.RunLive.IndexTest do chip = element(view, "#workorder-id-filter-chip") assert render(chip) =~ "Work order:" end + + test "signature filter chip appears when exit_reason filter is set", %{ + conn: conn, + project: project, + jobs: [job | _] + } do + {:ok, view, _html} = + live_async( + conn, + Routes.project_run_index_path(conn, :index, project.id, + filters: %{ + exit_reason: "fail", + error_type: "AdaptorError", + job_id: job.id + } + ) + ) + + assert has_element?(view, "#signature-filter-chip") + chip = element(view, "#signature-filter-chip") + + assert render(chip) =~ + "fail:AdaptorError @ #{LightningWeb.LiveHelpers.display_short_uuid(job.id)}" + end + + test "signature filter chip omits error type and job id when absent", %{ + conn: conn, + project: project + } do + {:ok, view, _html} = + live_async( + conn, + Routes.project_run_index_path(conn, :index, project.id, + filters: %{exit_reason: "lost"} + ) + ) + + chip = element(view, "#signature-filter-chip") + html = render(chip) + assert html =~ "lost" + refute html =~ "@" + end + + test "signature filter chip is absent when exit_reason filter is not set", + %{conn: conn, project: project} do + {:ok, view, _html} = + live_async( + conn, + Routes.project_run_index_path(conn, :index, project.id) + ) + + refute has_element?(view, "#signature-filter-chip") + end end describe "cancel work orders" do From 4ef2d84c126716bf61ba78ceb10cbe660a95ca06 Mon Sep 17 00:00:00 2001 From: Lucy Macartney Date: Tue, 8 Sep 2026 14:56:36 -0500 Subject: [PATCH 05/20] View button on each triage row Each row now links to the history page filtered to exactly the work orders it counts, where the existing "retry all" can act on the group. job_id joins the signature so a job deleted and recreated with the same name doesn't collide with the row it replaced, and the adaptor renders without its version since a merged row can span more than one. A rejected row links through history's existing rejected status filter instead of the signature keys, since a rejected work order never got a run for the signature filter to match against server-side. --- CHANGELOG.md | 4 +- assets/js/health/WorkflowHealth.tsx | 3 + assets/js/health/charts/TriageTable.tsx | 122 ++++++++++-- assets/js/health/types.ts | 7 +- assets/test/health/WorkflowHealth.test.tsx | 1 + .../test/health/charts/TriageTable.test.tsx | 188 +++++++++++------- 6 files changed, 244 insertions(+), 81 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 54564db0a2..184e77f173 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,7 +49,9 @@ and this project adheres to - A workflow health page at `/projects/:project_id/w/:workflow_id/health`, summarising one workflow over a selectable window (last 24 hours, 7 days, or 30 days): a donut of work order outcomes, a breakdown of the failing ones, and - a triage table grouping failures by error signature, heaviest first. The page + a triage table grouping failures by error signature, heaviest first. Each row + has a View button linking to the history page filtered to just the work orders + behind it, where the existing "retry all" can retry the group. The page refreshes itself as that workflow's work orders settle, at most once every 30 seconds. Reachable from the workflows list via a "Health" link in each row's Actions column. diff --git a/assets/js/health/WorkflowHealth.tsx b/assets/js/health/WorkflowHealth.tsx index da0db09df5..a6ebe2160b 100644 --- a/assets/js/health/WorkflowHealth.tsx +++ b/assets/js/health/WorkflowHealth.tsx @@ -125,6 +125,9 @@ export const HealthContent = ({ )} diff --git a/assets/js/health/charts/TriageTable.tsx b/assets/js/health/charts/TriageTable.tsx index 7357dd3c39..e60b8e0d0c 100644 --- a/assets/js/health/charts/TriageTable.tsx +++ b/assets/js/health/charts/TriageTable.tsx @@ -1,11 +1,12 @@ import type { FailureSignature } from '../types'; /** - * Failed work orders grouped by error signature, heaviest first. - * - * Purely informational — there is nothing to act on here, so no row is a link - * or a control. The signature grammar is: - * `exitReason:errorType [@ stepName [adaptor@version]]`. + * Failed work orders grouped by error signature, heaviest first. Each row + * links to the history page filtered to the work orders it counts, where the + * existing "retry all" can act on the group. The signature grammar is: + * `exitReason:errorType [@ stepName [adaptor]]` — the adaptor renders without + * its version, since a merged row can span more than one (see `job_id` on + * `FailureSignature`). */ // One sentence per error type the worker can report, written to hold @@ -61,19 +62,27 @@ const TIPS: Record = { interface TriageTableProps { signatures: FailureSignature[]; emptyMessage: string; + projectId: string; + workflowId: string; + /** `window.from` off the same response — the picked range's start. */ + from: string; } -export const TriageTable = ({ signatures, emptyMessage }: TriageTableProps) => { +export const TriageTable = ({ + signatures, + emptyMessage, + projectId, + workflowId, + from, +}: TriageTableProps) => { if (signatures.length === 0) { return

{emptyMessage}

; } return ( - // A rename forks a job's history into a signature per name, so a long-lived - // workflow can list far more rows than it has ways of breaking. Capped in - // height rather than in rows: the tail is still worth reading, just not - // worth pushing the rest of the page down for. `max-h` over a row count so - // a short list keeps the card short. + // Capped in height rather than in rows: the tail is still worth reading, + // just not worth pushing the rest of the page down for. `max-h` over a + // row count so a short list keeps the card short.
@@ -84,16 +93,22 @@ export const TriageTable = ({ signatures, emptyMessage }: TriageTableProps) => { + {signatures.map(signature => ( + // job_id joins the key: a job deleted and recreated with the same + // name reads as two identical-looking signatures otherwise. @@ -107,6 +122,14 @@ export const TriageTable = ({ signatures, emptyMessage }: TriageTableProps) => { {tipFor(signature)}

+ ))} @@ -115,6 +138,67 @@ export const TriageTable = ({ signatures, emptyMessage }: TriageTableProps) => { ); }; +/** + * Lands on history filtered to exactly the work orders this row counts, where + * the existing "retry all" can act on the group. Not labelled with the row's + * count — the filter re-derives the count on every load, so the number moves. + * + * Nothing to link on a row whose `exit_reason` never resolved — that leaves + * neither a step nor a mappable run state to filter history on. + */ +const ViewButton = ({ + signature, + projectId, + workflowId, + from, +}: { + signature: FailureSignature; + projectId: string; + workflowId: string; + from: string; +}) => { + if (!signature.exit_reason) return null; + + return ( + + View + + ); +}; + +// A rejected work order never got a run, so the signature filter would fail +// closed on it server-side — history's existing `rejected` status filter is +// what actually matches these. `to_signature/2` gives every rejected row the +// same literal `exit_reason: "rejected"`, so that is the signal to switch. +const historyUrl = ( + projectId: string, + workflowId: string, + from: string, + signature: FailureSignature +) => { + const params = new URLSearchParams({ + 'filters[workflow_id]': workflowId, + 'filters[date_after]': from, + }); + + if (signature.exit_reason === 'rejected') { + params.set('filters[rejected]', 'true'); + } else { + params.set('filters[exit_reason]', signature.exit_reason); + if (signature.error_type) { + params.set('filters[error_type]', signature.error_type); + } + if (signature.job_id) { + params.set('filters[job_id]', signature.job_id); + } + } + + return `/projects/${projectId}/history?${params.toString()}`; +}; + // The parts are styled apart rather than concatenated server-side: the error // type is the bit worth scanning down the column for. const Signature = ({ signature }: { signature: FailureSignature }) => ( @@ -123,11 +207,25 @@ const Signature = ({ signature }: { signature: FailureSignature }) => ( {errorTypeOf(signature)} {signature.step_name && @ {signature.step_name}} {signature.adaptor && ( - [{signature.adaptor}] + + {' '} + [{packageNameOf(signature.adaptor)}] + )}

); +// A row is keyed and labelled by `job_id`, not by (job_id, adaptor) — see +// "Why `job_id`" in the plan — so a row spanning an adaptor bump mid-window is +// labelled from its newest failing snapshot. Rendering that snapshot's version +// would head older failures with a version that isn't theirs, so only the +// package name renders. Strips everything from the last '@' that isn't the +// scope's leading one, so a scoped package's own '@' survives. +const packageNameOf = (adaptor: string) => { + const lastAt = adaptor.lastIndexOf('@'); + return lastAt > 0 ? adaptor.slice(0, lastAt) : adaptor; +}; + // A step can finish without reporting a type, and a worker can report one as an // empty string. The signature still has to say something, and `default` is the // tip written for exactly that case — hence `||`, which catches '' as well as diff --git a/assets/js/health/types.ts b/assets/js/health/types.ts index dac76b71da..06f028c570 100644 --- a/assets/js/health/types.ts +++ b/assets/js/health/types.ts @@ -44,12 +44,17 @@ export interface Outcomes { * One row of the triage table: the parts of an error signature and * the number of work orders that carry it. `step_name` and `adaptor` are null * for a work order whose run failed before reaching a step, or never ran at - * all; `error_type` is null when nothing reported one. + * all; `error_type` is null when nothing reported one. `job_id` is the same + * story as `step_name`/`adaptor` — null for a run-level row and for a + * rejected one — but it is the key the history filter matches on, since + * matching on the resolved name would need a snapshot lookup the filter + * doesn't do. */ export interface FailureSignature { count: number; exit_reason: string; error_type: string | null; + job_id: string | null; step_name: string | null; adaptor: string | null; } diff --git a/assets/test/health/WorkflowHealth.test.tsx b/assets/test/health/WorkflowHealth.test.tsx index f3efdb717b..da78917fd9 100644 --- a/assets/test/health/WorkflowHealth.test.tsx +++ b/assets/test/health/WorkflowHealth.test.tsx @@ -25,6 +25,7 @@ const failureSignatures = { count: 98, exit_reason: 'fail', error_type: 'RuntimeError', + job_id: 'a1b2c3d4-0000-0000-0000-000000000000', step_name: 'Map-beneficiary', adaptor: '@openfn/language-common@2.0.0', }, diff --git a/assets/test/health/charts/TriageTable.test.tsx b/assets/test/health/charts/TriageTable.test.tsx index db3d500bb7..11aadd45ee 100644 --- a/assets/test/health/charts/TriageTable.test.tsx +++ b/assets/test/health/charts/TriageTable.test.tsx @@ -10,6 +10,7 @@ const signature = ( count: 62, exit_reason: 'fail', error_type: 'RuntimeError', + job_id: 'a1b2c3d4-0000-0000-0000-000000000000', step_name: 'Map-beneficiary', adaptor: '@openfn/language-common@2.0.0', ...overrides, @@ -19,14 +20,25 @@ const rowText = (name: string | RegExp) => within(screen.getByRole('row', { name })).getByRole('cell', { name }) .textContent; +// Every test renders the same workflow at the same window, since only the +// signature varies between them. +const table = (signatures: FailureSignature[], emptyMessage = 'No failures') => + render( + + ); + describe('TriageTable', () => { - test('renders the full signature grammar for a step-level failure', () => { - render( - - ); + test('renders the full signature grammar for a step-level failure, adaptor version dropped', () => { + table([signature()]); expect(rowText(/RuntimeError/)).toContain( - 'fail:RuntimeError @ Map-beneficiary [@openfn/language-common@2.0.0]' + 'fail:RuntimeError @ Map-beneficiary [@openfn/language-common]' ); expect(screen.getByRole('cell', { name: '62' })).toBeVisible(); expect( @@ -37,19 +49,15 @@ describe('TriageTable', () => { // A run that crashed before any step has no job to name, so the grammar's // optional clause drops rather than rendering an empty ` @ []`. test('drops the step clause when nothing reached a step', () => { - render( - - ); + table([ + signature({ + exit_reason: 'crash', + error_type: 'CompileError', + job_id: null, + step_name: null, + adaptor: null, + }), + ]); const text = rowText(/CompileError/); expect(text).toContain('crash:CompileError'); @@ -61,19 +69,15 @@ describe('TriageTable', () => { // signature off, so the server labels it outright — and the tip has to be // there, or the row reads as a bug in the page. test('names a rejected work order and tips it', () => { - render( - - ); + table([ + signature({ + exit_reason: 'rejected', + error_type: 'RunLimitExceeded', + job_id: null, + step_name: null, + adaptor: null, + }), + ]); expect(rowText(/RunLimitExceeded/)).toContain('rejected:RunLimitExceeded'); expect( @@ -84,12 +88,7 @@ describe('TriageTable', () => { }); test('shows the tip written for the error type', () => { - render( - - ); + table([signature({ error_type: 'OOMError' })]); expect( screen.getByText( @@ -101,15 +100,10 @@ describe('TriageTable', () => { // A crashing step reports these, never `RuntimeCrash` — see TIPS. Untipped, // the most common crash there is falls through to "no recognised error type". test('tips the raw JS names that reach Lightning in place of RuntimeCrash', () => { - render( - - ); + table([ + signature({ exit_reason: 'crash', error_type: 'ReferenceError' }), + signature({ exit_reason: 'crash', error_type: 'SyntaxError' }), + ]); expect(screen.getByText(/often a typo or a missing import/)).toBeVisible(); expect(screen.getByText(/wasn't in the format it expected/)).toBeVisible(); @@ -119,15 +113,10 @@ describe('TriageTable', () => { // set — and a step can finish without reporting one at all. Neither may // render a blank tip. test('falls back to the default tip for an unmapped or missing type', () => { - render( - - ); + table([ + signature({ error_type: 'SomeUnmappedError' }), + signature({ error_type: null, step_name: 'Verify-cedula' }), + ]); const fallback = 'The step failed without a recognised error type; check its logs.'; @@ -140,12 +129,7 @@ describe('TriageTable', () => { // `??` let that through, rendering the signature as a bare `fail:` and a // "Tip: " with no sentence after it. test('treats an empty error type as a missing one', () => { - render( - - ); + table([signature({ error_type: '' })]); expect(rowText(/unknown/)).toContain('fail:unknown @ Map-beneficiary'); expect( @@ -156,14 +140,84 @@ describe('TriageTable', () => { }); test('shows the empty message when nothing failed', () => { - render( - - ); + table([], 'No failures in the last 30 days'); expect(screen.getByText('No failures in the last 30 days')).toBeVisible(); expect(screen.queryByRole('table')).not.toBeInTheDocument(); }); + + describe('View button', () => { + // The normal case: a step-level row, keyed and filtered on job_id. + test('links a normal row to history filtered on the signature', () => { + table([signature()]); + + const link = screen.getByRole('link', { name: 'View' }); + expect(link).toHaveAttribute( + 'href', + '/projects/proj-1/history' + + '?filters%5Bworkflow_id%5D=wf-1' + + '&filters%5Bdate_after%5D=2026-08-01T10%3A00%3A00Z' + + '&filters%5Bexit_reason%5D=fail' + + '&filters%5Berror_type%5D=RuntimeError' + + '&filters%5Bjob_id%5D=a1b2c3d4-0000-0000-0000-000000000000' + ); + }); + + // A run that crashed before reaching a step has no job to key on, so the + // filter must omit job_id — matching the filter's own reading of an + // absent job_id as "the run-level row" rather than matching nothing. + test('omits job_id and error_type for a row with no step', () => { + table([ + signature({ + exit_reason: 'crash', + error_type: null, + job_id: null, + step_name: null, + adaptor: null, + }), + ]); + + const link = screen.getByRole('link', { name: 'View' }); + expect(link).toHaveAttribute( + 'href', + '/projects/proj-1/history' + + '?filters%5Bworkflow_id%5D=wf-1' + + '&filters%5Bdate_after%5D=2026-08-01T10%3A00%3A00Z' + + '&filters%5Bexit_reason%5D=crash' + ); + }); + + // A rejected row has no run to key a signature filter on — that filter + // fails closed server-side — so it links to history's own `rejected` + // status filter instead, not to the signature's own fields. + test('links a rejected row to the rejected status filter, not the signature', () => { + table([ + signature({ + exit_reason: 'rejected', + error_type: 'RunLimitExceeded', + job_id: null, + step_name: null, + adaptor: null, + }), + ]); + + const link = screen.getByRole('link', { name: 'View' }); + expect(link).toHaveAttribute( + 'href', + '/projects/proj-1/history' + + '?filters%5Bworkflow_id%5D=wf-1' + + '&filters%5Bdate_after%5D=2026-08-01T10%3A00%3A00Z' + + '&filters%5Brejected%5D=true' + ); + expect(link.getAttribute('href')).not.toContain('error_type'); + expect(link.getAttribute('href')).not.toContain('exit_reason'); + }); + + // Nothing to filter history on without a resolved exit_reason. + test('renders no button when exit_reason never resolved', () => { + table([signature({ exit_reason: '' })]); + + expect(screen.queryByRole('link', { name: 'View' })).toBeNull(); + }); + }); }); From bcb8cb9770ae97d848960c73575caae3214fd0dd Mon Sep 17 00:00:00 2001 From: Lucy Macartney Date: Tue, 8 Sep 2026 15:37:50 -0500 Subject: [PATCH 06/20] Polish the triage table: view button, alignment, scrollbar View is now a small filled pill with an arrow instead of a plain text link, easier to spot as an action. Work orders and View are middle- aligned in the row; only the two-line signature/tip cell stays pinned to the top. The scroll region bleeds out to the card's own edge so the scrollbar sits flush against it instead of floating in the middle of the card's padding, and the sticky header gets a z-index so a row's content can't paint above it while scrolling. --- assets/js/health/charts/TriageTable.tsx | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/assets/js/health/charts/TriageTable.tsx b/assets/js/health/charts/TriageTable.tsx index e60b8e0d0c..6fad3c1d68 100644 --- a/assets/js/health/charts/TriageTable.tsx +++ b/assets/js/health/charts/TriageTable.tsx @@ -83,9 +83,13 @@ export const TriageTable = ({ // Capped in height rather than in rows: the tail is still worth reading, // just not worth pushing the rest of the page down for. `max-h` over a // row count so a short list keeps the card short. -
+ // + // `-mr-6 pr-4` bleeds the scroll region out to the card's own edge (the + // card is `p-6`), so the scrollbar sits flush against it instead of + // floating in the middle of the card's padding. +
Signature + Actions +
+ +
- + - @@ -110,12 +114,12 @@ export const TriageTable = ({ signature.adaptor, signature.job_id, ].join('|')} - className="border-b border-gray-100 align-top last:border-0" + className="border-b border-gray-100 last:border-0" > - + {/* Nothing to link on a row whose `exit_reason` never resolved: + that leaves neither a step nor a mappable run state to filter + history on. */} ))} @@ -146,33 +148,16 @@ export const TriageTable = ({ * Lands on history filtered to exactly the work orders this row counts, where * the existing "retry all" can act on the group. Not labelled with the row's * count — the filter re-derives the count on every load, so the number moves. - * - * Nothing to link on a row whose `exit_reason` never resolved — that leaves - * neither a step nor a mappable run state to filter history on. */ -const ViewButton = ({ - signature, - projectId, - workflowId, - from, -}: { - signature: FailureSignature; - projectId: string; - workflowId: string; - from: string; -}) => { - if (!signature.exit_reason) return null; - - return ( - - View - - - ); -}; +const ViewButton = ({ href }: { href: string }) => ( + + View + + +); // A rejected work order never got a run, so the signature filter would fail // closed on it server-side — history's existing `rejected` status filter is @@ -228,12 +213,12 @@ const Signature = ({ signature }: { signature: FailureSignature }) => (

); -// A row is keyed and labelled by `job_id`, not by (job_id, adaptor) — see -// "Why `job_id`" in the plan — so a row spanning an adaptor bump mid-window is -// labelled from its newest failing snapshot. Rendering that snapshot's version -// would head older failures with a version that isn't theirs, so only the -// package name renders. Strips everything from the last '@' that isn't the -// scope's leading one, so a scoped package's own '@' survives. +// A row is keyed and labelled by `job_id`, not by (job_id, adaptor), so a row +// spanning an adaptor bump mid-window is labelled from its newest failing +// snapshot. Rendering that snapshot's version would head older failures with a +// version that isn't theirs, so only the package name renders. Strips +// everything from the last '@' that isn't the scope's leading one, so a scoped +// package's own '@' survives. const packageNameOf = (adaptor: string) => { const lastAt = adaptor.lastIndexOf('@'); return lastAt > 0 ? adaptor.slice(0, lastAt) : adaptor; From e08a4ce5bafb8a9d8f312e720c7c993ddd7ca1f2 Mon Sep 17 00:00:00 2001 From: Lucy Macartney Date: Tue, 8 Sep 2026 17:01:50 -0500 Subject: [PATCH 13/20] Explain the grouping key without the back-reference The comment contrasted the key with "the whole map", which only means something to someone who saw the previous implementation. --- lib/lightning/workflows/stats.ex | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/lightning/workflows/stats.ex b/lib/lightning/workflows/stats.ex index 1c6c69c819..a81791faac 100644 --- a/lib/lightning/workflows/stats.ex +++ b/lib/lightning/workflows/stats.ex @@ -315,9 +315,9 @@ defmodule Lightning.Workflows.Stats do # Two groups can collapse into one signature — a crashed run and a failed run # whose steps both reported `fail`, say, or the same job renamed mid-window — # so the fold happens after the coalesce, not in the `group_by`. Grouping key - # is the triple that actually identifies a failure, not the whole map: the - # label (`step_name`, `adaptor`) is expected to differ between rows a rename - # merges, and `lock_version` never repeats. + # is the triple that identifies a failure: the label (`step_name`, `adaptor`) + # is expected to differ between rows a rename merges, and `lock_version` + # never repeats. defp merge_counts(signatures) do signatures |> Enum.group_by(&{&1.exit_reason, &1.error_type, &1.job_id}) From 3a425877ae16c828d9c3fa733bee100ce223b59b Mon Sep 17 00:00:00 2001 From: Lucy Macartney Date: Tue, 8 Sep 2026 17:44:39 -0500 Subject: [PATCH 14/20] Read an absent search-field set as "no preference" from_uri/1 always set search_fields, so a link naming none of the four flags landed with an empty list and every search term then matched nothing. Each unticked box still arrives marked false, so "none present" can only mean a link that never mentioned them. Reverts the flags the triage View link carried to work around this. It was the first link built in the browser rather than through to_uri_params/1, so it was the first to hit this. --- assets/js/health/charts/TriageTable.tsx | 8 ---- .../test/health/charts/TriageTable.test.tsx | 12 ------ lib/lightning/workorders/search_params.ex | 37 ++++++++++--------- .../workorders/search_params_test.exs | 28 ++++++++++++++ 4 files changed, 47 insertions(+), 38 deletions(-) diff --git a/assets/js/health/charts/TriageTable.tsx b/assets/js/health/charts/TriageTable.tsx index fc96e8265a..e45febdb5e 100644 --- a/assets/js/health/charts/TriageTable.tsx +++ b/assets/js/health/charts/TriageTable.tsx @@ -172,14 +172,6 @@ const historyUrl = ( const params = new URLSearchParams({ 'filters[workflow_id]': workflowId, 'filters[date_after]': from, - // SearchParams.from_uri/1 reads the search-field flags out of the query - // string and put_new's the result, so an absent set means `search_fields: - // []` rather than the schema default, and every later search term matches - // nothing. to_uri_params/1 fills these in for every server-built link. - 'filters[id]': 'true', - 'filters[body]': 'true', - 'filters[log]': 'true', - 'filters[dataclip_name]': 'true', }); if (signature.exit_reason === 'rejected') { diff --git a/assets/test/health/charts/TriageTable.test.tsx b/assets/test/health/charts/TriageTable.test.tsx index 21e468e64f..11aadd45ee 100644 --- a/assets/test/health/charts/TriageTable.test.tsx +++ b/assets/test/health/charts/TriageTable.test.tsx @@ -157,10 +157,6 @@ describe('TriageTable', () => { '/projects/proj-1/history' + '?filters%5Bworkflow_id%5D=wf-1' + '&filters%5Bdate_after%5D=2026-08-01T10%3A00%3A00Z' + - '&filters%5Bid%5D=true' + - '&filters%5Bbody%5D=true' + - '&filters%5Blog%5D=true' + - '&filters%5Bdataclip_name%5D=true' + '&filters%5Bexit_reason%5D=fail' + '&filters%5Berror_type%5D=RuntimeError' + '&filters%5Bjob_id%5D=a1b2c3d4-0000-0000-0000-000000000000' @@ -187,10 +183,6 @@ describe('TriageTable', () => { '/projects/proj-1/history' + '?filters%5Bworkflow_id%5D=wf-1' + '&filters%5Bdate_after%5D=2026-08-01T10%3A00%3A00Z' + - '&filters%5Bid%5D=true' + - '&filters%5Bbody%5D=true' + - '&filters%5Blog%5D=true' + - '&filters%5Bdataclip_name%5D=true' + '&filters%5Bexit_reason%5D=crash' ); }); @@ -215,10 +207,6 @@ describe('TriageTable', () => { '/projects/proj-1/history' + '?filters%5Bworkflow_id%5D=wf-1' + '&filters%5Bdate_after%5D=2026-08-01T10%3A00%3A00Z' + - '&filters%5Bid%5D=true' + - '&filters%5Bbody%5D=true' + - '&filters%5Blog%5D=true' + - '&filters%5Bdataclip_name%5D=true' + '&filters%5Brejected%5D=true' ); expect(link.getAttribute('href')).not.toContain('error_type'); diff --git a/lib/lightning/workorders/search_params.ex b/lib/lightning/workorders/search_params.ex index 9ccb1778ab..112e392409 100644 --- a/lib/lightning/workorders/search_params.ex +++ b/lib/lightning/workorders/search_params.ex @@ -113,25 +113,26 @@ defmodule Lightning.WorkOrders.SearchParams do end defp from_uri(params) do - statuses = - Enum.map(params, fn {key, value} -> - if key in @statuses and value in [true, "true"] do - key - end - end) - |> Enum.reject(&is_nil/1) - - search_fields = - Enum.map(params, fn {key, value} -> - if key in @search_fields and value in [true, "true"] do - key - end - end) - |> Enum.reject(&is_nil/1) - params - |> Map.put_new("status", statuses) - |> Map.put_new("search_fields", search_fields) + |> Map.put_new("status", selected(params, @statuses)) + |> put_search_fields(params) + end + + # A URL with none of the four search-field flags in it hasn't turned them + # off — it just didn't mention them. Leaving the key out lets the schema + # default (all four) stand; setting it to `[]` makes every search term match + # nothing. The form ships a hidden `false` beside every checkbox, so an + # actual "all off" still arrives as four keys rather than as none. + defp put_search_fields(uri_params, params) do + if Enum.any?(@search_fields, &Map.has_key?(params, &1)) do + Map.put_new(uri_params, "search_fields", selected(params, @search_fields)) + else + uri_params + end + end + + defp selected(params, keys) do + for {key, value} <- params, key in keys, value in [true, "true"], do: key end def to_uri_params(search_params) do diff --git a/test/lightning/workorders/search_params_test.exs b/test/lightning/workorders/search_params_test.exs index 5e4414545f..2b0410cdfd 100644 --- a/test/lightning/workorders/search_params_test.exs +++ b/test/lightning/workorders/search_params_test.exs @@ -40,6 +40,34 @@ defmodule Lightning.WorkOrders.SearchParamsTest do workflow_id: "babd29f7-bf15-4a66-af21-51209217ebd4" } == SearchParams.new(params) end + + # A server-built link naming a workflow and a date carries no search-field + # flags. Reading that as "search nothing" makes the search box on the + # landed page match nothing at all. + test "falls back to every search field when the params name none of them" do + params = + SearchParams.new(%{ + "workflow_id" => "babd29f7-bf15-4a66-af21-51209217ebd4", + "search_term" => "hello" + }) + + assert params.search_fields == [:id, :body, :log, :dataclip_name] + end + + # And an unticked box still arrives, marked false, so turning them all off + # is a different thing from a link that never mentioned them. + test "searches nothing when every search field is present and false" do + params = + SearchParams.new(%{ + "id" => "false", + "body" => "false", + "log" => "false", + "dataclip_name" => "false", + "search_term" => "hello" + }) + + assert params.search_fields == [] + end end describe "from_map/1" do From 776bc8dd674d984deaa8072f31b5a5db500c9665 Mon Sep 17 00:00:00 2001 From: Lucy Macartney Date: Wed, 9 Sep 2026 09:35:47 -0500 Subject: [PATCH 15/20] Say once that a cancelled work order is not a failure Query.failure_states/0 existed only to take :cancelled back off WorkOrder.failure_states/0, and it was that list's only caller. Cancelled is not a failure anywhere in the app, so the exclusion belongs on WorkOrder itself and the callers can ask it directly. --- lib/lightning/invocation.ex | 4 ++-- lib/lightning/invocation/query.ex | 6 ------ lib/lightning/workflows/stats.ex | 5 +---- lib/lightning/workorders/workorder.ex | 5 +++-- 4 files changed, 6 insertions(+), 14 deletions(-) diff --git a/lib/lightning/invocation.ex b/lib/lightning/invocation.ex index f7dd106c2c..ec31106e14 100644 --- a/lib/lightning/invocation.ex +++ b/lib/lightning/invocation.ex @@ -759,7 +759,7 @@ defmodule Lightning.Invocation do ) from([workorder: wo] in query, - where: wo.state in ^Query.failure_states(), + where: wo.state in ^WorkOrder.failure_states(), where: exists( from(r in subquery(latest_run_for_workorder()), @@ -778,7 +778,7 @@ defmodule Lightning.Invocation do case Map.fetch(@reason_states, exit_reason) do {:ok, state} -> from([workorder: wo] in query, - where: wo.state in ^Query.failure_states(), + where: wo.state in ^WorkOrder.failure_states(), where: exists( from(r in subquery(latest_run_for_workorder()), diff --git a/lib/lightning/invocation/query.ex b/lib/lightning/invocation/query.ex index 7746d9f2cf..b4ea6abac0 100644 --- a/lib/lightning/invocation/query.ex +++ b/lib/lightning/invocation/query.ex @@ -40,12 +40,6 @@ defmodule Lightning.Invocation.Query do ) end - # Narrower than `WorkOrder.failure_states/0`: `:cancelled` is final but not a - # failure — someone stopped it on purpose. Shared so the history filter - # narrows on the same set `Stats` does. - @spec failure_states() :: [atom()] - def failure_states, do: WorkOrder.failure_states() -- [:cancelled] - @doc """ Appends `exit_reason != "success"` to a query of `Step`. diff --git a/lib/lightning/workflows/stats.ex b/lib/lightning/workflows/stats.ex index a81791faac..eb5083b76c 100644 --- a/lib/lightning/workflows/stats.ex +++ b/lib/lightning/workflows/stats.ex @@ -35,9 +35,6 @@ defmodule Lightning.Workflows.Stats do @final_states WorkOrder.final_states() @zero_counts Map.new(@final_states, &{&1, 0}) - # Shared with the history filter, which needs the same set `Stats` narrows. - @failure_states Query.failure_states() - @doc """ Work order counts by final state over the last `days_back` days. """ @@ -179,7 +176,7 @@ defmodule Lightning.Workflows.Stats do on: r.work_order_id == wo.id, where: wo.workflow_id == ^workflow_id and wo.last_activity > ^since and - wo.state in ^@failure_states, + wo.state in ^WorkOrder.failure_states(), distinct: wo.id, order_by: [asc: wo.id], select: %{ diff --git a/lib/lightning/workorders/workorder.ex b/lib/lightning/workorders/workorder.ex index abea2d6e05..98464acf18 100644 --- a/lib/lightning/workorders/workorder.ex +++ b/lib/lightning/workorders/workorder.ex @@ -50,9 +50,10 @@ defmodule Lightning.WorkOrder do @doc """ Returns the list of failure states for a work order. - These are all final states except `:success`. + Every final state except `:success` and `:cancelled` — a cancelled work + order stopped because someone stopped it, not because anything failed. """ - def failure_states, do: final_states() -- [:success] + def failure_states, do: final_states() -- [:success, :cancelled] @derive {Jason.Encoder, only: [ From 52eabfa9ac714f76ea57ec14d4a3c3e052a11945 Mon Sep 17 00:00:00 2001 From: Lucy Macartney Date: Wed, 9 Sep 2026 09:46:53 -0500 Subject: [PATCH 16/20] Name the signature filter fields for the signature exit_reason, error_type and job_id read as three independent history filters, but they are one failure signature and only filter_by_signature/2 reads them. The signature_ prefix says so at every call site, from the URL the triage View link builds through to the query. --- assets/js/health/charts/TriageTable.tsx | 6 ++--- .../test/health/charts/TriageTable.test.tsx | 11 ++++----- lib/lightning/invocation.ex | 22 +++++++++-------- lib/lightning/workorders/search_params.ex | 24 +++++++++---------- lib/lightning_web/live/run_live/index.ex | 6 ++--- .../live/run_live/index.html.heex | 17 +++++++------ test/lightning/invocation_test.exs | 13 ++++++---- .../live/run_live/index_test.exs | 12 +++++----- 8 files changed, 60 insertions(+), 51 deletions(-) diff --git a/assets/js/health/charts/TriageTable.tsx b/assets/js/health/charts/TriageTable.tsx index e45febdb5e..59ae331aa1 100644 --- a/assets/js/health/charts/TriageTable.tsx +++ b/assets/js/health/charts/TriageTable.tsx @@ -177,12 +177,12 @@ const historyUrl = ( if (signature.exit_reason === 'rejected') { params.set('filters[rejected]', 'true'); } else { - params.set('filters[exit_reason]', signature.exit_reason); + params.set('filters[signature_exit_reason]', signature.exit_reason); if (signature.error_type) { - params.set('filters[error_type]', signature.error_type); + params.set('filters[signature_error_type]', signature.error_type); } if (signature.job_id) { - params.set('filters[job_id]', signature.job_id); + params.set('filters[signature_job_id]', signature.job_id); } } diff --git a/assets/test/health/charts/TriageTable.test.tsx b/assets/test/health/charts/TriageTable.test.tsx index 11aadd45ee..758d638dfa 100644 --- a/assets/test/health/charts/TriageTable.test.tsx +++ b/assets/test/health/charts/TriageTable.test.tsx @@ -157,9 +157,9 @@ describe('TriageTable', () => { '/projects/proj-1/history' + '?filters%5Bworkflow_id%5D=wf-1' + '&filters%5Bdate_after%5D=2026-08-01T10%3A00%3A00Z' + - '&filters%5Bexit_reason%5D=fail' + - '&filters%5Berror_type%5D=RuntimeError' + - '&filters%5Bjob_id%5D=a1b2c3d4-0000-0000-0000-000000000000' + '&filters%5Bsignature_exit_reason%5D=fail' + + '&filters%5Bsignature_error_type%5D=RuntimeError' + + '&filters%5Bsignature_job_id%5D=a1b2c3d4-0000-0000-0000-000000000000' ); }); @@ -183,7 +183,7 @@ describe('TriageTable', () => { '/projects/proj-1/history' + '?filters%5Bworkflow_id%5D=wf-1' + '&filters%5Bdate_after%5D=2026-08-01T10%3A00%3A00Z' + - '&filters%5Bexit_reason%5D=crash' + '&filters%5Bsignature_exit_reason%5D=crash' ); }); @@ -209,8 +209,7 @@ describe('TriageTable', () => { '&filters%5Bdate_after%5D=2026-08-01T10%3A00%3A00Z' + '&filters%5Brejected%5D=true' ); - expect(link.getAttribute('href')).not.toContain('error_type'); - expect(link.getAttribute('href')).not.toContain('exit_reason'); + expect(link.getAttribute('href')).not.toContain('signature_'); }); // Nothing to filter history on without a resolved exit_reason. diff --git a/lib/lightning/invocation.ex b/lib/lightning/invocation.ex index ec31106e14..dc9a011cdc 100644 --- a/lib/lightning/invocation.ex +++ b/lib/lightning/invocation.ex @@ -730,20 +730,22 @@ defmodule Lightning.Invocation do end) # A triage row's "View" button, scoped to exactly the work orders it - # counted. `exit_reason` switches the filter on; a present `job_id` reads - # as the step-level row, an absent one as the run-level row (no failing - # step) — safe because `steps.job_id` is `NOT NULL`. + # counted. `signature_exit_reason` switches the filter on; a present + # `signature_job_id` reads as the step-level row, an absent one as the + # run-level row (no failing step) — safe because `steps.job_id` is + # `NOT NULL`. # # Carries `wo.state in failure_states()` itself: a *successful* work order # can still hold a `fail` step in its latest run (an `on_job_failure` # handler that ran fine), so without this a signature filter would match # work orders the triage row never counted, and bulk retry would follow. - defp filter_by_signature(query, %SearchParams{exit_reason: nil}), do: query + defp filter_by_signature(query, %SearchParams{signature_exit_reason: nil}), + do: query defp filter_by_signature(query, %SearchParams{ - exit_reason: exit_reason, - error_type: error_type, - job_id: job_id + signature_exit_reason: exit_reason, + signature_error_type: error_type, + signature_job_id: job_id }) when is_binary(job_id) do step_match = @@ -771,9 +773,9 @@ defmodule Lightning.Invocation do end defp filter_by_signature(query, %SearchParams{ - exit_reason: exit_reason, - error_type: error_type, - job_id: nil + signature_exit_reason: exit_reason, + signature_error_type: error_type, + signature_job_id: nil }) do case Map.fetch(@reason_states, exit_reason) do {:ok, state} -> diff --git a/lib/lightning/workorders/search_params.ex b/lib/lightning/workorders/search_params.ex index 112e392409..c0ffae08fd 100644 --- a/lib/lightning/workorders/search_params.ex +++ b/lib/lightning/workorders/search_params.ex @@ -18,9 +18,9 @@ defmodule Lightning.WorkOrders.SearchParams do :wo_date_before, :sort_by, :sort_direction, - :exit_reason, - :error_type, - :job_id + :signature_exit_reason, + :signature_error_type, + :signature_job_id ] @derive {Jason.Encoder, only: @fields} @@ -55,9 +55,9 @@ defmodule Lightning.WorkOrders.SearchParams do wo_date_before: DateTime.t(), sort_by: String.t(), sort_direction: String.t(), - exit_reason: String.t(), - error_type: String.t(), - job_id: Ecto.UUID.t() + signature_exit_reason: String.t(), + signature_error_type: String.t(), + signature_job_id: Ecto.UUID.t() } @primary_key false @@ -80,12 +80,12 @@ defmodule Lightning.WorkOrders.SearchParams do field(:sort_direction, :string) # The failure signature the workflow health page's triage row draws its - # "View" button from. `exit_reason` switches the filter on; a present - # `job_id` is a step-level row, an absent one a run-level row. See - # `Lightning.Invocation.filter_by_signature/2`. - field(:exit_reason, :string) - field(:error_type, :string) - field(:job_id, :binary_id) + # "View" button from. `signature_exit_reason` switches the filter on; a + # present `signature_job_id` is a step-level row, an absent one a + # run-level row. See `Lightning.Invocation.filter_by_signature/2`. + field(:signature_exit_reason, :string) + field(:signature_error_type, :string) + field(:signature_job_id, :binary_id) end # Raises on invalid input. A malformed filter is only reachable by hand-editing diff --git a/lib/lightning_web/live/run_live/index.ex b/lib/lightning_web/live/run_live/index.ex index 4f64164840..414b4304cd 100644 --- a/lib/lightning_web/live/run_live/index.ex +++ b/lib/lightning_web/live/run_live/index.ex @@ -47,9 +47,9 @@ defmodule LightningWeb.RunLive.Index do rejected: :boolean, sort_by: :string, sort_direction: :string, - exit_reason: :string, - error_type: :string, - job_id: :string + signature_exit_reason: :string, + signature_error_type: :string, + signature_job_id: :string } @empty_page %{ diff --git a/lib/lightning_web/live/run_live/index.html.heex b/lib/lightning_web/live/run_live/index.html.heex index 030508d371..82993f0305 100644 --- a/lib/lightning_web/live/run_live/index.html.heex +++ b/lib/lightning_web/live/run_live/index.html.heex @@ -257,18 +257,21 @@ <% end %> - <%!-- Failure signature chip (only when exit_reason filter is active) --%> - <% exit_reason = get_change(@filters_changeset, :exit_reason) %> + <%!-- Failure signature chip (only when the signature filter is active) --%> + <% exit_reason = + get_change(@filters_changeset, :signature_exit_reason) %> <%= if exit_reason do %> - <% error_type = get_change(@filters_changeset, :error_type) - job_id = get_change(@filters_changeset, :job_id) %> + <% error_type = + get_change(@filters_changeset, :signature_error_type) + + job_id = get_change(@filters_changeset, :signature_job_id) %> <.filter_chip id="signature-filter-chip" active={true} clear_fields={[ - {:exit_reason, nil}, - {:error_type, nil}, - {:job_id, nil} + {:signature_exit_reason, nil}, + {:signature_error_type, nil}, + {:signature_job_id, nil} ]} > {exit_reason}{if error_type, diff --git a/test/lightning/invocation_test.exs b/test/lightning/invocation_test.exs index ceb268a930..c8507c1566 100644 --- a/test/lightning/invocation_test.exs +++ b/test/lightning/invocation_test.exs @@ -1037,9 +1037,9 @@ defmodule Lightning.InvocationTest do defp signature_params(exit_reason, error_type, job_id) do SearchParams.new(%{ "status" => SearchParams.status_list(), - "exit_reason" => exit_reason, - "error_type" => error_type, - "job_id" => job_id + "signature_exit_reason" => exit_reason, + "signature_error_type" => error_type, + "signature_job_id" => job_id }) end @@ -1152,7 +1152,12 @@ defmodule Lightning.InvocationTest do wo = workorder(workflow, trigger, :failed) params = SearchParams.new(%{"status" => SearchParams.status_list()}) - assert %{exit_reason: nil, error_type: nil, job_id: nil} = params + + assert %{ + signature_exit_reason: nil, + signature_error_type: nil, + signature_job_id: nil + } = params found = project diff --git a/test/lightning_web/live/run_live/index_test.exs b/test/lightning_web/live/run_live/index_test.exs index 711b79ba3e..0137402816 100644 --- a/test/lightning_web/live/run_live/index_test.exs +++ b/test/lightning_web/live/run_live/index_test.exs @@ -972,7 +972,7 @@ defmodule LightningWeb.RunLive.IndexTest do assert render(chip) =~ "Work order:" end - test "signature filter chip appears when exit_reason filter is set", %{ + test "signature filter chip appears when the signature filter is set", %{ conn: conn, project: project, jobs: [job | _] @@ -982,9 +982,9 @@ defmodule LightningWeb.RunLive.IndexTest do conn, Routes.project_run_index_path(conn, :index, project.id, filters: %{ - exit_reason: "fail", - error_type: "AdaptorError", - job_id: job.id + signature_exit_reason: "fail", + signature_error_type: "AdaptorError", + signature_job_id: job.id } ) ) @@ -1004,7 +1004,7 @@ defmodule LightningWeb.RunLive.IndexTest do live_async( conn, Routes.project_run_index_path(conn, :index, project.id, - filters: %{exit_reason: "lost"} + filters: %{signature_exit_reason: "lost"} ) ) @@ -1014,7 +1014,7 @@ defmodule LightningWeb.RunLive.IndexTest do refute html =~ "@" end - test "signature filter chip is absent when exit_reason filter is not set", + test "signature filter chip is absent when the signature filter is not set", %{conn: conn, project: project} do {:ok, view, _html} = live_async( From 15537fe1b8fbb83b1fbf2b2e01d3aabfd77a4d0b Mon Sep 17 00:00:00 2001 From: Lucy Macartney Date: Wed, 9 Sep 2026 09:47:47 -0500 Subject: [PATCH 17/20] Name the job in the signature chip, not its id The chip read "fail:AdaptorError @ 1a2b3c4d", which names nothing anyone recognises. It resolves the id to the job's name instead, scoped to the project the page is on so an id from elsewhere resolves to nothing and the short id still shows. The lookup sits in handle_params, so it runs once a page rather than once a render. --- lib/lightning/jobs.ex | 22 +++++++++++++++ lib/lightning_web/live/run_live/index.ex | 3 ++ .../live/run_live/index.html.heex | 2 +- .../live/run_live/index_test.exs | 28 +++++++++++++++++-- 4 files changed, 52 insertions(+), 3 deletions(-) diff --git a/lib/lightning/jobs.ex b/lib/lightning/jobs.ex index b256157713..1f2baf00ba 100644 --- a/lib/lightning/jobs.ex +++ b/lib/lightning/jobs.ex @@ -163,6 +163,28 @@ defmodule Lightning.Jobs do |> Repo.preload(:credential) end + @doc """ + Gets the name of a job in a project. + + Returns `nil` when the id names no job in that project — including when it + is not a UUID at all, since it reaches here from a query string. + """ + def get_job_name(project_id, job_id) do + case Ecto.UUID.cast(job_id) do + {:ok, id} -> + Repo.one( + from(j in Job, + join: w in assoc(j, :workflow), + where: j.id == ^id and w.project_id == ^project_id, + select: j.name + ) + ) + + :error -> + nil + end + end + @doc """ Creates a job. diff --git a/lib/lightning_web/live/run_live/index.ex b/lib/lightning_web/live/run_live/index.ex index 414b4304cd..f53a1b7779 100644 --- a/lib/lightning_web/live/run_live/index.ex +++ b/lib/lightning_web/live/run_live/index.ex @@ -9,6 +9,7 @@ defmodule LightningWeb.RunLive.Index do alias Lightning.Invocation alias Lightning.Invocation.Step + alias Lightning.Jobs alias Lightning.Policies.Permissions alias Lightning.Policies.ProjectUsers alias Lightning.Run @@ -185,6 +186,8 @@ defmodule LightningWeb.RunLive.Index do page_title: "History", step: %Step{}, filters_changeset: filters_changeset(filters), + signature_job_name: + Jobs.get_job_name(project.id, filters["signature_job_id"]), pagination_path: &pagination_path(socket, project, &1, filters), page: @empty_page, async_page: AsyncResult.loading() diff --git a/lib/lightning_web/live/run_live/index.html.heex b/lib/lightning_web/live/run_live/index.html.heex index 82993f0305..49aedf2056 100644 --- a/lib/lightning_web/live/run_live/index.html.heex +++ b/lib/lightning_web/live/run_live/index.html.heex @@ -276,7 +276,7 @@ > {exit_reason}{if error_type, do: ":#{error_type}"}{if job_id, - do: " @ #{display_short_uuid(job_id)}"} + do: " @ #{@signature_job_name || display_short_uuid(job_id)}"} <% end %> diff --git a/test/lightning_web/live/run_live/index_test.exs b/test/lightning_web/live/run_live/index_test.exs index 0137402816..a71404082d 100644 --- a/test/lightning_web/live/run_live/index_test.exs +++ b/test/lightning_web/live/run_live/index_test.exs @@ -992,8 +992,32 @@ defmodule LightningWeb.RunLive.IndexTest do assert has_element?(view, "#signature-filter-chip") chip = element(view, "#signature-filter-chip") - assert render(chip) =~ - "fail:AdaptorError @ #{LightningWeb.LiveHelpers.display_short_uuid(job.id)}" + assert render(chip) =~ "fail:AdaptorError @ #{job.name}" + end + + # The name is read back from the id, so the read is scoped to the project + # the page is on — a hand-edited id from elsewhere names nothing here. + test "signature filter chip falls back to the id for a job outside the project", + %{conn: conn, project: project} do + other_job = insert(:job, workflow: build(:workflow)) + + {:ok, view, _html} = + live_async( + conn, + Routes.project_run_index_path(conn, :index, project.id, + filters: %{ + signature_exit_reason: "fail", + signature_job_id: other_job.id + } + ) + ) + + chip = render(element(view, "#signature-filter-chip")) + + refute chip =~ other_job.name + + assert chip =~ + LightningWeb.LiveHelpers.display_short_uuid(other_job.id) end test "signature filter chip omits error type and job id when absent", %{ From 72e6240ada000e03f452acdfc6a64552763933e1 Mon Sep 17 00:00:00 2001 From: Lucy Macartney Date: Wed, 9 Sep 2026 09:48:13 -0500 Subject: [PATCH 18/20] Open the triage View link in a new tab The health page is a list people work down row by row, and following a row's View button used to lose the page. rel="noopener" comes with it: without it the opened tab holds a live handle on the page that opened it. --- assets/js/health/charts/TriageTable.tsx | 2 ++ assets/test/health/charts/TriageTable.test.tsx | 3 +++ 2 files changed, 5 insertions(+) diff --git a/assets/js/health/charts/TriageTable.tsx b/assets/js/health/charts/TriageTable.tsx index 59ae331aa1..ad4a21ce44 100644 --- a/assets/js/health/charts/TriageTable.tsx +++ b/assets/js/health/charts/TriageTable.tsx @@ -152,6 +152,8 @@ export const TriageTable = ({ const ViewButton = ({ href }: { href: string }) => ( View diff --git a/assets/test/health/charts/TriageTable.test.tsx b/assets/test/health/charts/TriageTable.test.tsx index 758d638dfa..ad92ec1e6a 100644 --- a/assets/test/health/charts/TriageTable.test.tsx +++ b/assets/test/health/charts/TriageTable.test.tsx @@ -161,6 +161,9 @@ describe('TriageTable', () => { '&filters%5Bsignature_error_type%5D=RuntimeError' + '&filters%5Bsignature_job_id%5D=a1b2c3d4-0000-0000-0000-000000000000' ); + // The health page is a dashboard people read row by row — the row they + // came from has to still be there when they come back. + expect(link).toHaveAttribute('target', '_blank'); }); // A run that crashed before reaching a step has no job to key on, so the From bbc1de6eec0a1bb4cd246561b67ea68bd4977bc3 Mon Sep 17 00:00:00 2001 From: Lucy Macartney Date: Wed, 9 Sep 2026 09:48:51 -0500 Subject: [PATCH 19/20] Say why the View link ticks no status Ticking the status the reason names would drop rows the triage row counted: a run's state and its steps' exit reasons are separate reports from the worker, so a fail: row also counts work orders that ended crashed or killed. The signature filter already restricts to failures on its own. --- assets/js/health/charts/TriageTable.tsx | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/assets/js/health/charts/TriageTable.tsx b/assets/js/health/charts/TriageTable.tsx index ad4a21ce44..41d91806cb 100644 --- a/assets/js/health/charts/TriageTable.tsx +++ b/assets/js/health/charts/TriageTable.tsx @@ -165,6 +165,12 @@ const ViewButton = ({ href }: { href: string }) => ( // closed on it server-side — history's existing `rejected` status filter is // what actually matches these. `to_signature/2` gives every rejected row the // same literal `exit_reason: "rejected"`, so that is the signal to switch. +// +// No status is ticked for the other rows: the signature filter carries +// `wo.state in failure_states()` itself, so the group is already exactly the +// row's, and a status the reason names would only subtract from it — a `fail:` +// row counts every work order whose latest run holds a step that failed, +// whatever state the run itself ended in. const historyUrl = ( projectId: string, workflowId: string, From 237b1888f5aa5811eb1de483c4fd09aa14cf3d03 Mon Sep 17 00:00:00 2001 From: Lucy Macartney Date: Wed, 9 Sep 2026 10:10:09 -0500 Subject: [PATCH 20/20] Name the concept an error signature The code answered to three names for one thing: filter_by_signature/2 in Invocation, the signature_* fields on SearchParams, and Stats.failure_signatures/2 with the FailureSignature type. Error signature is what the team says out loud, and it says which part of a failure the signature is made of, since every row in that table is a failure already. Nothing crosses the wire: the /health/failures path and the failures and signatures response keys are unchanged. --- assets/js/health/WorkflowHealth.tsx | 4 +- assets/js/health/charts/TriageTable.tsx | 21 +++++----- assets/js/health/types.ts | 6 +-- assets/test/health/WorkflowHealth.test.tsx | 10 ++--- .../test/health/charts/TriageTable.test.tsx | 18 ++++----- lib/lightning/invocation.ex | 38 ++++++++++--------- lib/lightning/workflows/stats.ex | 2 +- lib/lightning/workorders/search_params.ex | 27 ++++++------- .../api/workflow_health_controller.ex | 4 +- lib/lightning_web/live/run_live/index.ex | 10 ++--- .../live/run_live/index.html.heex | 19 +++++----- lib/lightning_web/router.ex | 2 +- test/lightning/invocation_test.exs | 16 ++++---- test/lightning/workflows/stats_test.exs | 38 +++++++++---------- .../live/run_live/index_test.exs | 37 +++++++++--------- 15 files changed, 128 insertions(+), 124 deletions(-) diff --git a/assets/js/health/WorkflowHealth.tsx b/assets/js/health/WorkflowHealth.tsx index a6ebe2160b..82760c86b9 100644 --- a/assets/js/health/WorkflowHealth.tsx +++ b/assets/js/health/WorkflowHealth.tsx @@ -6,7 +6,7 @@ import { FailureBreakdownDonut } from './charts/FailureBreakdownDonut'; import { OutcomesDonut } from './charts/OutcomesDonut'; import { TriageTable } from './charts/TriageTable'; import { DEFAULT_DAYS, RangePicker } from './RangePicker'; -import type { FailureSignatures, Outcomes } from './types'; +import type { ErrorSignatures, Outcomes } from './types'; import { FAILURE_STATES } from './types'; import type { Query } from './useHealthQuery'; import { healthBase, useHealthQuery } from './useHealthQuery'; @@ -58,7 +58,7 @@ export const HealthContent = ({ const base = healthBase(projectId, workflowId); const outcomes = useHealthQuery(`${base}/outcomes?days=${days}`); - const signatures = useHealthQuery( + const signatures = useHealthQuery( `${base}/failures?days=${days}` ); diff --git a/assets/js/health/charts/TriageTable.tsx b/assets/js/health/charts/TriageTable.tsx index 41d91806cb..9cc0d4e6bc 100644 --- a/assets/js/health/charts/TriageTable.tsx +++ b/assets/js/health/charts/TriageTable.tsx @@ -1,4 +1,4 @@ -import type { FailureSignature } from '../types'; +import type { ErrorSignature } from '../types'; /** * Failed work orders grouped by error signature, heaviest first. Each row @@ -6,7 +6,7 @@ import type { FailureSignature } from '../types'; * existing "retry all" can act on the group. The signature grammar is: * `exitReason:errorType [@ stepName [adaptor]]` — the adaptor renders without * its version, since a merged row can span more than one (see `job_id` on - * `FailureSignature`). + * `ErrorSignature`). */ // One sentence per error type the worker can report, written to hold @@ -60,7 +60,7 @@ const TIPS: Record = { }; interface TriageTableProps { - signatures: FailureSignature[]; + signatures: ErrorSignature[]; emptyMessage: string; projectId: string; workflowId: string; @@ -175,7 +175,7 @@ const historyUrl = ( projectId: string, workflowId: string, from: string, - signature: FailureSignature + signature: ErrorSignature ) => { const params = new URLSearchParams({ 'filters[workflow_id]': workflowId, @@ -185,12 +185,12 @@ const historyUrl = ( if (signature.exit_reason === 'rejected') { params.set('filters[rejected]', 'true'); } else { - params.set('filters[signature_exit_reason]', signature.exit_reason); + params.set('filters[error_signature_exit_reason]', signature.exit_reason); if (signature.error_type) { - params.set('filters[signature_error_type]', signature.error_type); + params.set('filters[error_signature_error_type]', signature.error_type); } if (signature.job_id) { - params.set('filters[signature_job_id]', signature.job_id); + params.set('filters[error_signature_job_id]', signature.job_id); } } @@ -199,7 +199,7 @@ const historyUrl = ( // The parts are styled apart rather than concatenated server-side: the error // type is the bit worth scanning down the column for. -const Signature = ({ signature }: { signature: FailureSignature }) => ( +const Signature = ({ signature }: { signature: ErrorSignature }) => (

{signature.exit_reason}: {errorTypeOf(signature)} @@ -228,8 +228,7 @@ const packageNameOf = (adaptor: string) => { // empty string. The signature still has to say something, and `default` is the // tip written for exactly that case — hence `||`, which catches '' as well as // null, where `??` would render a bare `fail:` and a tip with no sentence. -const errorTypeOf = ({ error_type }: FailureSignature) => - error_type || 'unknown'; +const errorTypeOf = ({ error_type }: ErrorSignature) => error_type || 'unknown'; -const tipFor = ({ error_type }: FailureSignature) => +const tipFor = ({ error_type }: ErrorSignature) => (error_type && TIPS[error_type]) || TIPS['default']; diff --git a/assets/js/health/types.ts b/assets/js/health/types.ts index 06f028c570..23f7acbe4b 100644 --- a/assets/js/health/types.ts +++ b/assets/js/health/types.ts @@ -50,7 +50,7 @@ export interface Outcomes { * matching on the resolved name would need a snapshot lookup the filter * doesn't do. */ -export interface FailureSignature { +export interface ErrorSignature { count: number; exit_reason: string; error_type: string | null; @@ -60,7 +60,7 @@ export interface FailureSignature { } /** The `failures` response, heaviest signature first. */ -export interface FailureSignatures { +export interface ErrorSignatures { window: { from: string; to: string }; - signatures: FailureSignature[]; + signatures: ErrorSignature[]; } diff --git a/assets/test/health/WorkflowHealth.test.tsx b/assets/test/health/WorkflowHealth.test.tsx index da78917fd9..5202bd4592 100644 --- a/assets/test/health/WorkflowHealth.test.tsx +++ b/assets/test/health/WorkflowHealth.test.tsx @@ -18,7 +18,7 @@ const outcomes = { }, }; -const failureSignatures = { +const errorSignatures = { window: outcomes.window, signatures: [ { @@ -32,7 +32,7 @@ const failureSignatures = { ], }; -const both = { outcomes, failures: failureSignatures }; +const both = { outcomes, failures: errorSignatures }; const ERROR = 'Could not load workflow stats. Refresh to try again.'; @@ -232,7 +232,7 @@ describe('WorkflowHealth', () => { window: { from: '2026-08-30T10:00:00Z', to: '2026-08-31T10:00:00Z' }, }; - mount({ outcomes: dayWide, failures: failureSignatures }); + mount({ outcomes: dayWide, failures: errorSignatures }); expect( await screen.findByText('Last 24 hours · 1,287 work orders') @@ -260,7 +260,7 @@ describe('WorkflowHealth', () => { expect(screen.queryByText('cancelled')).not.toBeInTheDocument(); }); - test('lists the failure signatures in the triage table', async () => { + test('lists the error signatures in the triage table', async () => { mount(both); expect( @@ -283,7 +283,7 @@ describe('WorkflowHealth', () => { }); test('degrades both donuts when the outcomes request fails', async () => { - mount({ outcomes: 500, failures: failureSignatures }); + mount({ outcomes: 500, failures: errorSignatures }); // Both donuts read the same response, so both degrade. expect(await screen.findAllByText(ERROR)).toHaveLength(2); diff --git a/assets/test/health/charts/TriageTable.test.tsx b/assets/test/health/charts/TriageTable.test.tsx index ad92ec1e6a..db5a23bddf 100644 --- a/assets/test/health/charts/TriageTable.test.tsx +++ b/assets/test/health/charts/TriageTable.test.tsx @@ -2,11 +2,11 @@ import { render, screen, within } from '@testing-library/react'; import { describe, expect, test } from 'vitest'; import { TriageTable } from '#/health/charts/TriageTable'; -import type { FailureSignature } from '#/health/types'; +import type { ErrorSignature } from '#/health/types'; const signature = ( - overrides: Partial = {} -): FailureSignature => ({ + overrides: Partial = {} +): ErrorSignature => ({ count: 62, exit_reason: 'fail', error_type: 'RuntimeError', @@ -22,7 +22,7 @@ const rowText = (name: string | RegExp) => // Every test renders the same workflow at the same window, since only the // signature varies between them. -const table = (signatures: FailureSignature[], emptyMessage = 'No failures') => +const table = (signatures: ErrorSignature[], emptyMessage = 'No failures') => render( { '/projects/proj-1/history' + '?filters%5Bworkflow_id%5D=wf-1' + '&filters%5Bdate_after%5D=2026-08-01T10%3A00%3A00Z' + - '&filters%5Bsignature_exit_reason%5D=fail' + - '&filters%5Bsignature_error_type%5D=RuntimeError' + - '&filters%5Bsignature_job_id%5D=a1b2c3d4-0000-0000-0000-000000000000' + '&filters%5Berror_signature_exit_reason%5D=fail' + + '&filters%5Berror_signature_error_type%5D=RuntimeError' + + '&filters%5Berror_signature_job_id%5D=a1b2c3d4-0000-0000-0000-000000000000' ); // The health page is a dashboard people read row by row — the row they // came from has to still be there when they come back. @@ -186,7 +186,7 @@ describe('TriageTable', () => { '/projects/proj-1/history' + '?filters%5Bworkflow_id%5D=wf-1' + '&filters%5Bdate_after%5D=2026-08-01T10%3A00%3A00Z' + - '&filters%5Bsignature_exit_reason%5D=crash' + '&filters%5Berror_signature_exit_reason%5D=crash' ); }); @@ -212,7 +212,7 @@ describe('TriageTable', () => { '&filters%5Bdate_after%5D=2026-08-01T10%3A00%3A00Z' + '&filters%5Brejected%5D=true' ); - expect(link.getAttribute('href')).not.toContain('signature_'); + expect(link.getAttribute('href')).not.toContain('error_signature_'); }); // Nothing to filter history on without a resolved exit_reason. diff --git a/lib/lightning/invocation.ex b/lib/lightning/invocation.ex index dc9a011cdc..f0fb929aeb 100644 --- a/lib/lightning/invocation.ex +++ b/lib/lightning/invocation.ex @@ -623,7 +623,7 @@ defmodule Lightning.Invocation do |> filter_by_wo_date_before(search_params.wo_date_before) |> filter_by_date_after(search_params.date_after) |> filter_by_date_before(search_params.date_before) - |> filter_by_signature(search_params) + |> filter_by_error_signature(search_params) |> filter_by_body_or_log_or_id( search_params.search_fields, search_params.search_term @@ -723,29 +723,31 @@ defmodule Lightning.Invocation do # `exit_reason` back into the state it came from. `"rejected"` is not a # value in that map — it is `to_signature/2`'s own literal for a work order # that never got a run — so it naturally misses here and falls through to - # `filter_by_signature/2`'s fail-closed branch, exactly like any other - # exit_reason no run can actually be in. + # `filter_by_error_signature/2`'s fail-closed branch, exactly like any + # other exit_reason no run can actually be in. @reason_states Map.new(Run.state_reasons(), fn {state, reason} -> {reason, state} end) # A triage row's "View" button, scoped to exactly the work orders it - # counted. `signature_exit_reason` switches the filter on; a present - # `signature_job_id` reads as the step-level row, an absent one as the - # run-level row (no failing step) — safe because `steps.job_id` is - # `NOT NULL`. + # counted. `error_signature_exit_reason` switches the filter on; a + # present `error_signature_job_id` reads as the step-level row, an + # absent one as the run-level row (no failing step) — safe because + # `steps.job_id` is `NOT NULL`. # # Carries `wo.state in failure_states()` itself: a *successful* work order # can still hold a `fail` step in its latest run (an `on_job_failure` # handler that ran fine), so without this a signature filter would match # work orders the triage row never counted, and bulk retry would follow. - defp filter_by_signature(query, %SearchParams{signature_exit_reason: nil}), - do: query - - defp filter_by_signature(query, %SearchParams{ - signature_exit_reason: exit_reason, - signature_error_type: error_type, - signature_job_id: job_id + defp filter_by_error_signature(query, %SearchParams{ + error_signature_exit_reason: nil + }), + do: query + + defp filter_by_error_signature(query, %SearchParams{ + error_signature_exit_reason: exit_reason, + error_signature_error_type: error_type, + error_signature_job_id: job_id }) when is_binary(job_id) do step_match = @@ -772,10 +774,10 @@ defmodule Lightning.Invocation do ) end - defp filter_by_signature(query, %SearchParams{ - signature_exit_reason: exit_reason, - signature_error_type: error_type, - signature_job_id: nil + defp filter_by_error_signature(query, %SearchParams{ + error_signature_exit_reason: exit_reason, + error_signature_error_type: error_type, + error_signature_job_id: nil }) do case Map.fetch(@reason_states, exit_reason) do {:ok, state} -> diff --git a/lib/lightning/workflows/stats.ex b/lib/lightning/workflows/stats.ex index eb5083b76c..024e2acf58 100644 --- a/lib/lightning/workflows/stats.ex +++ b/lib/lightning/workflows/stats.ex @@ -78,7 +78,7 @@ defmodule Lightning.Workflows.Stats do sum past the failure total the outcomes donut draws, which is the trade: a second broken branch is its own thing to fix, not fallout from the first. """ - def failure_signatures( + def error_signatures( %Workflow{id: workflow_id}, days_back \\ @default_days_back ) diff --git a/lib/lightning/workorders/search_params.ex b/lib/lightning/workorders/search_params.ex index c0ffae08fd..6b3a082247 100644 --- a/lib/lightning/workorders/search_params.ex +++ b/lib/lightning/workorders/search_params.ex @@ -18,9 +18,9 @@ defmodule Lightning.WorkOrders.SearchParams do :wo_date_before, :sort_by, :sort_direction, - :signature_exit_reason, - :signature_error_type, - :signature_job_id + :error_signature_exit_reason, + :error_signature_error_type, + :error_signature_job_id ] @derive {Jason.Encoder, only: @fields} @@ -55,9 +55,9 @@ defmodule Lightning.WorkOrders.SearchParams do wo_date_before: DateTime.t(), sort_by: String.t(), sort_direction: String.t(), - signature_exit_reason: String.t(), - signature_error_type: String.t(), - signature_job_id: Ecto.UUID.t() + error_signature_exit_reason: String.t(), + error_signature_error_type: String.t(), + error_signature_job_id: Ecto.UUID.t() } @primary_key false @@ -79,13 +79,14 @@ defmodule Lightning.WorkOrders.SearchParams do field(:sort_by, :string) field(:sort_direction, :string) - # The failure signature the workflow health page's triage row draws its - # "View" button from. `signature_exit_reason` switches the filter on; a - # present `signature_job_id` is a step-level row, an absent one a - # run-level row. See `Lightning.Invocation.filter_by_signature/2`. - field(:signature_exit_reason, :string) - field(:signature_error_type, :string) - field(:signature_job_id, :binary_id) + # The error signature the workflow health page's triage row draws its + # "View" button from. `error_signature_exit_reason` switches the + # filter on; a present `error_signature_job_id` is a step-level row, + # an absent one a run-level row. See + # `Lightning.Invocation.filter_by_error_signature/2`. + field(:error_signature_exit_reason, :string) + field(:error_signature_error_type, :string) + field(:error_signature_job_id, :binary_id) end # Raises on invalid input. A malformed filter is only reachable by hand-editing diff --git a/lib/lightning_web/controllers/api/workflow_health_controller.ex b/lib/lightning_web/controllers/api/workflow_health_controller.ex index 3176c8978f..4def997ef4 100644 --- a/lib/lightning_web/controllers/api/workflow_health_controller.ex +++ b/lib/lightning_web/controllers/api/workflow_health_controller.ex @@ -27,10 +27,10 @@ defmodule LightningWeb.API.WorkflowHealthController do ) end - def failure_signatures(conn, _params) do + def error_signatures(conn, _params) do json( conn, - Workflows.Stats.failure_signatures( + Workflows.Stats.error_signatures( conn.assigns.workflow, conn.assigns.days_back ) diff --git a/lib/lightning_web/live/run_live/index.ex b/lib/lightning_web/live/run_live/index.ex index f53a1b7779..79614fd706 100644 --- a/lib/lightning_web/live/run_live/index.ex +++ b/lib/lightning_web/live/run_live/index.ex @@ -48,9 +48,9 @@ defmodule LightningWeb.RunLive.Index do rejected: :boolean, sort_by: :string, sort_direction: :string, - signature_exit_reason: :string, - signature_error_type: :string, - signature_job_id: :string + error_signature_exit_reason: :string, + error_signature_error_type: :string, + error_signature_job_id: :string } @empty_page %{ @@ -186,8 +186,8 @@ defmodule LightningWeb.RunLive.Index do page_title: "History", step: %Step{}, filters_changeset: filters_changeset(filters), - signature_job_name: - Jobs.get_job_name(project.id, filters["signature_job_id"]), + error_signature_job_name: + Jobs.get_job_name(project.id, filters["error_signature_job_id"]), pagination_path: &pagination_path(socket, project, &1, filters), page: @empty_page, async_page: AsyncResult.loading() diff --git a/lib/lightning_web/live/run_live/index.html.heex b/lib/lightning_web/live/run_live/index.html.heex index 49aedf2056..bc0fe2f6e2 100644 --- a/lib/lightning_web/live/run_live/index.html.heex +++ b/lib/lightning_web/live/run_live/index.html.heex @@ -257,26 +257,27 @@ <% end %> - <%!-- Failure signature chip (only when the signature filter is active) --%> + <%!-- Error signature chip (only when the signature filter is active) --%> <% exit_reason = - get_change(@filters_changeset, :signature_exit_reason) %> + get_change(@filters_changeset, :error_signature_exit_reason) %> <%= if exit_reason do %> <% error_type = - get_change(@filters_changeset, :signature_error_type) + get_change(@filters_changeset, :error_signature_error_type) - job_id = get_change(@filters_changeset, :signature_job_id) %> + job_id = get_change(@filters_changeset, :error_signature_job_id) %> <.filter_chip - id="signature-filter-chip" + id="error-signature-filter-chip" active={true} clear_fields={[ - {:signature_exit_reason, nil}, - {:signature_error_type, nil}, - {:signature_job_id, nil} + {:error_signature_exit_reason, nil}, + {:error_signature_error_type, nil}, + {:error_signature_job_id, nil} ]} > {exit_reason}{if error_type, do: ":#{error_type}"}{if job_id, - do: " @ #{@signature_job_name || display_short_uuid(job_id)}"} + do: + " @ #{@error_signature_job_name || display_short_uuid(job_id)}"} <% end %> diff --git a/lib/lightning_web/router.ex b/lib/lightning_web/router.ex index 65c146970b..cddc51ddfa 100644 --- a/lib/lightning_web/router.ex +++ b/lib/lightning_web/router.ex @@ -115,7 +115,7 @@ defmodule LightningWeb.Router do get "/projects/:project_id/workflows/:workflow_id/health/failures", API.WorkflowHealthController, - :failure_signatures + :error_signatures end ## Collections diff --git a/test/lightning/invocation_test.exs b/test/lightning/invocation_test.exs index c8507c1566..af40cee29f 100644 --- a/test/lightning/invocation_test.exs +++ b/test/lightning/invocation_test.exs @@ -1030,16 +1030,16 @@ defmodule Lightning.InvocationTest do end end - # The triage row's "View" button: `filter_by_signature/2` inside + # The triage row's "View" button: `filter_by_error_signature/2` inside # `search_workorders_query/2`. Exercised through `search_workorders_for_export_query/2` # since it applies no destructive-action side filter of its own. - describe "filter_by_signature/2" do + describe "filter_by_error_signature/2" do defp signature_params(exit_reason, error_type, job_id) do SearchParams.new(%{ "status" => SearchParams.status_list(), - "signature_exit_reason" => exit_reason, - "signature_error_type" => error_type, - "signature_job_id" => job_id + "error_signature_exit_reason" => exit_reason, + "error_signature_error_type" => error_type, + "error_signature_job_id" => job_id }) end @@ -1154,9 +1154,9 @@ defmodule Lightning.InvocationTest do params = SearchParams.new(%{"status" => SearchParams.status_list()}) assert %{ - signature_exit_reason: nil, - signature_error_type: nil, - signature_job_id: nil + error_signature_exit_reason: nil, + error_signature_error_type: nil, + error_signature_job_id: nil } = params found = diff --git a/test/lightning/workflows/stats_test.exs b/test/lightning/workflows/stats_test.exs index 3451a9a432..ebc76735ce 100644 --- a/test/lightning/workflows/stats_test.exs +++ b/test/lightning/workflows/stats_test.exs @@ -125,7 +125,7 @@ defmodule Lightning.Workflows.StatsTest do insert_run(workflow, trigger, :cancelled) assert %{cancelled: 1, failed: 0} = Stats.outcomes(workflow).counts - assert %{signatures: []} = Stats.failure_signatures(workflow) + assert %{signatures: []} = Stats.error_signatures(workflow) end # The review comment this whole unit change is for: retrying a failure until @@ -147,7 +147,7 @@ defmodule Lightning.Workflows.StatsTest do end assert %{success: 1, failed: 0} = Stats.outcomes(workflow).counts - assert %{signatures: []} = Stats.failure_signatures(workflow) + assert %{signatures: []} = Stats.error_signatures(workflow) end test "ignores work orders belonging to another workflow", %{ @@ -183,7 +183,7 @@ defmodule Lightning.Workflows.StatsTest do assert Stats.outcomes(workflow) == first end - describe "failure_signatures/2" do + describe "error_signatures/2" do defp failed_run(workflow, trigger, attrs, steps \\ []) do {wo_attrs, run_attrs} = Keyword.split(attrs, [:last_activity]) state = Keyword.get(run_attrs, :state, :failed) @@ -239,7 +239,7 @@ defmodule Lightning.Workflows.StatsTest do step(job, exit_reason: "fail", error_type: "RuntimeError") ]) - assert %{signatures: [signature]} = Stats.failure_signatures(workflow) + assert %{signatures: [signature]} = Stats.error_signatures(workflow) assert signature == %{ count: 1, @@ -271,7 +271,7 @@ defmodule Lightning.Workflows.StatsTest do }) |> Repo.update!() - assert %{signatures: [signature]} = Stats.failure_signatures(workflow) + assert %{signatures: [signature]} = Stats.error_signatures(workflow) assert signature.step_name == job.name assert signature.adaptor == job.adaptor @@ -307,7 +307,7 @@ defmodule Lightning.Workflows.StatsTest do step(job, exit_reason: "fail", error_type: "RuntimeError") ]) - assert %{signatures: [signature]} = Stats.failure_signatures(workflow) + assert %{signatures: [signature]} = Stats.error_signatures(workflow) assert %{count: 2, job_id: ^job_id, step_name: "Renamed"} = signature end @@ -325,7 +325,7 @@ defmodule Lightning.Workflows.StatsTest do [step(job, exit_reason: "lost", error_type: nil)] ) - assert %{signatures: [signature]} = Stats.failure_signatures(workflow) + assert %{signatures: [signature]} = Stats.error_signatures(workflow) assert signature.exit_reason == "lost" assert signature.error_type == "LostAfterStart" assert signature.step_name == job.name @@ -337,7 +337,7 @@ defmodule Lightning.Workflows.StatsTest do } do failed_run(workflow, trigger, state: :crashed, error_type: "CompileError") - assert %{signatures: [signature]} = Stats.failure_signatures(workflow) + assert %{signatures: [signature]} = Stats.error_signatures(workflow) assert signature == %{ count: 1, @@ -358,7 +358,7 @@ defmodule Lightning.Workflows.StatsTest do } do work_order(workflow, trigger, state: :rejected) - assert %{signatures: [signature]} = Stats.failure_signatures(workflow) + assert %{signatures: [signature]} = Stats.error_signatures(workflow) assert signature == %{ count: 1, @@ -388,7 +388,7 @@ defmodule Lightning.Workflows.StatsTest do ]) assert %{signatures: [%{count: 2, error_type: nil}]} = - Stats.failure_signatures(workflow) + Stats.error_signatures(workflow) end # And on the run's own error type, which the step falls through to: it is @@ -402,7 +402,7 @@ defmodule Lightning.Workflows.StatsTest do failed_run(workflow, trigger, state: :crashed, error_type: nil) assert %{signatures: [%{count: 2, exit_reason: "crash", error_type: nil}]} = - Stats.failure_signatures(workflow) + Stats.error_signatures(workflow) end test "groups matching work orders and sorts the heaviest first", %{ @@ -420,7 +420,7 @@ defmodule Lightning.Workflows.StatsTest do failed_run(workflow, trigger, state: :crashed, error_type: "CompileError") assert %{signatures: [first, second]} = - Stats.failure_signatures(workflow) + Stats.error_signatures(workflow) assert %{count: 3, error_type: "RuntimeError"} = first assert %{count: 1, error_type: "CompileError"} = second @@ -453,7 +453,7 @@ defmodule Lightning.Workflows.StatsTest do run.(job_a, "FirstAttempt", DateTime.add(now, -60)) run.(job_b, "Retry", now) - assert %{signatures: [signature]} = Stats.failure_signatures(workflow) + assert %{signatures: [signature]} = Stats.error_signatures(workflow) assert %{count: 1, error_type: "Retry", step_name: name} = signature assert name == job_b.name end @@ -481,7 +481,7 @@ defmodule Lightning.Workflows.StatsTest do ) ]) - assert %{signatures: signatures} = Stats.failure_signatures(workflow) + assert %{signatures: signatures} = Stats.error_signatures(workflow) assert [ %{count: 1, error_type: "Earlier", step_name: job_a.name}, @@ -506,7 +506,7 @@ defmodule Lightning.Workflows.StatsTest do ]) assert %{signatures: [%{count: 1, error_type: "RuntimeError"}]} = - Stats.failure_signatures(workflow) + Stats.error_signatures(workflow) end test "ignores steps that succeeded and work orders that succeeded", %{ @@ -529,7 +529,7 @@ defmodule Lightning.Workflows.StatsTest do insert_run(workflow, trigger, :success) assert %{signatures: [%{count: 1, error_type: "RuntimeError"}]} = - Stats.failure_signatures(workflow) + Stats.error_signatures(workflow) end test "still finds a failing step that never stamped a start time", %{ @@ -544,7 +544,7 @@ defmodule Lightning.Workflows.StatsTest do ]) assert %{signatures: [%{error_type: "RuntimeError", step_name: name}]} = - Stats.failure_signatures(workflow) + Stats.error_signatures(workflow) assert name == job.name end @@ -561,7 +561,7 @@ defmodule Lightning.Workflows.StatsTest do last_activity: days_ago(31) ) - assert %{signatures: []} = Stats.failure_signatures(workflow) + assert %{signatures: []} = Stats.error_signatures(workflow) end defp two_jobs(workflow) do @@ -584,7 +584,7 @@ defmodule Lightning.Workflows.StatsTest do # deleting a hardcoded list of keys. Stats.outcomes(workflow) Stats.outcomes(workflow, 7) - Stats.failure_signatures(workflow) + Stats.error_signatures(workflow) other = insert(:simple_workflow) Stats.outcomes(other) diff --git a/test/lightning_web/live/run_live/index_test.exs b/test/lightning_web/live/run_live/index_test.exs index a71404082d..6305e5c43f 100644 --- a/test/lightning_web/live/run_live/index_test.exs +++ b/test/lightning_web/live/run_live/index_test.exs @@ -972,7 +972,7 @@ defmodule LightningWeb.RunLive.IndexTest do assert render(chip) =~ "Work order:" end - test "signature filter chip appears when the signature filter is set", %{ + test "error signature filter chip appears when the filter is set", %{ conn: conn, project: project, jobs: [job | _] @@ -982,22 +982,22 @@ defmodule LightningWeb.RunLive.IndexTest do conn, Routes.project_run_index_path(conn, :index, project.id, filters: %{ - signature_exit_reason: "fail", - signature_error_type: "AdaptorError", - signature_job_id: job.id + error_signature_exit_reason: "fail", + error_signature_error_type: "AdaptorError", + error_signature_job_id: job.id } ) ) - assert has_element?(view, "#signature-filter-chip") - chip = element(view, "#signature-filter-chip") + assert has_element?(view, "#error-signature-filter-chip") + chip = element(view, "#error-signature-filter-chip") assert render(chip) =~ "fail:AdaptorError @ #{job.name}" end # The name is read back from the id, so the read is scoped to the project # the page is on — a hand-edited id from elsewhere names nothing here. - test "signature filter chip falls back to the id for a job outside the project", + test "error signature filter chip falls back to the id for a job outside the project", %{conn: conn, project: project} do other_job = insert(:job, workflow: build(:workflow)) @@ -1006,13 +1006,13 @@ defmodule LightningWeb.RunLive.IndexTest do conn, Routes.project_run_index_path(conn, :index, project.id, filters: %{ - signature_exit_reason: "fail", - signature_job_id: other_job.id + error_signature_exit_reason: "fail", + error_signature_job_id: other_job.id } ) ) - chip = render(element(view, "#signature-filter-chip")) + chip = render(element(view, "#error-signature-filter-chip")) refute chip =~ other_job.name @@ -1020,25 +1020,26 @@ defmodule LightningWeb.RunLive.IndexTest do LightningWeb.LiveHelpers.display_short_uuid(other_job.id) end - test "signature filter chip omits error type and job id when absent", %{ - conn: conn, - project: project - } do + test "error signature filter chip omits error type and job id when absent", + %{ + conn: conn, + project: project + } do {:ok, view, _html} = live_async( conn, Routes.project_run_index_path(conn, :index, project.id, - filters: %{signature_exit_reason: "lost"} + filters: %{error_signature_exit_reason: "lost"} ) ) - chip = element(view, "#signature-filter-chip") + chip = element(view, "#error-signature-filter-chip") html = render(chip) assert html =~ "lost" refute html =~ "@" end - test "signature filter chip is absent when the signature filter is not set", + test "error signature filter chip is absent when the filter is not set", %{conn: conn, project: project} do {:ok, view, _html} = live_async( @@ -1046,7 +1047,7 @@ defmodule LightningWeb.RunLive.IndexTest do Routes.project_run_index_path(conn, :index, project.id) ) - refute has_element?(view, "#signature-filter-chip") + refute has_element?(view, "#error-signature-filter-chip") end end

Work orders @@ -93,7 +97,7 @@ export const TriageTable = ({ Signature + Actions
{signature.count.toLocaleString()} +

Tip: @@ -162,9 +166,10 @@ const ViewButton = ({ return ( View + ); }; From 945b3fafded2989fb1d13247b9b3c0aaf0766feb Mon Sep 17 00:00:00 2001 From: Lucy Macartney Date: Tue, 8 Sep 2026 15:54:55 -0500 Subject: [PATCH 07/20] Drop a changelog note for a bug nobody saw ship The workflow health page is still unreleased, so a Changed entry describing a fix to it is noise, not history: nobody saw the split- row bug this PR corrects. The Added entry for the page already describes the fixed behaviour on its own. --- CHANGELOG.md | 6 ------ 1 file changed, 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 184e77f173..0276314cff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,12 +21,6 @@ and this project adheres to first character of a string, which breaks names in many languages. Lightning does not normalise anything today, but #4577 adds it on every name, so the runtime moves first. -- The workflow health page's triage table now merges two rows that describe the - same failure into one, correcting a split count on a shipped page: a step - reporting an empty error type instead of none, and a job renamed or - adaptor-bumped mid-window, both used to draw two identical-looking rows with - the count divided between them. Merged rows are labelled from the most recent - snapshot to fail. ### Added From 09770f4fbad8110863f2399f62e224cf0f0b0452 Mon Sep 17 00:00:00 2001 From: Lucy Macartney Date: Tue, 8 Sep 2026 16:06:02 -0500 Subject: [PATCH 08/20] Shorten the signature filter tests Three fixture helpers replace the hand-built work order and run in every test. Drops the duplicate query in the no-op test: an absent key and a nil value cast to the same field, so both runs asked the same question. --- test/lightning/invocation_test.exs | 221 +++++++---------------------- 1 file changed, 48 insertions(+), 173 deletions(-) diff --git a/test/lightning/invocation_test.exs b/test/lightning/invocation_test.exs index e839c2b590..42bf850eec 100644 --- a/test/lightning/invocation_test.exs +++ b/test/lightning/invocation_test.exs @@ -1053,6 +1053,35 @@ defmodule Lightning.InvocationTest do |> MapSet.new() end + defp workorder(workflow, trigger, state) do + insert(:workorder, + workflow: workflow, + trigger: trigger, + dataclip: insert(:dataclip), + state: state + ) + end + + # The run carries the work order's own state: every case here is a work + # order that ran once and stopped there. + defp ran_wo(workflow, trigger, state, steps \\ []) do + wo = workorder(workflow, trigger, state) + + insert(:run, + work_order: wo, + starting_trigger: trigger, + dataclip: insert(:dataclip), + state: state, + steps: steps + ) + + wo + end + + defp failing_step(job, error_type) do + build(:step, job: job, exit_reason: "fail", error_type: error_type) + end + test "matches exactly the work orders behind a step-level signature, and none of a rejected or lost row" do project = insert(:project) @@ -1062,86 +1091,19 @@ defmodule Lightning.InvocationTest do other_job = insert(:job, workflow: workflow) crashed_wo = - insert(:workorder, - workflow: workflow, - trigger: trigger, - dataclip: insert(:dataclip), - state: :crashed - ) - - insert(:run, - work_order: crashed_wo, - starting_trigger: trigger, - dataclip: insert(:dataclip), - state: :crashed, - steps: [ - build(:step, job: job, exit_reason: "fail", error_type: "RuntimeError") - ] - ) + ran_wo(workflow, trigger, :crashed, [failing_step(job, "RuntimeError")]) failed_wo = - insert(:workorder, - workflow: workflow, - trigger: trigger, - dataclip: insert(:dataclip), - state: :failed - ) - - insert(:run, - work_order: failed_wo, - starting_trigger: trigger, - dataclip: insert(:dataclip), - state: :failed, - steps: [ - build(:step, job: job, exit_reason: "fail", error_type: "RuntimeError") - ] - ) + ran_wo(workflow, trigger, :failed, [failing_step(job, "RuntimeError")]) # Same reason and error type, different job — must not match. - other_job_wo = - insert(:workorder, - workflow: workflow, - trigger: trigger, - dataclip: insert(:dataclip), - state: :failed - ) + ran_wo(workflow, trigger, :failed, [ + failing_step(other_job, "RuntimeError") + ]) - insert(:run, - work_order: other_job_wo, - starting_trigger: trigger, - dataclip: insert(:dataclip), - state: :failed, - steps: [ - build(:step, - job: other_job, - exit_reason: "fail", - error_type: "RuntimeError" - ) - ] - ) + workorder(workflow, trigger, :rejected) - _rejected_wo = - insert(:workorder, - workflow: workflow, - trigger: trigger, - dataclip: insert(:dataclip), - state: :rejected - ) - - lost_wo = - insert(:workorder, - workflow: workflow, - trigger: trigger, - dataclip: insert(:dataclip), - state: :lost - ) - - insert(:run, - work_order: lost_wo, - starting_trigger: trigger, - dataclip: insert(:dataclip), - state: :lost - ) + lost_wo = ran_wo(workflow, trigger, :lost) assert signature_matches(project, "fail", "RuntimeError", job.id) == MapSet.new([crashed_wo.id, failed_wo.id]) @@ -1157,53 +1119,27 @@ defmodule Lightning.InvocationTest do assert signature_matches(project, "lost", nil) == MapSet.new([lost_wo.id]) end + # Guards every unfiltered history search, not just the View button: the + # signature filter sits in the query behind search, bulk retry, bulk cancel + # and export, and its run-level branch fails closed. A nil signature that + # stopped being a no-op would empty the history page for everyone. test "with all three fields nil is a no-op" do project = insert(:project) %{workflow: workflow, trigger: trigger} = build_workflow(project: project) - wo = - insert(:workorder, - workflow: workflow, - trigger: trigger, - dataclip: insert(:dataclip), - state: :failed - ) - - insert(:run, - work_order: wo, - starting_trigger: trigger, - dataclip: insert(:dataclip), - state: :failed - ) + wo = workorder(workflow, trigger, :failed) params = SearchParams.new(%{"status" => SearchParams.status_list()}) - assert params.exit_reason == nil - assert params.error_type == nil - assert params.job_id == nil + assert %{exit_reason: nil, error_type: nil, job_id: nil} = params - without_signature = + found = project |> Invocation.search_workorders_for_export_query(params) |> Repo.all() |> Enum.map(& &1.id) - with_explicit_nils = - SearchParams.new(%{ - "status" => SearchParams.status_list(), - "exit_reason" => nil, - "error_type" => nil, - "job_id" => nil - }) - - with_signature = - project - |> Invocation.search_workorders_for_export_query(with_explicit_nils) - |> Repo.all() - |> Enum.map(& &1.id) - - assert without_signature == [wo.id] - assert with_signature == without_signature + assert found == [wo.id] end test "search_workorders_for_retry/2 scopes a bulk retry to the signature" do @@ -1213,40 +1149,9 @@ defmodule Lightning.InvocationTest do build_workflow(project: project) matching_wo = - insert(:workorder, - workflow: workflow, - trigger: trigger, - dataclip: insert(:dataclip), - state: :failed - ) + ran_wo(workflow, trigger, :failed, [failing_step(job, "RuntimeError")]) - insert(:run, - work_order: matching_wo, - starting_trigger: trigger, - dataclip: insert(:dataclip), - state: :failed, - steps: [ - build(:step, job: job, exit_reason: "fail", error_type: "RuntimeError") - ] - ) - - other_wo = - insert(:workorder, - workflow: workflow, - trigger: trigger, - dataclip: insert(:dataclip), - state: :failed - ) - - insert(:run, - work_order: other_wo, - starting_trigger: trigger, - dataclip: insert(:dataclip), - state: :failed, - steps: [ - build(:step, job: job, exit_reason: "fail", error_type: "CompileError") - ] - ) + ran_wo(workflow, trigger, :failed, [failing_step(job, "CompileError")]) found = Invocation.search_workorders_for_retry( @@ -1263,21 +1168,7 @@ defmodule Lightning.InvocationTest do %{workflow: workflow, trigger: trigger, job: job} = build_workflow(project: project) - wo = - insert(:workorder, - workflow: workflow, - trigger: trigger, - dataclip: insert(:dataclip), - state: :failed - ) - - insert(:run, - work_order: wo, - starting_trigger: trigger, - dataclip: insert(:dataclip), - state: :failed, - steps: [build(:step, job: job, exit_reason: "fail", error_type: "")] - ) + wo = ran_wo(workflow, trigger, :failed, [failing_step(job, "")]) assert signature_matches(project, "fail", nil, job.id) == MapSet.new([wo.id]) @@ -1289,23 +1180,7 @@ defmodule Lightning.InvocationTest do %{workflow: workflow, trigger: trigger, job: job} = build_workflow(project: project) - success_wo = - insert(:workorder, - workflow: workflow, - trigger: trigger, - dataclip: insert(:dataclip), - state: :success - ) - - insert(:run, - work_order: success_wo, - starting_trigger: trigger, - dataclip: insert(:dataclip), - state: :success, - steps: [ - build(:step, job: job, exit_reason: "fail", error_type: "RuntimeError") - ] - ) + ran_wo(workflow, trigger, :success, [failing_step(job, "RuntimeError")]) assert signature_matches(project, "fail", "RuntimeError", job.id) == MapSet.new() From aa2334e99aec07ee47a5e667d94200c391bfbdc7 Mon Sep 17 00:00:00 2001 From: Lucy Macartney Date: Tue, 8 Sep 2026 16:16:40 -0500 Subject: [PATCH 09/20] Keep the signature coalescing next to its caller exit_reason/2 and error_type/2 only ever ran for the health page's triage rows, so they move back into Stats as defp. The three that the history filter also calls stay in Query, two of them with a comment instead of a doc block. --- lib/lightning/invocation/query.ex | 51 +++---------------------------- lib/lightning/workflows/stats.ex | 21 +++++++++++-- 2 files changed, 24 insertions(+), 48 deletions(-) diff --git a/lib/lightning/invocation/query.ex b/lib/lightning/invocation/query.ex index 5fd7c28e9a..7746d9f2cf 100644 --- a/lib/lightning/invocation/query.ex +++ b/lib/lightning/invocation/query.ex @@ -40,15 +40,9 @@ defmodule Lightning.Invocation.Query do ) end - @doc """ - Work order states the workflow health page's triage table treats as a - failure. - - Narrower than `WorkOrder.failure_states/0`: `:cancelled` is final but not a - failure — someone stopped it on purpose, their own outcome rather than the - red wedge — so it is excluded here rather than in the schema's own list. - Shared so the history filter narrows on the same set `Stats` does. - """ + # Narrower than `WorkOrder.failure_states/0`: `:cancelled` is final but not a + # failure — someone stopped it on purpose. Shared so the history filter + # narrows on the same set `Stats` does. @spec failure_states() :: [atom()] def failure_states, do: WorkOrder.failure_states() -- [:cancelled] @@ -65,14 +59,8 @@ defmodule Lightning.Invocation.Query do from(s in query, where: s.exit_reason != "success") end - @doc """ - Appends the tiebreak for "the run that speaks for a work order" to a query - with a `:run` binding: most recently finished first, ties broken by id. - - A one-liner, but shared so the health page's bulk `DISTINCT ON` and the - history filter's correlated per-work-order lookup can't drift apart on - what "latest" means. - """ + # Shared so the health page's bulk `DISTINCT ON` and the history filter's + # correlated per-work-order lookup can't drift apart on what "latest" means. @spec order_by_run_recency(Ecto.Queryable.t()) :: Ecto.Queryable.t() def order_by_run_recency(query) do from([run: r] in query, @@ -80,35 +68,6 @@ defmodule Lightning.Invocation.Query do ) end - @doc """ - The reason a failure is reported under, coalescing a step's own - `exit_reason` with what the run's terminal state implies for one that - never reported. - - `mark_steps_lost/1` stamps a step's `exit_reason` and nothing else, so a - crashed run with no step at all has to fall back to `Run.state_reasons/0` - — the worker's own words for each terminal state, kept in one place so the - map is not typed out twice. - """ - @spec exit_reason(String.t() | nil, atom() | nil) :: String.t() | nil - def exit_reason(step_exit_reason, run_state) do - step_exit_reason || Map.get(Run.state_reasons(), run_state) - end - - @doc """ - The error type a failure is reported under, coalescing a step's own - `error_type` with the run's — treating an empty string as missing on both, - since `"" || x` returns `""` (empty string is truthy in Elixir) and would - otherwise split one failure into two identical-looking rows. - """ - @spec error_type(String.t() | nil, String.t() | nil) :: String.t() | nil - def error_type(step_error_type, run_error_type) do - blank_to_nil(step_error_type) || blank_to_nil(run_error_type) - end - - defp blank_to_nil(""), do: nil - defp blank_to_nil(other), do: other - @doc """ Runs for a specific project, or all runs available to the requesting user """ diff --git a/lib/lightning/workflows/stats.ex b/lib/lightning/workflows/stats.ex index c79b24b23f..1c6c69c819 100644 --- a/lib/lightning/workflows/stats.ex +++ b/lib/lightning/workflows/stats.ex @@ -286,8 +286,8 @@ defmodule Lightning.Workflows.Stats do %{ count: row.count, - exit_reason: Query.exit_reason(row.exit_reason, row.run_state), - error_type: Query.error_type(row.error_type, row.run_error_type), + exit_reason: exit_reason(row.exit_reason, row.run_state), + error_type: error_type(row.error_type, row.run_error_type), job_id: row.job_id, step_name: step_name, adaptor: adaptor, @@ -295,6 +295,23 @@ defmodule Lightning.Workflows.Stats do } end + # `mark_steps_lost/1` stamps a step's `exit_reason` and nothing else, so a + # crashed run with no step at all falls back to `Run.state_reasons/0` — the + # worker's own words for each terminal state. + defp exit_reason(step_exit_reason, run_state) do + step_exit_reason || Map.get(Run.state_reasons(), run_state) + end + + # An empty string is missing on both sides: `"" || x` returns `""` (empty + # string is truthy in Elixir), which would split one failure into two + # identical-looking rows. + defp error_type(step_error_type, run_error_type) do + blank_to_nil(step_error_type) || blank_to_nil(run_error_type) + end + + defp blank_to_nil(""), do: nil + defp blank_to_nil(other), do: other + # Two groups can collapse into one signature — a crashed run and a failed run # whose steps both reported `fail`, say, or the same job renamed mid-window — # so the fold happens after the coalesce, not in the `group_by`. Grouping key From 2c1190706666906c17f75eef2d4663bf61e22ec1 Mon Sep 17 00:00:00 2001 From: Lucy Macartney Date: Tue, 8 Sep 2026 16:43:15 -0500 Subject: [PATCH 10/20] Read a blank run error type as a missing one when filtering The triage row nils an empty error_type before grouping, and so does the step-level branch of the signature filter. The run-level branch compared the raw column, so a crashed run with error_type "" was counted in the row but never matched by its own View button. --- lib/lightning/invocation.ex | 2 +- test/lightning/invocation_test.exs | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/lib/lightning/invocation.ex b/lib/lightning/invocation.ex index d479d22cb5..f7dd106c2c 100644 --- a/lib/lightning/invocation.ex +++ b/lib/lightning/invocation.ex @@ -786,7 +786,7 @@ defmodule Lightning.Invocation do where: r.state == ^state, where: fragment( - "? IS NOT DISTINCT FROM ?", + "nullif(?, '') IS NOT DISTINCT FROM ?", r.error_type, type(^error_type, :string) ), diff --git a/test/lightning/invocation_test.exs b/test/lightning/invocation_test.exs index 42bf850eec..ceb268a930 100644 --- a/test/lightning/invocation_test.exs +++ b/test/lightning/invocation_test.exs @@ -1119,6 +1119,27 @@ defmodule Lightning.InvocationTest do assert signature_matches(project, "lost", nil) == MapSet.new([lost_wo.id]) end + # Mirrors `stats_test.exs`, "treats an empty error type on the run the + # same as a missing one": the row reports `nil`, so the filter has to + # read `""` as `nil` too or the View button lands on an empty page. + test "reads an empty error type on the run as a missing one" do + project = insert(:project) + + %{workflow: workflow, trigger: trigger} = build_workflow(project: project) + + wo = workorder(workflow, trigger, :crashed) + + insert(:run, + work_order: wo, + starting_trigger: trigger, + dataclip: insert(:dataclip), + state: :crashed, + error_type: "" + ) + + assert signature_matches(project, "crash", nil) == MapSet.new([wo.id]) + end + # Guards every unfiltered history search, not just the View button: the # signature filter sits in the query behind search, bulk retry, bulk cancel # and export, and its run-level branch fails closed. A nil signature that From 8652a21bc86576aa737983dff610650b5f6a9bfc Mon Sep 17 00:00:00 2001 From: Lucy Macartney Date: Tue, 8 Sep 2026 16:43:17 -0500 Subject: [PATCH 11/20] Carry the search field flags in the triage View link History derives search_fields from the query string and put_new's the result, so a link carrying none of the four keys lands with an empty list rather than the schema default, and the search box then matches nothing. Every server-built link fills these in via to_uri_params/1. --- assets/js/health/charts/TriageTable.tsx | 8 ++++++++ assets/test/health/charts/TriageTable.test.tsx | 12 ++++++++++++ 2 files changed, 20 insertions(+) diff --git a/assets/js/health/charts/TriageTable.tsx b/assets/js/health/charts/TriageTable.tsx index 6fad3c1d68..84488e097e 100644 --- a/assets/js/health/charts/TriageTable.tsx +++ b/assets/js/health/charts/TriageTable.tsx @@ -187,6 +187,14 @@ const historyUrl = ( const params = new URLSearchParams({ 'filters[workflow_id]': workflowId, 'filters[date_after]': from, + // SearchParams.from_uri/1 reads the search-field flags out of the query + // string and put_new's the result, so an absent set means `search_fields: + // []` rather than the schema default, and every later search term matches + // nothing. to_uri_params/1 fills these in for every server-built link. + 'filters[id]': 'true', + 'filters[body]': 'true', + 'filters[log]': 'true', + 'filters[dataclip_name]': 'true', }); if (signature.exit_reason === 'rejected') { diff --git a/assets/test/health/charts/TriageTable.test.tsx b/assets/test/health/charts/TriageTable.test.tsx index 11aadd45ee..21e468e64f 100644 --- a/assets/test/health/charts/TriageTable.test.tsx +++ b/assets/test/health/charts/TriageTable.test.tsx @@ -157,6 +157,10 @@ describe('TriageTable', () => { '/projects/proj-1/history' + '?filters%5Bworkflow_id%5D=wf-1' + '&filters%5Bdate_after%5D=2026-08-01T10%3A00%3A00Z' + + '&filters%5Bid%5D=true' + + '&filters%5Bbody%5D=true' + + '&filters%5Blog%5D=true' + + '&filters%5Bdataclip_name%5D=true' + '&filters%5Bexit_reason%5D=fail' + '&filters%5Berror_type%5D=RuntimeError' + '&filters%5Bjob_id%5D=a1b2c3d4-0000-0000-0000-000000000000' @@ -183,6 +187,10 @@ describe('TriageTable', () => { '/projects/proj-1/history' + '?filters%5Bworkflow_id%5D=wf-1' + '&filters%5Bdate_after%5D=2026-08-01T10%3A00%3A00Z' + + '&filters%5Bid%5D=true' + + '&filters%5Bbody%5D=true' + + '&filters%5Blog%5D=true' + + '&filters%5Bdataclip_name%5D=true' + '&filters%5Bexit_reason%5D=crash' ); }); @@ -207,6 +215,10 @@ describe('TriageTable', () => { '/projects/proj-1/history' + '?filters%5Bworkflow_id%5D=wf-1' + '&filters%5Bdate_after%5D=2026-08-01T10%3A00%3A00Z' + + '&filters%5Bid%5D=true' + + '&filters%5Bbody%5D=true' + + '&filters%5Blog%5D=true' + + '&filters%5Bdataclip_name%5D=true' + '&filters%5Brejected%5D=true' ); expect(link.getAttribute('href')).not.toContain('error_type'); From 552a2e83f1e67f009f320a9db7237bdfc79616c0 Mon Sep 17 00:00:00 2001 From: Lucy Macartney Date: Tue, 8 Sep 2026 16:49:36 -0500 Subject: [PATCH 12/20] Give the View button a finished href ViewButton took four props to build one URL, all of them already at the call site. It takes the href now, and the row keeps the guard: no exit reason, no link. Also drops a comment's pointer to a plan file the reader of this file cannot open. --- assets/js/health/charts/TriageTable.tsx | 61 ++++++++++--------------- 1 file changed, 23 insertions(+), 38 deletions(-) diff --git a/assets/js/health/charts/TriageTable.tsx b/assets/js/health/charts/TriageTable.tsx index 84488e097e..fc96e8265a 100644 --- a/assets/js/health/charts/TriageTable.tsx +++ b/assets/js/health/charts/TriageTable.tsx @@ -126,13 +126,15 @@ export const TriageTable = ({ {tipFor(signature)}

- + {signature.exit_reason && ( + + )}