Skip to content

Store workflow output - #14062

Open
erlenlh wants to merge 6 commits into
mainfrom
store-workflow-output
Open

Store workflow output#14062
erlenlh wants to merge 6 commits into
mainfrom
store-workflow-output

Conversation

@erlenlh

@erlenlh erlenlh commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Issue
Resolves #13320

Approach
Second of two PRs splitting the original "Store workflow output" work. The base
PR (#14301) captures workflow job
output and writes it to the ERT log; this one makes that output available to the
GUI and ties it to the experiment it belongs to.

  • Adds WorkflowEvent, carrying the output of a single job invocation so it can
    travel over the status queue like the other run model events. It is a plain
    BaseModel rather than a RunModelEvent because PRE_EXPERIMENT hooks run
    before an ensemble exists, so the mandatory iteration and run_id of
    RunModelEvent are not always available.
  • Persists the events alongside the experiment, one JSON object per line in
    workflow_events.jsonl. Appending keeps every hook of a run in one file, in
    the order they ran.
  • Emits an event per job invocation from run_workflows. Workflows skipped
    because the user cancelled are reported as cancelled rather than dropped, so
    the run is not misrepresented as having completed.

Events are held back until an experiment exists, since PRE_EXPERIMENT hooks run
before storage is created, and are flushed in a finally block so output
survives a workflow that stops the experiment. Failing to persist is logged but
does not abort the experiment.

Events are consumed by the GUI in the follow-up PRs in this stack.

  • PR title captures the intent of the changes, and is fitting for release notes.
  • Added appropriate release note label
  • Commit history is consistent and clean, in line with the contribution guidelines.
  • Make sure unit tests pass locally after every commit (git rebase -i main --exec 'just rapid-tests')

When applicable

  • When screenshots are changed: Review screenshot-PR in ert-testdata,
    merge screenshot-PR in ert-testdata before merging this PR.
  • When there are user facing changes: Updated documentation
  • New behavior or changes to existing untested code: Ensured that unit tests are added (See Ground Rules).
  • Large PR: Prepare changes in small commits for more convenient review
  • Bug fix: Add regression test for the bug
  • Bug fix: Add backport label to latest release (format: 'backport release-branch-name')

@codecov-commenter

codecov-commenter commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.25%. Comparing base (7bda171) to head (e8f9d2b).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #14062      +/-   ##
==========================================
+ Coverage   91.23%   91.25%   +0.02%     
==========================================
  Files         497      497              
  Lines       36277    36330      +53     
==========================================
+ Hits        33097    33154      +57     
+ Misses       3180     3176       -4     
Flag Coverage Δ
cli-tests 36.07% <84.61%> (+0.08%) ⬆️
fuzz 43.85% <56.92%> (+0.03%) ⬆️
gui-tests 58.74% <84.61%> (+0.04%) ⬆️
performance-and-unit-tests 81.21% <100.00%> (+0.03%) ⬆️
test 45.72% <81.53%> (+0.06%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/ert/run_models/event.py 99.00% <100.00%> (+0.17%) ⬆️
src/ert/run_models/run_model.py 93.37% <100.00%> (+0.89%) ⬆️
src/ert/storage/local_experiment.py 96.42% <100.00%> (+0.07%) ⬆️

... and 1 file with indirect coverage changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds structured capture and persistence of workflow-job output (stdout/stderr) so that workflows hooked into experiment runs can be logged to <UPDATE_LOG_PATH>/<run_id>/workflows.log and appended to the experiment’s storage directory workflows.log. It introduces a dedicated status event for workflow output, and wires both CLI and GUI monitors to write these logs.

Changes:

  • Capture stdout/stderr for internal and external workflow jobs (including failure cases) and surface per-invocation results.
  • Introduce RunModelWorkflowLogEvent with a stable log-entry format, write it to update-log output paths, and append it to experiment storage.
  • Add unit/UI tests and docs describing the new workflow logging behavior.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/ert/config/ert_script.py Adds thread-aware stdout/stderr capture proxy, plus ExternalScriptError handling to avoid ERT-internal stack traces for external job exit failures.
src/ert/config/external_ert_script.py Writes subprocess stdout/stderr to current streams (so ErtScript capture can record it) and raises ExternalScriptError with an exit-code message.
src/ert/workflow_runner.py Adds WorkflowJobResult and WorkflowRunner.jobResults() to preserve per-invocation output (including repeated job runs).
src/ert/run_models/event.py Introduces RunModelWorkflowLogEvent with as_log_entry() and write_as_log() for update-log persistence.
src/ert/run_models/run_model.py Emits workflow log events per job, buffers log entries until an experiment exists, and appends them to experiment storage.
src/ert/storage/local_experiment.py Adds workflow_log_path and append_workflow_log() to persist workflow log entries alongside the experiment.
src/ert/cli/monitor.py Writes workflow log events to <UPDATE_LOG_PATH>/<run_id>/workflows.log during CLI runs.
src/ert/gui/experiments/run_dialog.py Writes workflow log events to the GUI output path.
tests/ert/unit_tests/workflow_runner/test_workflow_runner.py Adds coverage for external-job stdout capture, exit-code error reporting, and per-invocation job results.
tests/ert/unit_tests/workflow_runner/test_ert_script.py Adds coverage for stdout/stderr capture, failure behavior, and concurrent/threaded capture isolation.
tests/ert/unit_tests/run_models/test_workflow_log_event.py New tests for RunModelWorkflowLogEvent log formatting and file append semantics.
tests/ert/unit_tests/run_models/test_status_events_serialization.py Extends status-event serialization coverage to include RunModelWorkflowLogEvent.
tests/ert/unit_tests/run_models/test_base_run_model.py Adds tests asserting workflow log event emission and storage persistence behavior across hooks/iterations.
tests/ert/unit_tests/gui/experiments/test_run_dialog.py Adds GUI test to verify workflow log events are written to the output path.
tests/ert/ui_tests/cli/test_cli.py Adds CLI UI test ensuring workflow output is written into the update log path.
docs/ert/reference/workflows/complete_workflows.rst Documents that hooked workflow output is also written to update-log and experiment storage logs.
docs/ert/reference/configuration/keywords.rst Documents the workflow log location/format under UPDATE_LOG_PATH, including storage-copy behavior and hook scope.
Suppressed comments (1)

tests/ert/unit_tests/workflow_runner/test_ert_script.py:183

  • Thread exceptions can be missed, and the join loop doesn’t assert that worker threads actually finished. Add an is_alive() assertion after each join so this test fails deterministically instead of leaking threads on failure paths.
    for thread in threads:
        thread.start()
    for thread in threads:
        thread.join(timeout=10)

Comment thread tests/ert/unit_tests/workflow_runner/test_workflow_runner.py Outdated
Comment thread tests/ert/unit_tests/workflow_runner/test_ert_script.py Outdated
Comment thread docs/ert/reference/workflows/complete_workflows.rst Outdated
@erlenlh
erlenlh force-pushed the store-workflow-output branch from 69e54f9 to 53b84d8 Compare August 3, 2026 14:00
Comment thread docs/ert/reference/workflows/complete_workflows.rst Outdated
@erlenlh
erlenlh force-pushed the store-workflow-output branch from 11a454c to 74d8bfe Compare August 5, 2026 07:42
@codspeed-hq

codspeed-hq Bot commented Aug 5, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 36 untouched benchmarks


Comparing store-workflow-output (780734f) with main (79e5551)

Open in CodSpeed

@erlenlh
erlenlh force-pushed the store-workflow-output branch from 03e0e55 to 598ec95 Compare August 10, 2026 07:49
@erlenlh
erlenlh requested a balanced review from Copilot August 20, 2026 09:00

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/ert/config/ert_script.py:63

  • The linked issue requires workflow stdout/stderr to be rerouted away from the user's terminal, but this forwarding path deliberately writes every captured byte back to the original stream. As a result, internal and external workflow output still appears in the terminal in addition to the log, so #13320's requested behavior is not implemented. Suppress forwarding for a thread with an active capture while continuing to forward writes from unrelated threads, and update the passthrough test accordingly.
        if self.wrapped is None:
            return len(s)
        return self.wrapped.write(s)

src/ert/run_models/run_model.py:947

  • This clears all pending events even when append_workflow_events() raises. A transient storage error therefore permanently drops buffered PRE_EXPERIMENT and current-hook output, and later hooks cannot retry it. Clear only events known to have been committed; because the append can partially succeed, this needs an atomic append or a commit result rather than simply retaining or clearing the whole batch.
        except Exception:
            logger.exception("Failed to persist workflow events to storage")
        self._pending_workflow_events = []

docs/ert/reference/workflows/complete_workflows.rst:116

  • This documents a GUI tab that this PR does not add: there is no workflow-event reader or “Workflow events” tab under src/ert/gui, and the PR description explicitly defers frontend consumption to a later PR. Users following these instructions cannot view the stored output as described; document the file as reserved for the future GUI instead.
in the GUI, select the experiment in the *Experiments* tool, then select the
*Workflow events* tab.

The GUI will show a list of all workflow jobs that have
been run for that experiment, and clicking on a job will show its output.

Comment thread docs/ert/reference/workflows/complete_workflows.rst Outdated
@erlenlh
erlenlh force-pushed the store-workflow-output branch 2 times, most recently from 597ce7a to a5b9131 Compare August 27, 2026 07:07
@erlenlh erlenlh mentioned this pull request Aug 27, 2026
10 tasks
@erlenlh
erlenlh changed the base branch from main to log-workflow-output August 27, 2026 07:21
Comment thread docs/ert/reference/workflows/complete_workflows.rst
Comment thread docs/ert/reference/workflows/complete_workflows.rst Outdated
@erlenlh
erlenlh force-pushed the store-workflow-output branch 4 times, most recently from e72d575 to 5aeeefe Compare August 31, 2026 12:47
@erlenlh
erlenlh force-pushed the store-workflow-output branch from 5aeeefe to c95c925 Compare September 1, 2026 12:06
@erlenlh
erlenlh force-pushed the store-workflow-output branch from c95c925 to dedec36 Compare September 3, 2026 07:37
Base automatically changed from log-workflow-output to main September 8, 2026 07:53
@erlenlh
erlenlh force-pushed the store-workflow-output branch 2 times, most recently from 1f2a10b to 54de5c6 Compare September 8, 2026 09:47
erlenlh and others added 6 commits September 9, 2026 12:18
Workflow job output is written to the ERT log, but the GUI has no way to
show it, since it never sees the log.

Add WorkflowEvent carrying the output of a single job invocation, so it
can travel over the status queue like the other run model events. It is
a plain BaseModel rather than a RunModelEvent because PRE_EXPERIMENT
hooks run before an ensemble exists, so the mandatory iteration and
run_id of RunModelEvent are not always available.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Workflow output lives only in the ERT log, which the GUI cannot read and
which is not tied to the experiment the workflow ran for.

Record the events alongside the experiment, one JSON object per line in
workflow_events.jsonl, so the output of a hooked workflow can be found
again from the experiment it belongs to. Appending keeps every hook of a
run in one file, in the order they ran.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Hooked workflows produce output that is only in the ERT log, so the GUI
cannot show what a workflow did, neither while it runs nor afterwards.

Emit a WorkflowEvent per job invocation from run_workflows and persist
them to the experiment. Workflows skipped because the user cancelled are
reported as cancelled rather than dropped, so the run is not
misrepresented as having completed.

Events are held back until an experiment exists, since PRE_EXPERIMENT
hooks run before storage is created, and are flushed in a finally block
so output survives a workflow that stops the experiment. Failing to
persist is logged but does not abort the experiment.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Test names introduced in this PR used "a", "an" and "the", which
does not follow this repo's naming convention and was flagged
repeatedly on the sibling PR #14301. Rename them to read as plain
behaviour specifications.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The "Complete workflows" heading underline was shorter than the
title text, which RST requires to match. Restore it to the correct
length.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This PR does not add a GUI "Workflow events" tab; that is planned
for a follow-up PR in the same stack. Remove the description of it
here so the docs don't describe a feature that doesn't exist yet.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@erlenlh
erlenlh force-pushed the store-workflow-output branch from 780734f to e8f9d2b Compare September 9, 2026 10:18

Workflows hooked in with :code:`HOOK_WORKFLOW` are in addition recorded
alongside the experiment they belong to, in
:code:`<ENSPATH>/experiments/<experiment_id>/workflow_events.jsonl`. That file

@xjules xjules Sep 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jsonl -> typo
Now I read the previous comment 😆

_start_iteration: int = PrivateAttr(default=0)
_max_parallelism_violation: ParallelismViolation = ParallelismViolation()
_workflow_runner: WorkflowRunner | None = PrivateAttr(default=None)
_workflow_run_id: uuid.UUID = PrivateAttr(default_factory=uuid.uuid4)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess that these two can be None?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Store output from workflows

6 participants