P1 — failure branch trusts total_files and discards the actual ERROR-row count, so a stale/low total_files silently under-reports failures (the inverse of the bug being fixed).

The new branch returns total_files - successful and never looks at real ERROR rows. If total_files lags the true row count (counter drift — exactly what UN-3652/UN-3657 are about — or a partial-write race during abort), genuine failures vanish and reappear as phantom in-progress via the UI's total - successful - failed.

Example: total_files=2 but 5 rows exist (3 COMPLETED, 2 ERROR) → max(0, 2-3)=0 failed; the old code returned 2. Two real failures disappear, silently.

Secondary: ExecutionStatus.is_failure swallows ValueError and returns False for any unrecognized status (data_models.py:576-581), so a malformed/legacy status string silently takes the else row-count path — reintroducing phantom-in-progress for exactly the corrupt data most worth flagging.

Suggested hardening — take the larger of the derived value and the real ERROR count, and don't let the divergence go unrecorded:

error_rows = obj.file_executions.filter(status=ExecutionStatus.ERROR.value).count()
derived = total - self.get_successful_files(obj)
if derived != error_rows:
    logger.warning("exec %s failed-file reconcile mismatch: total=%s successful=%s error_rows=%s", obj.id, total, ..., error_rows)
return max(0, derived, error_rows)

(There is no logger in this module today — its siblings views/execution.py and workflow_v2/serializers.py both define one.)

Broader fix worth considering: the partition invariant successful + failed + in_progress == total is currently split across this serializer (two terms) and the frontend (DetailedLogs.jsx derives the third). Emitting an explicit in_progress_files = SerializerMethodField (= max(0, total - successful - failed)) and having the UI render it would let the server own all three buckets, making this whole class of phantom-count bug impossible regardless of how failed is computed.

P2 — max(0, …) silently clamps the impossible successful > total state with no signal. successful > total means more files completed than the run claims to have had — an impossible count that indicates real counter corruption (double-counted completions, total_files written too low at dispatch, concurrent updates). The clamp returns 0 and erases the most diagnostic inconsistency there is. test_successful_over_total_is_clamped_not_negative even enshrines this silent 0 as correct.

Keep the clamp as the safe return value, but logger.warning(...) the anomaly first (execution id + both counts) so the drift is debuggable months later instead of invisible. This is consistent with the project's error-surfacing conventions.

P3 — docstring precision / verbosity.

1. "a no-op whenever the rows already account for every file" is imprecise: the new branch (total - COMPLETED) equals the old (ERROR count) only when completed + errored == total. A file with a row that is still PENDING/EXECUTING does account for the file in the plain reading, yet the new code now counts it as failed — so behavior does change there. Suggest: "a no-op whenever every file already has a terminal COMPLETED/ERROR row."
2. "...untouched on every transport" — this method reads only obj.status/total_files/row counts and has no transport (Celery vs PG) dependency; mentioning transport implies a conditional that doesn't exist. Drop it.
3. 14 lines of prose for a 3-line method, with the rationale repeated again in the test-module docstring. Consider trimming to ~6 lines, keeping the load-bearing "why" (UI derives in-progress as total - successful - failed). Ideally anchor that claim to the specific frontend component so it doesn't rot silently.

P2 — os.environ.setdefault("DJANGO_SETTINGS_MODULE", …) is a no-op when the var is already exported, which is extremely common in dev shells (backend.settings.dev, or an enterprise backend.settings.cloud). A developer with DJANGO_SETTINGS_MODULE already set gets their module instead of backend.settings.test, breaking collection (reproduced: ModuleNotFoundError: No module named 'backend.settings.cloud' on a checkout with that env). Force the assignment: os.environ["DJANGO_SETTINGS_MODULE"] = "backend.settings.test".

P2 — django.setup() contradicts the module docstring's "no test database is required" (line 10) and diverges from this repo's DB-free test convention. django.setup() imports the full backend.settings.base stack and runs every app's AppConfig.ready() — which (a) requires a complete prod-like env (from a clean checkout with only sample.test.env it aborts on missing CELERY_BROKER_BASE_URL/INDEXING_FLAG_TTL), and (b) actually opens a live DB connection during startup (execution_log_utils.ready()). So the bootstrap is neither env-free nor DB-free as claimed, even though the serializer methods under test are pure.

Sibling DB-free tests avoid this by stubbing the models in sys.modules before import (usage_v2/tests/test_helper.py, pg_queue/tests/test_producer.py). Recommend following that pattern — stub workflow_manager.workflow_v2.models (+ enums), drop django.setup(), and the docstring becomes true. (Relevant since there is no backend pytest job in CI, so the clean-environment fragility is the primary failure mode.)

P3 — test gaps that would lock the intended contract:

1. COMPLETED-not-reconciled asymmetry is untested. This case only covers completed + errored == total. The deliberate decision not to reconcile COMPLETED is invisible: add _execution(COMPLETED, total=3, completed=1, errored=1) asserting failed == 1 (the ERROR-row count), not total - completed == 2, so a future "fix" that reconciles COMPLETED fails here.
2. Assert the real invariant, not just failed. test_stopped_run/test_error_run_partial assert only get_failed_files; they don't verify the actual contract (total - successful - failed == 0, i.e. zero phantom in-progress). Add that derived assertion so the test fails if either count regresses.
3. Unknown/garbage status (is_failure's swallowed ValueError → else path) is the untested branch most worth pinning: _execution("GARBAGE", 5, completed=1, errored=1) should return 1 and must never raise (a raise would 500 the executions list endpoint).

Minor: the 4b180f05 exec id in the line-55 comment is an opaque one-off production UUID — the rest of the comment already describes the scenario; consider dropping the id.