Add Task execution and failure introspection (v0.8.0) - #514
Open
dotsdl wants to merge 19 commits into
Open
Conversation
Implements the v0.8.0 introspection design (milestone 9): durable execution provenance, live progress reporting, failure introspection (tracebacks, per-unit logs, stdout/stderr), and compute share reporting. Addresses #106, #211, #347, #195, #415, #295, #349, #389. - Durable provenance: an immutable TaskProvenance record per execution attempt (created at claim, finalized at result/expiry/deregistration/ release, race-safe against late results), hostname on compute service registrations, and Task.datetime_status_changed / Task.reason indicators with centralized status writes across every mutation site. - Live progress: an alchemiscale-owned execute_DAG mirroring gufe's semantics plus unit-attempt hooks (guarded by a gufe-equivalence suite), driving event-driven, fire-and-forget progress reporting. - Failure introspection: fast bulk tracebacks; per-unit log capture over the gufekey logger namespace plus gufe-native stdout/stderr capture; ProtocolUnitResultRef nodes and artifact retrieval; and graceful ProtocolDAG creation-failure handling (Task -> error with reason rather than killing the compute service). - Compute share: get_scope_compute_share. - Client methods, API routes, ComputeServiceSettings, a v04_to_v05 index migration, Sphinx docs, news fragments, and unit + integration tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- compute API set_task_result: tolerate a missing/`"None"` compute_service_id (the client serializes an absent id as the string "None"); skip provenance finalization instead of raising ValueError -> 500. Fixes the existing test_set_task_result / test_set_task_result_failure integration tests. - equivalence suite: pass `mapping=None` to every `Protocol.create()` call; the pinned gufe requires `mapping` as a keyword-only argument with no default (a newer local gufe had masked this). - docs: replace the unresolvable `gufe...ProtocolUnit.logger` intersphinx attr references with plain literals, so the docs build (fail_on_warning) passes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #514 +/- ##
==========================================
+ Coverage 80.83% 81.20% +0.37%
==========================================
Files 31 33 +2
Lines 4916 5705 +789
==========================================
+ Hits 3974 4633 +659
- Misses 942 1072 +130 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The Sphinx build (fail_on_warning) flagged autodoc'd docstrings with "inline interpreted text ... start-string without end-string": napoleon's field-list rendering surfaces reStructuredText's markup rules, which reject a closing backtick immediately followed by a letter/apostrophe (`Task`s, `Task`'s) and slash-joined inline markup (`_scoped_key`/`_gufe_key`). Reworded the affected docstrings (get_tasks_details, get_tasks_progress, get_task_tracebacks, get_task_stdout/stderr, get_scope_compute_share, ProtocolUnitResultRef) to keep the code markup while satisfying the markup rules. Verified zero inline-markup warnings across all changed docstrings by running each through napoleon + docutils. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The RTD Sphinx build (older/stricter than local) still flagged TaskProvenance:49 --- backtick-plural (`ProtocolUnit`s) inside a napoleon Attributes section, a context my local docutils reproduction couldn't see (the `.. attribute::` directive masked the inline content). Reworded that and, defensively, the other backtick-plural/apostrophe occurrences in the new docstrings so the render is robust to the stricter build. Pre-existing methods (get_task_results etc.) are untouched; they use the same style in plain summary prose and build cleanly on main. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- tests/unit/test_introspection_records.py: to_dict/from_dict round-trips (including None branches) for TaskProvenance and the client-facing record models (TaskAttempt, TaskClaim, TaskDetails, TaskTracebacks, TaskUnitTraceback, ProtocolDAGResultRec, ProtocolUnitResultRec), plus the datetime coercion helpers and the ProtocolUnitResultRef gufe round-trip. - tests/integration/storage/test_objectstore_introspection.py: push/pull round-trips for the per-unit log and stdout/stderr artifact methods (moto-mocked), including UTF-8 errors="replace" decoding, empty-prefix pulls, and invalid-stream-name validation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
End-to-end (client -> API -> Neo4j/S3) coverage for the new user-facing methods: get_task_history, get_tasks_details, get_task_result_recs, get_result_unit_recs, get_result_unit_logs/stdout/stderr, get_result_logs, get_task_stdout/stderr, get_task_tracebacks, get_tasks_progress, get_scope_compute_share, and set_tasks_status(reason=...). State (claimed provenance, results, per-unit refs, and per-unit artifacts) is set up via the n4js/s3os handles as the existing result tests do, then each method is called through the real client and asserted. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
get_scope_compute_share at the org level requires org-level scope access; the test identity holds only the specific project scope, so the org-level query 401'd (correct authorization behavior). Query at the project scope the identity holds instead; the org-level branch is already covered by the statestore integration test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…are levels) Nudge patch coverage over the threshold by exercising the last uncovered branches in already-passing tests: - get_result_logs(order='time') interleave path (with a timestamped and an untimestamped log line, hitting both sort keys); - get_scope_compute_share campaign-level and project-level grouping, plus the no-org ValueError. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ire tests
- H1 (HIGH): the /computeservice/{id}/progress route declared Body(embed=True)
while the compute client sends the bare map, so every live-progress push
422'd (silently, swallowed at DEBUG) and the whole feature returned None in
production. Drop embed so the route accepts the bare map per the design's
transport spec; also skip malformed entries instead of 500'ing.
- M2: add end-to-end compute-client tests through the API for
update_task_progress (would have caught H1), set_task_error (the /error
DAG-creation-failure path), and set_task_result_unit_logs (the artifact
upload route).
- M1: rename the migration v04_to_v05 -> v07_to_v08. Migration names track
release versions (v03_to_v04 shipped in v0.4.0); this is the v0.7 -> v0.8
upgrade, so as named an operator upgrading 0.7->0.8 would never run it and
the TaskProvenance indexes would never be created. Update the CLI command
and document the migration in docs/operations.rst.
- N4: constrain the `limit` query param on /history and /tracebacks to ge=1
(422 instead of a 500 from a negative Cypher LIMIT).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Every TaskProvenance access in the introspection queries is Task-anchored -- `(t)<-[:PROVENANCE_OF]-(tp:TaskProvenance ...)` -- starting from a Task pinned by its `_scoped_key` (already covered by the GufeTokenizable uniqueness constraint) and expanding the relationship to that Task's handful of attempt records. The `compute_service_id`/`datetime_claimed` label indexes are never consulted by these anchored traversals; they'd only help a query that scans the TaskProvenance label globally (e.g. future pruning or #180), of which there are none yet. Drop the migration entirely (module, CLI command, and operations docs); we'll add indexes if a global provenance query ever proves them critical. No data migration is required for this release regardless. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Introduce `protocol_unit_result_location` in `storage/objectstore.py` as the
single source of truth for the per-unit-result artifact layout
(`.../{pdr_key}/units/{unit_result_key}`), and use it at both storage sites
(`compute.api.set_task_result` and
`Neo4jStore.add_protocol_unit_result_refs`) instead of re-deriving the path
inline in each. This removes the duplicated derivation that could silently
drift and leave pushed stdout/stderr unretrievable.
Retrieval continues to read the authoritative `ProtocolUnitResultRef.location`
directly: unlike `ProtocolDAGResult`s, unit refs have no legacy/heterogeneous
data and their stored location is the actual write path, so a
reconstruct-then-fallback mechanism would be inert.
Add a unit test pinning the constructor's layout and GufeKey/str acceptance.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
TaskProvenance now carries a ScopedKey so it is a first-class scoped entity, authorized through the standard `validate_scopes(sk.scope, token)` path rather than relying on an anchoring Task -- which matters for future global/cross-task provenance queries (pruning, #180) that have no Task to borrow a scope check from. Like `Task`, it tokenizes on a uuid (via `_gufe_tokenize`), not its contents, so its GufeKey/ScopedKey is fixed at creation and unaffected by the in-place mutations (outcome/datetime_end/progress) applied over the attempt's life. Content-addressing would be neither stable nor unique per attempt. Creation moves out of the atomic CLAIM_QUERY: the query now claims and returns the registration's hostname/manager_name, and `claim_taskhub_tasks` builds a TaskProvenance for each genuinely-claimed Task via the normal keyed-node path (`_keyed_chain_to_subgraph` + `merge_subgraph`) and creates the PROVENANCE_OF edge, all in the same transaction so claim and provenance stay atomic. `datetime_claimed` still stores as a native neo4j DateTime, so ordering/coercion reads are unchanged. Adds a unit test for the uuid identity/round-trip and an integration test for the ScopedKey + get_gufe round-trip. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fill in the empty `datetime_created` description on `Task` and document the previously-undocumented `datetime_status_changed`, `reason`, `creator`, and `extends` attributes. Add the missing `scope` attribute (inherited from `ObjectStoreRef`) to `ProtocolUnitResultRef`. Docstring-only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ProtocolUnitResultRef is a mutable GufeTokenizable -- its has_logs/ has_stdout/has_stderr flags are flipped in place via Cypher as artifacts arrive. Under the inherited content-based tokenization, a reconstructed ref would recompute a key from its (now-mutated) content that no longer matched its stored `_scoped_key` -- safe before only by never relying on that recomputed key. Switch it to a uuid `_gufe_tokenize`, matching `Task`/`TaskProvenance`, so the GufeKey/ScopedKey is fixed at creation and independent of the mutable flags by construction. `_key` now round-trips through `__init__`/`_to_dict` so lookups by `_scoped_key` stay stable. Nothing relied on the old content-determined key: the refs map and idempotency short-circuit key off `obj_key`, not the ref's own key. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The client-facing record models (`TaskAttempt`, `TaskClaim`, `TaskDetails`, `TaskUnitTraceback`, `TaskTracebacks`, `ProtocolDAGResultRec`, `ProtocolUnitResultRec`) hand-rolled per-field coercion in `from_dict` and per-field serialization in `to_dict`. Replace that with three reusable pydantic v2 `Annotated` field types -- `Datetime`, `SK`, `GK` -- carrying `BeforeValidator`/`PlainSerializer`, so coercion happens on any construction (not just `from_dict`) and enums/nested models/ISO datetimes are handled natively. `to_dict`/`from_dict` become thin wrappers over `model_dump(mode="json")`/`model_validate`, preserving the public interface and call sites. The wire JSON is unchanged: `Datetime` serializes with `isoformat()` so UTC stays `+00:00` rather than pydantic's default `Z`. A new `TestWireShapeStability` pins the exact wire shape of each model (including the datetime-offset guard and inbound coercion of raw wire/neo4j values). Drops the now-unused `_iso` helper; `_coerce_datetime` remains (it backs the `Datetime` validator and is still used by the state store). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`pull_protocol_unit_result_streams` used a redundant private `_get_filename_prefix_contents` helper identical to the existing public `S3ObjectStore.iter_contents`; use `iter_contents` and drop the helper. Minor docstring clarifications in the state store and interface client. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Logs were retrievable at the unit and result (ProtocolDAGResult) tiers, but
stdout/stderr only at the unit and task tiers -- an accidental asymmetry.
The result is the coherent aggregation boundary (one execution attempt's
DAG), which is why logs uses it; task-level aggregation instead concatenates
raw streams across independent attempts, which is noisier and less useful.
Replace `get_task_stdout`/`get_task_stderr` (and their
`/tasks/{task}/stdout|stderr` routes) with `get_result_stdout`/
`get_result_stderr` (`/protocoldagresultrefs/{pdrr}/stdout|stderr`),
mirroring `get_result_logs`. This yields a consistent unit + result matrix
for logs/stdout/stderr; a caller wanting everything for a Task iterates
`get_task_result_recs`, the same drill-down logs already requires. Removal
is safe: the introspection surface is unreleased (0.8.0).
Updates the integration test and the docs accordingly.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Give `get_task_history`, `get_tasks_details`, `get_tasks_progress`, and
`get_task_tracebacks` a `visualize: bool = True` kwarg that renders a
rich-formatted view (side-effect display; the raw records are still
returned), matching the existing `get_*_status` convention.
- history: per-attempt table with color-coded outcome, hostname, claimed,
a compact duration ("how long did it run"), progress, and a result flag.
- details: table across tasks; a not-found (None) entry is shown, not dropped.
- progress: real rich progress bars; non-reporting tasks marked.
- tracebacks: each unit traceback in a bordered panel.
Consistency/cleanup: extract a shared status color palette and an outcome
palette, and refactor `_visualize_status` onto it. Drop `expand=True` on the
wide tables (it starved flexible columns to zero width) and use minute
precision + a duration column so they read cleanly at typical widths. All
user-derived text (reasons, tracebacks) is wrapped in `rich.text.Text` so
bracketed content (e.g. `[Errno 2]`) renders literally instead of being
parsed as markup.
Adds tests/unit/test_client_visualize.py smoke-testing every renderer
against edge cases (open attempts, not-found tasks, zero-total progress,
empty input, markup-hostile tracebacks).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements the accepted v0.8.0 Task execution and failure introspection design (milestone 9). The goal is to let users understand what is happening with their
Tasks: where and how long they've run, how far along they are, why they failed, and how much compute aScopeis getting.Addresses #106, #211, #347, #195, #415, #295, #349, #389.
What's included
1. Durable execution provenance (#106, #211)
TaskProvenancerecord per execution attempt, created at claim time and finalized when the attempt ends (complete/error/expired/released). It survives claim teardown and registration expiry, so "how many times has thisTaskrun, and where?" is answerable.(task, compute_service_id), a late result overwrites only its own prematurely-expiredrecord and never touches another service's open one.hostnameonComputeServiceRegistration(copied onto every provenance record); newTask.datetime_status_changedandTask.reasonindicators, written through a single centralized helper at every status-mutation site (claim,set_task_*, expiry, deregistration, restart-renew).get_task_history,get_tasks_details.2. Live progress reporting (#415)
execute_DAG(alchemiscale.compute.execute) mirroring gufe's execution semantics plus unit-attempt start/end hooks — the single seam progress and log capture plug into. A behavioral-equivalence test suite runs identical DAGs through both implementations and asserts equivalentProtocolDAGResults.TaskProvenance, so they survive claim teardown.get_tasks_progress; progress also appears in history/details.3. Failure introspection (#347, #349, #295, #195)
get_task_tracebacks).gufekeylogger namespace, plus gufe-native per-unitstdout/stderrcapture;ProtocolUnitResultRefnodes (withCONTAINS/UNIT_DEPENDS_ONtopology) and object-store artifacts.get_task_result_recs→get_result_unit_recs→get_result_unit_logs/get_result_unit_stdout/get_result_unit_stderr, plus renderingsget_result_logs,get_task_stdout/get_task_stderr.ProtocolDAGcreation-failure handling: theTaskis set toerrorwith the traceback inTask.reasonand the service continues, instead of the failure killing the loop and silently bouncing Tasks back towaiting.4. Compute share reporting (#389)
get_scope_compute_share(scope)— the fraction of currently-runningTasks aScopeconsumes relative to its same-level siblings (only the aggregate is disclosed).Also: new
ComputeServiceSettings(hostname,capture_streams,capture_logs,gufekey_loglevel,log_cap_bytes,progress_push_timeout), av04_to_v05index migration (idempotent, no data migration required), Sphinx docs (handling_errors.rst, a newintrospection.rst, compute settings), andnews/fragments.Data model changes
All additions are optional-valued, so no data migration is required — pre-existing
Tasks simply have empty history andnullindicators.TaskProvenancenode +PROVENANCE_OFedgesProtocolUnitResultRefnode +CONTAINS/UNIT_DEPENDS_ONedgesTask.datetime_status_changed,Task.reasonComputeServiceRegistration.hostnameTesting
tests/unit/compute/): a gufe-equivalence suite for the executor (success / failure / retries / retry-then-success / interrupts / cache-resume / stream-dir parity / hooks) proving byte-for-byte behavioral parity with the pinned gufeexecute_DAG, plus log-capture unit tests.tests/integration/storage/test_statestore_introspection.py, 22 tests) cover the provenance lifecycle, the late-result overwrite rules, status/reason indicators (including the idempotent-no-op guard), history/details/tracebacks/progress, unit refs, and compute share. These require the Neo4j/grolt harness and will run in CI.Notes for reviewers
datetime_status_changed/reasonagainst no-op re-sets using a CypherCASEthat reads the pre-clause status.test_status_write_idempotent_noop_preserves_indicatorsvalidates this against real Neo4j._pu_to_pur/_get_valid_unit_resultsto minimize divergence; the equivalence suite is the guard to run on every gufe upgrade.Tasksystem to retainProtocolDAG, uploadProtocolUnitResults andResultFiles as they complete #180; several choices (unit-attempt hooks,ProtocolUnitResultRefshape, always recordinglocation+ recompute-first/location-fallback retrieval) were made to carry over cheaply.🤖 Generated with Claude Code