Skip to content

fix(migration): stop the report claiming PASSED when the red step is outside every phase (#1141) - #1142

Merged
rafaelgiln merged 1 commit into
mainfrom
fix/issue-1141-report-job-status-reconcile
Jul 30, 2026
Merged

fix(migration): stop the report claiming PASSED when the red step is outside every phase (#1141)#1142
rafaelgiln merged 1 commit into
mainfrom
fix/issue-1141-report-job-status-reconcile

Conversation

@rafaelgiln

Copy link
Copy Markdown
Collaborator

Closes #1141. Follow-up to #1120 (PR #1139) — the other half of the same defect.

The gap

#1139 taught generate_report.py to reconcile the runner's steps.<id>.outcome against what each phase recorded, so a phase that crashed before writing its verdict can no longer render as Result: PASSED. Only three steps carry an id, so only three steps were reconciled.

The steps between them carry none:

Line Step How it fails
migration-test.yml:146 Resolve nightly version from PyPI explicit exit 1 when PyPI returns no pre-release
migration-test.yml:167 Install Langflow nightly uv resolution failure
migration-test.yml:181 Start Langflow nightly (same database) process dies immediately
migration-test.yml:190 Wait for Langflow nightly (includes migration) 180 s timeout

A failure there stops the job, so Verify migration via API / via UI never run and their outcomes arrive as the empty string. declared_outcomes() filters those out, the state file still holds a fully-passing latest phase, assess() finds nothing wrong — and the report printed Result: PASSED into an issue titled "Langflow Migration Test Failed".

The nightly failing to boot against a migrated database is exactly what this workflow exists to catch, and it landed in that gap.

The fix

Generate report now also receives JOB_STATUS: ${{ job.status }} — the same expression the summary step below it already uses. A job the runner reports as failure/cancelled, whose report found no failure and no integrity problem, is an integrity problem: the failure is real and lives outside every phase the report can see.

Two properties, both tested:

  • Raised only when nothing else attributes the failure. A phase that recorded a fail, or one declared failure by the runner, says it better; duplicating it would be noise.
  • Never reddens a healthy run. Same state + a green (or unknown) job status still yields PASSED. If the job is red, something failed — a report saying PASSED is wrong by definition, so there is no false-positive direction here.

Chosen over adding id + PHASE_OUTCOME_* to the four steps (issue option 2), which covers only today's steps and degrades silently the moment a fifth is added.

Also in this PR:

Validation

Unit tests — 21 → 27, all green (python-units lane, added in #1139):

Test Asserts
test_a_red_job_with_no_phase_failure_is_not_a_pass the #1141 shape → FAILED + an entry naming outside every phase and Read the job log
test_the_same_state_still_passes_when_the_job_is_green same state, only the job status differs (success, None) → PASSED, integrity == []
test_a_cancelled_job_is_not_a_pass cancelled is not a pass either; the message names it
test_a_red_job_is_not_reported_twice_when_a_phase_already_owns_the_failure a recorded fail, and run #115's declared failure, each yield exactly one entry — the phase's
test_job_status_is_parsed_from_the_env "Failure"failure, whitespace trimmed, unset/blank → None (unknown, never a status)
test_the_rendered_report_states_both_verdicts header + ## Result: FAILED + section, in the rendered markdown

Force-fail — 5 mutations, each dropping the intended test:

FF: remove the job-status reconcile block (pre-fix code)  → 3 failed (red-job, cancelled, rendered)
FF: drop the double-attribution guard                     → 1 failed (not-reported-twice)
FF: treat a green job as not-ok (over-reddening)          → 1 failed (still-passes-when-green)
FF: stop normalising JOB_STATUS                           → 1 failed (env parsing)
FF: drop the job-status header line                       → 1 failed (rendered)

Reverted afterwards: no mutation markers in the diff, 27 passed.

End-to-end through the real env path (the unit tests pass a dict to declared_outcomes(); CI goes through os.environ), driving the script exactly as the step does:

**Job status (runner):** `failure`
## Result: FAILED
## Unaccounted failures
- **(outside every phase)**: the runner reports this job `failure`, but no verification
  phase recorded or was declared a failure — so the cause is a step this report does not
  cover (resolving, installing or booting the nightly, or the alembic migration timing
  out). Read the job log; this report cannot attribute it.

Counter-check: three phases green + JOB_STATUS=success## Result: PASSED.

Reproduce locally:

uv run --with pytest --no-project python -m pytest \
  tests/github-workflows/migration \
  --ignore=tests/github-workflows/migration/test_ui_migration.py -q     # 27 passed

cat > /tmp/state.json <<'JSON'
{"flow_name":"witness","flow_id":"1",
 "phases":{"latest":{"steps":{"create":{"status":"pass"},"execute":{"status":"pass"}}}}}
JSON
env -i PATH="$PATH" STATE_FILE=/tmp/state.json REPORT_FILE=/tmp/report.md \
  PHASE_OUTCOME_latest=success PHASE_OUTCOME_nightly_api= PHASE_OUTCOME_nightly_ui= \
  JOB_STATUS=failure python3 tests/github-workflows/migration/generate_report.py

Other gates: tsc --noEmit 0 errors; ESLint 0 errors (warnings pre-existing); the workflow YAML parses and the report step's env carries all four variables.

Not proven here: that GitHub actually populates JOB_STATUS at that point in the job — the same residual as #1139's PHASE_OUTCOME_*, which no local harness can close. A workflow_dispatch of migration-test.yml proves both at once; what to watch in the Print report step is **Job status (runner):** being present and ## Result: agreeing with the job's own status. No spec files are touched, so nothing here creates flows.

🤖 Generated with Claude Code

…outside every phase (#1141)

#1120 taught the report to reconcile the runner's `steps.<id>.outcome` against
what each phase recorded, so a phase that crashed before writing its verdict can
no longer render as `Result: PASSED`. Only three steps carry an `id`, so only
three steps were reconciled.

The steps BETWEEN them carry none: resolving the nightly version from PyPI,
installing it, starting it, and waiting out the alembic migration. A failure
there stops the job, so phases 2 and 3 never run and their outcomes arrive
empty — `declared_outcomes()` filters those out, the state file still holds a
fully-passing `latest` phase, and the report printed `Result: PASSED` into an
issue titled "Langflow Migration Test Failed". The nightly failing to boot
against a migrated database is exactly what this workflow exists to catch, and
it landed in that gap.

The workflow now also hands over `JOB_STATUS` (`job.status`, already used by the
summary step below it). A job the runner reports as `failure`/`cancelled`, whose
report found no failure and no integrity problem, is itself an integrity
problem: the failure is real and lives outside every phase the report can see.
It is only raised when nothing else attributes the failure — the phase-level
message says it better and duplicating it would be noise.

Reconciling the job status covers the four steps that exist today and any added
later, which per-step `id`s would not.

Also: `## Unaccounted phases` is now `## Unaccounted failures`, since the
section can hold an entry that belongs to no phase; the header states the
runner's verdict next to the report's, the pair that contradicted each other;
and `__pycache__/` is ignored — running the Python tests locally left untracked
noise in every subsequent diff.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@rafaelgiln rafaelgiln added qa-infra QA testing infrastructure: workflows, automation, evidence, tracking follow-up Approved exception: follow-up of merged work (ROADMAP Intake) labels Jul 30, 2026
@rafaelgiln
rafaelgiln merged commit 4dbc5a1 into main Jul 30, 2026
7 of 8 checks passed
@rafaelgiln
rafaelgiln deleted the fix/issue-1141-report-job-status-reconcile branch July 30, 2026 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

follow-up Approved exception: follow-up of merged work (ROADMAP Intake) qa-infra QA testing infrastructure: workflows, automation, evidence, tracking

Projects

None yet

Development

Successfully merging this pull request may close these issues.

migration-test: the report still says "Result: PASSED" when the failure is in one of the 4 steps between phase_latest and phase_nightly_api

1 participant