Skip to content

Unify definition of a "failed work order" everywhere - #5153

Open
lmac-1 wants to merge 11 commits into
frank/con-106from
unify-failed-definition
Open

Unify definition of a "failed work order" everywhere#5153
lmac-1 wants to merge 11 commits into
frank/con-106from
unify-failed-definition

Conversation

@lmac-1

@lmac-1 lmac-1 commented Sep 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

This PR makes "a failed work order" mean the same thing everywhere it's counted: the workflow list, its failure link, the workflow health page, and the digest email.

Before this, each of those screens counted a different set of states as a failure and measured its "last 30 days" from a different date. The same work order could show as failed on one screen and fine on another, and a work order retried after sitting untouched for 40 days could drop out of the count on some screens but not others.

What changes:

  • A cancelled work order is never a failure, anywhere. Someone stopped it on purpose.
  • A rejected work order (its webhook was dropped because the project was over its run limit) is a failure everywhere, including the digest email, where it used to be invisible.
  • Every "last 30 days" window is measured from when a work order last did something, not when it was created, so a stale work order retried today counts as recent on every screen.
  • The dashboard counts and the history links beside them now open the same window, so a count and the page it opens agree.
  • The "Work Orders in failed state" percentage is now a share of the work orders that finished, not of every work order. A project that cancels heavily no longer reports a near-zero failure rate.
  • No new UI. Failure counts get smaller in places and move around a bit, and they finally agree with each other.

Closes CON-177

Validation steps

  1. On a workflow, create a work order that fails and a separate one you cancel.
  2. Open the workflow list. The failure count for that workflow should read 1, not 2.
  3. Click that count. The history view it opens should show only the real failure, not the cancelled one.
  4. On the project dashboard, check the "Work Orders in failed state" card. The count should exclude the cancelled work order, and the percentage below it should read as a share of the work orders that finished, not of every work order in the window.
  5. Check the "Latest failure" date next to the count. It should show the real failure's date, even if you cancelled the other work order more recently.
  6. Retry a work order older than 30 days. It should now count toward the workflow's total, since it was active today, and it should be the one named as the latest failure.
  7. Click any count on the dashboard. The history view it opens should show exactly that many work orders, not one or two more.
  8. Trigger a digest email for a project with a rejected work order (a webhook that arrived while the project was over its run limit). It should now be counted and named as a failure in the email copy.
  9. Cancel a work order and confirm it no longer shows up in a digest email's failed count.

Additional notes for the reviewer

  • Found and fixed a real bug along the way: the "Latest failure" chip on the workflow list could show a cancelled work order as the latest failure, because the code only double-checked when the newest work order was exactly :success.
  • A second bug surfaced during review: the query narrowed to the last 30 days by last_activity but picked its row by inserted_at, so a work order retried today could lose to an older one and the chip could name a failure the count had already moved past. Both now use last_activity.
  • DashboardStats carried two copies of every query, one per workflow and one batched. Only the batched set is reachable from the page, so the same sort bug existed twice and the tests asserted against the copy nothing renders. The unreachable copy is deleted rather than fixed.
  • The 30-day window now has one definition, DashboardStats.window_start/0, read by both the queries and the history links. Previously the counts used 30 days and the links asked for one calendar month.
  • The digest email lists its failure states sorted rather than in schema declaration order, so recipients read (crashed, exception, failed, killed, lost, rejected etc.) instead of leading with rejected. Sorting needs no per-state handling as states are added.
  • The project dashboard's "Work Orders in failed state" card had its own hand-typed list of failure states, separate from the one on the per-workflow row. Both now come from the same source, so they can't drift apart again.
  • Not changed: lib/lightning/pipeline/failure_alerter.ex still alerts on any non-success run, cancellations included. That's a decision about run-level alerting on a different surface, and it's out of scope here.
  • Not changed on purpose: a cancelled work order shows in neither the digest email's successful nor failed count. Pending and running ones already didn't either, so this isn't new. Considered adding a cancelled count, decided against it: on its own it still wouldn't make the numbers sum to a workflow's total work orders, and nobody's asked for that total.

AI Usage

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

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

You can read more details in our
Responsible AI Policy

Pre-submission checklist

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

@lmac-1
WorkOrder owns the failure definition
98755fa 
failure_states/0 now excludes :cancelled as well as :success, and a new
outcome/1 buckets any state into success/cancelled/pending/failed,
exhaustive over states() by construction. Behaviour-neutral: stats.ex is
still the only caller, and it subtracts cancelled itself.
@lmac-1
Health page stops subtracting cancelled
17373ff 
WorkOrder.failure_states/0 already excludes it now, so the narrowing
here was redundant. Behaviour-neutral.
@lmac-1
DashboardStats buckets work orders by outcome/1
841ac98 
count_workorders/1 and batch_count_workorders/1 now derive their bucket
from WorkOrder.outcome/1 instead of hand-rolled state checks, so
cancelled work orders get their own bucket instead of counting as
failed. aggregate_metrics/2 adds cancelled back into its total so the
"Work Orders" card total doesn't shrink, only the failed slice does.

Also fixes the "Latest failure" chip: it only re-checked the exclusion
query when the newest work order was exactly :success, so a cancelled
one displayed as "Latest failure" outright. The guard is now
outcome(state) != :failed, in both the batched path the workflow list
actually uses and the equivalent private helper for the (currently
uncalled) single-workflow path.
@lmac-1
DashboardStats window moves to last_activity
e8ba255 
filter_days_ago/2 becomes /3, taking the date column (default
:inserted_at). The four work-order queries pass :last_activity;
count_runs/1 and count_steps/1 keep the default, folding their inline
date math into the same helper along the way.

A work order retried after sitting stale no longer falls out of the
30-day count depending which screen you're looking at. Run and
work-order counts will no longer tie for a retried work order — the old
run is outside the run window, the new one inside. Correct and
unavoidable, per the plan.

Fixed an existing test pinned to the old inserted_at boundary, and
added the plan's test covering cancelled-not-failed, the window shift,
and the latest-failure chip together.
@lmac-1
Dashboard links derive their filters instead of hand-typing them
b5db860 
failed_wo_filters and its duplicate in project_metrics/1 (failed_filters)
now build their state list from WorkOrder.failure_states() instead of a
hand-typed list that had cancelled and not rejected. All four dashboard
filter links move from wo_date_after (inserted_at) to date_after
(last_activity), so a link's window matches the count it sits under, now
that commit 4 moved those counts to last_activity.

The plan only named the per-row link and left wo_filters on
wo_date_after "unless the total card moves too" - it does, so wo_filters
moves too, and project_metrics's identical hand-typed duplicate gets the
same fix.
@lmac-1
Digest email derives its failure list from WorkOrder
a498e76 
build_failed_status_params/1 and the digest copy's parenthetical both
switch from Lightning.Run.failure_states() to
Lightning.WorkOrder.failure_states(). This is a real fix, not a rename:
the params this builds filter work orders, not runs, and Run has no
:rejected state (a rejected work order never gets a run), so those
were silently missing from the digest's failed count. The digest now
also stops counting cancelled.

Run.failure_states/0 has no callers left - deleted. Leaving a second
function with the same name is how this drifted in the first place.

Updated the test helper that attaches runs to each failure state:
:rejected work orders don't get a run, matching how a real one is
created. Added a CHANGELOG entry (issue link left as a TODO for you
to fill in).
@github-project-automation github-project-automation Bot moved this to New Issues in Core Sep 8, 2026
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown

Security Review

⚠️ Review did not complete. See the workflow run.

@lmac-1
lmac-1 force-pushed the unify-failed-definition branch from 42bfc70 to a498e76 Compare September 8, 2026 22:40
@lmac-1 lmac-1 changed the title Unify definition of a "failed work order" across the site Unify definition of a "failed work order" everywhere Sep 8, 2026
@codecov

codecov Bot commented Sep 8, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.8%. Comparing base (5593a47) to head (604fdaa).

Additional details and impacted files 
@@              Coverage Diff              @@
##           frank/con-106   #5153   +/-   ##
=============================================
  Coverage           90.8%   90.8%           
=============================================
  Files                422     422           
  Lines              20875   20850   -25     
=============================================
- Hits               18954   18937   -17     
+ Misses              1921    1913    -8

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

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

@lmac-1
Dashboard stats sort by the same column they filter on
572a06e 
batch_get_last_workorders filtered the 30-day window on last_activity
but sorted on inserted_at, so the work order shown as "last failure"
was not the most recent one in the window. A work order created 45 days
ago and retried today sorted below one created 20 days ago, making the
row disagree with the failure count beside it.

Delete get_workflow_stats/1 and its five private helpers. Each was a
duplicate of a batch_* function and nothing called it, so the same sort
bug existed twice and the tests asserted the new failure behaviour
against the copy the dashboard never runs. Tests now go through
get_workflows_stats/1.

Give runs and steps a three-key empty default; only work orders have a
:cancelled bucket.
@lmac-1
lmac-1 requested a review from midigofrank September 8, 2026 23:04
@lmac-1 lmac-1 mentioned this pull request Sep 8, 2026
Open
7 tasks
@lmac-1
Dashboard counts and links share one window definition
6557c59 
The cards counted a 30-day window while the history links they sit on
asked for one calendar month, so a link could open a wider range than
the number beside it. Add DashboardStats.window_start/0 and read it from
both the queries and the links. filter_days_ago/3 drops its days
argument; every caller passed @days_back.

failed_percentage divides by success + failed instead of by every work
order. Cancelled work orders left the numerator when failure was
redefined but stayed in the denominator, so a project that cancels
heavily reported a near-zero failure rate.
@lmac-1
Digest email lists failure states alphabetically
0eda32c 
failure_states/0 returns schema declaration order, which puts :rejected
first. Recipients opened the digest led by the one state that means
nothing outside the team: a webhook dropped for exceeding the run limit.

Sort the list and restore the "etc." the hardcoded copy had. Sorting needs
no per-state handling, so a new failure state slots in without anyone
touching this. The list stays derived from failure_states/0, so cancelled
still cannot appear in it.
@lmac-1
Keep the failed percentage over all work orders
d464807 
Dividing by success + failed made the number defensible in the
cancel-heavy case but unreadable in the ordinary one: a workflow with one
success, one failure and one cancelled work order showed 50% beside a
page listing three. The card names no denominator, so any figure other
than "of all work orders" has to be guessed at.

Which denominator is right is a design question for the dashboard rework,
not for a change about what counts as a failure. The count still excludes
cancelled; only the percentage goes back.
@lmac-1
Merge frank/con-106 into unify-failed-definition
604fdaa 
Both branches dropped :cancelled from WorkOrder.failure_states/0. Kept our
version, which also carries outcome/1. Took main's stats.ex, where the
error_signatures rework already inlines the calls our @failure_states and
@state_reasons attributes stood in for.
@lmac-1
lmac-1 force-pushed the unify-failed-definition branch from 50b896e to 604fdaa Compare September 9, 2026 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@midigofrank midigofrank Awaiting requested review from midigofrank

Assignees

No one assigned

Labels

None yet

Projects

Core
Status: New Issues

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@lmac-1